Pre-release label in git tag

[asbjornu]

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

[ruhullahshah]

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?

[asbjornu]

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.

[ruhullahshah]

@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.

[asbjornu]

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]

Also, when thinking of how the test is formulated and #1255 (comment), isn't mode: Mainline more appropriate for the given use case? I'll test how it affects the version number.

[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. 🤷‍♂️

[ruhullahshah]

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.

[asbjornu]

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.

[stale]

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]

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.

[ermshiperete]

#1844/ #1845 seem to be related

[stale]

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.

[arturcic]

Closed in favor of #3445