fix(git): correct canonical_root for worktrees and subdirectory projects#749
Merged
Conversation
detect_changes returned an empty impacted_symbols set for projects indexed inside a git worktree or from a repo subdirectory (#659): canonical_root was computed wrong. `git rev-parse --git-common-dir` emits a path relative to the directory passed via -C (input_path), NOT to worktree_root, and the "/.git" suffix was stripped textually without resolving ".." components. So e.g. "../.git" joined against worktree_root, or "/ws/scripts/../.git" strip, left an unresolved ".." and never matched the project's stored root path. Fix (src/git/git_context.c): join the relative --git-common-dir against input_path (the -C dir) instead of worktree_root, and realpath()/_fullpath()- normalize the result BEFORE stripping "/.git" (the .git dir always exists for a valid repo, so resolution succeeds). This is consistent with the existing realpath'd root_path used for prefix matching (mcp.c). Reproduce-first: tests/test_git_context.c. The subdirectory case is the genuine guard — a repo indexed from a subdir yields a *relative* --git-common-dir, so the unfixed code leaves an un-normalized "<root>/subdir/.." (verified RED; GREEN after the realpath fix). The linked-worktree case is a supporting invariant, not the #659 reproducer: git that emits an *absolute* worktree --git-common-dir (e.g. 2.48.x) doesn't manifest the bug, so it passes either way; it still enforces the worktree->main-root invariant. All three git-based tests SKIP_PLATFORM on Windows (the CI shell there cannot init a repo via system()). Distilled from #672 (thanks @anivaryam); that PR's production fix is taken verbatim. Its repo_root/subdir tests ran git unconditionally and failed the Windows CI leg ("failed to init git repo"); here all three git-based tests are Windows-skipped so ci-ok is green cross-platform. Supersedes #696, which only realpath'd after the strip without changing the join base and so still yielded the workspace parent for the subdirectory case. Closes #659 Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com> Co-Authored-By: anivaryam <anivaryam.dev@gmail.com>
This was referenced Jul 1, 2026
On Windows every git_context test SKIP_PLATFORMs, leaving git_run/make_git_repo as unused static functions -> -Wunused-function under -Werror failed the Windows build. Guard the two helpers (only called from the non-Windows test bodies) so the file compiles clean on Windows while the tests still run on Linux/macOS. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Replace the manual join(input_path, --git-common-dir) + realpath/_fullpath with `git rev-parse --path-format=absolute --git-common-dir` (git 2.31+), which returns git's own absolute, canonical common-dir. Because a main repo and its linked worktree both query the same git binary, they resolve to the IDENTICAL path, so canonical_root is consistent across worktrees and platforms with no platform-specific path canonicalization. This is the truly platform-agnostic #659 fix: - git resolves the relative ".." internally, so a project indexed from a subdirectory gets the real repo root (the #659 bug). - No realpath/_fullpath, so the msys-vs-native path-representation divergence that broke the worktree==main-root invariant (test_pipeline.c git_context_linked_worktree) on the Windows CI leg is eliminated — both sides get git's single canonical representation. Falls back to the previous relative-join derivation (Unix realpath) when git is older than 2.31 and doesn't support --path-format (abs_common_dir empty). The input_path-join fix and the Windows test-harness SKIP_PLATFORM guards from the earlier commits on this branch remain for that fallback path. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com> Co-Authored-By: anivaryam <anivaryam.dev@gmail.com>
2 tasks
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
1 participant
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.
Distilled from #672 (thanks @anivaryam for diagnosing #659), reworked to be platform-agnostic.
Bug (#659)
detect_changesreturned an emptyimpacted_symbolsfor projects indexed inside a git worktree or from a repo subdirectory:canonical_rootwas computed wrong.git rev-parse --git-common-diremits a path relative to the-Cdirectory, and the/.gitsuffix was stripped textually without resolving.., so the derived root never matched the project's stored root path.Fix — let git canonicalize
Instead of manually joining the relative common-dir and normalizing it ourselves (
realpath/_fullpath), capture git's own absolute canonical common-dir:--path-format=absolute(git 2.31+) makes the path absolute and canonical.derive_canonical_rootuses it directly (strip/.git). Why this is the right fix:..internally, so a subdirectory (or worktree) yields the real repo root.worktree == main-rootinvariant (test_pipeline.c git_context_linked_worktree) on all platforms, including the MSYS2 Windows CI where our earlier_fullpathapproach diverged (msys/tmp→D:\a\_temp\msys64\tmpvs git's nativeD:\tmpfor the same directory).realpath/_fullpath/#ifdefon the primary path.Fallback: for git < 2.31 (no
--path-format, capture returns empty) it falls back to the relative-join derivation (Unixrealpath).Reproduce-first (verified locally, git 2.48.1 / macOS)
tests/test_git_context.c—canonical_root_subdiris the genuine guard: a repo indexed from a subdirectory. RED without the fix ("<root>/.." != "<root>"), GREEN with it; full suite green. The git-based testsSKIP_PLATFORMon Windows (that CI shell can'tgit initviasystem()with a POSIX redirect); the cross-platform correctness is covered by the pre-existinggit_context_linked_worktreeinvariant test, which now passes on Windows too.Why a distillation of #672
#672 diagnosed #659 correctly; its own fix (manual
input_pathjoin +_fullpath) couldn't pass the Windows CI leg because_fullpathcanonicalizes msys paths inconsistently with git. This PR keeps the diagnosis and reproduce case but derives the path from git itself, which is correct on every platform. Supersedes #696 (incomplete). Co-authored with @anivaryam.Closes #659
Source:
git rev-parse --path-format(git 2.31+).