GitVersion icon indicating copy to clipboard operation
GitVersion copied to clipboard

[Improvement] Separate SemVerSource from CommitCountSource

Open asbjornu opened this issue 3 years ago • 8 comments

Currently, the term VersionSource is used both for calculation of the SemVer and for commit counting. The VersionSource output variables all point to the the source used for commit counting, not the source used for the resulting SemVer.

Detailed Description

The conflation of commit count source and SemVer source was introduced in #478, with the reasoning given in #465 that to avoid resetting the commit count, the oldest tag is used as the source for commit counting. The oldest tag is however not used as a source for the resulting SemVer, leading to the codebase operating on two different version sources.

Context

The conflation of commit count source and SemVer source is confusing, as #1830 and #2394 is proof of. A way to remove this confusion is to split VersionSource into two different concepts: SemVerSource and CommitCountSource.

Possible Implementation

The entire codebase needs to be refactored and every mention of VersionSource needs to be renamed to either SemVerSource or CommitCountSource depending on its usage.

asbjornu avatar Mar 09 '22 21:03 asbjornu

Wonder if this would solve #2795 ?

wsy-cloud avatar Mar 23 '22 12:03 wsy-cloud

Hi @asbjornu.

I would like to understand the problem and viewed the initial bug #465 and the actual fix for that #478 and tried to create an integration test which reproduce this issue. Could you please take a look and tell me what the expectation is? And also what was generated before this bug fix.

    [Test]
    public void __Just_A_Test__()
    {
        var configuration = ConfigBuilder.New.Build();

        using var fixture = new EmptyRepositoryFixture();

        fixture.Repository.MakeATaggedCommit("1.2.0");

        fixture.BranchTo("develop");
        fixture.MakeACommit(); // one
        fixture.MakeACommit(); // two

        // ✅ succeeds as expected
        fixture.AssertFullSemver("1.3.0-alpha.2", configuration);

        fixture.Checkout("main");
        fixture.BranchTo("hotfix/1.2.1");
        fixture.MakeACommit(); // three
        fixture.MakeACommit(); // four
        fixture.ApplyTag("1.2.1-beta.1");

        // ✅ succeeds as expected
        fixture.AssertFullSemver("1.2.1-beta.1", configuration);

        fixture.Checkout("main");
        fixture.MergeNoFF("hotfix/1.2.1");
        fixture.ApplyTag("1.2.1");

        // ✅ succeeds as expected
        fixture.AssertFullSemver("1.2.1", configuration);

        fixture.Checkout("develop");

        // ✅ succeeds as expected
        fixture.AssertFullSemver("1.3.0-alpha.2", configuration);

        fixture.MergeNoFF("hotfix/1.2.1");

        // ❌ expected: 1.3.0-alpha.2 or 1.3.0-alpha.3 or 1.3.0-alpha.4 or 1.3.0-alpha.5 ???
        fixture.AssertFullSemver("1.3.0-alpha.???", configuration);
    }

HHobeck avatar Sep 22 '22 05:09 HHobeck

What GitVersion did before #478 is impossible for me to answer. I have no idea. 🤷🏼 When it comes to the test you've written, I think 1.3.0-alpha.5 makes sense because develop has 2 direct commits, hotfix/1.2.1 has two direct commits and then develop has the merge-commit from hotfix/1.2.1 as well. That amounts to a total of 5 commits on develop, 3 more than what develop had before the merge and the version was successfully asserted to be 1.3.0-alpha.2.

asbjornu avatar Sep 27 '22 11:09 asbjornu

And this is the reason why the bugfix has been implemented and you want to separate the SemVerSource from CommitCountSource? Means in this example the SemVerSource is 5 and the CommitCountSource is 3? Or what is the your expectation?

HHobeck avatar Sep 27 '22 13:09 HHobeck

And this is the reason why the bugfix has been implemented and you want to separate the SemVerSource from CommitCountSource? Means in this example the SemVerSource is 5 and the CommitCountSource is 3? Or what is the your expectation?

The output of the SemVer variable is 1.3.0-alpha.5 in this example already because we are using the variable CommitsSinceVersionSource:

{
  "AssemblySemFileVer": "1.3.0.0",
  "AssemblySemVer": "1.3.0.0",
  "BranchName": "develop",
  "BuildMetaData": null,
  "CommitDate": "2024-01-11",
  "CommitsSinceVersionSource": 5,
  "EscapedBranchName": "develop",
  "FullBuildMetaData": "Branch.develop.Sha.3f7d98e22589faf4352f21b3735f8d83a9c99181",
  "FullSemVer": "1.3.0-alpha.5",
  "InformationalVersion": "1.3.0-alpha.5+Branch.develop.Sha.3f7d98e22589faf4352f21b3735f8d83a9c99181",
  "Major": 1,
  "MajorMinorPatch": "1.3.0",
  "Minor": 3,
  "Patch": 0,
  "PreReleaseLabel": "alpha",
  "PreReleaseLabelWithDash": "-alpha",
  "PreReleaseNumber": 5,
  "PreReleaseTag": "alpha.5",
  "PreReleaseTagWithDash": "-alpha.5",
  "SemVer": "1.3.0-alpha.5",
  "Sha": "3f7d98e22589faf4352f21b3735f8d83a9c99181",
  "ShortSha": "3f7d98e",
  "UncommittedChanges": 0,
  "VersionSourceSha": "45530eca118a96112469bde8a632db9310fdae7e",
  "WeightedPreReleaseNumber": 5
}

Thus everything is good right!? Could you give me an example where the SemVerSource and the CommitCountSource is not the same? It would be good to have clear acceptance criteria for this improvement.

HHobeck avatar Jan 11 '24 12:01 HHobeck

I think this test illustrates the problem very well:

[TestCase("0.3.0-alpha.1+4", false)]
[TestCase("0.3.0-alpha.1+1", true)]
public void VersionSource(string semanticVersion, bool removeReleaseBranchAfterMerging)
{
    // Arrange
    var configuration = GitFlowConfigurationBuilder.New
        .WithBranch("develop", _ => _
            .WithVersioningMode(VersioningMode.ManualDeployment)
            .WithTrackMergeMessage(false)
        ).Build();

    // Act
    using var fixture = new EmptyRepositoryFixture("main");

    fixture.MakeATaggedCommit("0.1.0");
    fixture.BranchTo("develop");
    fixture.MakeACommit();
    fixture.BranchTo("release/0.2.0");
    fixture.MakeACommit();
    fixture.MergeTo("main", removeReleaseBranchAfterMerging);
    fixture.ApplyTag("0.2.0");
    fixture.MergeTo("develop");

    // Assert
    fixture.AssertFullSemver(semanticVersion, configuration);
}

I'm still not sure how the idea of introducing two new variables with name SemVerSource and the CommitCountSource can solve the problem. Obviously the root problem is that two different version sources resulting in the same next version number. Like you already mentioned the algorithm takes the oldest version source. That is the reason why we getting 0.3.0-alpha.1+4 instead of the 0.3.0-alpha.1+1.

HHobeck avatar Jan 11 '24 15:01 HHobeck