diff --git a/GitIntegration.Test/Execution/RunCommandGitProcessRunnerTests.cs b/GitIntegration.Test/Execution/RunCommandGitProcessRunnerTests.cs index dd491a0..0778f32 100644 --- a/GitIntegration.Test/Execution/RunCommandGitProcessRunnerTests.cs +++ b/GitIntegration.Test/Execution/RunCommandGitProcessRunnerTests.cs @@ -293,6 +293,24 @@ public void EnvironmentOverlayForcesNonInteractiveEnglishGit() Assert.AreEqual("C", RunCommandGitProcessRunner.EnvironmentOverlay["LC_ALL"]); } + [TestMethod] + [DataRow("GIT_DIR")] + [DataRow("GIT_WORK_TREE")] + [DataRow("GIT_INDEX_FILE")] + [DataRow("GIT_OBJECT_DIRECTORY")] + [DataRow("GIT_ALTERNATE_OBJECT_DIRECTORIES")] + [DataRow("GIT_COMMON_DIR")] + [DataRow("GIT_NAMESPACE")] + [DataRow("GIT_PREFIX")] + [DataRow("GIT_DIFF_OPTS")] + public void EnvironmentOverlayRemovesVariablesThatOverrideTheArguments(string name) + { + // A null value makes ktsu.RunCommand remove the variable from the child's environment, so a + // value inherited from a git hook cannot beat -C or -U (ktsu-dev/GitIntegration#139). + Assert.IsTrue(RunCommandGitProcessRunner.EnvironmentOverlay.TryGetValue(name, out string? value)); + Assert.IsNull(value); + } + public TestContext TestContext { get; set; } = null!; /// An that invokes its callback on the reporting thread. diff --git a/GitIntegration.Test/Integration/GitInheritedEnvironmentTests.cs b/GitIntegration.Test/Integration/GitInheritedEnvironmentTests.cs new file mode 100644 index 0000000..23e6bba --- /dev/null +++ b/GitIntegration.Test/Integration/GitInheritedEnvironmentTests.cs @@ -0,0 +1,137 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.GitIntegration.Test; + +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; + +using ktsu.Semantics.Paths; +using ktsu.Semantics.Strings; + +/// +/// Runs the verbs with git's own environment variables set in the calling process, as they are +/// inside a git hook. GIT_DIR and GIT_INDEX_FILE used to beat -C, so every +/// verb read the hook's repository, and GIT_DIFF_OPTS beat the pinned -U, so +/// Patch() could return zero-context hunks (ktsu-dev/GitIntegration#139). +/// +/// +/// The variables are set on this test process, which every other test shares, so the class must not +/// run alongside them. +/// +[TestClass] +[TestCategory("Integration")] +[DoNotParallelize] +public class GitInheritedEnvironmentTests +{ + private static readonly GitAuthorName AuthorName = "Fixture Author".As(); + private static readonly GitAuthorEmail AuthorEmail = "fixture@example.com".As(); + + [TestMethod] + public async Task InheritedGitDirAndIndexFileDoNotRedirectTheVerbsAsync() + { + CancellationToken cancellationToken = TestContext.CancellationTokenSource.Token; + await IntegrationGitFixture.RequireGitAsync(cancellationToken).ConfigureAwait(false); + + using TemporaryRepository other = new(); + using TemporaryRepository target = new(); + GitClient client = IntegrationGitFixture.CreateClient(); + + _ = await SeedAsync(client, other, "A.txt", "a\n", cancellationToken).ConfigureAwait(false); + _ = await SeedAsync(client, target, "B.txt", "b\n", cancellationToken).ConfigureAwait(false); + target.WriteFile("B.txt", "b changed\n"); + + string otherGitDir = Path.Join(other.RootPath, ".git"); + + using (new EnvironmentScope(new() + { + ["GIT_DIR"] = otherGitDir, + ["GIT_INDEX_FILE"] = Path.Join(otherGitDir, "index"), + })) + { + GitRepository opened = await client.OpenAsync(target.Root).ConfigureAwait(false); + + GitStatus status = await opened.Status().ExecuteAsync(cancellationToken).ConfigureAwait(false); + GitPatch patch = await opened.Patch().ExecuteAsync(cancellationToken).ConfigureAwait(false); + + GitStatusEntry entry = status.Entries.Single(); + Assert.AreEqual("B.txt", entry.Path.ToString(), "Status must describe the repository that was opened, not the one GIT_DIR names"); + Assert.AreEqual(GitFileState.Modified, entry.WorkTreeState); + Assert.AreEqual("B.txt", patch.Files.Single().Path.ToString(), "Patch must describe the repository that was opened, not the one GIT_DIR names"); + } + } + + [TestMethod] + public async Task InheritedGitDiffOptsDoesNotStripPatchContextAsync() + { + CancellationToken cancellationToken = TestContext.CancellationTokenSource.Token; + await IntegrationGitFixture.RequireGitAsync(cancellationToken).ConfigureAwait(false); + + using TemporaryRepository repository = new(); + GitClient client = IntegrationGitFixture.CreateClient(); + + _ = await SeedAsync(client, repository, "n.txt", "1\n2\n3\n4\n5\n", cancellationToken).ConfigureAwait(false); + repository.WriteFile("n.txt", "1\n2\nX\n4\n5\n"); + + using (new EnvironmentScope(new() { ["GIT_DIFF_OPTS"] = "-u0" })) + { + GitRepository opened = await client.OpenAsync(repository.Root).ConfigureAwait(false); + + GitFilePatch file = (await opened.Patch().WithContext(3) + .ForPath("n.txt".As()) + .ExecuteAsync(cancellationToken).ConfigureAwait(false)).Files.Single(); + GitHunk hunk = file.Hunks.Single(); + + Assert.AreEqual(1, hunk.OldStart, "The hunk should carry the context WithContext(3) asked for, not GIT_DIFF_OPTS's zero lines"); + Assert.AreEqual(5, hunk.OldCount); + + GitResult applied = await opened.Apply(file.PatchFor(file.Hunks)).ToIndex() + .TryExecuteAsync(cancellationToken).ConfigureAwait(false); + + Assert.IsTrue(applied.Success, "A patch read with context should stage cleanly"); + } + } + + private static async Task SeedAsync( + GitClient client, TemporaryRepository repository, string fileName, string contents, CancellationToken cancellationToken) + { + GitInitResult init = await client.Init(repository.Root) + .WithInitialBranch("main".As()) + .ExecuteAsync(cancellationToken).ConfigureAwait(false); + await IntegrationGitFixture.ConfigureIdentityAsync( + init.Repository, AuthorName, AuthorEmail, cancellationToken).ConfigureAwait(false); + + repository.WriteFile(fileName, contents); + _ = await init.Repository.Add().All().ExecuteAsync(cancellationToken).ConfigureAwait(false); + _ = await init.Repository.Commit("seed".As()).ExecuteAsync(cancellationToken).ConfigureAwait(false); + return init.Repository; + } + + /// Sets environment variables on this process and restores their previous values on dispose. + private sealed class EnvironmentScope : IDisposable + { + private readonly Dictionary _previous = new(StringComparer.Ordinal); + + public EnvironmentScope(Dictionary values) + { + foreach ((string name, string value) in values) + { + _previous[name] = Environment.GetEnvironmentVariable(name); + Environment.SetEnvironmentVariable(name, value); + } + } + + public void Dispose() + { + foreach ((string name, string? value) in _previous) + { + Environment.SetEnvironmentVariable(name, value); + } + } + } + + public TestContext TestContext { get; set; } = null!; +} diff --git a/GitIntegration/Execution/RunCommandGitProcessRunner.cs b/GitIntegration/Execution/RunCommandGitProcessRunner.cs index b5abb0d..fafdd93 100644 --- a/GitIntegration/Execution/RunCommandGitProcessRunner.cs +++ b/GitIntegration/Execution/RunCommandGitProcessRunner.cs @@ -42,9 +42,20 @@ public sealed class RunCommandGitProcessRunner(GitOptions options) : IGitProcess /// in this library — and every parser built on it — silently locale-dependent. /// /// - /// Both were impossible before ktsu.RunCommand 1.5.0, which added - /// . The entries are an overlay, so every - /// other variable the calling process had is inherited unchanged. + /// The null entries remove variables that would override the arguments this library + /// passes. GIT_DIR, GIT_WORK_TREE, GIT_INDEX_FILE and the other + /// repository-locating variables take priority over -C, and git exports them to hooks. A + /// tool that runs from a hook and opens a different repository would otherwise read, and write, + /// the hook's repository. GIT_DIFF_OPTS overrides the -U that Patch() always + /// passes, so it could produce zero-context hunks that Apply rejects + /// (ktsu-dev/GitIntegration#139). GIT_EXTERNAL_DIFF is already neutralised by + /// --no-ext-diff. + /// + /// + /// None of this was possible before ktsu.RunCommand 1.5.0, which added + /// and removes a variable whose value is + /// null. The entries are an overlay, so every other variable the calling process had is + /// inherited unchanged. /// /// internal static IReadOnlyDictionary EnvironmentOverlay { get; } = @@ -52,6 +63,15 @@ public sealed class RunCommandGitProcessRunner(GitOptions options) : IGitProcess { ["GIT_TERMINAL_PROMPT"] = "0", ["LC_ALL"] = "C", + ["GIT_DIR"] = null, + ["GIT_WORK_TREE"] = null, + ["GIT_INDEX_FILE"] = null, + ["GIT_OBJECT_DIRECTORY"] = null, + ["GIT_ALTERNATE_OBJECT_DIRECTORIES"] = null, + ["GIT_COMMON_DIR"] = null, + ["GIT_NAMESPACE"] = null, + ["GIT_PREFIX"] = null, + ["GIT_DIFF_OPTS"] = null, }; ///