Merge pull request #150: Cleanups following gvfs-helper

Here are a few leftovers from #71 

* There were a few more "readobject" strings in the codebase.

* The `PackfileMaintenanceStepTests` were disabled, but now they are re-enabled and adjusted to the new gvfs-helper environment.

* The `PrefetchVerb` no longer does anything but the commits and trees prefetch, so drop the `--commits` option.

Scalar.Common/ScalarConstants.cs

@@ -145,27 +145,21 @@ public static class Logs
 
             public static class Hooks
             {
-                public const string ReadObjectName = "read-object";
                 public const string QueryWatchmanName = "query-watchman";
                 public const string FsMonitorWatchmanSampleName = "fsmonitor-watchman.sample";
 
                 public static readonly string Root = Path.Combine(DotGit.Root, "hooks");
-                public static readonly string ReadObjectPath = Path.Combine(Hooks.Root, ReadObjectName);
                 public static readonly string QueryWatchmanPath = Path.Combine(Hooks.Root, QueryWatchmanName);
                 public static readonly string FsMonitorWatchmanSamplePath = Path.Combine(Hooks.Root, FsMonitorWatchmanSampleName);
             }
 
             public static class Info
             {
                 public const string Name = "info";
-                public const string ExcludeName = "exclude";
-                public const string AlwaysExcludeName = "always_exclude";
                 public const string SparseCheckoutName = "sparse-checkout";
 
                 public static readonly string Root = Path.Combine(DotGit.Root, Info.Name);
                 public static readonly string SparseCheckoutPath = Path.Combine(Info.Root, Info.SparseCheckoutName);
-                public static readonly string ExcludePath = Path.Combine(Info.Root, ExcludeName);
-                public static readonly string AlwaysExcludePath = Path.Combine(Info.Root, AlwaysExcludeName);
             }
 
             public static class Objects

Scalar.FunctionalTests/Tests/EnlistmentPerFixture/PackfileMaintenanceStepTests.cs

