fix: resolve the git repository from cwd instead of the inherited environment - #2255
Open
dylanpulver wants to merge 2 commits into
Open
fix: resolve the git repository from cwd instead of the inherited environment#2255dylanpulver wants to merge 2 commits into
dylanpulver wants to merge 2 commits into
Conversation
✅ Deploy Preview for changesets ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
🦋 Changeset detectedLatest commit: f185876 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2255 +/- ##
==========================================
+ Coverage 84.53% 84.73% +0.20%
==========================================
Files 80 80
Lines 3168 3177 +9
Branches 890 893 +3
==========================================
+ Hits 2678 2692 +14
+ Misses 433 428 -5
Partials 57 57 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2027
The environment variable alone reproduces it, no hook or worktree needed:
Git runs hooks with
GIT_DIRpointing at the worktree-specific git directory and does not setGIT_WORK_TREE(an ordinary checkout gets neither, which is why only worktree pushes are affected). WithGIT_DIRset andGIT_WORK_TREEunset, git treatscwditself as the work tree root. Calls in@changesets/gitpasscwdbut inherit the environment, so the inheritedGIT_DIRwins.filterChangesetsSinceRefpassescwdas the.changesetdirectory, sogit diff --name-only --diff-filter=dlooks for each committed path under.changeset/<path>, finds nothing, classifies every file as deleted, and filters them all out.The fix
The exec wrapper strips the inherited variables that would redirect git away from the repository containing
cwd. Routing every call through one helper covers the other git-shelling commands, not just the one in the report.tinyexecmerges withprocess.env, so variables are dropped by overriding withundefined; keys match case-insensitively for Windows.Only
GIT_DIRis needed for this bug. I included the rest of the repository/work-tree/object-store/index set because leaving them keeps the same failure alive for apre-commithook that setsGIT_INDEX_FILE. Happy to narrow.packages/release-utils/src/gitUtils.tsspawns git the same way and I left it alone, since it runs in the action on an ordinary checkout where neither variable is set.Alternatives rejected
Setting
GIT_WORK_TREEalongside the inheritedGIT_DIR, as the issue suggests, is circular: finding the correct value needs the real worktree root, and asking git for it needs an already-correct environment. Setting it tocwdis wrong in exactly the failing case.Honouring the pair when a caller sets both is not coherent:
@manypkg/get-packagesreadspackage.jsonfromcwdregardless, so aGIT_WORK_TREEpointing elsewhere means git and the filesystem disagree about which project is being inspected.Fixing it locally in
@changesets/readwould leave the wrapper sensitive toGIT_INDEX_FILE, and any future caller passing a subdirectory would reintroduce it.Testing
Three tests in
packages/git/src/index.test.ts, all red onmain:getCurrentCommitIdreporting thecwdrepository's HEAD rather than the oneGIT_DIRpoints at, andgetChangedChangesetFilesSinceReffinding the changeset both withcwdat.changesetand in a realgit worktree addworktree.End to end, a real
pre-pushhook runningchangeset status --since=origin/mainfrom a worktree rejects the push onmainand succeeds here, same commit and hook.pnpm types:check,pnpm lint,pnpm formatclean; git, read and status suites pass. The pack/publish e2e snapshot tests fail for me, but identically onmainwith the same counts, so that is my local package manager versions. Changeset included.