[Bug] GitVersionTask: VersionSource is set to wrong tag

[fdiewald]

Describe the bug
Using the default configuration of GitVersionTask v5.3.7 the VersionSource ends up being not the latest tagged commit on master but rather the one before that.

Image of the test repository branch history: (blue: develop, red: master, green: release branches)

Output of gitversion /diag:

INFO [08/26/20 16:21:25:50] Dumping commit graph: 
INFO [08/26/20 16:21:25:62] *   4c6828d 2 hours ago  (HEAD -> develop, origin/develop)
|\  
| *   27f83ef 2 hours ago  (tag: 0.4.0, origin/master, master)
| |\  
| | * 925521e 2 hours ago 
| |/  
|/|   
* | ac89157 2 hours ago 
* | b6e93d6 2 hours ago 
|\| 
| *   b6df2d3 2 hours ago  (tag: 0.3.0)
| |\  
| | * 27277e7 3 hours ago 
| |/  
|/|   
* | ae5d80a 3 hours ago 
* | 73ffb7d 3 hours ago 
|\| 
| *   a3fe19c 3 hours ago  (tag: 0.2.0)
| |\  
| | * 9a40911 3 hours ago 
| |/  
|/|   
* | c074d61 4 hours ago 
* | 00cdcbe 5 hours ago 
|/  
* 18dbaa2 5 hours ago  (tag: 0.1.0)

