Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Improvement] Separate SemVerSource from CommitCountSource #3041

Open
asbjornu opened this issue Mar 9, 2022 · 8 comments
Open

[Improvement] Separate SemVerSource from CommitCountSource #3041

asbjornu opened this issue Mar 9, 2022 · 8 comments

Comments

@asbjornu
Copy link
Member

asbjornu commented Mar 9, 2022

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
Copy link

Wonder if this would solve #2795 ?

@HHobeck
Copy link
Contributor

HHobeck commented Sep 22, 2022

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
Copy link
Member Author

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
Copy link
Contributor

HHobeck commented Sep 27, 2022

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
Copy link
Contributor

HHobeck commented Jan 11, 2024

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
Copy link
Contributor

HHobeck commented Jan 11, 2024

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 HHobeck modified the milestones: 6.x, 7.x Jan 15, 2024
@kkgthb
Copy link

kkgthb commented May 20, 2024

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
Copy link
Contributor

HHobeck commented May 22, 2024

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

Maybe this issue goes in the direction you mentioned:

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants