HighestTagBaseVersionStrategy critical bug

[GeertvanHorrik]

I have a repository (Catel.Fody), which has these commits:

Somehow it picks tags on the specific branch, but not sure why it would do that.

The previous versions of GitVersion worked fine.

[GeertvanHorrik]

btw. I use sourcetree for following gitflow, so it's something that affects all sourcetree users if this is by design.

[JakeGinnivan]

v3 is practically a rewrite which is why I have been asking for more people to test.

I am not really clear of the issue, could you elaborate with the version you are getting and what you expect?

[GeertvanHorrik]

The previous version gave me 2.6.0-unstable-001 or something like that. Now it says 1.9.0-unstable-73.

[GeertvanHorrik]

Another thing is that it now defaults to 0.1.0 instead of 0.2.0, but that can be bypassed by using the config.

[GeertvanHorrik]

Here is an example how I tag using SourceTree (purple is master, blue is develop).

[JakeGinnivan]

Yeah, I just cloned and had a look. This is listed as a breaking change, we no longer track the version of master on develop. GitVersion has been made more stupid in that regard.

This may be an issue of my understanding of GitFlow, but shouldn't the release branch be merged into develop as well as master?

[GeertvanHorrik]

Yep, but only if it has changes ;-)

A good example is Catel what I did this morning:

1. Create release branch
2. Modify something
3. Finish release => merged to both develop and master

If there are no changes, no need to merge anything back to develop.

[JakeGinnivan]

Understand.. GitVersion in 3.0 basically only looks at what is in it's history tree. It makes it much easier to follow what it is doing and means we don't have different classes for calculating versions for each branch.

I am thinking maybe we just create a GitFlowDevelopBranchBaseVersionStrategy class. Damn that is a long class name. But it would have this specific logic.

Or we make it generic and have a config value on the develop branch saying it tracks masters version.

@andreasohlund any other ideas?

[JakeGinnivan]

Another thing is that it now defaults to 0.1.0 instead of 0.2.0, but that can be bypassed by using the config.

I think that is expected, I think starting at 0.2.0 is wrong?

[GeertvanHorrik]

Agree, so I won't complain about that ;-)

[GeertvanHorrik]

I think I found a nice workaround. I added this method:

        public static bool IsDirectMergeFromCommit(this Tag tag, Commit commit)
        {
            var targetCommit = tag.Target as Commit;
            if (targetCommit != null)
            {
                foreach (var parent in targetCommit.Parents)
                {
                    if (string.Equals(parent.Id.Sha, commit.Id.Sha, StringComparison.OrdinalIgnoreCase))
                    {
                        return true;
                    }
                }
            }

            return false;
        }

It checks if the tag is the result of a specific commit (we are currently investigating). This will ensure that we only use tags coming from the branch we are really investigating. It results in the following:

What are your thoughts. Am I missing something here?

[JakeGinnivan]

What branch are you running on? From what I understand the issue is this:

using (var fixture = new EmptyRepositoryFixture())
{
   fixture.repo.MakeACommit();
   fixture.repo.Branch("develop").Checkout();
   fixture.repo.MakeACommit();
   fixture.repo.Branch("release/1.0.0").Checkout();
   fixture.repo.Checkout("master");
   fixture.repo.MergeNoFF("release/1.0.0");
   fixture.repo.TagCommit("1.0.0");

   fixture.repo.Checkout("develop");
   fixture.AssertFullSemVer("1.0.0-unstable.1");
}

But that fails as the version on develop has not bumped and is still 0.1.0-unstable-2 or something like that?

[GeertvanHorrik]

/url https://github.com/Catel/Catel.Fody /b develop /output buildserver

[GeertvanHorrik]

This is a fixture-only issue I hope?

[JakeGinnivan]

Yeah. So the issue is, the tags on master are not on develop. So it no longer discovers them.

If so, we need to re-implement the logic which tracks tags on master and bumps develop accordingly. I suggest we create a GitFlowDevelopBranchBaseVersionStrategy, take the v2 logic which bumps when master is tagged and this base version strategy only works when the current branch is develop.

[JakeGinnivan]

This is a fixture-only issue I hope?

No idea, unsure of the context. The fixture is creating an empty git repo via libgit2.

[GeertvanHorrik]

