Merge pull request #14 from AshleighAdams/fetch-depth-bugfix

GitRepoInspector: Deepen(): Fetch from the point of the deepest commit.

src/Verlite.Core/Command.cs

@@ -7,7 +7,44 @@
 namespace Verlite
 {
 	/// <summary>
-	/// The command class
+	/// An interface to run commands.
+	/// </summary>
+	public interface ICommandRunner
+	{
+		/// <summary>
+		/// Asynchronously execute a command.
+		/// </summary>
+		/// <param name="directory">The working directory in which to start the executable.</param>
+		/// <param name="command">The command to execute.</param>
+		/// <param name="args">Arguments to pass to the command.</param>
+		/// <param name="envVars">The enviornment variables to start the process with.</param>
+		/// <exception cref="CommandException">Thrown if the process returns a non-zero exit code.</exception>
+		/// <returns>A task that completes upon the process exiting, containing the standard out and error streams.</returns>
+		Task<(string stdout, string stderr)> Run(
+			string directory,
+			string command,
+			string[] args,
+			IDictionary<string, string>? envVars = null);
+	}
+
+	/// <summary>
+	/// Run commands using <see cref="Command.Run(string, string, string[], IDictionary{string, string}?)"/>
+	/// </summary>
+	public class SystemCommandRunner : ICommandRunner
+	{
+		/// <inheritdoc/>
+		public async Task<(string stdout, string stderr)> Run(
+			string directory,
+			string command,
+			string[] args,
+			IDictionary<string, string>? envVars = null)
+		{
+			return await Command.Run(directory, command, args, envVars);
+		}
+	}
+
+	/// <summary>
+	/// A class for executing commands.
 	/// </summary>
 	public static class Command
 	{

src/Verlite.Core/GitRepoInspector.cs

@@ -59,14 +59,20 @@ public sealed class GitRepoInspector : IRepoInspector
 		/// </summary>
 		/// <param name="path">The path of the Git repository.</param>
 		/// <param name="log">A logger for diagnostics.</param>
+		/// <param name="commandRunner">A command runner to use. Defaults to <see cref="SystemCommandRunner"/> if null is given.</param>
 		/// <exception cref="GitMissingOrNotGitRepoException">Thrown if the path is not a Git repository.</exception>
 		/// <returns>A task containing the Git repo inspector.</returns>
-		public static async Task<GitRepoInspector> FromPath(string path, ILogger? log = null)
+		public static async Task<GitRepoInspector> FromPath(string path, ILogger? log = null, ICommandRunner? commandRunner = null)
 		{
+			commandRunner ??= new SystemCommandRunner();
+
 			try
 			{
-				var (root, _) = await Command.Run(path, "git", new string[] { "rev-parse", "--show-toplevel" });
-				var ret = new GitRepoInspector(root, log);
+				var (root, _) = await commandRunner.Run(path, "git", new string[] { "rev-parse", "--show-toplevel" });
+				var ret = new GitRepoInspector(
+					root,
+					log,
+					commandRunner);
 				await ret.CacheParents();
 				return ret;
 			}
@@ -77,6 +83,7 @@ public static async Task<GitRepoInspector> FromPath(string path, ILogger? log =
 		}
 
 		private ILogger? Log { get; }
+		private ICommandRunner CommandRunner { get; }
 		/// <summary>
 		/// Can the Git repository be deepened to fetch commits not in the local repository.
 		/// </summary>
@@ -86,15 +93,16 @@ public static async Task<GitRepoInspector> FromPath(string path, ILogger? log =
 		/// </summary>
 		public string Root { get; }
 		private Dictionary<Commit, Commit> CachedParents { get; } = new();
-		private (int depth, bool shallow)? FetchDepth { get; set; }
+		private (int depth, bool shallow, Commit deepest)? FetchDepth { get; set; }
 
-		private GitRepoInspector(string root, ILogger? log)
+		private GitRepoInspector(string root, ILogger? log, ICommandRunner commandRunner)
 		{
 			Root = root;
 			Log = log;
+			CommandRunner = commandRunner;
 		}
 
-		private Task<(string stdout, string stderr)> Git(params string[] args) => Command.Run(Root, "git", args);
+		private Task<(string stdout, string stderr)> Git(params string[] args) => CommandRunner.Run(Root, "git", args);
 
 		/// <inheritdoc/>
 		public async Task<Commit?> GetHead()
@@ -140,14 +148,16 @@ private GitRepoInspector(string root, ILogger? log)
 			}
 		}
 
-		private async Task<(int depth, bool shallow)> MeasureDepth()
+		private async Task<(int depth, bool shallow, Commit deepestCommit)> MeasureDepth()
 		{
 			int depth = 0;
 			var current = await GetHead()
 				?? throw new InvalidOperationException("MeasureDepth(): Could not fetch head");
+			Commit deepest = current;
 
 			while (CachedParents.TryGetValue(current, out Commit parent))
 			{
+				deepest = current;
 				current = parent;
 				depth++;
 				Log?.Verbatim($"MeasureDepth(): Found parent {parent}, depth {depth}");
@@ -159,21 +169,23 @@ private GitRepoInspector(string root, ILogger? log)
 				if (commitObj is null)
 				{
 					Log?.Verbatim($"MeasureDepth() -> (depth: {depth}, shallow: true)");
-					return (depth, shallow: true);
+					return (depth, shallow: true, deepestCommit: deepest);
 				}
 
 				Commit? parent = ParseCommitObjectParent(commitObj);
 				if (parent is null)
 				{
 					Log?.Verbatim($"MeasureDepth() -> (depth: {depth}, shallow: false)");
-					return (depth, shallow: false);
+					return (depth, shallow: false, deepestCommit: current);
 				}
 
 				depth++;
+				deepest = current;
 				current = parent.Value;
 			}
 		}
 
+		private bool DeepenFromCommitSupported { get; set; } = true;
 		private async Task Deepen()
 		{
 			Debug.Assert(FetchDepth is null || FetchDepth.Value.shallow == true);
@@ -184,14 +196,28 @@ private async Task Deepen()
 
 			int wasDepth = FetchDepth.Value.depth;
 			int newDepth = Math.Max(32, FetchDepth.Value.depth * 2);
+			int deltaDepth = newDepth - wasDepth;
 
-			Log?.Normal($"Fetching depth {newDepth} (was {wasDepth})");
+			Log?.Normal($"Deepen(): Deepening to depth {newDepth} (+{deltaDepth} commits, was {wasDepth})");
 
 			try
 			{
-				_ = await Git("fetch", $"--depth={newDepth}");
+				if (DeepenFromCommitSupported)
+					_ = await Git("fetch", "origin", FetchDepth.Value.deepest.Id, $"--depth={deltaDepth}");
+				else
+					_ = await Git("fetch", $"--depth={newDepth}");
+
 				await CacheParents();
 			}
+			catch (CommandException ex)
+				when (
+					DeepenFromCommitSupported &&
+					ex.StandardError.Contains("error: Server does not allow request for unadvertised object"))
+			{
+				Log?.Normal($"Deepen(): From commit not supported, falling back to old method.");
+				DeepenFromCommitSupported = false;
+				await Deepen();
+			}
 			catch (CommandException ex)
 			{
 				throw new AutoDeepenException(ex);

src/Verlite.Core/PublicAPI.Shipped.txt

@@ -13,7 +13,7 @@ override Verlite.TaggedVersion.GetHashCode() -> int
 static Verlite.Command.Run(string! directory, string! command, string![]! args, System.Collections.Generic.IDictionary<string!, string!>? envVars = null) -> System.Threading.Tasks.Task<(string! stdout, string! stderr)>!
 static Verlite.Commit.operator !=(Verlite.Commit left, Verlite.Commit right) -> bool
 static Verlite.Commit.operator ==(Verlite.Commit left, Verlite.Commit right) -> bool
-static Verlite.GitRepoInspector.FromPath(string! path, Verlite.ILogger? log = null) -> System.Threading.Tasks.Task<Verlite.GitRepoInspector!>!
+static Verlite.GitRepoInspector.FromPath(string! path, Verlite.ILogger? log = null, Verlite.ICommandRunner? commandRunner = null) -> System.Threading.Tasks.Task<Verlite.GitRepoInspector!>!
 static Verlite.HeightCalculator.FromRepository(Verlite.IRepoInspector! repo, string! tagPrefix, bool queryRemoteTags, Verlite.ILogger? log = null) -> System.Threading.Tasks.Task<(int height, Verlite.TaggedVersion?)>!
 static Verlite.SemVer.ComparePrerelease(string! left, string! right) -> int
 static Verlite.SemVer.IsValidIdentifierCharacter(char input) -> bool
@@ -59,6 +59,8 @@ Verlite.GitRepoInspector.GetParent(Verlite.Commit commit) -> System.Threading.Ta
 Verlite.GitRepoInspector.GetTags(Verlite.QueryTarget queryTarget) -> System.Threading.Tasks.Task<Verlite.TagContainer!>!
 Verlite.GitRepoInspector.Root.get -> string!
 Verlite.HeightCalculator
+Verlite.ICommandRunner
+Verlite.ICommandRunner.Run(string! directory, string! command, string![]! args, System.Collections.Generic.IDictionary<string!, string!>? envVars = null) -> System.Threading.Tasks.Task<(string! stdout, string! stderr)>!
 Verlite.ILogger
 Verlite.ILogger.Normal(string! message) -> void
 Verlite.ILogger.Verbatim(string! message) -> void
@@ -94,6 +96,9 @@ Verlite.SemVer.Prerelease.get -> string?
 Verlite.SemVer.Prerelease.set -> void
 Verlite.SemVer.SemVer() -> void
 Verlite.SemVer.SemVer(int major, int minor, int patch, string? prerelease = null, string? buildMetadata = null) -> void
+Verlite.SystemCommandRunner
+Verlite.SystemCommandRunner.Run(string! directory, string! command, string![]! args, System.Collections.Generic.IDictionary<string!, string!>? envVars = null) -> System.Threading.Tasks.Task<(string! stdout, string! stderr)>!
+Verlite.SystemCommandRunner.SystemCommandRunner() -> void
 Verlite.Tag
 Verlite.Tag.Equals(Verlite.Tag other) -> bool
 Verlite.Tag.Name.get -> string!

tests/UnitTests/GitRepoInspectorTests.cs

@@ -53,9 +53,12 @@ public GitTestDirectory()
 			Directory.CreateDirectory(RootPath);
 		}
 
-		public Task<GitRepoInspector> MakeInspector()
+		public Task<GitRepoInspector> MakeInspector(ICommandRunner? commandRunner = null)
 		{
-			return GitRepoInspector.FromPath(RootPath);
+			return GitRepoInspector.FromPath(
+				path: RootPath,
+				log: null,
+				commandRunner);
 		}
 
 		public void Dispose()
@@ -391,5 +394,30 @@ public async Task FetchingTagInDeepCloneDoesNotMakeShallow()
 			var deeperParent = await repo.GetParent(deeperTag.PointsTo);
 			deeperParent.Should().Be(new Commit("b2000fc1f1d2e5f816cfa51a4ad8764048f22f0a"));
 		}
+
+		[Fact]
+		public async Task ShallowGitFetchFromCommitCanFallBack()
+		{
+			await TestRepo.Git("init");
+			await TestRepo.Git("commit", "--allow-empty", "-m", "first");
+			await TestRepo.Git("tag", "tag-one");
+			await TestRepo.Git("commit", "--allow-empty", "-m", "second");
+			await TestRepo.Git("tag", "tag-two");
+			await TestRepo.Git("commit", "--allow-empty", "-m", "third");
+
+			using var clone = new GitTestDirectory();
+			await clone.Git("clone", TestRepo.RootUri, ".", "--branch", "master", "--depth", "1");
+
+			var repo = await clone.MakeInspector(
+				new MockCommandRunnerWithOldRemoteGitVersion(
+					new SystemCommandRunner()));
+
+			repo.CanDeepen = true;
+			var head = await repo.GetHead();
+			var parent = await repo.GetParent(head.Value);
+			var parentsParent = await repo.GetParent(parent.Value);
+
+			parentsParent.Should().Be(new Commit("b2000fc1f1d2e5f816cfa51a4ad8764048f22f0a"));
+		}
 	}
 }

tests/UnitTests/Mocks/MockCommandRunnerWithOldRemoteGitVersion.cs

@@ -0,0 +1,33 @@
+using System.Collections.Generic;
+using System.Linq;
+using System.Threading.Tasks;
+
+using Verlite;
+
+namespace UnitTests
+{
+	public sealed class MockCommandRunnerWithOldRemoteGitVersion : ICommandRunner
+	{
+		private ICommandRunner BaseRunner { get; }
+		public MockCommandRunnerWithOldRemoteGitVersion(ICommandRunner baseRunner)
+		{
+			BaseRunner = baseRunner;
+		}
+
+		Task<(string stdout, string stderr)> ICommandRunner.Run(
+			string directory,
+			string command,
+			string[] args,
+			IDictionary<string, string>? envVars)
+		{
+			string? firstArg = args.Length > 0 ? args[0] : null;
+
+			return (command, firstArg, args) switch
+			{
+				("git", "fetch", _) when args.Contains("origin") =>
+					throw new CommandException(128, "", "error: Server does not allow request for unadvertised object a1b2c3"),
+				_ => BaseRunner.Run(directory, command, args, envVars),
+			};
+		}
+	}
+}