@@ -9,7 +9,6 @@
 namespace Scalar.FunctionalTests.Tests.EnlistmentPerFixture
 {
     [TestFixture]
-    [Category(Categories.NeedsUpdatesForNonVirtualizedMode)]
     public class PackfileMaintenanceStepTests : TestsWithEnlistmentPerFixture
     {
         private FileSystemRunner fileSystem;
@@ -26,57 +25,23 @@ public PackfileMaintenanceStepTests()
         private string PackRoot => this.Enlistment.GetPackRoot(this.fileSystem);
 
         [TestCase, Order(1)]
-        public void ExpireClonePack()
-        {
-            this.GetPackSizes(out int packCount, out long maxSize, out long totalSize);
-
-            // We should have at least two packs:
-            //
-            // 1. the pack-<hash>.pack from clone.
-            // 2. a prefetch-<timestamp>-<hash>.pack from prefetch.
-            //
-            // The prefetch pack is newer, and covers all the objects in the clone pack,
-            // so the clone pack will be expired when we run the step.
-
-            Directory.GetFiles(this.PackRoot, "*.keep")
-                .Count()
-                .ShouldEqual(1);
-
-            packCount.ShouldEqual(2, message: "Incorrect packfile layout for expire test");
-
-            // Ensure we have a multi-pack-index (not created on clone)
-            GitProcess.InvokeProcess(
-                this.Enlistment.RepoRoot,
-                $"multi-pack-index write --object-dir={this.GitObjectRoot}");
-
-            this.Enlistment.PackfileMaintenanceStep();
-
-            List<string> packs = this.GetPackfiles();
-
-            packs.Count.ShouldEqual(1, $"incorrect number of packs after first step: {packs.Count}");
-
-            Path.GetFileName(packs[0])
-                .StartsWith("prefetch-")
-                .ShouldBeTrue($"packsBetween[0] should start with 'prefetch-': {packs[0]}");
-        }
-
-        [TestCase, Order(2)]
         public void RepackAllToOnePack()
         {
-            // Create new pack(s) by prefetching blobs for a folder.
-            // This generates a number of packs, based on the processor number (for parallel downloads).
-            this.Enlistment.Prefetch($"--folders {Path.Combine("Scalar", "Scalar")}");
+            this.GetPackSizes(out int beforePrefetchPackCount, out long maxSize, out long totalSize);
+
+            // Cannot be sure of the count, but there should be two from the inital clone
+            beforePrefetchPackCount.ShouldBeAtLeast(2);
 
             // Create a multi-pack-index that covers the prefetch packs
-            // (The post-fetch job creates a multi-pack-index only after a --commits prefetch)
+            // (The post-fetch job creates a multi-pack-index only after a prefetch)
             GitProcess.InvokeProcess(
                 this.Enlistment.RepoRoot,
                 $"multi-pack-index write --object-dir={this.GitObjectRoot}");
 
             // Run the step to ensure we don't have any packs that will be expired during the repack step
             this.Enlistment.PackfileMaintenanceStep();
 
-            this.GetPackSizes(out int afterPrefetchPackCount, out long maxSize, out long totalSize);
+            this.GetPackSizes(out int afterPrefetchPackCount, out maxSize, out totalSize);
 
             // Cannot be sure of the count, as the prefetch uses parallel threads to get multiple packs
             afterPrefetchPackCount.ShouldBeAtLeast(2);
@@ -88,7 +53,7 @@ public void RepackAllToOnePack()
             packCount.ShouldEqual(afterPrefetchPackCount + 1, $"incorrect number of packs after repack step: {packCount}");
         }
 
-        [TestCase, Order(3)]
+        [TestCase, Order(2)]
         public void ExpireAllButOneAndKeep()
         {
             string prefetchPack = Directory.GetFiles(this.PackRoot, "prefetch-*.pack")

Scalar.FunctionalTests/Tests/EnlistmentPerFixture/PrefetchVerbTests.cs

@@ -27,7 +27,7 @@ public PrefetchVerbTests()
         [Category(Categories.MacTODO.TestNeedsToLockFile)]
         public void PrefetchCleansUpStalePrefetchLock()
         {
-            this.Enlistment.Prefetch("--commits");
+            this.Enlistment.Prefetch();
             this.PostFetchStepShouldComplete();
             string prefetchCommitsLockFile = Path.Combine(this.Enlistment.GetObjectRoot(this.fileSystem), "pack", PrefetchCommitsAndTreesLock);
             prefetchCommitsLockFile.ShouldNotExistOnDisk(this.fileSystem);
@@ -41,7 +41,7 @@ public void PrefetchCleansUpStalePrefetchLock()
                 .Count()
                 .ShouldEqual(1, "Incorrect number of .keep files in pack directory");
 
-            this.Enlistment.Prefetch("--commits");
+            this.Enlistment.Prefetch();
             this.PostFetchStepShouldComplete();
             prefetchCommitsLockFile.ShouldNotExistOnDisk(this.fileSystem);
         }

Scalar.FunctionalTests/Tests/EnlistmentPerFixture/PrefetchVerbWithoutSharedCacheTests.cs

@@ -48,7 +48,7 @@ private string TempPackRoot
         [TestCase, Order(1)]
         public void PrefetchCommitsToEmptyCache()
         {
-            this.Enlistment.Prefetch("--commits");
+            this.Enlistment.Prefetch();
             this.PostFetchJobShouldComplete();
 
             // Verify prefetch pack(s) are in packs folder and have matching idx file
@@ -72,7 +72,7 @@ public void PrefetchBuildsIdxWhenMissingFromPrefetchPack()
             idxPath.ShouldNotExistOnDisk(this.fileSystem);
 
             // Prefetch should rebuild the missing idx
-            this.Enlistment.Prefetch("--commits");
+            this.Enlistment.Prefetch();
             this.PostFetchJobShouldComplete();
 
             idxPath.ShouldBeAFile(this.fileSystem);
@@ -97,7 +97,7 @@ public void PrefetchCleansUpBadPrefetchPack()
             badPackPath.ShouldBeAFile(this.fileSystem).WithContents(badContents);
 
             // Prefetch should delete the bad pack
-            this.Enlistment.Prefetch("--commits");
+            this.Enlistment.Prefetch();
             this.PostFetchJobShouldComplete();
 
             badPackPath.ShouldNotExistOnDisk(this.fileSystem);
@@ -124,7 +124,7 @@ public void PrefetchCleansUpOldPrefetchPack()
             badPackPath.ShouldBeAFile(this.fileSystem).WithContents(badContents);
 
             // Prefetch should delete the bad pack and all packs after it
-            this.Enlistment.Prefetch("--commits");
+            this.Enlistment.Prefetch();
             this.PostFetchJobShouldComplete();
 
             badPackPath.ShouldNotExistOnDisk(this.fileSystem);
@@ -157,12 +157,12 @@ public void PrefetchFailsWhenItCannotRemoveABadPrefetchPack()
             // Open a handle to the bad pack that will prevent prefetch from being able to delete it
             using (FileStream stream = new FileStream(badPackPath, FileMode.Open, FileAccess.Read, FileShare.None))
             {
-                string output = this.Enlistment.Prefetch("--commits", failOnError: false);
+                string output = this.Enlistment.Prefetch(failOnError: false);
                 output.ShouldContain($"Unable to delete {badPackPath}");
             }
 
             // After handle is closed prefetch should succeed
-            this.Enlistment.Prefetch("--commits");
+            this.Enlistment.Prefetch();
             this.PostFetchJobShouldComplete();
 
             badPackPath.ShouldNotExistOnDisk(this.fileSystem);
@@ -190,12 +190,12 @@ public void PrefetchFailsWhenItCannotRemoveAPrefetchPackNewerThanBadPrefetchPack
             // Open a handle to a good pack that is newer than the bad pack, which will prevent prefetch from being able to delete it
             using (FileStream stream = new FileStream(prefetchPacks[0], FileMode.Open, FileAccess.Read, FileShare.None))
             {
-                string output = this.Enlistment.Prefetch("--commits", failOnError: false);
+                string output = this.Enlistment.Prefetch(failOnError: false);
                 output.ShouldContain($"Unable to delete {prefetchPacks[0]}");
             }
 
             // After handle is closed prefetch should succeed
-            this.Enlistment.Prefetch("--commits");
+            this.Enlistment.Prefetch();
             this.PostFetchJobShouldComplete();
 
             // The bad pack and all packs newer than it should not be on disk
@@ -227,12 +227,12 @@ public void PrefetchFailsWhenItCannotRemoveAPrefetchIdxNewerThanBadPrefetchPack(
             // Open a handle to a good idx that is newer than the bad pack, which will prevent prefetch from being able to delete it
             using (FileStream stream = new FileStream(newerIdxPath, FileMode.Open, FileAccess.Read, FileShare.None))
             {
-                string output = this.Enlistment.Prefetch("--commits", failOnError: false);
+                string output = this.Enlistment.Prefetch(failOnError: false);
                 output.ShouldContain($"Unable to delete {newerIdxPath}");
             }
 
             // After handle is closed prefetch should succeed
-            this.Enlistment.Prefetch("--commits");
+            this.Enlistment.Prefetch();
             this.PostFetchJobShouldComplete();
 
             // The bad pack and all packs newer than it should not be on disk
@@ -275,7 +275,7 @@ public void PrefetchCleansUpStaleTempPrefetchPacks()
             this.fileSystem.WriteAllText(otherFilePath, otherFileContents);
             otherFilePath.ShouldBeAFile(this.fileSystem).WithContents(otherFileContents);
 
-            this.Enlistment.Prefetch("--commits");
+            this.Enlistment.Prefetch();
             this.PostFetchJobShouldComplete();
 
             // Validate stale prefetch packs are cleaned up
@@ -307,7 +307,7 @@ public void PrefetchCleansUpOphanedLockFiles()
             // Re-mount so the post-fetch job runs
             this.Enlistment.MountScalar();
 
-            this.Enlistment.Prefetch("--commits");
+            this.Enlistment.Prefetch();
             this.PostFetchJobShouldComplete();
 
             this.fileSystem.FileExists(graphLockPath).ShouldBeFalse(nameof(graphLockPath));

Scalar.FunctionalTests/Tools/ScalarFunctionalTestEnlistment.cs

@@ -197,9 +197,9 @@ public bool TryMountScalar(out string output)
             return this.scalarProcess.TryMount(out output);
         }
 
-        public string Prefetch(string args, bool failOnError = true, string standardInput = null)
+        public string Prefetch(bool failOnError = true, string standardInput = null)
         {
-            return this.scalarProcess.Prefetch(args, failOnError, standardInput);
+            return this.scalarProcess.Prefetch(failOnError, standardInput);
         }
 
         public void Repair(bool confirm)

Scalar.FunctionalTests/Tools/ScalarProcess.cs

@@ -56,9 +56,9 @@ public bool TryMount(out string output)
             return this.IsEnlistmentMounted();
         }
 
-        public string Prefetch(string args, bool failOnError, string standardInput = null)
+        public string Prefetch(bool failOnError, string standardInput = null)
         {
-            return this.CallScalar("prefetch \"" + this.enlistmentRoot + "\" " + args, failOnError ? SuccessExitCode : DoNotCheckExitCode, standardInput: standardInput);
+            return this.CallScalar($"prefetch \"{this.enlistmentRoot}\"", failOnError ? SuccessExitCode : DoNotCheckExitCode, standardInput: standardInput);
         }
 
         public void Repair(bool confirm)

Scalar.UnitTests/Mock/Git/MockGitProcess.cs

@@ -78,7 +78,7 @@ protected override Result InvokeGitImpl(
             string command,
             string workingDirectory,
             string dotGitDirectory,
-            bool useReadObjectHook,
+            bool fetchMissingObjects,
             Action<StreamWriter> writeStdIn,
             Action<string> parseStdOutLine,
             int timeoutMs,

Scalar/CommandLine/CloneVerb.cs

@@ -264,7 +264,6 @@ private Result DoClone(string fullEnlistmentRootPathParameter, string normalized
                         this.enlistment,
                         verb =>
                         {
-                            verb.Commits = true;
                             verb.SkipVersionCheck = true;
                             verb.ResolvedCacheServer = this.cacheServer;
                             verb.ServerScalarConfig = this.serverScalarConfig;

Scalar/CommandLine/PrefetchVerb.cs

@@ -14,17 +14,6 @@ public class PrefetchVerb : ScalarVerb.ForExistingEnlistment
     {
         private const string PrefetchVerbName = "prefetch";
 
-        /// <remarks>
-        /// This does not change behavior, but we leave it here as it is called by a hook this way.
-        /// </remarks>
-        [Option(
-            'c',
-            "commits",
-            Required = false,
-            Default = true,
-            HelpText = "Fetch the latest set of commit and tree packs. This option cannot be used with any of the file- or folder-related options.")]
-        public bool Commits { get; set; }
-
         [Option(
             "verbose",
             Required = false,
@@ -64,7 +53,6 @@ protected override void Execute(ScalarEnlistment enlistment)
                 try
                 {
                     EventMetadata metadata = new EventMetadata();
-                    metadata.Add("Commits", this.Commits);
                     tracer.RelatedEvent(EventLevel.Informational, "PerformPrefetch", metadata);
 
                     GitObjectsHttpRequestor objectRequestor;