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

GitVersion should only consider tags matching current branch config (was: GitVersion fails to return correct version when tag exists on commit) #2340

Closed
DocZoid opened this issue Jun 24, 2020 · 25 comments

Comments

@DocZoid
Copy link

DocZoid commented Jun 24, 2020

Describe the bug
Tags have a too strong weight on the version evaluation which I feel is a bug. I am quite new to versioning and GitVersion, so I might be on the wrong track without knowing. I have read in some other issue that fast-forward merges for master are generally discouraged (forbidden?), and this is a part of my issue (#2). The proposed fix would as a side effect allow using fast-forward merges in my opinion.

There are two scenarios where the tag should not be used to evaluate the version:

  1. When I build a snapshot from the develop branch, tag the branch with the created version (e.g. 0.1.2-snapshot.2), and then branch this commit to a new release (release/0.1.2)
  2. When I build a release candidate from a release branch, tag the release branch with the new created version (e.g. 0.1.2-rc.1), and then make a fast-forward merge to master, essentially having master point to that tagged commit

Expected Behavior

  1. The resulting version should be 0.1.2-rc.1 (with the release configuration "tag: rc")
  2. The resulting version should be 0.1.2

Actual Behavior

  1. The resulting version is 0.1.2-snapshot.2 based on the tag
  2. The resulting version is 0.1.2-rc.1 based on the tag

Possible Fix

Multiple branches point to the same commit. This is confusing GitVersion, which takes tags from that commit, but it does not respect the branch configuration of the current branch when analyzing the tags, e.g. when I am working on a release branch, it should only look for previous version tags which are formatted like its own tag configuration in the release branch ("with "rc" in it, so tags 0.1.2-rc.1 should be considered, but not 0.1.2-snapshot.2, because of the configuration "tag: rc")

Steps to Reproduce

  1. assume or create config: "develop -> tag: snapshot", "release -> tag: rc"
  2. tag a commit in "develop" with a version tag, e.g. 0.1.2-snapshot.2
  3. branch that commit to release/0.1.2
  4. using release/0.1.2 as current branch, run GitVersion. Calculated version should be 0.1.2-rc.1 instead of 0.1.2-snapshot.2

Context

We have commit hooks checking every commit message. Therefor, it seems like an easy option to use fast-forward merges for master to avoid commits, but GitVersion is missing the mentioned configuration option to be able to handle fast-forward merges.
Also, I do not know how GitVersion is intended to be used when creating a release branch without adding an empty commit on that branch before building it, if the commit already has a tag. Deleting all tags locally would be a workaround, but it has a smell of nastiness.

Your Environment

  • Version Used: 5.3.3+Branch.master.Sha.08a43ede0b292c8ce2178b3d305587f151a2bdf4
  • Operating System and version (Windows 10, Ubuntu 18.04): Windows 10
  • Link to your project: -
  • Link to your CI build (if appropriate): -
@DocZoid DocZoid added the bug label Jun 24, 2020
@asbjornu
Copy link
Member

The workflow you are describing is not supported by GitVersion. The closest match you have may be Mainline mode, but tags are going to have the highest weight when figuring out a version number no matter what you do. This is by design and is not going to change.

@asbjornu asbjornu added question and removed bug labels Jun 25, 2020
@DocZoid
Copy link
Author

DocZoid commented Jun 25, 2020

Can you elaborate more on why the workflow is not supported? I use GitFlow, with develop creating snapshots (instead of alpha). Mainline mode does not suite our development needs.

I know that it is a design decision to build versions preferably by using the commit's tag, and I am not suggesting to remove it, but rather to improve it. Of course this can also be a configurable option for each branch. The option could work like that: (in the branch configuration) consider-tags-of-other-branches: <true/false>, or more general, but probably over-the-top: filter-tag-regex: .*. Setting the first option to false yields the current behaviour. True would create a regex <SemVer-regex>-<tag-param of branch>.* to filter the found tags before actually analyzing them (or only <SemVer-regex> if tag is set to '').

If that improvement is not an option (apart from possible minor bugs in my suggested implementation), please explain why, and what I could change to build a release (candidate) from an already tagged commit.

@asbjornu
Copy link
Member

Perhaps I'm not properly understanding your use-case. Please submit a PR with a RepositoryFixture that implements the behaviour you'd like to see.

@DocZoid
Copy link
Author

DocZoid commented Jul 16, 2020

I'd like to, but the way of working for such a change and the GitVersion internals are all new to me. But I'll try to get through it.

I just did a code analysis to find a place where I would assume the change to take place.

In TaggedCommitVersionStrategy.cs I see a comment /// Version is extracted from all tags on the branch which are valid, and not newer than the current commit.. I want to point out that the comment (and the following code examples) all state that the tags have to refer to the current branch, not the current commit. When using FF merges or when branching (and not committing after branching), commits (and their tags) may belong to multiple branches. This cannot be distinghuished in GIT, but it can in GitVersion, based on the configuration.

My intention is to have GitVersion focus more on the current branch and disregard tags on the same commit, but belonging to another branch (again, using the configuration to tell to which branch a tag belongs).

The essential code seems to be in GetTaggedVersions:

         var tagsOnBranch = currentBranch
               .Commits
               .SelectMany(commit => { return allTags.Where(t => IsValidTag(t.Item1, commit)); })
               .Select(t =>
               {
                   if (t.Item1.PeeledTarget() is Commit)
                       return new VersionTaggedCommit(t.Item1.PeeledTarget() as Commit, t.Item2, t.Item1.FriendlyName);

                   return null;
               })
               .Where(a => a != null)
               .Take(5)
               .ToList();

where t.Item1.FriendlyName refers to the tag name. I would want to filter that to have a prefix that matches the configured tag in the GitVersion branch configuration (e.g. "release: tag: rc" means if the current branch is release, all tags returned here should have the prefix "rc", meaning they actually belong to the current branch. Do not consider tags with other prefixes, like "snapshot.32", since we are on a release branch and do not want to build a snapshot, but a release candidate).

Is there a function somewhere that can tell me if a Git-tag matches a specific configured branch tag? How would I get the configured branch tag? Also, two special cases have to be considered: if the configured branch tag is empty (e.g. master branch), the match must yield true if the Git-tag has no prefix, and if the configured branch tag is "useBranchName", this has to be resolved.

@asbjornu
Copy link
Member

asbjornu commented Jul 16, 2020

Adding a test shouldn't require deep knowledge about the GitVersion codebase. They are pretty straight forward:

[Test]
public void TagPreReleaseWeightIsConfigured_HeadIsATaggedCommit_WeightedPreReleaseNumberShouldBeTheSameAsTheTagPreReleaseWeight()
{
// Arrange
var config = new ConfigurationBuilder()
.Add(new Config
{
AssemblyFileVersioningFormat = "{Major}.{Minor}.{Patch}.{WeightedPreReleaseNumber}",
TagPreReleaseWeight = 65535
})
.Build();
// Act
using var fixture = new BaseGitFlowRepositoryFixture("1.0.0");
fixture.MakeATaggedCommit("1.1.0");
var version = fixture.GetVersion(config);
// Assert
version.AssemblySemFileVer.ShouldBe("1.1.0.65535");
}

Start with writing a test that provokes the behaviour you're experiencing, then you can start deep-diving into the code and change things to see whether it fixes your problem or not. Without seeing a reproduction in a test, it's very hard for me to help you figure out where or how to fix your problem, so please start there. You can submit a draft pull request to allow feedback and discussion before the problem is solved and the PR is good for a final review and merge.

My intention is to have GitVersion focus more on the current branch and disregard tags on the same commit, but belonging to another branch (again, using the configuration to tell to which branch a tag belongs).

This sounds impossible. Git doesn't have a link between branch and tag references. To Git, they are basically the same: a reference to a commit. So as long as you have a tag reference to the same commit that is checked out, which branch was checked out when the commit was made, is irrelevant. You would need to add another commit to the current branch to deviate from the "tagged branch" so to say.

@DocZoid
Copy link
Author

DocZoid commented Jul 17, 2020

I'll try to get quite close to it for one example at least, based on some other tests. Apologies for likely mistakes.


        [Test]
        public void BetaBranchCreatedButStillOnTaggedAlphaCommitShouldCreateBetaVersion()
        {
            // Arrange
            var config = new Config
            {
                Branches =
                {
                    { "release", new BranchConfig { Tag = "beta" } },
                    { "develop", new BranchConfig { Tag = "alpha" } }
                }
            };

            // Act
            using var fixture = new BaseGitFlowRepositoryFixture("1.0.0");
            //not sure if needed
            //fixture.Checkout("master");
            //fixture.MergeNoFF("develop");
            fixture.Checkout("develop");
            fixture.MakeACommit("Feature commit 1");
            fixture.ApplyTag("1.1.0-alpha.1"); // assuming this has been build as 1.1.0-alpha.1
            fixture.BranchTo("release/1.1.0"); // about to be released
            //fixture.MakeACommit("Release commit 1"); // no additional empty commit in this scenario!
            fixture.Checkout("release/1.1.0"); // still on the same commit, but another branch, choosing to build same code as beta
            fixture.AssertFullSemver("1.1.0-beta.1", config); //will be 1.1.0-alpha.1, should be 1.1.0-beta.1. Tag is an "alpha" tag, only "beta" tags should count when on release branch. If no beta tag found, build new beta version on release branch.
            fixture.Checkout("develop"); // back to develop
            fixture.AssertFullSemver("1.1.0-alpha.1", config); //will be 1.1.0-alpha.1 based on tag (as before)

            var version = fixture.GetVersion(config);

            // Assert
            // done in-line
        }

@asbjornu
Copy link
Member

Sorry, but writing code in a comment is not the same as submitting a draft pull request, @DocZoid.

@DocZoid
Copy link
Author

DocZoid commented Jul 20, 2020

Sorry, but you are not being helpful either, @asbjornu .
I'm just getting as far as I can with the time I have. It might take some time until I have a pull request out, and I was hoping for some constructive feedback.

@asbjornu
Copy link
Member

asbjornu commented Jul 20, 2020

Sorry, but the constructive feedback is very hard to give on an island of code like that. If I had a proper draft pull request to look at, I would for instance know whether the code even compiled and the result of the test would be available to me. As presented, I would have to open my editor, paste in your code, compile it, execute tests, etc. That requires me to actually block out time to even know what I'm looking at here. If you are unable to allot the required time and energy to submit a draft pull request, I'm not going to be able to allot the required time and energy to give you feedback. Sorry.

@DocZoid DocZoid changed the title GitVersion fails to return correct version when tag exists on commit GitVersion should only consider tags matching current branch config (was: GitVersion fails to return correct version when tag exists on commit) Jul 21, 2020
@DocZoid
Copy link
Author

DocZoid commented Jul 21, 2020

I never said that I won't make a pull request, it's just small steps with a fail-fast mindset (if you tell me "that won't work because you have not considered <mutant unicorns>" I might as well give it up altogether).

Well, I have checked out the code and added a (hopefully improved) test. I have no access to push the commit - is it something I have to look up in GitHub or do you need to give me repo rights? I didn't use GitHub for contributing yet and the permissions doc page I found didn't help.

Edit: I believe I need collaborator rights

@arturcic
Copy link
Member

@DocZoid

Well, I have checked out the code and added a (hopefully improved) test. I have no access to push the commit - is it something I have to look up in GitHub or do you need to give me repo rights? I didn't use GitHub for contributing yet and the permissions doc page I found didn't help.

We don't give permissions to directly commit the changes to the repository. But all those that want to contribute usually fork the repository, make the adjustments in their fork then they create a pull request against the original repository.

@asbjornu
Copy link
Member

asbjornu commented Jul 21, 2020

You can read about how to fork a repository and then how to submit a pull request from a fork.

DocZoid pushed a commit to DocZoid/GitVersion that referenced this issue Jul 24, 2020
…mit should not consider the tag if it does not match to the current branch config
@DocZoid
Copy link
Author

DocZoid commented Jul 24, 2020

Getting there... do I work on master then, or do I still start a new branch?

@asbjornu
Copy link
Member

A separate branch would be best, as a pull request is not frozen in time, so whenever commits are added to the branch you create the pull request from, they will be added to the pull request. As master is a moving target, a separate branch gives more control and stability.

DocZoid pushed a commit to DocZoid/GitVersion that referenced this issue Jul 24, 2020
…mit should not consider the tag if it does not match to the current branch config
DocZoid pushed a commit to DocZoid/GitVersion that referenced this issue Oct 5, 2020
…mit should not consider the tag if it does not match to the current branch config
DocZoid pushed a commit to DocZoid/GitVersion that referenced this issue Oct 5, 2020
…mit should not consider the tag if it does not match to the current branch config
@DocZoid
Copy link
Author

DocZoid commented Oct 5, 2020

@asbjornu I finally have some time again to get back on this topic. Does this pull request match your expectations or am I missing something? What is your opinion on the shown behavior?

@asbjornu
Copy link
Member

asbjornu commented Oct 5, 2020

Thanks for the PR and test, @DocZoid. It is a great way to understand your use-case. From what I understand, you would like release branches to always yield the pre-release label (aka. "tag") beta even though the currently built commit is tagged with alpha?

I don't think we can support that, at least not without configuration. Tags always take presedence when they are available on the current commit and I'm not sure we want to change this, even with configuration. It's also a bit strange to create a release branch directly from a tagged commit without any commits in it. In Git Flow (which is one of a few flows GitVersion supports), release branches should be branched out from develop.

Which flow are you using?

@DocZoid
Copy link
Author

DocZoid commented Oct 6, 2020

Exactly, that is scenario 1 from my initial comment. I'll try to add a test for scenario 2 as well.

It has to be configurable for sure. I wrote

I know that it is a design decision to build versions preferably by using the commit's tag, and I am not suggesting to remove it, but rather to improve it. Of course this can also be a configurable option for each branch. The option could work like that: (in the branch configuration) consider-tags-of-other-branches: <true/false>, or more general, but probably over-the-top: filter-tag-regex: .*.

For example, the release branch configuration for the test would be filter-tag-regex: .*-beta.*. GitVersion knows that the current branch which is checked out is a release branch, therefor it can fetch the filter-tag-regex configuration of the current branch and apply the filter .*-beta.*to all known tags of the current commit. Only the tags remaining will be used for further version number calculation.
The default setting would be filter-tag-regex: .*, which yields the current behavior, that all tags found on the current commit are used for version number calculation.

It's also a bit strange to create a release branch directly from a tagged commit without any commits in it. In Git Flow (which is one of a few flows GitVersion supports), release branches should be branched out from develop.
Which flow are you using?

Actually we are using Git Flow, but only as a branching model, not using the commands. When creating a release branch, we do not create empty commits to branch out of develop because this would be a step which would have to be done automatically by our CI/CD build pipeline, and we would not want to have it commit automatically. This is also not supported by the repository permissions and commit hook configuration. Just using a regular branch (initiated by jira in our case) is a neat way to support Git Flow but not using the command line tool.

DocZoid pushed a commit to DocZoid/GitVersion that referenced this issue Oct 12, 2020
… commit, the tag should not be considered if it does not match to the current branch config
@stale
Copy link

stale bot commented Jan 6, 2021

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 stale bot added the stale label Jan 6, 2021
DocZoid pushed a commit to DocZoid/GitVersion that referenced this issue Jan 29, 2021
BomBastiG added a commit to DocZoid/GitVersion that referenced this issue Feb 16, 2021
there might be an easier solution
BomBastiG added a commit to DocZoid/GitVersion that referenced this issue Feb 16, 2021
BomBastiG added a commit to DocZoid/GitVersion that referenced this issue Feb 16, 2021
BomBastiG added a commit to DocZoid/GitVersion that referenced this issue Feb 16, 2021
@stale stale bot closed this as completed Feb 21, 2021
@asbjornu
Copy link
Member

Sorry for not answering this before, @DocZoid.

Actually we are using Git Flow, but only as a branching model, not using the commands.

I too use Git Flow "manually" so to speak, because I often don't agree with everything the automatic tooling does and prefer doing the individual steps myself. But by the sound of your workflow, it does not match entirely what Git Flow prescribes.

Just using a regular branch (initiated by jira in our case) is a neat way to support Git Flow but not using the command line tool.

If you're not adding any commits to the release branch, is it really a branch? Why not use a tag instead?

@asbjornu asbjornu reopened this Feb 11, 2022
@stale stale bot removed the stale label Feb 11, 2022
@NatMarchand
Copy link

NatMarchand commented Feb 15, 2022

Hello there !
We're encountering almost the same issue.
Here is a description of our way of working :
All developers work on a branch named main
At each commit, we compute the semver and use it to tag the docker image (and embed the version in assemblies).
We are working in mainline mode and on main every commit is tagged (with pre-release tag alpha).
Every commit is then deployed on a dev environment.

When we're feature ready, we create a release branch in order to stabilize and let the external QA happen. Commits on this branch are also built and tagged (without pre-release tag).
Then once built, they go to the QA environment then production.

We would like to end up with something like this :
image

However, as you can notice and as it is the case in this thread, when we create the release branch on the commit "second feature", it is already tagged 0.2.0-alpha.0. gitversion is then unable to produce a 0.2.0 but instead (with our configuration I can post if you want) output 0.2.0-alpha.

We've debugged and narrowed down to this line :

if (semver.CompareTo(taggedSemanticVersion, false) > 0)

taggedSemanticVersion is 0.2.0-alpha.0 (which is correct), semver is 0.2.0 (which is correct) but the comparison don't take in account the prerelease while comparing (the false parameter in CompareTo). Therefore it considers that 0.2.0-alpha.0 == 0.2.0 (which is false). If we switch the parameter to true, it works great. I can make a PR with such a change however, I'm unsure about consequences. What do you think ?

Addendum: I've just tested, this change breaks 5 tests

@asbjornu
Copy link
Member

asbjornu commented Mar 7, 2022

@NatMarchand, thanks for the detailed explanation. Is the following pseudocode a fair description of what's happening?

var semver = new SemVer("0.2.0");
var taggedSemanticVersion = new SemVer("0.2.0-alpha.0");

if (semver.CompareTo(taggedSemanticVersion, false) > 0)
{
    // "0.2.0-alpha.0" is considered to be equal to "0.2.0",
    // therefore, the `taggedSemanticVersion` is set to `null`
    // and `semver` is used instead.
}

I can't say exactly why the prerelease label would not be considered in this case. Can you name the tests that are failing when changing includePrerelease from false to true?

@asbjornu
Copy link
Member

asbjornu commented Mar 8, 2022

It seems like this may be a duplicate of #2241.

@HHobeck
Copy link
Contributor

HHobeck commented Mar 16, 2023

@DocZoid Could you please take a look to the following bugs: #3438 and #3436.

@HHobeck
Copy link
Contributor

HHobeck commented Mar 17, 2023

An integration test in #3441 gives the follwoting result:

[Test]
public void __Just_A_Test__()
{
    var configuration = GitFlowConfigurationBuilder.New
        .WithBranch("develop", builder => builder.WithLabel("snapshot"))
        .WithBranch("release", builder => builder.WithLabel("rc"))
        .Build();

    using var fixture = new EmptyRepositoryFixture();

    fixture.MakeACommit();
    fixture.BranchTo("develop");
    fixture.MakeACommit();
    fixture.MakeATaggedCommit("0.1.2-snapshot.2");
    fixture.BranchTo("release/0.1.2");

    // ✅ succeeds as expected
    fixture.AssertFullSemver("0.1.2-rc.1+0", configuration);
}

@HHobeck HHobeck closed this as completed Mar 17, 2023
@arturcic arturcic modified the milestones: 6.x, 6.0.0-beta.2 Apr 6, 2023
@arturcic
Copy link
Member

arturcic commented Apr 6, 2023

🎉 This issue has been resolved in version 6.0.0-beta.2 🎉
The release is available on:

Your GitReleaseManager bot 📦🚀

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