diff --git a/GVFS/GVFS.UnitTests/CommandLine/CloneVerbTests.cs b/GVFS/GVFS.UnitTests/CommandLine/CloneVerbTests.cs new file mode 100644 index 000000000..e8055ce5e --- /dev/null +++ b/GVFS/GVFS.UnitTests/CommandLine/CloneVerbTests.cs @@ -0,0 +1,97 @@ +using GVFS.Common; +using GVFS.CommandLine; +using GVFS.UnitTests.Mock.Common; +using NUnit.Framework; +using System; +using System.IO; + +namespace GVFS.UnitTests.CommandLine +{ + [TestFixture] + public class CloneVerbTests + { + private CloneVerb cloneVerb; + private string testDir; + + [SetUp] + public void Setup() + { + this.cloneVerb = new CloneVerb(); + this.testDir = Path.Combine(Path.GetTempPath(), "CloneVerbTests_" + Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(this.testDir); + } + + [TearDown] + public void TearDown() + { + if (Directory.Exists(this.testDir)) + { + Directory.Delete(this.testDir, recursive: true); + } + } + + [TestCase] + public void TryCreateEnlistmentFailsWithoutEnlistmentWhenTargetDirectoryIsNotEmpty() + { + File.WriteAllText(Path.Combine(this.testDir, "preexisting.txt"), "content"); + + CloneVerb.Result result = this.cloneVerb.TryCreateEnlistment( + this.testDir, + this.testDir, + out GVFSEnlistment enlistment); + + Assert.IsFalse(result.Success); + Assert.IsNull(enlistment); + StringAssert.Contains("exists and is not empty", result.ErrorMessage); + } + + [TestCase] + public void TryCreateEnlistmentDoesNotFailForEmptyTargetDirectory() + { + // testDir is created empty by Setup and never written to in this test. + CloneVerb.Result result = this.cloneVerb.TryCreateEnlistment( + this.testDir, + this.testDir, + out GVFSEnlistment enlistment); + + StringAssert.DoesNotContain("exists and is not empty", result.ErrorMessage ?? string.Empty); + } + + [TestCase] + public void TryCreateEnlistmentReportsNormalizedPathWhenItDiffersFromFullPath() + { + File.WriteAllText(Path.Combine(this.testDir, "preexisting.txt"), "content"); + string fullPath = this.testDir + Path.DirectorySeparatorChar; + + CloneVerb.Result result = this.cloneVerb.TryCreateEnlistment( + fullPath, + this.testDir, + out GVFSEnlistment enlistment); + + Assert.IsFalse(result.Success); + Assert.IsNull(enlistment); + StringAssert.Contains($"'{fullPath}'", result.ErrorMessage); + StringAssert.Contains($"['{this.testDir}']", result.ErrorMessage); + } + + // Regression test: this is the actual code path that used to throw a + // NullReferenceException when `gvfs clone` targeted a non-empty directory. + // TryCreateEnlistment (above) fails and returns a null enlistment; CloneVerb.Execute() + // used to unconditionally dereference that null enlistment to read the trustPackIndexes + // config, crashing instead of reporting the "exists and is not empty" error. Execute() + // itself cannot be unit-tested (it terminates the process via Environment.Exit()), so + // GetTrustPackIndexes was extracted as the smallest testable seam that reproduces the + // exact failure condition: a failed clone result with a null enlistment. + [TestCase] + public void GetTrustPackIndexesDoesNotThrowWhenCloneFailedAndEnlistmentIsNull() + { + MockTracer tracer = new MockTracer(); + CloneVerb.Result failedCloneResult = new CloneVerb.Result("Clone directory exists and is not empty"); + + bool trustPackIndexes = true; + Assert.DoesNotThrow(() => trustPackIndexes = this.cloneVerb.GetTrustPackIndexes(tracer, failedCloneResult, enlistment: null)); + + Assert.AreEqual(GVFSConstants.GitConfig.TrustPackIndexesDefault, trustPackIndexes); + } + } +} diff --git a/GVFS/GVFS/CommandLine/CloneVerb.cs b/GVFS/GVFS/CommandLine/CloneVerb.cs index 2e9bb5276..4fa935de1 100644 --- a/GVFS/GVFS/CommandLine/CloneVerb.cs +++ b/GVFS/GVFS/CommandLine/CloneVerb.cs @@ -302,14 +302,8 @@ public override void Execute() { tracer.RelatedError(cloneResult.ErrorMessage); } - else - { - trustPackIndexes = LibGit2Repo.GetConfigBoolOrDefault( - tracer, - enlistment.WorkingDirectoryBackingRoot, - GVFSConstants.GitConfig.TrustPackIndexes, - GVFSConstants.GitConfig.TrustPackIndexesDefault); - } + + trustPackIndexes = this.GetTrustPackIndexes(tracer, cloneResult, enlistment); } if (cloneResult.Success) @@ -418,7 +412,30 @@ private static bool IsForceCheckoutErrorCloneFailure(string checkoutError) return true; } - private Result TryCreateEnlistment( + /// + /// Determines whether pack indexes should be trusted for the newly cloned enlistment. + /// Only reads the enlistment's git config when indicates + /// the clone succeeded; is null when the clone failed + /// (e.g. TryCreateEnlistment failed because the target directory was not empty), and must + /// not be dereferenced in that case. This gating is what fixes the NullReferenceException + /// regression where `gvfs clone` into a non-empty directory used to crash instead of + /// reporting "exists and is not empty". + /// + internal bool GetTrustPackIndexes(ITracer tracer, Result cloneResult, GVFSEnlistment enlistment) + { + if (!cloneResult.Success) + { + return GVFSConstants.GitConfig.TrustPackIndexesDefault; + } + + return LibGit2Repo.GetConfigBoolOrDefault( + tracer, + enlistment.WorkingDirectoryBackingRoot, + GVFSConstants.GitConfig.TrustPackIndexes, + GVFSConstants.GitConfig.TrustPackIndexesDefault); + } + + internal Result TryCreateEnlistment( string fullEnlistmentRootPathParameter, string normalizedEnlistementRootPath, out GVFSEnlistment enlistment) @@ -883,7 +900,7 @@ private Result TryInitRepo(ITracer tracer, GitRefs refs, Enlistment enlistmentTo return new Result(true); } - private class Result + internal class Result { public Result(bool success) {