Merge pull request #134 from robmen/114-partialperf

Partial performance improvement for #114

Author: AArnott

src/NerdBank.GitVersioning.Tests/VersionOracleTests.cs

@@ -1,4 +1,5 @@
-using System.IO;
+using System;
+using System.IO;
 using System.Linq;
 using LibGit2Sharp;
 using Nerdbank.GitVersioning;
@@ -15,6 +16,14 @@ public VersionOracleTests(ITestOutputHelper logger)
 
     private string CommitIdShort => this.Repo.Head.Commits.First().Id.Sha.Substring(0, 10);
 
+    [Fact]
+    public void NotRepo()
+    {
+        // Seems safe to assume the system directory is not a repository.
+        var oracle = VersionOracle.Create(Environment.SystemDirectory);
+        Assert.Equal(0, oracle.VersionHeight);
+    }
+
     [Fact(Skip = "Unstable test. See issue #125")]
     public void Submodule_RecognizedWithCorrectVersion()
     {

src/NerdBank.GitVersioning/GitExtensions.cs

@@ -33,13 +33,18 @@ public static class GitExtensions
         /// </summary>
         /// <param name="commit">The commit to measure the height of.</param>
         /// <param name="repoRelativeProjectDirectory">The repo-relative project directory for which to calculate the version.</param>
+        /// <param name="baseVersion">Optional base version to calculate the height. If not specified, the base version will be calculated by scanning the repository.</param>
         /// <returns>The height of the commit. Always a positive integer.</returns>
-        public static int GetVersionHeight(this Commit commit, string repoRelativeProjectDirectory = null)
+        public static int GetVersionHeight(this Commit commit, string repoRelativeProjectDirectory = null, Version baseVersion = null)
         {
             Requires.NotNull(commit, nameof(commit));
             Requires.Argument(repoRelativeProjectDirectory == null || !Path.IsPathRooted(repoRelativeProjectDirectory), nameof(repoRelativeProjectDirectory), "Path should be relative to repo root.");
 
-            var baseVersion = VersionFile.GetVersion(commit, repoRelativeProjectDirectory)?.Version?.Version ?? Version0;
+            if (baseVersion == null)
+            {
+                baseVersion = VersionFile.GetVersion(commit, repoRelativeProjectDirectory)?.Version?.Version ?? Version0;
+            }
+
             int height = commit.GetHeight(c => CommitMatchesMajorMinorVersion(c, baseVersion, repoRelativeProjectDirectory));
             return height;
         }
@@ -170,7 +175,7 @@ public static Commit GetCommitFromTruncatedIdInteger(this Repository repo, int t
         /// <param name="commit">The commit whose ID and position in history is to be encoded.</param>
         /// <param name="repoRelativeProjectDirectory">The repo-relative project directory for which to calculate the version.</param>
         /// <param name="versionHeight">
-        /// The version height, previously calculated by a call to <see cref="GetVersionHeight(Commit, string)"/>
+        /// The version height, previously calculated by a call to <see cref="GetVersionHeight(Commit, string, Version)"/>
         /// with the same value for <paramref name="repoRelativeProjectDirectory"/>.
         /// </param>
         /// <returns>
@@ -188,7 +193,13 @@ public static Version GetIdAsVersion(this Commit commit, string repoRelativeProj
             Requires.Argument(repoRelativeProjectDirectory == null || !Path.IsPathRooted(repoRelativeProjectDirectory), nameof(repoRelativeProjectDirectory), "Path should be relative to repo root.");
 
             var versionOptions = VersionFile.GetVersion(commit, repoRelativeProjectDirectory);
-            return GetIdAsVersionHelper(commit, versionOptions, repoRelativeProjectDirectory, versionHeight);
+
+            if (!versionHeight.HasValue)
+            {
+                versionHeight = GetVersionHeight(commit, repoRelativeProjectDirectory);
+            }
+
+            return GetIdAsVersionHelper(commit, versionOptions, versionHeight.Value);
         }
 
         /// <summary>
@@ -198,7 +209,7 @@ public static Version GetIdAsVersion(this Commit commit, string repoRelativeProj
         /// <param name="repo">The repo whose ID and position in history is to be encoded.</param>
         /// <param name="repoRelativeProjectDirectory">The repo-relative project directory for which to calculate the version.</param>
         /// <param name="versionHeight">
-        /// The version height, previously calculated by a call to <see cref="GetVersionHeight(Commit, string)"/>
+        /// The version height, previously calculated by a call to <see cref="GetVersionHeight(Commit, string, Version)"/>
         /// with the same value for <paramref name="repoRelativeProjectDirectory"/>.
         /// </param>
         /// <returns>
@@ -219,7 +230,13 @@ public static Version GetIdAsVersion(this Repository repo, string repoRelativePr
             if (IsVersionFileChangedInWorkingCopy(repo, repoRelativeProjectDirectory, out committedVersionOptions, out workingCopyVersionOptions))
             {
                 // Apply ordinary logic, but to the working copy version info.
-                Version result = GetIdAsVersionHelper(headCommit, workingCopyVersionOptions, repoRelativeProjectDirectory, versionHeight);
+                if (!versionHeight.HasValue)
+                {
+                    var baseVersion = workingCopyVersionOptions?.Version?.Version;
+                    versionHeight = GetVersionHeight(headCommit, repoRelativeProjectDirectory, baseVersion);
+                }
+
+                Version result = GetIdAsVersionHelper(headCommit, workingCopyVersionOptions, versionHeight.Value);
                 return result;
             }
 
@@ -367,7 +384,7 @@ public static string FindLibGit2NativeBinaries(string basePath)
         /// <param name="expectedVersion">The version to test for in the commit</param>
         /// <param name="repoRelativeProjectDirectory">The repo-relative directory from which <paramref name="expectedVersion"/> was originally calculated.</param>
         /// <returns><c>true</c> if the <paramref name="commit"/> matches the major and minor components of <paramref name="expectedVersion"/>.</returns>
-        private static bool CommitMatchesMajorMinorVersion(Commit commit, Version expectedVersion, string repoRelativeProjectDirectory)
+        internal static bool CommitMatchesMajorMinorVersion(this Commit commit, Version expectedVersion, string repoRelativeProjectDirectory)
         {
             Requires.NotNull(commit, nameof(commit));
             Requires.NotNull(expectedVersion, nameof(expectedVersion));
@@ -494,11 +511,7 @@ private static void AddReachableCommitsFrom(Commit startingCommit, HashSet<Commi
         /// </summary>
         /// <param name="commit">The commit whose ID and position in history is to be encoded.</param>
         /// <param name="versionOptions">The version options applicable at this point (either from commit or working copy).</param>
-        /// <param name="repoRelativeProjectDirectory">The repo-relative project directory for which to calculate the version.</param>
-        /// <param name="versionHeight">
-        /// The version height, previously calculated by a call to <see cref="GetVersionHeight(Commit, string)"/>
-        /// with the same value for <paramref name="repoRelativeProjectDirectory"/>.
-        /// </param>
+        /// <param name="versionHeight">The version height, previously calculated by a call to <see cref="GetVersionHeight(Commit, string, Version)"/>.</param>
         /// <returns>
         /// A version whose <see cref="Version.Build"/> and
         /// <see cref="Version.Revision"/> components are calculated based on the commit.
@@ -509,7 +522,7 @@ private static void AddReachableCommitsFrom(Commit startingCommit, HashSet<Commi
         /// component is the first four bytes of the git commit id (forced to be a positive integer).
         /// </remarks>
         /// <returns></returns>
-        private static Version GetIdAsVersionHelper(Commit commit, VersionOptions versionOptions, string repoRelativeProjectDirectory, int? versionHeight)
+        internal static Version GetIdAsVersionHelper(this Commit commit, VersionOptions versionOptions, int versionHeight)
         {
             var baseVersion = versionOptions?.Version?.Version ?? Version0;
             int buildNumber = baseVersion.Build;
@@ -522,14 +535,8 @@ private static Version GetIdAsVersionHelper(Commit commit, VersionOptions versio
                 // The build number is set to the git height. This helps ensure that
                 // within a major.minor release, each patch has an incrementing integer.
                 // The revision is set to the first two bytes of the git commit ID.
-                if (!versionHeight.HasValue)
-                {
-                    versionHeight = commit != null
-                        ? commit.GetHeight(c => CommitMatchesMajorMinorVersion(c, baseVersion, repoRelativeProjectDirectory))
-                        : 0;
-                }
 
-                int adjustedVersionHeight = versionHeight.Value == 0 ? 0 : versionHeight.Value + (versionOptions?.BuildNumberOffset ?? 0);
+                int adjustedVersionHeight = versionHeight == 0 ? 0 : versionHeight + (versionOptions?.BuildNumberOffset ?? 0);
                 Verify.Operation(adjustedVersionHeight <= MaximumBuildNumberOrRevisionComponent, "Git height is {0}, which is greater than the maximum allowed {0}.", adjustedVersionHeight, MaximumBuildNumberOrRevisionComponent);
 
                 if (buildNumber < 0)

src/NerdBank.GitVersioning/VersionOracle.cs

@@ -5,9 +5,7 @@
     using System.Globalization;
     using System.IO;
     using System.Linq;
-    using System.Text;
     using System.Text.RegularExpressions;
-    using System.Threading.Tasks;
     using Validation;
 
     /// <summary>
@@ -21,9 +19,9 @@ public class VersionOracle
         private static readonly Regex NumericIdentifierRegex = new Regex(@"(?<![\w-])(\d+)(?![\w-])");
 
         /// <summary>
-        /// The cloud build suppport, if any.
+        /// The 0.0 version.
         /// </summary>
-        private readonly ICloudBuild cloudBuild;
+        private static readonly Version Version0 = new Version(0, 0);
 
         /// <summary>
         /// Initializes a new instance of the <see cref="VersionOracle"/> class.
@@ -51,24 +49,33 @@ public static VersionOracle Create(string projectDirectory, string gitRepoDirect
         /// </summary>
         public VersionOracle(string projectDirectory, LibGit2Sharp.Repository repo, ICloudBuild cloudBuild)
         {
-            this.cloudBuild = cloudBuild;
-            this.VersionOptions =
-                VersionFile.GetVersion(repo, projectDirectory) ??
-                VersionFile.GetVersion(projectDirectory);
-
             var repoRoot = repo?.Info?.WorkingDirectory?.TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar);
             var relativeRepoProjectDirectory = !string.IsNullOrWhiteSpace(repoRoot)
                 ? projectDirectory.Substring(repoRoot.Length).TrimStart(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar)
                 : null;
 
             var commit = repo?.Head.Commits.FirstOrDefault();
+
+            var committedVersion = VersionFile.GetVersion(commit, relativeRepoProjectDirectory);
+
+            var workingVersion = VersionFile.GetVersion(projectDirectory);
+
+            this.VersionOptions = committedVersion ?? workingVersion;
+
             this.GitCommitId = commit?.Id.Sha ?? cloudBuild?.GitCommitId ?? null;
-            this.VersionHeight = repo?.GetVersionHeight(relativeRepoProjectDirectory) ?? 0;
+            this.VersionHeight = CalculateVersionHeight(relativeRepoProjectDirectory, commit, committedVersion, workingVersion);
             this.BuildingRef = cloudBuild?.BuildingTag ?? cloudBuild?.BuildingBranch ?? repo?.Head.CanonicalName;
 
             // Override the typedVersion with the special build number and revision components, when available.
-            this.Version = repo?.GetIdAsVersion(relativeRepoProjectDirectory, this.VersionHeight) ?? this.VersionOptions?.Version.Version;
-            this.Version = this.Version ?? new Version(0, 0);
+            if (repo != null)
+            {
+                this.Version = GetIdAsVersion(commit, committedVersion, workingVersion, this.VersionHeight);
+            }
+            else
+            {
+                this.Version = this.VersionOptions?.Version.Version ?? Version0;
+            }
+
             this.VersionHeightOffset = this.VersionOptions?.BuildNumberOffset ?? 0;
 
             this.PrereleaseVersion = ReplaceMacros(this.VersionOptions?.Version.Prerelease ?? string.Empty);
@@ -391,5 +398,42 @@ private static string MakePrereleaseSemVer1Compliant(string prerelease, int padd
 
             return semver1;
         }
+
+        private static int CalculateVersionHeight(string relativeRepoProjectDirectory, LibGit2Sharp.Commit headCommit, VersionOptions committedVersion, VersionOptions workingVersion)
+        {
+            var headCommitVersion = committedVersion?.Version?.Version ?? Version0;
+
+            if (IsVersionFileChangedInWorkingTree(committedVersion, workingVersion))
+            {
+                var workingCopyVersion = workingVersion?.Version?.Version;
+
+                if (workingCopyVersion == null || !workingCopyVersion.Equals(headCommitVersion))
+                {
+                    // The working copy has changed the major.minor version.
+                    // So by definition the version height is 0, since no commit represents it yet.
+                    return 0;
+                }
+            }
+
+            return headCommit?.GetHeight(c => c.CommitMatchesMajorMinorVersion(headCommitVersion, relativeRepoProjectDirectory)) ?? 0;
+        }
+
+        private static Version GetIdAsVersion(LibGit2Sharp.Commit headCommit, VersionOptions committedVersion, VersionOptions workingVersion, int versionHeight)
+        {
+            var version = IsVersionFileChangedInWorkingTree(committedVersion, workingVersion) ? workingVersion : committedVersion;
+
+            return headCommit.GetIdAsVersionHelper(version, versionHeight);
+        }
+
+        private static bool IsVersionFileChangedInWorkingTree(VersionOptions committedVersion, VersionOptions workingVersion)
+        {
+            if (workingVersion != null)
+            {
+                return !EqualityComparer<VersionOptions>.Default.Equals(workingVersion, committedVersion);
+            }
+
+            // A missing working version is a change only if it was previously commited.
+            return committedVersion != null;
+        }
     }
 }