From e129c7c34ac35ae735f3cf5773468aea9513e68f Mon Sep 17 00:00:00 2001 From: Tom Glastonbury <1101693+tg73@users.noreply.github.com> Date: Fri, 24 Sep 2021 11:04:53 +0100 Subject: [PATCH 1/3] Make it easier to debug GitObjectId instances. --- src/NerdBank.GitVersioning/ManagedGit/GitObjectId.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/NerdBank.GitVersioning/ManagedGit/GitObjectId.cs b/src/NerdBank.GitVersioning/ManagedGit/GitObjectId.cs index 3e62f158..436ca439 100644 --- a/src/NerdBank.GitVersioning/ManagedGit/GitObjectId.cs +++ b/src/NerdBank.GitVersioning/ManagedGit/GitObjectId.cs @@ -116,6 +116,9 @@ public static GitObjectId ParseHex(ReadOnlySpan value) bytes[i >> 1] = (byte)(c1 + c2); } + + // Clear any cached sha. This can happen when debugging, and is very confusing. + objectId.sha = null; return objectId; } From 7a99e3c06e3530df0b0e8d41a2d0fff6567501e6 Mon Sep 17 00:00:00 2001 From: Tom Glastonbury <1101693+tg73@users.noreply.github.com> Date: Fri, 24 Sep 2021 11:10:06 +0100 Subject: [PATCH 2/3] Work in progress: - Height calculations now keep track of nearest relevant commit (managed only for now). - VersionOracle uses the nearest relevant commit. --- src/NerdBank.GitVersioning/GitContext.cs | 2 +- .../LibGit2/LibGit2Context.cs | 6 ++- .../Managed/ManagedGitContext.cs | 7 ++-- .../Managed/ManagedGitExtensions.cs | 37 +++++++++++++------ .../NoGit/NoGitContext.cs | 2 +- src/NerdBank.GitVersioning/VersionOracle.cs | 14 ++++++- 6 files changed, 49 insertions(+), 19 deletions(-) diff --git a/src/NerdBank.GitVersioning/GitContext.cs b/src/NerdBank.GitVersioning/GitContext.cs index e9d9114b..c2c92c8d 100644 --- a/src/NerdBank.GitVersioning/GitContext.cs +++ b/src/NerdBank.GitVersioning/GitContext.cs @@ -176,7 +176,7 @@ public static GitContext Create(string path, string? committish = null, bool wri /// A string that is at least in length but may be more as required to uniquely identify the git object identified by . public abstract string GetShortUniqueCommitId(int minLength); - internal abstract int CalculateVersionHeight(VersionOptions? committedVersion, VersionOptions? workingVersion); + internal abstract (int height, string? nearestRelevantCommit) CalculateVersionHeightAndNearestRelevantCommit(VersionOptions? committedVersion, VersionOptions? workingVersion); internal abstract Version GetIdAsVersion(VersionOptions? committedVersion, VersionOptions? workingVersion, int versionHeight); diff --git a/src/NerdBank.GitVersioning/LibGit2/LibGit2Context.cs b/src/NerdBank.GitVersioning/LibGit2/LibGit2Context.cs index 65cd8bba..599323d3 100644 --- a/src/NerdBank.GitVersioning/LibGit2/LibGit2Context.cs +++ b/src/NerdBank.GitVersioning/LibGit2/LibGit2Context.cs @@ -82,8 +82,11 @@ public override bool TrySelectCommit(string committish) /// public override string GetShortUniqueCommitId(int minLength) => this.Repository.ObjectDatabase.ShortenObjectId(this.Commit, minLength); - internal override int CalculateVersionHeight(VersionOptions? committedVersion, VersionOptions? workingVersion) + internal override (int height, string? nearestRelevantCommit) CalculateVersionHeightAndNearestRelevantCommit(VersionOptions? committedVersion, VersionOptions? workingVersion) { + throw new NotImplementedException("nearestRelevantCommit for libgit2"); + +#if false var headCommitVersion = committedVersion?.Version ?? SemVer0; if (IsVersionFileChangedInWorkingTree(committedVersion, workingVersion)) @@ -99,6 +102,7 @@ internal override int CalculateVersionHeight(VersionOptions? committedVersion, V } return LibGit2GitExtensions.GetVersionHeight(this); +#endif } internal override System.Version GetIdAsVersion(VersionOptions? committedVersion, VersionOptions? workingVersion, int versionHeight) diff --git a/src/NerdBank.GitVersioning/Managed/ManagedGitContext.cs b/src/NerdBank.GitVersioning/Managed/ManagedGitContext.cs index a6d4fab5..36fdce5f 100644 --- a/src/NerdBank.GitVersioning/Managed/ManagedGitContext.cs +++ b/src/NerdBank.GitVersioning/Managed/ManagedGitContext.cs @@ -86,7 +86,7 @@ public static ManagedGitContext Create(string path, string? committish = null) }; } - internal override int CalculateVersionHeight(VersionOptions? committedVersion, VersionOptions? workingVersion) + internal override (int height, string? nearestRelevantCommit) CalculateVersionHeightAndNearestRelevantCommit(VersionOptions? committedVersion, VersionOptions? workingVersion) { var headCommitVersion = committedVersion?.Version ?? SemVer0; @@ -98,11 +98,12 @@ internal override int CalculateVersionHeight(VersionOptions? committedVersion, V { // 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 (0, null); } } - return GitExtensions.GetVersionHeight(this); + var (height, nearestRelevantCommit) = GitExtensions.GetVersionHeight(this); + return (height, nearestRelevantCommit?.Sha.ToString()); } internal override Version GetIdAsVersion(VersionOptions? committedVersion, VersionOptions? workingVersion, int versionHeight) diff --git a/src/NerdBank.GitVersioning/Managed/ManagedGitExtensions.cs b/src/NerdBank.GitVersioning/Managed/ManagedGitExtensions.cs index e51bf4ea..305462d5 100644 --- a/src/NerdBank.GitVersioning/Managed/ManagedGitExtensions.cs +++ b/src/NerdBank.GitVersioning/Managed/ManagedGitExtensions.cs @@ -20,16 +20,18 @@ internal static class GitExtensions /// /// Gets the number of commits in the longest single path between /// the specified commit and the most distant ancestor (inclusive) - /// that set the version to the value at . + /// that set the version to the value at , + /// and the nearest commit that is part of that path, taking into + /// account path filters. /// /// The git context for which to calculate the height. /// Optional base version to calculate the height. If not specified, the base version will be calculated by scanning the repository. - /// The height of the commit. Always a positive integer. - internal static int GetVersionHeight(ManagedGitContext context, Version? baseVersion = null) + /// The height of the commit (always a positive integer), and a representing the nearest relevant commit, or if there is no relevant commit. + internal static (int height, GitCommit? nearestRelevantCommit) GetVersionHeight(ManagedGitContext context, Version? baseVersion = null) { if (context.Commit is null) { - return 0; + return (0, null); } var tracker = new GitWalkTracker(context); @@ -37,21 +39,21 @@ internal static int GetVersionHeight(ManagedGitContext context, Version? baseVer var versionOptions = tracker.GetVersion(context.Commit.Value); if (versionOptions is null) { - return 0; + return (0, null); } var baseSemVer = baseVersion is not null ? SemanticVersion.Parse(baseVersion.ToString()) : versionOptions.Version ?? SemVer0; + // TODO: What if versionHeightPosition is not set, but we still want to know the nearest relevant commit id? var versionHeightPosition = versionOptions.VersionHeightPosition; if (versionHeightPosition.HasValue) { - int height = GetHeight(context, c => CommitMatchesVersion(c, baseSemVer, versionHeightPosition.Value, tracker)); - return height; + return GetHeight(context, c => CommitMatchesVersion(c, baseSemVer, versionHeightPosition.Value, tracker)); } - return 0; + return (0, null); } /// @@ -94,11 +96,11 @@ private static bool CommitMatchesVersion(GitCommit commit, SemanticVersion expec /// May be null to count the height to the original commit. /// /// The height of the commit. Always a positive integer. - public static int GetHeight(ManagedGitContext context, Func? continueStepping = null) + public static (int height, GitCommit? nearestRelevantCommit) GetHeight(ManagedGitContext context, Func? continueStepping = null) { Verify.Operation(context.Commit.HasValue, "No commit is selected."); var tracker = new GitWalkTracker(context); - return GetCommitHeight(context.Repository, context.Commit.Value, tracker, continueStepping); + return (GetCommitHeight(context.Repository, context.Commit.Value, tracker, continueStepping), tracker.NearestCommit); } /// @@ -316,6 +318,10 @@ private class GitWalkTracker private readonly Dictionary heights = new Dictionary(); private readonly ManagedGitContext context; + private int nearestCommitHeight = -1; + + internal GitCommit? NearestCommit { get; private set; } + internal GitWalkTracker(ManagedGitContext context) { this.context = context; @@ -323,7 +329,16 @@ internal GitWalkTracker(ManagedGitContext context) internal bool TryGetVersionHeight(GitCommit commit, out int height) => this.heights.TryGetValue(commit.Sha, out height); - internal void RecordHeight(GitCommit commit, int height) => this.heights.Add(commit.Sha, height); + internal void RecordHeight(GitCommit commit, int height) + { + if ( height > this.nearestCommitHeight) + { + this.NearestCommit = commit; + this.nearestCommitHeight = height; + } + + this.heights.Add(commit.Sha, height); + } internal VersionOptions? GetVersion(GitCommit commit) { diff --git a/src/NerdBank.GitVersioning/NoGit/NoGitContext.cs b/src/NerdBank.GitVersioning/NoGit/NoGitContext.cs index 4d8324db..11298465 100644 --- a/src/NerdBank.GitVersioning/NoGit/NoGitContext.cs +++ b/src/NerdBank.GitVersioning/NoGit/NoGitContext.cs @@ -32,7 +32,7 @@ public NoGitContext(string workingTreePath) public override void Stage(string path) => throw new InvalidOperationException(NotAGitRepoMessage); public override string GetShortUniqueCommitId(int minLength) => throw new InvalidOperationException(NotAGitRepoMessage); public override bool TrySelectCommit(string committish) => throw new InvalidOperationException(NotAGitRepoMessage); - internal override int CalculateVersionHeight(VersionOptions? committedVersion, VersionOptions? workingVersion) => 0; + internal override (int height, string? nearestRelevantCommit) CalculateVersionHeightAndNearestRelevantCommit(VersionOptions? committedVersion, VersionOptions? workingVersion) => (0, null); internal override Version GetIdAsVersion(VersionOptions? committedVersion, VersionOptions? workingVersion, int versionHeight) => throw new NotImplementedException(); } } diff --git a/src/NerdBank.GitVersioning/VersionOracle.cs b/src/NerdBank.GitVersioning/VersionOracle.cs index 143fe4eb..054d392a 100644 --- a/src/NerdBank.GitVersioning/VersionOracle.cs +++ b/src/NerdBank.GitVersioning/VersionOracle.cs @@ -61,9 +61,10 @@ public VersionOracle(GitContext context, ICloudBuild? cloudBuild = null, int? ov } this.BuildingRef = cloudBuild?.BuildingTag ?? cloudBuild?.BuildingBranch ?? context.HeadCanonicalName; + string? nearestRelevantCommit = null; try { - this.VersionHeight = context.CalculateVersionHeight(this.CommittedVersion, this.WorkingVersion); + (this.VersionHeight, nearestRelevantCommit) = context.CalculateVersionHeightAndNearestRelevantCommit(this.CommittedVersion, this.WorkingVersion); } catch (GitException ex) when (context.IsShallow && ex.ErrorCode == GitException.ErrorCodes.ObjectNotFound) { @@ -78,6 +79,7 @@ public VersionOracle(GitContext context, ICloudBuild? cloudBuild = null, int? ov static Exception ThrowShallowClone(Exception inner) => throw new GitException("Shallow clone lacks the objects required to calculate version height. Use full clones or clones with a history at least as deep as the last version height resetting change.", inner) { iSShallowClone = true, ErrorCode = GitException.ErrorCodes.ObjectNotFound }; + this.VersionOptions = this.CommittedVersion ?? this.WorkingVersion; this.Version = this.VersionOptions?.Version?.Version ?? Version0; this.assemblyInformationalVersionComponentCount = this.VersionOptions?.VersionHeightPosition == SemanticVersion.Position.Revision ? 4 : 3; @@ -90,13 +92,21 @@ public VersionOracle(GitContext context, ICloudBuild? cloudBuild = null, int? ov this.CloudBuildNumberOptions = this.VersionOptions?.CloudBuild?.BuildNumberOrDefault ?? VersionOptions.CloudBuildNumberOptions.DefaultInstance; + if (!string.IsNullOrEmpty(nearestRelevantCommit) && context.GitCommitId != nearestRelevantCommit) + { + if (!context.TrySelectCommit(nearestRelevantCommit!)) + { + // This would be very unexpected, given that the context itself provided the commit id we're selecting. + throw new GitException("Failed to select the nearest relevant commit."); + } + } + // get the commit id abbreviation only if the commit id is set if (!string.IsNullOrEmpty(this.GitCommitId)) { var gitCommitIdShortFixedLength = this.VersionOptions?.GitCommitIdShortFixedLength ?? VersionOptions.DefaultGitCommitIdShortFixedLength; var gitCommitIdShortAutoMinimum = this.VersionOptions?.GitCommitIdShortAutoMinimum ?? 0; - // Get it from the git repository if there is a repository present and it is enabled. this.GitCommitIdShort = this.GitCommitId is object && gitCommitIdShortAutoMinimum > 0 ? this.context.GetShortUniqueCommitId(gitCommitIdShortAutoMinimum) : this.GitCommitId!.Substring(0, gitCommitIdShortFixedLength); From 3d0d77b3a45eb57e9e2251a666ce84f08eb731d9 Mon Sep 17 00:00:00 2001 From: Tom Glastonbury <1101693+tg73@users.noreply.github.com> Date: Fri, 24 Sep 2021 11:34:58 +0100 Subject: [PATCH 3/3] Replace comment removed by mistake. --- src/NerdBank.GitVersioning/VersionOracle.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/NerdBank.GitVersioning/VersionOracle.cs b/src/NerdBank.GitVersioning/VersionOracle.cs index 054d392a..26562f2f 100644 --- a/src/NerdBank.GitVersioning/VersionOracle.cs +++ b/src/NerdBank.GitVersioning/VersionOracle.cs @@ -107,6 +107,7 @@ public VersionOracle(GitContext context, ICloudBuild? cloudBuild = null, int? ov var gitCommitIdShortFixedLength = this.VersionOptions?.GitCommitIdShortFixedLength ?? VersionOptions.DefaultGitCommitIdShortFixedLength; var gitCommitIdShortAutoMinimum = this.VersionOptions?.GitCommitIdShortAutoMinimum ?? 0; + // Get it from the git repository if there is a repository present and it is enabled. this.GitCommitIdShort = this.GitCommitId is object && gitCommitIdShortAutoMinimum > 0 ? this.context.GetShortUniqueCommitId(gitCommitIdShortAutoMinimum) : this.GitCommitId!.Substring(0, gitCommitIdShortFixedLength);