From 73aeb4d8f440b279bb7466d0c76044415786c86d Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Fri, 17 Jan 2020 09:54:11 -0500 Subject: [PATCH 1/4] FetchStep: use '--prune' and refs/scalar/hidden Signed-off-by: Derrick Stolee --- Scalar.Common/Git/GitProcess.cs | 11 ++++++--- Scalar.Common/Maintenance/FetchStep.cs | 8 ------- Scalar.Common/ScalarConstants.cs | 24 +++++++++++++++---- .../Tests/GitRepoPerFixture/RunVerbTests.cs | 21 ++++++++++------ 4 files changed, 42 insertions(+), 22 deletions(-) diff --git a/Scalar.Common/Git/GitProcess.cs b/Scalar.Common/Git/GitProcess.cs index 24fb3d0f12..42650a5666 100644 --- a/Scalar.Common/Git/GitProcess.cs +++ b/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() diff --git a/Scalar.Common/Maintenance/FetchStep.cs b/Scalar.Common/Maintenance/FetchStep.cs index 298e7d66d7..ba02d58275 100644 --- a/Scalar.Common/Maintenance/FetchStep.cs +++ b/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; diff --git a/Scalar.Common/ScalarConstants.cs b/Scalar.Common/ScalarConstants.cs index f1f6afde19..f933acd207 100644 --- a/Scalar.Common/ScalarConstants.cs +++ b/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}"; + } } } } diff --git a/Scalar.FunctionalTests/Tests/GitRepoPerFixture/RunVerbTests.cs b/Scalar.FunctionalTests/Tests/GitRepoPerFixture/RunVerbTests.cs index 265a70d8a8..ee166e8422 100644 --- a/Scalar.FunctionalTests/Tests/GitRepoPerFixture/RunVerbTests.cs +++ b/Scalar.FunctionalTests/Tests/GitRepoPerFixture/RunVerbTests.cs @@ -96,13 +96,15 @@ 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.Enlistment.RunVerb("fetch"); @@ -110,20 +112,25 @@ public void FetchStep() 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 = "2797fbb8358bb2e0c12d6f3b42a60b43f7655edf"; + 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() From 101e32a5fb43b83b0c2460733628edcc8587c34c Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Mon, 20 Jan 2020 09:27:15 -0500 Subject: [PATCH 2/4] RetryOnException: report when retries all fail Signed-off-by: Derrick Stolee --- .../FileSystemRunners/SystemIORunner.cs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/Scalar.FunctionalTests/FileSystemRunners/SystemIORunner.cs b/Scalar.FunctionalTests/FileSystemRunners/SystemIORunner.cs index 4e97c194d3..adaa6083ce 100644 --- a/Scalar.FunctionalTests/FileSystemRunners/SystemIORunner.cs +++ b/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(Action action) where ExceptionType : Exception From 0675415d8dea1f677a39870e09ba8713e0ec3f99 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Mon, 20 Jan 2020 09:29:56 -0500 Subject: [PATCH 3/4] Add CommitId to Settings.Properties --- Scalar.FunctionalTests/Settings.cs | 2 ++ Scalar.FunctionalTests/Tests/GitRepoPerFixture/RunVerbTests.cs | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/Scalar.FunctionalTests/Settings.cs b/Scalar.FunctionalTests/Settings.cs index 7c18dc5a19..d3e83a739f 100644 --- a/Scalar.FunctionalTests/Settings.cs +++ b/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)) { diff --git a/Scalar.FunctionalTests/Tests/GitRepoPerFixture/RunVerbTests.cs b/Scalar.FunctionalTests/Tests/GitRepoPerFixture/RunVerbTests.cs index ee166e8422..6a99c34026 100644 --- a/Scalar.FunctionalTests/Tests/GitRepoPerFixture/RunVerbTests.cs +++ b/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; @@ -117,7 +118,7 @@ public void FetchStep() 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 = "2797fbb8358bb2e0c12d6f3b42a60b43f7655edf"; + string sha1 = Settings.Default.CommitId; this.fileSystem.WriteAllText(refsHiddenOriginFake, sha1); this.Enlistment.RunVerb("fetch"); From 1aa390c8e2673c88a9eae8559503ccfb8a4de53e Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Mon, 20 Jan 2020 10:43:49 -0500 Subject: [PATCH 4/4] Attempt removing test flakiness Signed-off-by: Derrick Stolee --- .../Tests/GitRepoPerFixture/RunVerbTests.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Scalar.FunctionalTests/Tests/GitRepoPerFixture/RunVerbTests.cs b/Scalar.FunctionalTests/Tests/GitRepoPerFixture/RunVerbTests.cs index 6a99c34026..74ed4808c3 100644 --- a/Scalar.FunctionalTests/Tests/GitRepoPerFixture/RunVerbTests.cs +++ b/Scalar.FunctionalTests/Tests/GitRepoPerFixture/RunVerbTests.cs @@ -24,6 +24,12 @@ public RunVerbTests() this.fileSystem = new SystemIORunner(); } + [SetUp] + public void Setup() + { + this.Enlistment.Unregister(); + } + [TestCase] [Order(1)] public void CommitGraphStep() @@ -106,6 +112,7 @@ public void FetchStep() 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");