GitVersion icon indicating copy to clipboard operation
GitVersion copied to clipboard

Pre-release label in git tag

Open asbjornu opened this issue 6 years ago • 12 comments

Rebase and conflict resolvement of #1261. Resolves #1261, fixes #1255.

asbjornu avatar Feb 20 '19 20:02 asbjornu

The reason for the observed incorrect version number is that GitVersion is somehow sensitive to tags that are of the form v5.0.0-rc.1 or v5.0.0-beta.1. In case the PreReleaseTag is dropped from the Git tag, for instance naming the tag simply as v5.0.0 leads to a successful execution of the testcase reported here.

I see that the chosen versioning strategy is correct. however, this part of the code causes an incorrect version increment, even though it is set to Patch. I think that this part of the code would generally hold true, for instance, bumping the version number on develop from 5.0.0-alpha.1 to 5.0.0-alpha.2, but in this case it is doing the wrong thing.

Any thoughts?

ruhullahshah avatar Feb 25 '19 15:02 ruhullahshah

Thanks for digging into this, @ruhullahshah. I'm a bit confused, since the tag should only take presedence if it is on the current commit. For the test case in this PR, there is a commit after the tag v5.0.0-rc.1 is added, so according to my understanding, we should end up on this case VersionField.Patch. However, that does not seem to be the case.

asbjornu avatar Feb 26 '19 10:02 asbjornu

@asbjornu You are welcome. Your argument is correct, if the tag name did not have a PreReleaseTag, in this case -rc.1. This line of code will not allow it to pass through. Try removing -rc.1 from the test and everything works as expected.

ruhullahshah avatar Feb 26 '19 17:02 ruhullahshah

Aha, I was caught yet again by the naming confusion discussed in #1054. I instinctively think of git tag every time I read PreReleaseTag, which is why I argue in #1054 that it should be called PreReleaseLabel instead. Okay, so in the presence of a pre-release label, the label's Number will be incremented instead of applying the chosen increment strategy. I'm not sure why that should be correct for any other commits than the one where the git tag is applied, though. So my thought is still the same: Why should the pre-release label affect the increment strategy at all?

asbjornu avatar Feb 27 '19 08:02 asbjornu

Also, when thinking of how the test is formulated and https://github.com/GitTools/GitVersion/issues/1255#issuecomment-465548324, isn't mode: Mainline more appropriate for the given use case? I'll test how it affects the version number.

asbjornu avatar Feb 27 '19 08:02 asbjornu

Nope, mode: Mainline does not support pre-release labels: System.NotSupportedException : Mainline development mode doesn't yet support pre-release tags on master.

I still think mode: Mainline is correct for the use case, but we don't support the given use case with Mainline. 🤷‍♂️

asbjornu avatar Feb 27 '19 09:02 asbjornu

Your questions are valid and relate more to the specifications and usage of GitVersion. It "might" be true that in this particular case, GitVersion is abused. However, I would like to demonstrate the general problem with a simpler test case:

[Test]
public void ShouldProvideTheCorrectVersionEvenIfPreReleaseLabelExistsInTheGitTag()
{
    using (var fixture = new EmptyRepositoryFixture())
    {
        fixture.Repository.MakeACommit();
        fixture.ApplyTag("1.0.0-oreo.1");
        fixture.BranchTo("develop");
        fixture.Repository.MakeACommit();
        fixture.AssertFullSemver("1.1.0-alpha.1");
    }
}

This simple test fails due to the presence of a PreReleaseLabel (I agree that this is a better naming convention) in the git tag

Is there some documentation, that talks about GitVersion and its assumptions on naming conventions of a git tag? If no, then in my opinion, this should be fixed or the documentation should be updated to declare the assumptions.

ruhullahshah avatar Feb 27 '19 13:02 ruhullahshah

Great point, @ruhullahshah. I've added your test to this PR as further proof that something needs to be done about this. I'm just not sure what.

asbjornu avatar Feb 27 '19 13:02 asbjornu

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. Thank you for your contributions.

stale[bot] avatar Sep 25 '19 12:09 stale[bot]

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. Thank you for your contributions.

stale[bot] avatar Dec 24 '19 12:12 stale[bot]

#1844/ #1845 seem to be related

ermshiperete avatar May 22 '20 06:05 ermshiperete

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. Thank you for your contributions.

stale[bot] avatar Aug 21 '20 20:08 stale[bot]

Closed in favor of #3445

arturcic avatar Mar 19 '23 09:03 arturcic