Merge pull request #298: FetchStep: use '--prune' and refs/scalar/hidden

Resolves #289.

1. When using our custom refspec, `--prune` does not delete remote refs, but _will_ delete our custom refs. So, use it! Delete our custom code for deleting the hidden refs directory.

2. Make the refspec use `ScalarConstants` a bit better, and update the refs to update `refs/scalar/hidden` instead of `refs/hidden`.

Scalar.Common/Git/GitProcess.cs

@@ -426,12 +426,17 @@ public Result ForceCheckoutAllFiles()
 
         public Result BackgroundFetch(string remote)
         {
-            // By using "--no-update-remote-refs", the user will see their remote refs update
-            // normally when they do a foreground fetch.
             // By using this refspec, we do not create local refs, but instead store them in the "hidden"
             // namespace. These refs are never visible to the user (unless they open the .git/refs dir)
             // but still allow us to run reachability questions like updating the commit-graph.
-            return this.InvokeGitInWorkingDirectoryRoot($"fetch {remote} --quiet --no-update-remote-refs +refs/heads/*:refs/{ScalarConstants.DotGit.Refs.Hidden.Name}/{remote}/*", fetchMissingObjects: true);
+            string refspec = $"+{ScalarConstants.DotGit.Refs.Heads.RefName}/*"
+                           + $":{ScalarConstants.DotGit.Refs.Scalar.Hidden.RefName}/{remote}/*";
+
+            // By using "--no-update-remote-refs", the user will see their remote refs update
+            // normally when they do a foreground fetch.
+            return this.InvokeGitInWorkingDirectoryRoot(
+                $"fetch {remote} --quiet --prune --no-update-remote-refs \"{refspec}\"",
+                fetchMissingObjects: true);
         }
 
         public string[] GetRemotes()

Scalar.Common/Maintenance/FetchStep.cs

@@ -158,14 +158,6 @@ private bool TryFetchUsingGitProtocol(GitProcess gitProcess, out string error)
 
             using (ITracer activity = this.Context.Tracer.StartActivity(nameof(GitProcess.BackgroundFetch), EventLevel.LogAlways))
             {
-                // Clear hidden refs to avoid arbitrarily large growth
-                string hiddenRefspace = Path.Combine(this.Context.Enlistment.WorkingDirectoryRoot, ScalarConstants.DotGit.Refs.Hidden.Root);
-
-                if (this.Context.FileSystem.DirectoryExists(hiddenRefspace))
-                {
-                    this.Context.FileSystem.DeleteDirectory(hiddenRefspace, recursive: true);
-                }
-
                 string[] remotes = gitProcess.GetRemotes();
                 bool response = true;
 

Scalar.Common/ScalarConstants.cs

@@ -165,12 +165,28 @@ public static class Pack
 
             public static class Refs
             {
-                public static readonly string Root = Path.Combine(DotGit.Root, "refs");
+                public static readonly string RefName = "refs";
+                public static string Root => Path.Combine(DotGit.Root, RefName);
 
-                public static class Hidden
+                public static class Heads
                 {
-                    public static readonly string Name = "hidden";
-                    public static readonly string Root = Path.Combine(DotGit.Refs.Root, Name);
+                    public static readonly string Name = "heads";
+                    public static string Root => Path.Combine(Refs.Root, Name);
+                    public static string RefName => $"{Refs.RefName}/{Name}";
+                }
+
+                public static class Scalar
+                {
+                    public static readonly string Name = "scalar";
+                    public static string Root => Path.Combine(Refs.Root, Name);
+                    public static string RefName => $"{Refs.RefName}/{Name}";
+
+                    public static class Hidden
+                    {
+                        public static readonly string Name = "hidden";
+                        public static string Root => Path.Combine(Scalar.Root, Name);
+                        public static string RefName => $"{Scalar.RefName}/{Name}";
+                    }
                 }
             }
         }

Scalar.FunctionalTests/FileSystemRunners/SystemIORunner.cs

@@ -4,6 +4,7 @@
 using System.Diagnostics;
 using System.IO;
 using System.Runtime.InteropServices;
+using System.Text;
 using System.Threading;
 
 namespace Scalar.FunctionalTests.FileSystemRunners
