From 87856a42d7519c86d84859a48ac62e5c8804d4bd Mon Sep 17 00:00:00 2001 From: Tyrie Vella Date: Fri, 28 Aug 2026 15:53:10 -0700 Subject: [PATCH] Document why GitProcess does not use Enlistment.DotGitRoot In a linked git worktree, Enlistment.DotGitRoot and the git directory that GitProcess passes as --git-dir point at different locations: Enlistment.DotGitRoot -> the shared .git directory of the main repo GitProcess git dir -> "\.git", a file holding "gitdir: /.git/worktrees/" The two values look inconsistent, but each is correct for its purpose. Git follows the gitdir: pointer, so the worktree .git file resolves the per-worktree state (HEAD, index, per-worktree refs, reflog) and, through commondir, the shared state (config, objects, packed-refs). DotGitRoot is used for shared state that GVFS reads or writes directly on disk (objects, objects/info/alternates, hooks). Making GitProcess use DotGitRoot would be a regression. Every command would bind to the main worktree. From inside a linked worktree, "rev-parse HEAD" and "name-rev --name-only HEAD" would report the main worktree's branch and commit. No behavior changes. This adds comments on both sides of the divergence, an internal GitDirPath accessor, and tests that pin the current values. The tests cover both linked-worktree and regular enlistment behavior, plus the null working-directory constructor case. Review feedback folded into this commit tightened the API surface and docs: GitDirPath is internal, DotGitRoot has property-level XML docs, the helper summary no longer assumes .git is always a directory, and the worktree test comment now claims only the string contract that it verifies. Assisted-by: Claude Opus 4.5 Signed-off-by: Tyrie Vella --- GVFS/GVFS.Common/Enlistment.cs | 4 ++++ GVFS/GVFS.Common/GVFSEnlistment.cs | 3 +++ GVFS/GVFS.Common/Git/GitProcess.cs | 24 +++++++++++++++---- .../Common/WorktreeEnlistmentTests.cs | 14 ++++++++++- GVFS/GVFS.UnitTests/Git/GitProcessTests.cs | 21 ++++++++++++++++ 5 files changed, 61 insertions(+), 5 deletions(-) diff --git a/GVFS/GVFS.Common/Enlistment.cs b/GVFS/GVFS.Common/Enlistment.cs index a9208b0ed..503a29b61 100644 --- a/GVFS/GVFS.Common/Enlistment.cs +++ b/GVFS/GVFS.Common/Enlistment.cs @@ -62,6 +62,10 @@ protected Enlistment( public string WorkingDirectoryRoot { get; } public string WorkingDirectoryBackingRoot { get; } + /// + /// Path to the shared repository metadata directory that GVFS reads or writes directly. + /// In a linked worktree this is the common git directory, not the worktree's .git file. + /// public string DotGitRoot { get; protected set; } public abstract string GitObjectsRoot { get; protected set; } public abstract string LocalObjectsRoot { get; protected set; } diff --git a/GVFS/GVFS.Common/GVFSEnlistment.cs b/GVFS/GVFS.Common/GVFSEnlistment.cs index 7cd441aad..77188b6e5 100644 --- a/GVFS/GVFS.Common/GVFSEnlistment.cs +++ b/GVFS/GVFS.Common/GVFSEnlistment.cs @@ -64,6 +64,9 @@ private GVFSEnlistment(string enlistmentRoot, string gitBinPath, GitAuthenticati // Override DotGitRoot to point to the shared .git directory. // The base constructor sets it to WorkingDirectoryBackingRoot/.git // which is a file (not directory) in worktrees. + // + // DotGitRoot is shared git state. Per-worktree paths must come from + // Worktree.WorktreeGitDir. this.DotGitRoot = worktreeInfo.SharedGitDir; this.DotGVFSRoot = Path.Combine(worktreeInfo.WorktreeGitDir, GVFSPlatform.Instance.Constants.DotGVFSRoot); diff --git a/GVFS/GVFS.Common/Git/GitProcess.cs b/GVFS/GVFS.Common/Git/GitProcess.cs index 43a40ced0..2ac36ae75 100644 --- a/GVFS/GVFS.Common/Git/GitProcess.cs +++ b/GVFS/GVFS.Common/Git/GitProcess.cs @@ -55,7 +55,7 @@ public class GitProcess : ICredentialStore private string gitBinPath; private string workingDirectoryRoot; - private string dotGitRoot; + private string gitDirPath; private Process executingProcess; private bool stopping; @@ -101,10 +101,25 @@ public GitProcess(string gitBinPath, string workingDirectoryRoot) if (this.workingDirectoryRoot != null) { - this.dotGitRoot = Path.Combine(this.workingDirectoryRoot, GVFSConstants.DotGit.Root); + // Deliberately not Enlistment.DotGitRoot. In a linked worktree, --git-dir must + // point at the worktree's .git file, not the gitdir target named inside it. Git + // follows the gitdir: pointer, resolves per-worktree HEAD/index/refs, and then + // follows commondir to shared state. + // Passing Enlistment.DotGitRoot would bind commands to the main worktree instead. + this.gitDirPath = Path.Combine(this.workingDirectoryRoot, GVFSConstants.DotGit.Root); } } + /// + /// Path passed as --git-dir to InvokeGitAgainstDotGitFolder. + /// In a linked worktree this is the worktree's .git file, not the gitdir target + /// named inside it or . + /// + internal string GitDirPath + { + get { return this.gitDirPath; } + } + public static string ExpireTimeDateString { get @@ -1152,7 +1167,8 @@ private Result InvokeGitInWorkingDirectoryRoot( } /// - /// Invokes git.exe against an enlistment's .git folder. + /// Invokes git.exe against this process's --git-dir path. In a linked worktree this is + /// the worktree's .git file, not the gitdir target named inside it. /// This method should be used only with git-commands that ignore the working directory /// private Result InvokeGitAgainstDotGitFolder(string command, bool usePreCommandHook = true) @@ -1174,7 +1190,7 @@ private Result InvokeGitAgainstDotGitFolder( return this.InvokeGitImpl( command, workingDirectory: Environment.SystemDirectory, - dotGitDirectory: this.dotGitRoot, + dotGitDirectory: this.gitDirPath, useReadObjectHook: false, writeStdIn: writeStdIn, parseStdOutLine: parseStdOutLine, diff --git a/GVFS/GVFS.UnitTests/Common/WorktreeEnlistmentTests.cs b/GVFS/GVFS.UnitTests/Common/WorktreeEnlistmentTests.cs index dd072b1e5..4351189d2 100644 --- a/GVFS/GVFS.UnitTests/Common/WorktreeEnlistmentTests.cs +++ b/GVFS/GVFS.UnitTests/Common/WorktreeEnlistmentTests.cs @@ -1,4 +1,5 @@ using GVFS.Common; +using GVFS.Common.Git; using GVFS.Tests.Should; using NUnit.Framework; using System.IO; @@ -91,6 +92,17 @@ public void DotGitRootPointsToSharedGitDir() enlistment.DotGitRoot.ShouldEqual(this.sharedGitDir); } + [TestCase] + public void GitProcessUsesWorktreeGitFileNotSharedGitDir() + { + // GitProcess passes the worktree's .git file as --git-dir. It does not pre-resolve + // the gitdir target named inside that file. This pins the string contract. + GVFSEnlistment enlistment = this.CreateWorktreeEnlistment(); + GitProcess gitProcess = new GitProcess(enlistment); + + gitProcess.GitDirPath.ShouldEqual(Path.Combine(this.worktreePath, ".git")); + } + [TestCase] public void WorkingDirectoryRootIsWorktreePath() { @@ -155,4 +167,4 @@ public void RepoUrlIsReadFromSharedConfig() enlistment.RepoUrl.ShouldEqual("https://mock/repo"); } } -} +} diff --git a/GVFS/GVFS.UnitTests/Git/GitProcessTests.cs b/GVFS/GVFS.UnitTests/Git/GitProcessTests.cs index 8182f8dfb..6c9258519 100644 --- a/GVFS/GVFS.UnitTests/Git/GitProcessTests.cs +++ b/GVFS/GVFS.UnitTests/Git/GitProcessTests.cs @@ -102,6 +102,27 @@ public void TryKillRunningProcess_NeverRan() error.ShouldBeNull(); } + [TestCase] + public void GitDirPathMatchesDotGitRootForRegularEnlistment() + { + // Regular enlistments do not have a separate worktree .git file, + // so GitProcess and Enlistment use the same .git directory path. + MockGVFSEnlistment enlistment = new MockGVFSEnlistment(); + GitProcess process = new GitProcess(enlistment); + + process.GitDirPath.ShouldEqual(enlistment.DotGitRoot); + } + + [TestCase] + public void GitDirPathIsNullWhenWorkingDirectoryRootIsNull() + { + // Some callers only need global Git operations. Those instances have no enlistment + // root and cannot pass a --git-dir path. + GitProcess process = new GitProcess("git.exe", workingDirectoryRoot: null); + + process.GitDirPath.ShouldBeNull(); + } + [TestCase] public void ResultHasNoErrors() {