Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 4 commits into from
Jan 20, 2020
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
73aeb4d
FetchStep: use '--prune' and refs/scalar/hidden
derrickstolee Jan 17, 2020
101e32a
RetryOnException: report when retries all fail
derrickstolee Jan 20, 2020
0675415
Add CommitId to Settings.Properties
derrickstolee Jan 20, 2020
1aa390c
Attempt removing test flakiness
derrickstolee Jan 20, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions Scalar.Common/Git/GitProcess.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
derrickstolee marked this conversation as resolved.
Show resolved Hide resolved
}

public string[] GetRemotes()
Expand Down
8 changes: 0 additions & 8 deletions Scalar.Common/Maintenance/FetchStep.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
derrickstolee marked this conversation as resolved.
Show resolved Hide resolved
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;

Expand Down
24 changes: 20 additions & 4 deletions Scalar.Common/ScalarConstants.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}";
derrickstolee marked this conversation as resolved.
Show resolved Hide resolved

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}";
}
}
}
}
Expand Down
13 changes: 10 additions & 3 deletions Scalar.FunctionalTests/FileSystemRunners/SystemIORunner.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.Diagnostics;
using System.IO;
using System.Runtime.InteropServices;
using System.Text;
using System.Threading;

namespace Scalar.FunctionalTests.FileSystemRunners
Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions Scalar.FunctionalTests/Settings.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand All @@ -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))
{
Expand Down
29 changes: 22 additions & 7 deletions Scalar.FunctionalTests/Tests/GitRepoPerFixture/RunVerbTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -23,6 +24,12 @@ public RunVerbTests()
this.fileSystem = new SystemIORunner();
}

[SetUp]
public void Setup()
{
this.Enlistment.Unregister();
}

[TestCase]
[Order(1)]
public void CommitGraphStep()
Expand Down Expand Up @@ -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);
derrickstolee marked this conversation as resolved.
Show resolved Hide resolved
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()
Expand Down