Well, the extension method I wrote does just that. It checks if the merge is coming from the branch we are investigating (thus develop in this case). The only issue I foresee is that if a merge from develop to another branch would also be tagged. I never did that, but is it something that can happen in GitFlow?

[GeertvanHorrik]

This fixture now succeeds:

[Test]
public void NoMergeBacksToDevelopInCaseThereAreNoChangesInReleaseBranch()
{
    using (var fixture = new EmptyRepositoryFixture(new Config()))
    {
        fixture.Repository.MakeACommit();
        fixture.Repository.CreateBranch("develop").Checkout();
        fixture.Repository.MakeCommits(3);
        var releaseBranch = fixture.Repository.CreateBranch("release/1.0.0");
        releaseBranch.Checkout();
        fixture.Repository.Checkout("master");
        fixture.Repository.MergeNoFF("release/1.0.0");
        fixture.Repository.ApplyTag("1.0.0");
        fixture.Repository.Checkout("develop");
        fixture.Repository.MakeACommit();

        fixture.Repository.Branches.Remove(releaseBranch);

        fixture.AssertFullSemver("1.1.0-unstable.1");
    }
}

[Test]
public void NoMergeBacksToDevelopInCaseThereAreChangesInReleaseBranch()
{
    using (var fixture = new EmptyRepositoryFixture(new Config()))
    {
        fixture.Repository.MakeACommit();
        fixture.Repository.CreateBranch("develop").Checkout();
        fixture.Repository.MakeCommits(3);
        var releaseBranch = fixture.Repository.CreateBranch("release/1.0.0");
        releaseBranch.Checkout();
        fixture.Repository.MakeACommit();

        // Merge to master
        fixture.Repository.Checkout("master");
        fixture.Repository.MergeNoFF("release/1.0.0");
        fixture.Repository.ApplyTag("1.0.0");

        // Merge to develop
        fixture.Repository.Checkout("develop");
        fixture.Repository.MergeNoFF("release/1.0.0");

        fixture.Repository.MakeACommit();
        fixture.Repository.Branches.Remove(releaseBranch);

        fixture.AssertFullSemver("1.1.0-unstable.1");
    }
}

Without my changes, the unit test without any changes in the release branch fails. With the changes it succeeds.

[JakeGinnivan]

Cool. Can you throw a PR up? Just jumped off the train and will be busy until I head back to London tomorrow. Will sort it out then.

Sent from my Windows Phone

---

From: Geert van Horrikmailto:notifications@github.com
Sent: ý14/ý03/ý2015 14:34
To: ParticularLabs/GitVersionmailto:GitVersion@noreply.github.com
Cc: Jake Ginnivanmailto:jake@ginnivan.net
Subject: Re: [GitVersion] HighestTagBaseVersionStrategy critical bug (#389)

This fixture now succeeds:

[Test]
public void NoMergeBacksToDevelopInCaseThereAreNoChangesInReleaseBranch()
{
using (var fixture = new EmptyRepositoryFixture(new Config()))
{
fixture.Repository.MakeACommit();
fixture.Repository.CreateBranch("develop").Checkout();
fixture.Repository.MakeCommits(3);
var releaseBranch = fixture.Repository.CreateBranch("release/1.0.0");
releaseBranch.Checkout();
fixture.Repository.Checkout("master");
fixture.Repository.MergeNoFF("release/1.0.0");
fixture.Repository.ApplyTag("1.0.0");
fixture.Repository.Checkout("develop");
fixture.Repository.MakeACommit();

    fixture.Repository.Branches.Remove(releaseBranch);

    fixture.AssertFullSemver("1.1.0-unstable.1");
}

}

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/389#issuecomment-80510098.

[GeertvanHorrik]

Will try. Need to make sure I didn't break anything else.

[JakeGinnivan]

Safest is a new base version finder hardcoded to develop. Or an opt in config value which turns on that logic (can just enable for develop).

Config approach is probably better then can opt any branch into that logic.

Sent from my Windows Phone

---

From: Geert van Horrikmailto:notifications@github.com
Sent: ý14/ý03/ý2015 14:45
To: ParticularLabs/GitVersionmailto:GitVersion@noreply.github.com
Cc: Jake Ginnivanmailto:jake@ginnivan.net
Subject: Re: [GitVersion] HighestTagBaseVersionStrategy critical bug (#389)

Will try. Need to make sure I didn't break anything else.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/389#issuecomment-80516122.

[GeertvanHorrik]

Fixed by #392