INFO [08/26/20 16:21:25:62] Working directory: D:\Workspace\GitVersionTest
INFO [08/26/20 16:21:25:67] Project root is: D:\Workspace\GitVersionTest\
INFO [08/26/20 16:21:25:67] DotGit directory is: D:\Workspace\GitVersionTest\.git
INFO [08/26/20 16:21:25:76] Using latest commit on specified branch
INFO [08/26/20 16:21:25:79] Running against branch: develop (4c6828daf637c00e93f7e1d7b47e029a0d867bd2)
INFO [08/26/20 16:21:25:79] Begin: Calculating base versions
  INFO [08/26/20 16:21:25:87] Found commit [4c6828daf637c00e93f7e1d7b47e029a0d867bd2] matching merge message format: Default
  INFO [08/26/20 16:21:25:87] Found commit [4c6828daf637c00e93f7e1d7b47e029a0d867bd2] matching merge message format: Default
  INFO [08/26/20 16:21:25:87] Found commit [4c6828daf637c00e93f7e1d7b47e029a0d867bd2] matching merge message format: Default
  INFO [08/26/20 16:21:25:87] Merge message 'Merge branch 'release/0.4.0'': 0.4.0 with commit count source 27f83ef810557a4f92a310c5ecd3d37e946a9c50
  INFO [08/26/20 16:21:25:88] Merge message 'Merge branch 'release/0.3.0'': 0.3.0 with commit count source b6df2d3925f42079b32ba28321368f4905159926
  INFO [08/26/20 16:21:25:88] Merge message 'Merge branch 'release/0.2.0'': 0.2.0 with commit count source a3fe19c9fcbc8e3b32238c31e321e8b2bc800b57
  INFO [08/26/20 16:21:25:89] Git tag '0.4.0': 0.4.0 with commit count source 27f83ef810557a4f92a310c5ecd3d37e946a9c50
  INFO [08/26/20 16:21:25:89] Git tag '0.3.0': 0.3.0 with commit count source b6df2d3925f42079b32ba28321368f4905159926
  INFO [08/26/20 16:21:25:89] Git tag '0.2.0': 0.2.0 with commit count source a3fe19c9fcbc8e3b32238c31e321e8b2bc800b57
  INFO [08/26/20 16:21:25:89] Git tag '0.1.0': 0.1.0 with commit count source 18dbaa2bda565ca554463be8f7797343501cb0d3
  INFO [08/26/20 16:21:25:90] Git tag '0.4.0': 0.4.0 with commit count source 27f83ef810557a4f92a310c5ecd3d37e946a9c50
  INFO [08/26/20 16:21:25:91] Git tag '0.3.0': 0.3.0 with commit count source b6df2d3925f42079b32ba28321368f4905159926
  INFO [08/26/20 16:21:25:91] Git tag '0.2.0': 0.2.0 with commit count source a3fe19c9fcbc8e3b32238c31e321e8b2bc800b57
  INFO [08/26/20 16:21:25:91] Git tag '0.1.0': 0.1.0 with commit count source 18dbaa2bda565ca554463be8f7797343501cb0d3
  INFO [08/26/20 16:21:25:92] Found multiple base versions which will produce the same SemVer (0.5.0), taking oldest source for commit counting (Merge message 'Merge branch 'release/0.4.0'')
  INFO [08/26/20 16:21:25:92] Base version used: Merge message 'Merge branch 'release/0.4.0'': 0.4.0 with commit count source b6df2d3925f42079b32ba28321368f4905159926
  INFO [08/26/20 16:21:25:92] End: Calculating base versions (Took: 126.65ms)
  INFO [08/26/20 16:21:25:92] 5 commits found between b6df2d3925f42079b32ba28321368f4905159926 and 4c6828daf637c00e93f7e1d7b47e029a0d867bd2
  INFO [08/26/20 16:21:25:93] Begin: Getting version tags from branch 'refs/heads/develop'.
  INFO [08/26/20 16:21:25:94] End: Getting version tags from branch 'refs/heads/develop'. (Took: 9.33ms)
{
  "Major":0,
  "Minor":5,
  "Patch":0,
  "PreReleaseTag":"alpha.5",
  "PreReleaseTagWithDash":"-alpha.5",
  "PreReleaseLabel":"alpha",
  "PreReleaseNumber":5,
  "WeightedPreReleaseNumber":5,
  "BuildMetaData":"",
  "BuildMetaDataPadded":"",
  "FullBuildMetaData":"Branch.develop.Sha.4c6828daf637c00e93f7e1d7b47e029a0d867bd2",
  "MajorMinorPatch":"0.5.0",
  "SemVer":"0.5.0-alpha.5",
  "LegacySemVer":"0.5.0-alpha5",
  "LegacySemVerPadded":"0.5.0-alpha0005",
  "AssemblySemVer":"0.5.0.0",
  "AssemblySemFileVer":"0.5.0.0",
  "FullSemVer":"0.5.0-alpha.5",
  "InformationalVersion":"0.5.0-alpha.5+Branch.develop.Sha.4c6828daf637c00e93f7e1d7b47e029a0d867bd2",
  "BranchName":"develop",
  "EscapedBranchName":"develop",
  "Sha":"4c6828daf637c00e93f7e1d7b47e029a0d867bd2",
  "ShortSha":"4c6828d",
  "NuGetVersionV2":"0.5.0-alpha0005",
  "NuGetVersion":"0.5.0-alpha0005",
  "NuGetPreReleaseTagV2":"alpha0005",
  "NuGetPreReleaseTag":"alpha0005",
  "VersionSourceSha":"b6df2d3925f42079b32ba28321368f4905159926",
  "CommitsSinceVersionSource":5,
  "CommitsSinceVersionSourcePadded":"0005",
  "CommitDate":"2020-08-26"
}
  INFO [08/26/20 16:21:25:97] Done writing 

Expected Behavior

I would expect the VersionSource to be the 0.4.0 tag

Actual Behavior

VersionSource is determined to be the 0.3.0 tag

I tried my hand on this fixture thing but i have no idea if i did it right:

[Test]
    public void VersionSource()
    {
        // Arrange
        var config = new ConfigurationBuilder().Build();

        // Act
        using var fixture = new BaseGitFlowRepositoryFixture("0.1.0");
        fixture.Checkout("master");
        var initialCommit = fixture.Repository.Head.Tip;
        fixture.MergeNoFF("develop");
        fixture.Checkout("develop");
        fixture.MakeACommit("Feature commit 1");
        fixture.BranchTo("release/0.2.0");
        fixture.MakeACommit("Release commit 1");
        fixture.Checkout("master");
        fixture.MergeNoFF("release/0.2.0");
        fixture.ApplyTag("0.2.0");
        var tag = fixture.Repository.Head.Tip;
        fixture.Checkout("develop");
        fixture.MergeNoFF("master");
        var version = fixture.GetVersion(config);
        version.VersionSourceSha.ShouldBe(tag.Sha);
    }

• Version Used:
• Operating System and version (Windows 10, Ubuntu 18.04): Windows 10 Version 10.0.17763 Build 17763

[asbjornu]

Does the provided test fail?

[fdiewald]

yes

[asbjornu]

Sorry for taking so long and thank you so much for providing a failing test!

I've now run the test and agree it seems weird that the version source is the first and not the last tag. This seems to be a duplicate of #1830, where it is correctly pointed out that the error comes from the following piece of code:

GitVersion/src/GitVersion.Core/VersionCalculation/BaseVersionCalculator.cs

Lines 56 to 73 in 34b21cb

	static Versions CompareVersions(Versions versions1, Versions version2)
	{
	if (versions1.Version.BaseVersionSource == null)
	{
	return version2;
	}
	if (version2.Version.BaseVersionSource == null)
	{
	return versions1;
	}
	return versions1.Version.BaseVersionSource.When < version2.Version.BaseVersionSource.When ? versions1 : version2;
	}
	var oldest = matchingVersionsOnceIncremented.Aggregate(CompareVersions);
	baseVersionWithOldestSource = oldest.Version;
	maxVersion = oldest;
	this.log.Info($"Found multiple base versions which will produce the same SemVer ({maxVersion.IncrementedVersion}), taking oldest source for commit counting ({baseVersionWithOldestSource.Source})");

If I swap the following comparison, the provided test turns green.

GitVersion/src/GitVersion.Core/VersionCalculation/BaseVersionCalculator.cs

Line 67 in 34b21cb

return versions1.Version.BaseVersionSource.When < version2.Version.BaseVersionSource.When ? versions1 : version2;

This swap also turns a lot of existing tests red, but only because the commit count is lowered due to a newer tag being taken as a version source. I need to dig a bit more into why the current logic is the way it is before we go ahead and change this, as it can be viewed as a breaking change.

[asbjornu]

Okay, so the logic of BaseVersionCalculator taking the oldest tag as version source is from all the way back to #478. The reasoning behind it is given in #465 and the TL;DR is that to avoid resetting the commit count, the oldest tag is used as the source for commit counting.

I think this may be conflating two concepts into one, as the version source for the above test is the 0.2.0 tag (as evident by the produced SemVer being 0.3.0+6), but the commit count source is the initial 0.1.0 tag. So perhaps the right thing to do would be to split these into two different sources: SemVerSource and CommitCountSource?

I'm not sure, but I do understand both the reasoning and why it's confusing that it is the way it is. Now, the question is: Why do you need VersionSource to point to the 0.2.0 tag? Could we improve the current situation through documentation instead of breaking code changes?

[asbjornu]

I'm tracking this in #3041.

[HHobeck]

Thank you very much providing the integration test because it gives me the possibility to dig into the issue very easily and I think I have found the problem.

Actually we have the TaggedCommitVersionStrategy and the MergeMessageVersionStrategy version strategy classes which are relevant for this issue. The first version strategy detects a base version with the source e3399990 and the second with 0529a4e5 which is quite obviously. Because both strategies are resulting in the same incremented version of 0.3.0-alpha.1 only one wins. I'm not sure how the bug fix will look like but I think it might be an idea to order the result. ;)

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. After 30 days from now, it will be closed if no further activity occurs.