From cb5eca3132bdd649858d468fe3a869954a4ea53e Mon Sep 17 00:00:00 2001 From: Hardy Hobeck Date: Mon, 26 Feb 2024 23:30:36 +0100 Subject: [PATCH 1/3] Fix GitVersion calculates the wrong version after main is merged back to develop. --- .../IntegrationTests/OtherScenarios.cs | 83 +++++++++++++++++++ .../IncrementStrategyFinder.cs | 66 ++++++++++----- 2 files changed, 129 insertions(+), 20 deletions(-) diff --git a/src/GitVersion.Core.Tests/IntegrationTests/OtherScenarios.cs b/src/GitVersion.Core.Tests/IntegrationTests/OtherScenarios.cs index 306ae3def6..6ccb0c86b2 100644 --- a/src/GitVersion.Core.Tests/IntegrationTests/OtherScenarios.cs +++ b/src/GitVersion.Core.Tests/IntegrationTests/OtherScenarios.cs @@ -1218,4 +1218,87 @@ public void EnsurePreventIncrementWhenCurrentCommitTaggedOnReleaseBranchAndIncre fixture.AssertFullSemver(semVersion, configuration); } + + [Test] + public void EnsureVersionAfterMainIsMergedBackToDevelopIsCorrect() + { + using EmptyRepositoryFixture fixture = new("main"); + + fixture.MakeATaggedCommit("1.0.0"); + fixture.BranchTo("develop"); + fixture.MakeACommit("A"); + + // ✅ succeeds as expected + fixture.AssertFullSemver("1.1.0-alpha.1"); + + fixture.Checkout("main"); + fixture.MakeACommit("B"); + fixture.BranchTo("hotfix/just-a-hotfix"); + fixture.MakeACommit("C +semver: major"); + fixture.MergeTo("main", removeBranchAfterMerging: true); + fixture.Checkout("develop"); + fixture.MakeACommit("D"); + fixture.Checkout("main"); + fixture.MakeACommit("E"); + fixture.ApplyTag("1.0.1"); + fixture.Checkout("develop"); + fixture.MergeNoFF("main"); + + // ✅ succeeds as expected + fixture.AssertFullSemver("1.1.0-alpha.7"); + } + + [TestCase(false, "2.0.0-alpha.3")] + [TestCase(true, "2.0.0-alpha.2")] + public void EnsureVersionAfterMainIsMergedBackToDevelopIsCorrectForTrunkBased(bool applyTag, string semanticVersion) + { + var configuration = GitFlowConfigurationBuilder.New + .WithVersionStrategy(VersionStrategies.TrunkBased) + .Build(); + + using EmptyRepositoryFixture fixture = new("main"); + + fixture.MakeACommit("A"); + fixture.ApplyTag("1.0.0"); + fixture.BranchTo("develop"); + fixture.MakeACommit("B +semver: major"); + + // ✅ succeeds as expected + fixture.AssertFullSemver("2.0.0-alpha.1", configuration); + + fixture.Checkout("main"); + fixture.MakeACommit("C"); + if (applyTag) fixture.ApplyTag("1.0.1"); + fixture.Checkout("develop"); + fixture.MergeNoFF("main"); + + // ✅ succeeds as expected + fixture.AssertFullSemver(semanticVersion, configuration); + } + + [TestCase(false, "2.0.0-alpha.3")] + [TestCase(true, "2.0.0-alpha.3")] + public void EnsureVersionAfterMainIsMergedBackToDevelopIsCorrectForGitFlow(bool applyTag, string semanticVersion) + { + var configuration = GitFlowConfigurationBuilder.New.Build(); + + using EmptyRepositoryFixture fixture = new("main"); + + fixture.MakeACommit("A"); + fixture.ApplyTag("1.0.0"); + fixture.BranchTo("develop"); + fixture.MakeACommit("B +semver: major"); + + // ✅ succeeds as expected + fixture.AssertFullSemver("2.0.0-alpha.1", configuration); + + fixture.Checkout("main"); + fixture.MakeACommit("C"); + if (applyTag) fixture.ApplyTag("1.0.1"); + fixture.Checkout("develop"); + fixture.MergeNoFF("main"); + + // ✅ succeeds as expected + fixture.AssertFullSemver(semanticVersion, configuration); + } } diff --git a/src/GitVersion.Core/VersionCalculation/IncrementStrategyFinder.cs b/src/GitVersion.Core/VersionCalculation/IncrementStrategyFinder.cs index 14ae152aa8..7804acc09c 100644 --- a/src/GitVersion.Core/VersionCalculation/IncrementStrategyFinder.cs +++ b/src/GitVersion.Core/VersionCalculation/IncrementStrategyFinder.cs @@ -75,26 +75,21 @@ public VersionField DetermineIncrementedField( } private VersionField? FindCommitMessageIncrement( - EffectiveConfiguration configuration, ICommit? baseCommit, ICommit? currentCommit, string? label) + EffectiveConfiguration configuration, ICommit? baseVersionSource, ICommit currentCommit, string? label) { if (configuration.CommitMessageIncrementing == CommitMessageIncrementMode.Disabled) { return null; } - //get tags with valid version - depends on configuration (see #3757) - var targetShas = new Lazy>(() => - this.taggedSemanticVersionRepository.GetTaggedSemanticVersions(configuration.TagPrefix, configuration.SemanticVersionFormat) - .SelectMany(_ => _).Where(_ => _.Value.IsMatchForBranchSpecificLabel(label)).Select(_ => _.Tag.TargetSha).ToHashSet() + IEnumerable commits = GetCommitHistory( + tagPrefix: configuration.TagPrefix, + semanticVersionFormat: configuration.SemanticVersionFormat, + baseVersionSource: baseVersionSource, + currentCommit: currentCommit, + label: label ); - var commits = GetIntermediateCommits(baseCommit, currentCommit); - // consider commit messages since latest tag only (see #3071) - commits = commits - .Reverse() - .TakeWhile(x => !targetShas.Value.Contains(x.Sha)) - .Reverse(); - if (configuration.CommitMessageIncrementing == CommitMessageIncrementMode.MergeMessageOnly) { commits = commits.Where(c => c.Parents.Count() > 1); @@ -114,6 +109,41 @@ private static Regex TryGetRegexOrDefault(string? messageRegex, Regex defaultReg ? defaultRegex : CompiledRegexCache.GetOrAdd(messageRegex, pattern => new(pattern, RegexOptions.Compiled | RegexOptions.IgnoreCase)); + public IReadOnlyCollection GetCommitHistory( + string? tagPrefix, SemanticVersionFormat semanticVersionFormat, ICommit? baseVersionSource, ICommit currentCommit, string? label) + { + var targetShas = new Lazy>(() => + this.taggedSemanticVersionRepository.GetTaggedSemanticVersions(tagPrefix, semanticVersionFormat) + .SelectMany(_ => _).Where(_ => _.Value.IsMatchForBranchSpecificLabel(label)).Select(_ => _.Tag.TargetSha).ToHashSet() + ); + + var intermediateCommits = GetIntermediateCommits(baseVersionSource, currentCommit).ToArray(); + + var commitLog = intermediateCommits.ToDictionary(element => element.Id.Sha); + + foreach (var item in intermediateCommits.Reverse()) + { + if (!commitLog.ContainsKey(item.Sha)) continue; + + if (targetShas.Value.Contains(item.Sha)) + { + void RemoveCommitFromHistory(ICommit commit) + { + if (!commitLog.ContainsKey(commit.Sha)) return; + + commitLog.Remove(commit.Sha); + foreach (var item in commit.Parents) + { + RemoveCommitFromHistory(item); + } + } + RemoveCommitFromHistory(item); + } + } + + return commitLog.Values; + } + /// /// Get the sequence of commits in a repository between a (exclusive) /// and a particular (inclusive) @@ -149,7 +179,7 @@ private Dictionary GetHeadCommitsMap(ICommit? headCommit) => /// private ICommit[] GetHeadCommits(ICommit? headCommit) => this.headCommitsCache.GetOrAdd(headCommit?.Sha ?? "NULL", () => - GetCommitsReacheableFromHead(repository, headCommit).ToArray()); + GetCommitsReacheableFromHead(headCommit).ToArray()); private VersionField? GetIncrementFromCommit(ICommit commit, Regex majorRegex, Regex minorRegex, Regex patchRegex, Regex none) => this.commitIncrementCache.GetOrAdd(commit.Sha, () => @@ -164,11 +194,7 @@ private ICommit[] GetHeadCommits(ICommit? headCommit) => return null; } - /// - /// Query a for the sequence of commits from the beginning to a particular - /// (inclusive) - /// - private static IEnumerable GetCommitsReacheableFromHead(IGitRepository repo, ICommit? headCommit) + private IEnumerable GetCommitsReacheableFromHead(ICommit? headCommit) { var filter = new CommitFilter { @@ -176,7 +202,7 @@ private static IEnumerable GetCommitsReacheableFromHead(IGitRepository SortBy = CommitSortStrategies.Topological | CommitSortStrategies.Reverse }; - return repo.Commits.QueryBy(filter); + return repository.Commits.QueryBy(filter); } public IEnumerable GetMergedCommits(ICommit mergeCommit, int index) @@ -202,7 +228,7 @@ private static ICommit GetMergedHead(ICommit mergeCommit) { var parents = mergeCommit.Parents.Skip(1).ToList(); if (parents.Count > 1) - throw new NotSupportedException("Mainline development does not support more than one merge source in a single commit yet"); + throw new NotSupportedException("GitVersion does not support more than one merge source in a single commit yet"); return parents.Single(); } From 85701ff9e60b259fee49d20b14a0831239fadf4d Mon Sep 17 00:00:00 2001 From: Hardy Hobeck Date: Wed, 28 Feb 2024 14:00:20 +0100 Subject: [PATCH 2/3] Integrate code review remarks --- .../IncrementStrategyFinder.cs | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/GitVersion.Core/VersionCalculation/IncrementStrategyFinder.cs b/src/GitVersion.Core/VersionCalculation/IncrementStrategyFinder.cs index 7804acc09c..f4fe8265d4 100644 --- a/src/GitVersion.Core/VersionCalculation/IncrementStrategyFinder.cs +++ b/src/GitVersion.Core/VersionCalculation/IncrementStrategyFinder.cs @@ -109,7 +109,7 @@ private static Regex TryGetRegexOrDefault(string? messageRegex, Regex defaultReg ? defaultRegex : CompiledRegexCache.GetOrAdd(messageRegex, pattern => new(pattern, RegexOptions.Compiled | RegexOptions.IgnoreCase)); - public IReadOnlyCollection GetCommitHistory( + private IReadOnlyCollection GetCommitHistory( string? tagPrefix, SemanticVersionFormat semanticVersionFormat, ICommit? baseVersionSource, ICommit currentCommit, string? label) { var targetShas = new Lazy>(() => @@ -121,23 +121,22 @@ public IReadOnlyCollection GetCommitHistory( var commitLog = intermediateCommits.ToDictionary(element => element.Id.Sha); - foreach (var item in intermediateCommits.Reverse()) + foreach (var intermediateCommit in intermediateCommits.Reverse()) { - if (!commitLog.ContainsKey(item.Sha)) continue; + if (!commitLog.ContainsKey(intermediateCommit.Sha)) continue; - if (targetShas.Value.Contains(item.Sha)) + if (targetShas.Value.Contains(intermediateCommit.Sha)) { - void RemoveCommitFromHistory(ICommit commit) + void RemoveCommitFromHistory(ICommit commit, HashSet traversedCommits) { - if (!commitLog.ContainsKey(commit.Sha)) return; + if (!traversedCommits.Add(commit) || !commitLog.Remove(commit.Sha)) return; - commitLog.Remove(commit.Sha); - foreach (var item in commit.Parents) + foreach (var parentCommit in commit.Parents) { - RemoveCommitFromHistory(item); + RemoveCommitFromHistory(parentCommit, traversedCommits); } } - RemoveCommitFromHistory(item); + RemoveCommitFromHistory(intermediateCommit, new HashSet()); } } From 475f65c5c7020ce314d70af5cbebd5a73e746ed9 Mon Sep 17 00:00:00 2001 From: Hardy Hobeck Date: Wed, 28 Feb 2024 18:11:45 +0100 Subject: [PATCH 3/3] Change recursive call in incrementStrategyFinder --- .../IncrementStrategyFinder.cs | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/GitVersion.Core/VersionCalculation/IncrementStrategyFinder.cs b/src/GitVersion.Core/VersionCalculation/IncrementStrategyFinder.cs index f4fe8265d4..93c04fa163 100644 --- a/src/GitVersion.Core/VersionCalculation/IncrementStrategyFinder.cs +++ b/src/GitVersion.Core/VersionCalculation/IncrementStrategyFinder.cs @@ -112,7 +112,7 @@ private static Regex TryGetRegexOrDefault(string? messageRegex, Regex defaultReg private IReadOnlyCollection GetCommitHistory( string? tagPrefix, SemanticVersionFormat semanticVersionFormat, ICommit? baseVersionSource, ICommit currentCommit, string? label) { - var targetShas = new Lazy>(() => + var targetShas = new Lazy>(() => this.taggedSemanticVersionRepository.GetTaggedSemanticVersions(tagPrefix, semanticVersionFormat) .SelectMany(_ => _).Where(_ => _.Value.IsMatchForBranchSpecificLabel(label)).Select(_ => _.Tag.TargetSha).ToHashSet() ); @@ -123,20 +123,21 @@ private IReadOnlyCollection GetCommitHistory( foreach (var intermediateCommit in intermediateCommits.Reverse()) { - if (!commitLog.ContainsKey(intermediateCommit.Sha)) continue; - - if (targetShas.Value.Contains(intermediateCommit.Sha)) + if (targetShas.Value.Contains(intermediateCommit.Sha) && commitLog.Remove(intermediateCommit.Sha)) { - void RemoveCommitFromHistory(ICommit commit, HashSet traversedCommits) + var parentCommits = intermediateCommit.Parents.ToList(); + while (parentCommits.Count != 0) { - if (!traversedCommits.Add(commit) || !commitLog.Remove(commit.Sha)) return; - - foreach (var parentCommit in commit.Parents) + List temporaryList = new(); + foreach (var parentCommit in parentCommits) { - RemoveCommitFromHistory(parentCommit, traversedCommits); + if (commitLog.Remove(parentCommit.Sha)) + { + temporaryList.AddRange(parentCommit.Parents); + } } + parentCommits = temporaryList; } - RemoveCommitFromHistory(intermediateCommit, new HashSet()); } }