diff --git a/GVFS/GVFS.Build/GVFS.props b/GVFS/GVFS.Build/GVFS.props index 06459df094..14173fa84b 100644 --- a/GVFS/GVFS.Build/GVFS.props +++ b/GVFS/GVFS.Build/GVFS.props @@ -3,7 +3,7 @@ 0.2.173.2 - 2.20190805.1 + 2.20190806.1 diff --git a/GVFS/GVFS.Common/Maintenance/PackfileMaintenanceStep.cs b/GVFS/GVFS.Common/Maintenance/PackfileMaintenanceStep.cs index 9ea9e68c6b..998b82d6b2 100644 --- a/GVFS/GVFS.Common/Maintenance/PackfileMaintenanceStep.cs +++ b/GVFS/GVFS.Common/Maintenance/PackfileMaintenanceStep.cs @@ -27,7 +27,8 @@ namespace GVFS.Common.Maintenance public class PackfileMaintenanceStep : GitMaintenanceStep { public const string PackfileLastRunFileName = "pack-maintenance.time"; - public const string DefaultBatchSize = "2g"; + public const string DefaultBatchSize = "2g"; + private const string MultiPackIndexLock = "multi-pack-index.lock"; private readonly bool forceRun; private readonly string batchSize; @@ -105,15 +106,20 @@ protected override void PerformMaintenance() activity.RelatedWarning($"Skipping {nameof(PackfileMaintenanceStep)} due to git pids {string.Join(",", processIds)}", Keywords.Telemetry); return; } - } - - this.GetPackFilesInfo(out int beforeCount, out long beforeSize, out bool hasKeep); + } + + this.GetPackFilesInfo(out int beforeCount, out long beforeSize, out bool hasKeep); if (!hasKeep) { activity.RelatedWarning(this.CreateEventMetadata(), "Skipping pack maintenance due to no .keep file."); return; - } + } + + string multiPackIndexLockPath = Path.Combine(this.Context.Enlistment.GitPackRoot, MultiPackIndexLock); + this.Context.FileSystem.TryDeleteFile(multiPackIndexLockPath); + + this.RunGitCommand((process) => process.WriteMultiPackIndex(this.Context.Enlistment.GitObjectsRoot), nameof(GitProcess.WriteMultiPackIndex)); // If a LibGit2Repo is active, then it may hold handles to the .idx and .pack files we want // to delete during the 'git multi-pack-index expire' step. If one starts during the step, diff --git a/GVFS/GVFS.Common/Maintenance/PostFetchStep.cs b/GVFS/GVFS.Common/Maintenance/PostFetchStep.cs index acb2cbea36..f279ad47d2 100644 --- a/GVFS/GVFS.Common/Maintenance/PostFetchStep.cs +++ b/GVFS/GVFS.Common/Maintenance/PostFetchStep.cs @@ -10,7 +10,6 @@ namespace GVFS.Common.Maintenance public class PostFetchStep : GitMaintenanceStep { private const string CommitGraphChainLock = "commit-graph-chain.lock"; - private const string MultiPackIndexLock = "multi-pack-index.lock"; private List packIndexes; public PostFetchStep(GVFSContext context, List packIndexes, bool requireObjectCacheLock = true) @@ -23,20 +22,6 @@ public PostFetchStep(GVFSContext context, List packIndexes, bool require protected override void PerformMaintenance() { - using (ITracer activity = this.Context.Tracer.StartActivity("TryWriteMultiPackIndex", EventLevel.Informational)) - { - string multiPackIndexLockPath = Path.Combine(this.Context.Enlistment.GitPackRoot, MultiPackIndexLock); - this.Context.FileSystem.TryDeleteFile(multiPackIndexLockPath); - - this.RunGitCommand((process) => process.WriteMultiPackIndex(this.Context.Enlistment.GitObjectsRoot), nameof(GitProcess.WriteMultiPackIndex)); - - GitProcess.Result verifyResult = this.RunGitCommand((process) => process.VerifyMultiPackIndex(this.Context.Enlistment.GitObjectsRoot), nameof(GitProcess.VerifyMultiPackIndex)); - if (!this.Stopping && verifyResult.ExitCodeIsFailure) - { - this.LogErrorAndRewriteMultiPackIndex(activity); - } - } - if (this.packIndexes == null || this.packIndexes.Count == 0) { this.Context.Tracer.RelatedInfo(this.Area + ": Skipping commit-graph write due to no new packfiles"); diff --git a/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/PrefetchVerbTests.cs b/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/PrefetchVerbTests.cs index d259df44b6..396d2b8f7e 100644 --- a/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/PrefetchVerbTests.cs +++ b/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/PrefetchVerbTests.cs @@ -15,7 +15,6 @@ namespace GVFS.FunctionalTests.Tests.EnlistmentPerFixture public class PrefetchVerbTests : TestsWithEnlistmentPerFixture { private const string PrefetchCommitsAndTreesLock = "prefetch-commits-trees.lock"; - private const string MultiPackIndexLock = "multi-pack-index.lock"; private const string LsTreeTypeInPathBranchName = "FunctionalTests/20181105_LsTreeTypeInPath"; private FileSystemRunner fileSystem; @@ -137,27 +136,6 @@ public void PrefetchCleansUpStalePrefetchLock() prefetchCommitsLockFile.ShouldNotExistOnDisk(this.fileSystem); } - [TestCase, Order(11)] - [Category(Categories.MacTODO.TestNeedsToLockFile)] // PostFetchStepShouldComplete waits for a lock file - public void PrefetchCleansUpPackDir() - { - string multiPackIndexLockFile = Path.Combine(this.Enlistment.GetPackRoot(this.fileSystem), MultiPackIndexLock); - string oldGitTempFile = Path.Combine(this.Enlistment.GetPackRoot(this.fileSystem), "tmp_midx_XXXX"); - string oldKeepFile = Path.Combine(this.Enlistment.GetPackRoot(this.fileSystem), "prefetch-00000000-HASH.keep"); - - this.fileSystem.WriteAllText(multiPackIndexLockFile, this.Enlistment.EnlistmentRoot); - this.fileSystem.WriteAllText(oldGitTempFile, this.Enlistment.EnlistmentRoot); - this.fileSystem.WriteAllText(oldKeepFile, this.Enlistment.EnlistmentRoot); - - this.Enlistment.Prefetch("--commits"); - this.Enlistment.PostFetchStep(); - oldGitTempFile.ShouldNotExistOnDisk(this.fileSystem); - oldKeepFile.ShouldNotExistOnDisk(this.fileSystem); - - this.PostFetchStepShouldComplete(); - multiPackIndexLockFile.ShouldNotExistOnDisk(this.fileSystem); - } - [TestCase, Order(12)] public void PrefetchFilesFromFileListFile() { @@ -236,9 +214,6 @@ private void PostFetchStepShouldComplete() } while (this.fileSystem.FileExists(objectCacheLock)); - ProcessResult midxResult = GitProcess.InvokeProcess(this.Enlistment.RepoRoot, "multi-pack-index verify --object-dir=\"" + objectDir + "\""); - midxResult.ExitCode.ShouldEqual(0); - // A commit graph is not always generated, but if it is, then we want to ensure it is in a good state if (this.fileSystem.FileExists(Path.Combine(objectDir, "info", "commit-graphs", "commit-graph-chain"))) { diff --git a/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/PrefetchVerbWithoutSharedCacheTests.cs b/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/PrefetchVerbWithoutSharedCacheTests.cs index 0cb15b8257..d94fe70376 100644 --- a/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/PrefetchVerbWithoutSharedCacheTests.cs +++ b/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/PrefetchVerbWithoutSharedCacheTests.cs @@ -71,12 +71,6 @@ public void PrefetchBuildsIdxWhenMissingFromPrefetchPack() this.fileSystem.DeleteFile(idxPath); idxPath.ShouldNotExistOnDisk(this.fileSystem); - // Remove midx that contains references to the pack - string midxPath = Path.Combine(this.Enlistment.GetObjectRoot(this.fileSystem), "pack", "multi-pack-index"); - File.SetAttributes(midxPath, FileAttributes.Normal); - this.fileSystem.DeleteFile(midxPath); - midxPath.ShouldNotExistOnDisk(this.fileSystem); - // Prefetch should rebuild the missing idx this.Enlistment.Prefetch("--commits"); this.PostFetchJobShouldComplete(); @@ -296,13 +290,10 @@ public void PrefetchCleansUpStaleTempPrefetchPacks() [TestCase, Order(9)] public void PrefetchCleansUpOphanedLockFiles() { - // Multi-pack-index write happens even if the prefetch downloads nothing, while // the commit-graph write happens only when the prefetch downloads at least one pack string graphPath = Path.Combine(this.Enlistment.GetObjectRoot(this.fileSystem), "info", "commit-graphs", "commit-graph-chain"); string graphLockPath = graphPath + ".lock"; - string midxPath = Path.Combine(this.PackRoot, "multi-pack-index"); - string midxLockPath = midxPath + ".lock"; this.fileSystem.CreateEmptyFile(graphLockPath); @@ -312,7 +303,6 @@ public void PrefetchCleansUpOphanedLockFiles() // Force deleting the prefetch packs to make the prefetch non-trivial. this.fileSystem.DeleteDirectory(this.PackRoot); this.fileSystem.CreateDirectory(this.PackRoot); - this.fileSystem.CreateEmptyFile(midxLockPath); // Re-mount so the post-fetch job runs this.Enlistment.MountGVFS(); @@ -321,9 +311,7 @@ public void PrefetchCleansUpOphanedLockFiles() this.PostFetchJobShouldComplete(); this.fileSystem.FileExists(graphLockPath).ShouldBeFalse(nameof(graphLockPath)); - this.fileSystem.FileExists(midxLockPath).ShouldBeFalse(nameof(midxLockPath)); this.fileSystem.FileExists(graphPath).ShouldBeTrue(nameof(graphPath)); - this.fileSystem.FileExists(midxPath).ShouldBeTrue(nameof(midxPath)); } private void PackShouldHaveIdxFile(string pathPath) @@ -406,9 +394,6 @@ private void PostFetchJobShouldComplete() Thread.Sleep(500); } - ProcessResult midxResult = GitProcess.InvokeProcess(this.Enlistment.RepoRoot, "multi-pack-index verify --object-dir=\"" + objectDir + "\""); - midxResult.ExitCode.ShouldEqual(0); - ProcessResult graphResult = GitProcess.InvokeProcess(this.Enlistment.RepoRoot, "commit-graph verify --shallow --object-dir=\"" + objectDir + "\""); graphResult.ExitCode.ShouldEqual(0); } diff --git a/GVFS/GVFS.UnitTests/Maintenance/PackfileMaintenanceStepTests.cs b/GVFS/GVFS.UnitTests/Maintenance/PackfileMaintenanceStepTests.cs index 9cd7461e0c..10e0e23b47 100644 --- a/GVFS/GVFS.UnitTests/Maintenance/PackfileMaintenanceStepTests.cs +++ b/GVFS/GVFS.UnitTests/Maintenance/PackfileMaintenanceStepTests.cs @@ -21,8 +21,8 @@ public class PackfileMaintenanceStepTests private const string KeepName = "pack-3.keep"; private MockTracer tracer; private MockGitProcess gitProcess; - private GVFSContext context; - + private GVFSContext context; + private string ExpireCommand => $"multi-pack-index expire --object-dir=\"{this.context.Enlistment.GitObjectsRoot}\""; private string VerifyCommand => $"-c core.multiPackIndex=true multi-pack-index verify --object-dir=\"{this.context.Enlistment.GitObjectsRoot}\""; private string WriteCommand => $"-c core.multiPackIndex=true multi-pack-index write --object-dir=\"{this.context.Enlistment.GitObjectsRoot}\""; @@ -39,11 +39,12 @@ public void PackfileMaintenanceIgnoreTimeRestriction() this.tracer.StartActivityTracer.RelatedErrorEvents.Count.ShouldEqual(0); this.tracer.StartActivityTracer.RelatedWarningEvents.Count.ShouldEqual(0); List commands = this.gitProcess.CommandsRun; - commands.Count.ShouldEqual(4); - commands[0].ShouldEqual(this.ExpireCommand); - commands[1].ShouldEqual(this.VerifyCommand); - commands[2].ShouldEqual(this.RepackCommand); - commands[3].ShouldEqual(this.VerifyCommand); + commands.Count.ShouldEqual(5); + commands[0].ShouldEqual(this.WriteCommand); + commands[1].ShouldEqual(this.ExpireCommand); + commands[2].ShouldEqual(this.VerifyCommand); + commands[3].ShouldEqual(this.RepackCommand); + commands[4].ShouldEqual(this.VerifyCommand); } [TestCase] @@ -82,11 +83,12 @@ public void PackfileMaintenancePassTimeRestriction() this.tracer.StartActivityTracer.RelatedErrorEvents.Count.ShouldEqual(0); this.tracer.StartActivityTracer.RelatedWarningEvents.Count.ShouldEqual(0); List commands = this.gitProcess.CommandsRun; - commands.Count.ShouldEqual(4); - commands[0].ShouldEqual(this.ExpireCommand); - commands[1].ShouldEqual(this.VerifyCommand); - commands[2].ShouldEqual(this.RepackCommand); - commands[3].ShouldEqual(this.VerifyCommand); + commands.Count.ShouldEqual(5); + commands[0].ShouldEqual(this.WriteCommand); + commands[1].ShouldEqual(this.ExpireCommand); + commands[2].ShouldEqual(this.VerifyCommand); + commands[3].ShouldEqual(this.RepackCommand); + commands[4].ShouldEqual(this.VerifyCommand); } [TestCase] @@ -130,13 +132,14 @@ public void PackfileMaintenanceRewriteOnBadVerify() this.tracer.StartActivityTracer.RelatedWarningEvents.Count.ShouldEqual(2); List commands = this.gitProcess.CommandsRun; - commands.Count.ShouldEqual(6); - commands[0].ShouldEqual(this.ExpireCommand); - commands[1].ShouldEqual(this.VerifyCommand); - commands[2].ShouldEqual(this.WriteCommand); - commands[3].ShouldEqual(this.RepackCommand); - commands[4].ShouldEqual(this.VerifyCommand); - commands[5].ShouldEqual(this.WriteCommand); + commands.Count.ShouldEqual(7); + commands[0].ShouldEqual(this.WriteCommand); + commands[1].ShouldEqual(this.ExpireCommand); + commands[2].ShouldEqual(this.VerifyCommand); + commands[3].ShouldEqual(this.WriteCommand); + commands[4].ShouldEqual(this.RepackCommand); + commands[5].ShouldEqual(this.VerifyCommand); + commands[6].ShouldEqual(this.WriteCommand); } [TestCase] @@ -222,8 +225,11 @@ private void TestSetup(DateTime lastRun, bool failOnVerify = false) // Create and return Context this.tracer = new MockTracer(); - this.context = new GVFSContext(this.tracer, fileSystem, repository, enlistment); - + this.context = new GVFSContext(this.tracer, fileSystem, repository, enlistment); + + this.gitProcess.SetExpectedCommandResult( + this.WriteCommand, + () => new GitProcess.Result(string.Empty, string.Empty, GitProcess.Result.SuccessCode)); this.gitProcess.SetExpectedCommandResult( this.ExpireCommand, () => new GitProcess.Result(string.Empty, string.Empty, GitProcess.Result.SuccessCode)); diff --git a/GVFS/GVFS.UnitTests/Maintenance/PostFetchStepTests.cs b/GVFS/GVFS.UnitTests/Maintenance/PostFetchStepTests.cs index ae983fd833..4e527e6058 100644 --- a/GVFS/GVFS.UnitTests/Maintenance/PostFetchStepTests.cs +++ b/GVFS/GVFS.UnitTests/Maintenance/PostFetchStepTests.cs @@ -18,47 +18,28 @@ public class PostFetchStepTests private MockGitProcess gitProcess; private GVFSContext context; - private string MultiPackIndexWriteCommand => $"-c core.multiPackIndex=true multi-pack-index write --object-dir=\"{this.context.Enlistment.GitObjectsRoot}\""; - private string MultiPackIndexVerifyCommand => $"-c core.multiPackIndex=true multi-pack-index verify --object-dir=\"{this.context.Enlistment.GitObjectsRoot}\""; private string CommitGraphWriteCommand => $"commit-graph write --stdin-packs --split --size-multiple=4 --object-dir \"{this.context.Enlistment.GitObjectsRoot}\""; private string CommitGraphVerifyCommand => $"commit-graph verify --shallow --object-dir \"{this.context.Enlistment.GitObjectsRoot}\""; [TestCase] - public void WriteMultiPackIndexNoGraphOnEmptyPacks() + public void DontWriteGraphOnEmptyPacks() { this.TestSetup(); - this.gitProcess.SetExpectedCommandResult( - this.MultiPackIndexWriteCommand, - () => new GitProcess.Result(string.Empty, string.Empty, GitProcess.Result.SuccessCode)); - this.gitProcess.SetExpectedCommandResult( - this.MultiPackIndexVerifyCommand, - () => new GitProcess.Result(string.Empty, string.Empty, GitProcess.Result.SuccessCode)); - PostFetchStep step = new PostFetchStep(this.context, new List()); step.Execute(); - this.tracer.StartActivityTracer.RelatedErrorEvents.Count.ShouldEqual(0); - this.tracer.StartActivityTracer.RelatedWarningEvents.Count.ShouldEqual(0); this.tracer.RelatedInfoEvents.Count.ShouldEqual(1); List commands = this.gitProcess.CommandsRun; - commands.Count.ShouldEqual(2); - commands[0].ShouldEqual(this.MultiPackIndexWriteCommand); - commands[1].ShouldEqual(this.MultiPackIndexVerifyCommand); + commands.Count.ShouldEqual(0); } [TestCase] - public void WriteMultiPackIndexAndGraphWithPacks() + public void WriteGraphWithPacks() { this.TestSetup(); - this.gitProcess.SetExpectedCommandResult( - this.MultiPackIndexWriteCommand, - () => new GitProcess.Result(string.Empty, string.Empty, GitProcess.Result.SuccessCode)); - this.gitProcess.SetExpectedCommandResult( - this.MultiPackIndexVerifyCommand, - () => new GitProcess.Result(string.Empty, string.Empty, GitProcess.Result.SuccessCode)); this.gitProcess.SetExpectedCommandResult( this.CommitGraphWriteCommand, () => new GitProcess.Result(string.Empty, string.Empty, GitProcess.Result.SuccessCode)); @@ -69,51 +50,13 @@ public void WriteMultiPackIndexAndGraphWithPacks() PostFetchStep step = new PostFetchStep(this.context, new List() { "pack" }, requireObjectCacheLock: false); step.Execute(); - this.tracer.StartActivityTracer.RelatedErrorEvents.Count.ShouldEqual(0); - this.tracer.StartActivityTracer.RelatedWarningEvents.Count.ShouldEqual(0); this.tracer.RelatedInfoEvents.Count.ShouldEqual(0); List commands = this.gitProcess.CommandsRun; - commands.Count.ShouldEqual(4); - commands[0].ShouldEqual(this.MultiPackIndexWriteCommand); - commands[1].ShouldEqual(this.MultiPackIndexVerifyCommand); - commands[2].ShouldEqual(this.CommitGraphWriteCommand); - commands[3].ShouldEqual(this.CommitGraphVerifyCommand); - } - - [TestCase] - public void RewriteMultiPackIndexOnBadVerify() - { - this.TestSetup(); - - this.gitProcess.SetExpectedCommandResult( - this.MultiPackIndexWriteCommand, - () => new GitProcess.Result(string.Empty, string.Empty, GitProcess.Result.SuccessCode)); - this.gitProcess.SetExpectedCommandResult( - this.MultiPackIndexVerifyCommand, - () => new GitProcess.Result(string.Empty, string.Empty, GitProcess.Result.GenericFailureCode)); - this.gitProcess.SetExpectedCommandResult( - this.CommitGraphWriteCommand, - () => new GitProcess.Result(string.Empty, string.Empty, GitProcess.Result.SuccessCode)); - this.gitProcess.SetExpectedCommandResult( - this.CommitGraphVerifyCommand, - () => new GitProcess.Result(string.Empty, string.Empty, GitProcess.Result.SuccessCode)); - - PostFetchStep step = new PostFetchStep(this.context, new List() { "pack" }, requireObjectCacheLock: false); - step.Execute(); - - this.tracer.StartActivityTracer.RelatedErrorEvents.Count.ShouldEqual(0); - this.tracer.StartActivityTracer.RelatedWarningEvents.Count.ShouldEqual(1); - - List commands = this.gitProcess.CommandsRun; - - commands.Count.ShouldEqual(5); - commands[0].ShouldEqual(this.MultiPackIndexWriteCommand); - commands[1].ShouldEqual(this.MultiPackIndexVerifyCommand); - commands[2].ShouldEqual(this.MultiPackIndexWriteCommand); - commands[3].ShouldEqual(this.CommitGraphWriteCommand); - commands[4].ShouldEqual(this.CommitGraphVerifyCommand); + commands.Count.ShouldEqual(2); + commands[0].ShouldEqual(this.CommitGraphWriteCommand); + commands[1].ShouldEqual(this.CommitGraphVerifyCommand); } [TestCase] @@ -121,12 +64,6 @@ public void RewriteCommitGraphOnBadVerify() { this.TestSetup(); - this.gitProcess.SetExpectedCommandResult( - this.MultiPackIndexWriteCommand, - () => new GitProcess.Result(string.Empty, string.Empty, GitProcess.Result.SuccessCode)); - this.gitProcess.SetExpectedCommandResult( - this.MultiPackIndexVerifyCommand, - () => new GitProcess.Result(string.Empty, string.Empty, GitProcess.Result.SuccessCode)); this.gitProcess.SetExpectedCommandResult( this.CommitGraphWriteCommand, () => new GitProcess.Result(string.Empty, string.Empty, GitProcess.Result.SuccessCode)); @@ -141,12 +78,10 @@ public void RewriteCommitGraphOnBadVerify() this.tracer.StartActivityTracer.RelatedWarningEvents.Count.ShouldEqual(1); List commands = this.gitProcess.CommandsRun; - commands.Count.ShouldEqual(5); - commands[0].ShouldEqual(this.MultiPackIndexWriteCommand); - commands[1].ShouldEqual(this.MultiPackIndexVerifyCommand); + commands.Count.ShouldEqual(3); + commands[0].ShouldEqual(this.CommitGraphWriteCommand); + commands[1].ShouldEqual(this.CommitGraphVerifyCommand); commands[2].ShouldEqual(this.CommitGraphWriteCommand); - commands[3].ShouldEqual(this.CommitGraphVerifyCommand); - commands[4].ShouldEqual(this.CommitGraphWriteCommand); } private void TestSetup()