From c246cc6b1c1e052246a41e227f5807e65144f43a Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 16:31:10 +0000 Subject: [PATCH 1/2] Report AlreadyExisted for a submodule, worktree or separate-git-dir target [patch] Init's probe treated any absolute --git-dir answer as "a repository above the target", but git prints one too when the target's .git is a file pointing elsewhere. git re-initializes those repositories and ignores --initial-branch, while AlreadyExisted said a fresh one was created. Ask rev-parse for --is-bare-repository and --show-cdup as well, and treat an empty cdup (the working-tree root) as the repository being at the target. Fixes ktsu-dev/GitIntegration#138 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01W83XxSnDX5Fu4sVPDKjXem --- CLAUDE.md | 5 +- .../Builders/GitInitBuilderTests.cs | 37 ++++++++-- .../Integration/GitRoundTripTests.cs | 68 +++++++++++++++++++ GitIntegration/Builders/GitInitBuilder.cs | 55 +++++++++++++-- 4 files changed, 152 insertions(+), 13 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 4f952e0..777cc31 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -210,7 +210,10 @@ Non-obvious, load-bearing design points: 4. **`Init` probes before running, so `GitInitResult.AlreadyExisted` can tell a caller whether a repository was already there.** `git init` is idempotent and announces the difference only in prose, and it silently ignores `--initial-branch` when re-initialising — the probe (a - `rev-parse --git-dir` check) is the only way to know either fact. + `rev-parse --is-bare-repository --git-dir --show-cdup` check) is the only way to know either + fact. `--git-dir` alone is not enough: it prints an absolute path both for an ancestor + repository and for a submodule, linked worktree or `--separate-git-dir` repository rooted at the + target, and only the empty `--show-cdup` tells the second apart. 5. **`Clone`'s destination check is advisory.** Git enforces the same rule itself; the pre-check exists only so a doomed clone fails before paying its network cost, and it is deliberately racy — diff --git a/GitIntegration.Test/Builders/GitInitBuilderTests.cs b/GitIntegration.Test/Builders/GitInitBuilderTests.cs index 293390c..bdfe810 100644 --- a/GitIntegration.Test/Builders/GitInitBuilderTests.cs +++ b/GitIntegration.Test/Builders/GitInitBuilderTests.cs @@ -93,9 +93,9 @@ public async Task ReportsAnExistingRepositoryAsAlreadyExistingAsync() { // git init on an existing repository exits 0 and only says "Reinitialized" in prose, so the // probe is the sole machine-readable signal. ".git" is what --git-dir prints for a non-bare - // repository at exactly this path. + // repository at exactly this path, and --show-cdup prints an empty line at its root. ScriptedGitProcessRunner runner = new ScriptedGitProcessRunner() - .Then(standardOutput: ".git\n") + .Then(standardOutput: "false\n.git\n\n") .Then(standardOutput: "Reinitialized existing Git repository in /dev/new-repo/.git/\n"); GitInitBuilder builder = new(runner, Target); @@ -109,7 +109,7 @@ public async Task StillRunsInitWhenTheRepositoryAlreadyExistsAsync() { // The probe reports, it does not gate: git init is idempotent and running it is harmless. ScriptedGitProcessRunner runner = new ScriptedGitProcessRunner() - .Then(standardOutput: ".git\n") + .Then(standardOutput: "false\n.git\n\n") .Then(standardOutput: "Reinitialized existing Git repository\n"); GitInitBuilder builder = new(runner, Target); @@ -126,7 +126,7 @@ public async Task ReportsAFreshRepositoryWhenTheProbeFindsOnlyAnAncestorReposito // above it: a real repository exists, but not at this path, so init here creates a new one // nested inside it. That must not be reported as AlreadyExisted. ScriptedGitProcessRunner runner = new ScriptedGitProcessRunner() - .Then(standardOutput: (OperatingSystem.IsWindows() ? @"C:\dev\.git" : "/dev/.git") + "\n") + .Then(standardOutput: "false\n" + (OperatingSystem.IsWindows() ? @"C:\dev\.git" : "/dev/.git") + "\n../\n") .Then(standardOutput: "Initialized empty Git repository in /dev/new-repo/sub/.git/\n"); GitInitBuilder builder = new(runner, Target); @@ -142,7 +142,7 @@ public async Task ReportsABareRepositoryAtTheTargetAsAlreadyExistingAsync() // --is-inside-work-tree got backwards, since a bare repository has no working tree to be // inside. ScriptedGitProcessRunner runner = new ScriptedGitProcessRunner() - .Then(standardOutput: ".\n") + .Then(standardOutput: "true\n.\n") .Then(standardOutput: "Reinitialized existing Git repository in /dev/new-repo/\n"); GitInitBuilder builder = new(runner, Target); @@ -151,6 +151,33 @@ public async Task ReportsABareRepositoryAtTheTargetAsAlreadyExistingAsync() Assert.IsTrue(result.AlreadyExisted); } + [TestMethod] + public async Task ReportsARepositoryWhoseGitDirectoryLivesElsewhereAsAlreadyExistingAsync() + { + // A submodule, a linked worktree or a --separate-git-dir repository has a .git file at the + // target pointing elsewhere, so --git-dir prints an absolute path just as it does for an + // ancestor. The empty --show-cdup line is what says the working tree is rooted here. + ScriptedGitProcessRunner runner = new ScriptedGitProcessRunner() + .Then(standardOutput: "false\n" + (OperatingSystem.IsWindows() ? @"C:\elsewhere\sg" : "/elsewhere/sg") + "\n\n") + .Then(standardOutput: "Reinitialized existing Git repository in /elsewhere/sg/\n"); + GitInitBuilder builder = new(runner, Target); + + GitInitResult result = await builder.ExecuteAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + Assert.IsTrue(result.AlreadyExisted); + } + + [TestMethod] + public void ReadsTheProbeOutputForEveryLayout() + { + Assert.IsTrue(GitInitBuilder.IsRepositoryRoot("false\n.git"), "non-bare repository at the target"); + Assert.IsTrue(GitInitBuilder.IsRepositoryRoot("true\n."), "bare repository at the target"); + Assert.IsTrue(GitInitBuilder.IsRepositoryRoot("false\n/elsewhere/sg"), ".git file at the target"); + Assert.IsFalse(GitInitBuilder.IsRepositoryRoot("false\n/dev/.git\n../"), "subdirectory of a repository"); + Assert.IsFalse(GitInitBuilder.IsRepositoryRoot("true\n/dev/bare"), "subdirectory of a bare repository"); + Assert.IsFalse(GitInitBuilder.IsRepositoryRoot(string.Empty), "no output"); + } + [TestMethod] public async Task ThrowsWhenInitItselfFailsAsync() { diff --git a/GitIntegration.Test/Integration/GitRoundTripTests.cs b/GitIntegration.Test/Integration/GitRoundTripTests.cs index 5366994..20f2bb5 100644 --- a/GitIntegration.Test/Integration/GitRoundTripTests.cs +++ b/GitIntegration.Test/Integration/GitRoundTripTests.cs @@ -3,6 +3,7 @@ namespace ktsu.GitIntegration.Test; using System.Collections.Generic; +using System.IO; using System.Linq; using System.Threading; using System.Threading.Tasks; @@ -84,6 +85,73 @@ public async Task InitReportsAnExistingRepositoryAsAlreadyExistingAsync() Assert.IsTrue(second.AlreadyExisted); } + [TestMethod] + public async Task InitReportsASeparateGitDirRepositoryAsAlreadyExistingAsync() + { + // The target's .git is a file pointing at a git directory elsewhere, so --git-dir prints an + // absolute path, as it does for an ancestor; git re-initializes it all the same. + CancellationToken cancellationToken = TestContext.CancellationTokenSource.Token; + await IntegrationGitFixture.RequireGitAsync(cancellationToken).ConfigureAwait(false); + + using TemporaryRepository temporary = new(); + using TemporaryRepository separateGitDir = new(); + GitRepository repository = await InitializeAsync(temporary, cancellationToken).ConfigureAwait(false); + IGitProcessRunner runner = repository.ProcessRunner!; + + string gitDir = Path.Combine(separateGitDir.Root.WeakString, "sg"); + _ = await new GitTextBuilder(runner, temporary.Root, "init", "--separate-git-dir", gitDir, "s") + .ExecuteAsync(cancellationToken).ConfigureAwait(false); + + GitInitResult init = await IntegrationGitFixture.CreateClient() + .Init(Path.Combine(temporary.Root.WeakString, "s").As()) + .ExecuteAsync(cancellationToken).ConfigureAwait(false); + + Assert.IsTrue(init.AlreadyExisted); + } + + [TestMethod] + public async Task InitReportsALinkedWorktreeAsAlreadyExistingAsync() + { + CancellationToken cancellationToken = TestContext.CancellationTokenSource.Token; + await IntegrationGitFixture.RequireGitAsync(cancellationToken).ConfigureAwait(false); + + using TemporaryRepository temporary = new(); + using TemporaryRepository worktreeParent = new(); + GitRepository repository = await InitializeAsync(temporary, cancellationToken).ConfigureAwait(false); + + temporary.WriteFile("a.txt", "one\n"); + _ = await repository.Add().All().ExecuteAsync(cancellationToken).ConfigureAwait(false); + _ = await repository.Commit("c1".As()).ExecuteAsync(cancellationToken).ConfigureAwait(false); + + string worktree = Path.Combine(worktreeParent.Root.WeakString, "wt"); + _ = await new GitTextBuilder(repository.ProcessRunner!, repository.LocalPath, "worktree", "add", worktree) + .ExecuteAsync(cancellationToken).ConfigureAwait(false); + + GitInitResult init = await IntegrationGitFixture.CreateClient() + .Init(worktree.As()) + .ExecuteAsync(cancellationToken).ConfigureAwait(false); + + Assert.IsTrue(init.AlreadyExisted); + } + + [TestMethod] + public async Task InitReportsASubdirectoryOfARepositoryAsFreshAsync() + { + CancellationToken cancellationToken = TestContext.CancellationTokenSource.Token; + await IntegrationGitFixture.RequireGitAsync(cancellationToken).ConfigureAwait(false); + + using TemporaryRepository temporary = new(); + _ = await InitializeAsync(temporary, cancellationToken).ConfigureAwait(false); + + temporary.WriteFile("sub/a.txt", "one\n"); + + GitInitResult init = await IntegrationGitFixture.CreateClient() + .Init(Path.Combine(temporary.Root.WeakString, "sub").As()) + .ExecuteAsync(cancellationToken).ConfigureAwait(false); + + Assert.IsFalse(init.AlreadyExisted); + } + [TestMethod] public async Task AddAndCommitProduceAReadableCommitAsync() { diff --git a/GitIntegration/Builders/GitInitBuilder.cs b/GitIntegration/Builders/GitInitBuilder.cs index 31bb222..8cea348 100644 --- a/GitIntegration/Builders/GitInitBuilder.cs +++ b/GitIntegration/Builders/GitInitBuilder.cs @@ -129,21 +129,62 @@ public override async Task> TryExecuteAsync(Cancellatio private async Task ProbeAsync(CancellationToken cancellationToken) { - // Asks "is there a repository at exactly this path", via --git-dir, deliberately not via + // Asks "is there a repository at exactly this path", deliberately not via // GitProbes.IsWorkTreeAsync's --is-inside-work-tree: that answers a different question, // whether the path is inside *some* working tree. That wrongly reports AlreadyExisted = // true for a plain subdirectory of an existing repository (git init there creates a // nested repository), and wrongly reports AlreadyExisted = false for an existing bare - // repository (git init there only prints a re-init warning). --git-dir discriminates all - // four cases: ".git" (a non-bare repository at exactly this path), "." (a bare repository - // at exactly this path), an absolute path (a repository exists, but as an ancestor, not - // here), or a non-zero exit (no repository, or the directory does not exist). + // repository (git init there only prints a re-init warning). // // TryExecuteAsync, because failure is the expected answer: the directory may hold no // repository, or may not exist at all, and both exit 128 and both mean "not yet". - GitResult probe = await new GitTextBuilder(Runner, _targetPath, "rev-parse", "--git-dir") + GitResult probe = await new GitTextBuilder( + Runner, _targetPath, "rev-parse", "--is-bare-repository", "--git-dir", "--show-cdup") .TryExecuteAsync(cancellationToken).ConfigureAwait(false); - return probe.Success && probe.Value is ".git" or "."; + return probe.Success && probe.Value is not null && IsRepositoryRoot(probe.Value); + } + + /// + /// Reads the probe's answer: whether the repository git found is rooted at the target itself. + /// + /// + /// + /// --git-dir alone cannot tell. It prints .git for an ordinary repository at the + /// target and . for a bare one, but an absolute path both for a repository rooted above + /// the target and for one at the target whose .git is a file pointing elsewhere — a + /// submodule, a linked worktree, or a --separate-git-dir repository. git re-initializes + /// the latter, so it is the working-tree root, not the git directory, that decides. + /// + /// + /// --show-cdup gives that root relative to the target: empty at the root, ../ + /// and so on below it. It prints nothing at all outside a working tree, and the output is + /// trimmed, so a missing line and an empty one read the same; the bare check comes first + /// because a subdirectory of a bare repository is the one place that difference would matter. + /// + /// + /// The trimmed output of the probe. + /// when a repository already exists at the target. + internal static bool IsRepositoryRoot(string output) + { + string[] lines = Ensure.NotNull(output).Split('\n'); + + if (lines.Length < 2) + { + return false; + } + + // "." is the target itself being a git directory: a bare repository, or a .git directory. + if (string.Equals(lines[1].TrimEnd('\r'), ".", StringComparison.Ordinal)) + { + return true; + } + + if (string.Equals(lines[0].TrimEnd('\r'), "true", StringComparison.Ordinal)) + { + return false; + } + + return lines.Length < 3 || lines[2].TrimEnd('\r').Length == 0; } } From b48090be544bb1c1bb80e10fd56e1dd6684ee940 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 16:34:22 +0000 Subject: [PATCH 2/2] Build test paths with Path.Join, as the other integration tests do Path.Combine drops earlier arguments when a later one is rooted; Path.Join never does. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01W83XxSnDX5Fu4sVPDKjXem --- GitIntegration.Test/Integration/GitRoundTripTests.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/GitIntegration.Test/Integration/GitRoundTripTests.cs b/GitIntegration.Test/Integration/GitRoundTripTests.cs index 20f2bb5..db04a7c 100644 --- a/GitIntegration.Test/Integration/GitRoundTripTests.cs +++ b/GitIntegration.Test/Integration/GitRoundTripTests.cs @@ -98,12 +98,12 @@ public async Task InitReportsASeparateGitDirRepositoryAsAlreadyExistingAsync() GitRepository repository = await InitializeAsync(temporary, cancellationToken).ConfigureAwait(false); IGitProcessRunner runner = repository.ProcessRunner!; - string gitDir = Path.Combine(separateGitDir.Root.WeakString, "sg"); + string gitDir = Path.Join(separateGitDir.RootPath, "sg"); _ = await new GitTextBuilder(runner, temporary.Root, "init", "--separate-git-dir", gitDir, "s") .ExecuteAsync(cancellationToken).ConfigureAwait(false); GitInitResult init = await IntegrationGitFixture.CreateClient() - .Init(Path.Combine(temporary.Root.WeakString, "s").As()) + .Init(Path.Join(temporary.RootPath, "s").As()) .ExecuteAsync(cancellationToken).ConfigureAwait(false); Assert.IsTrue(init.AlreadyExisted); @@ -123,7 +123,7 @@ public async Task InitReportsALinkedWorktreeAsAlreadyExistingAsync() _ = await repository.Add().All().ExecuteAsync(cancellationToken).ConfigureAwait(false); _ = await repository.Commit("c1".As()).ExecuteAsync(cancellationToken).ConfigureAwait(false); - string worktree = Path.Combine(worktreeParent.Root.WeakString, "wt"); + string worktree = Path.Join(worktreeParent.RootPath, "wt"); _ = await new GitTextBuilder(repository.ProcessRunner!, repository.LocalPath, "worktree", "add", worktree) .ExecuteAsync(cancellationToken).ConfigureAwait(false); @@ -146,7 +146,7 @@ public async Task InitReportsASubdirectoryOfARepositoryAsFreshAsync() temporary.WriteFile("sub/a.txt", "one\n"); GitInitResult init = await IntegrationGitFixture.CreateClient() - .Init(Path.Combine(temporary.Root.WeakString, "sub").As()) + .Init(Path.Join(temporary.RootPath, "sub").As()) .ExecuteAsync(cancellationToken).ConfigureAwait(false); Assert.IsFalse(init.AlreadyExisted);