[Improvement] Separate SemVerSource from CommitCountSource

asbjornu commented 2 years ago

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.

wsy-cloud commented 2 years ago

Wonder if this would solve #2795 ?

HHobeck commented 2 years ago

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);
    }

asbjornu commented 2 years ago

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.

HHobeck commented 2 years ago

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 commented 8 months ago

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 commented 8 months ago

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.

kkgthb commented 4 months ago

Means in this example the SemVerSource is 5 and the CommitCountSource is 3?

@HHobeck , that's what I ended up reading this issue in hopes of.

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, I'd like to use your lovely tool a little differently than you intended -- it's so close, but not quite what I'd wanted to do.

If commit number 123456's $gitversion_output.Sha -eq $gitversion_output.VersionSourceSha, then I want to go ahead and return the value of $gitversion_output.MajorMinorPatch as a suggested version number for the rest of my PowerShell module autobuild process.
But even if that's not the case not, if commit number 123456's $gitversion_output.MajorMinorPatch is a "fresh SemVer increment" based on, say, my minor-version-bump-message regex and the commit message attached to 123456, then I also want to return the value of $gitversion_output.MajorMinorPatch as a suggested version number for the rest of my PowerShell module build-publish process.
Otherwise, I'd like to no-op and return a null value. (I plan to parse null values to no-op and not waste CI/CD minutes running ModuleBuilder if there's no commit or tag to indicate that anything substantive changed in the source code -- e.g. if it was just a "docs" commit and I've overridden my commit message standards such that I don't consider "docs" substantive for this purpose.)

At the moment, GitVersion doesn't return me any data on which to distinguish a tagless commit that bumped the SemVer from a tagless commit that came after the tagless commit that bumped the SemVer. Meaning I can't say, "hey, this is a semver-bumping commit; go ahead and return the suggested version number."

HHobeck commented 4 months ago

@kkgthb: Please create a new discussion if it is not related to this issue.

Maybe this issue goes in the direction you mentioned:

https://github.com/GitTools/GitVersion/issues/3812

Still I miss a description about the motivation. Why would you tried the build or whatever differently?

GitTools / GitVersion