@@ -245,22 +246,28 @@ private static extern bool WindowsCreateHardLink(
 
         private static void RetryOnException(Action action)
         {
+            StringBuilder message = new StringBuilder();
+            message.AppendLine("Failed to perform action with inner exceptions:");
             for (int i = 0; i < 10; i++)
             {
                 try
                 {
                     action();
-                    break;
+                    return;
                 }
-                catch (IOException)
+                catch (IOException e)
                 {
                     Thread.Sleep(500);
+                    message.AppendLine(e.Message);
                 }
-                catch (UnauthorizedAccessException)
+                catch (UnauthorizedAccessException e)
                 {
                     Thread.Sleep(500);
+                    message.AppendLine(e.Message);
                 }
             }
+
+            throw new Exception(message.ToString());
         }
 
         private void ShouldFail<ExceptionType>(Action action) where ExceptionType : Exception

Scalar.FunctionalTests/Settings.cs

@@ -21,6 +21,7 @@ public static class Default
             public static string PathToBash { get; set; }
             public static string PathToScalar { get; set; }
             public static string Commitish { get; set; }
+            public static string CommitId { get; set; }
             public static string ControlGitRepoRoot { get; set; }
             public static string EnlistmentRoot { get; set; }
             public static string PathToGit { get; set; }
@@ -33,6 +34,7 @@ public static void Initialize()
 
                 RepoToClone = @"https://gvfs.visualstudio.com/ci/_git/ForTests";
                 Commitish = @"FunctionalTests/20180214";
+                CommitId = "2797fbb8358bb2e0c12d6f3b42a60b43f7655edf";
 
                 if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
                 {

Scalar.FunctionalTests/Tests/GitRepoPerFixture/RunVerbTests.cs

@@ -1,5 +1,6 @@
 using NUnit.Framework;
 using Scalar.FunctionalTests.FileSystemRunners;
+using Scalar.FunctionalTests.Properties;
 using Scalar.FunctionalTests.Tools;
 using Scalar.Tests.Should;
 using System.Collections.Generic;
@@ -23,6 +24,12 @@ public RunVerbTests()
             this.fileSystem = new SystemIORunner();
         }
 
+        [SetUp]
+        public void Setup()
+        {
+            this.Enlistment.Unregister();
+        }
+
         [TestCase]
         [Order(1)]
         public void CommitGraphStep()
@@ -96,34 +103,42 @@ public void FetchStep()
         {
             string refsRoot = Path.Combine(this.Enlistment.RepoRoot, ".git", "refs");
             string refsHeads = Path.Combine(refsRoot, "heads");
-            string refsRemotes = Path.Combine(refsRoot, "remotes");
-            string refsHidden = Path.Combine(refsRoot, "hidden");
+            string refsRemotesOrigin = Path.Combine(refsRoot, "remotes", "origin");
+            string refsHidden = Path.Combine(refsRoot, "scalar", "hidden");
+            string refsHiddenOriginFake = Path.Combine(refsHidden, "origin", "fake");
 
             // Removing refs makes the next fetch need to download a new pack
             this.fileSystem.DeleteDirectory(refsHeads);
-            this.fileSystem.DeleteDirectory(refsRemotes);
+            this.fileSystem.DeleteDirectory(refsRemotesOrigin);
             this.fileSystem.DeleteDirectory(this.PackRoot);
+            this.fileSystem.CreateDirectory(this.PackRoot);
+            this.fileSystem.DirectoryExists(refsRemotesOrigin).ShouldBeFalse($"{refsRemotesOrigin} was not deleted");
 
             this.Enlistment.RunVerb("fetch");
 
             this.GetPackSizes(out int countAfterFetch, out _, out _, out _);
 
             countAfterFetch.ShouldEqual(1, "fetch should download one pack");
 
-            this.fileSystem.DirectoryExists(refsHidden).ShouldBeTrue("background fetch should have created refs/hidden/*");
+            this.fileSystem.DirectoryExists(refsHidden).ShouldBeTrue("background fetch should have created refs/scalar/hidden/*");
             this.fileSystem.DirectoryExists(refsHeads).ShouldBeFalse("background fetch should not have created refs/heads/*");
-            this.fileSystem.DirectoryExists(refsRemotes).ShouldBeFalse("background fetch should not have created refs/remotes/*");
+            this.fileSystem.DirectoryExists(refsRemotesOrigin).ShouldBeFalse("background fetch should not have created refs/remotes/origin/*");
+
+            // This is the SHA-1 for the master branch
+            string sha1 = Settings.Default.CommitId;
+            this.fileSystem.WriteAllText(refsHiddenOriginFake, sha1);
 
             this.Enlistment.RunVerb("fetch");
 
             this.fileSystem.DirectoryExists(refsHeads).ShouldBeFalse("background fetch should not have created refs/heads/*");
-            this.fileSystem.DirectoryExists(refsRemotes).ShouldBeFalse("background fetch should not have created refs/remotes/*");
+            this.fileSystem.DirectoryExists(refsRemotesOrigin).ShouldBeFalse("background fetch should not have created refs/remotes/origin/*");
+
+            this.fileSystem.FileExists(refsHiddenOriginFake).ShouldBeFalse("background fetch should clear deleted refs from refs/scalar/hidden");
 
             this.GetPackSizes(out int countAfterFetch2, out _, out _, out _);
             countAfterFetch2.ShouldEqual(1, "sceond fetch should not download a pack");
         }
 
-
         [TestCase]
         [Order(5)]
         public void AllSteps()