Fix NullReferenceException when cloning into a non-empty directory

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);
+        }
+    }
+}

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(
+        /// <summary>
+        /// Determines whether pack indexes should be trusted for the newly cloned enlistment.
+        /// Only reads the enlistment's git config when <paramref name="cloneResult"/> indicates
+        /// the clone succeeded; <paramref name="enlistment"/> 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".
+        /// </summary>
+        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)
             {