[Bug] Version not generated correct when creating a feature branch from a release branch

[HHobeck]

Describe the bug
Hi there. I'm using the GitFlow branching strategy descripted on this page https://gitversion.net/docs/learn/branching-strategies/gitflow/examples. Because I want to ensure the stability of the develop and release branch direct committing is not allowed. Thus I need to go via feature branches and got an unexpected version generation on the following scenario:

[Test]
public void __Just_A_Test__()
{
    using var fixture = new EmptyRepositoryFixture();
    fixture.Repository.MakeATaggedCommit("1.0.0");
    fixture.BranchTo("develop");
    fixture.Repository.MakeACommit();
    fixture.BranchTo("release/1.1.0");
    fixture.Repository.MakeACommit();
    fixture.BranchTo("feature/just-a-test");
    fixture.Checkout("develop");
    fixture.Repository.MakeACommit();
    fixture.Checkout("feature/just-a-test");
    fixture.MergeNoFF("develop");
    fixture.Repository.MakeACommit();
    var configuration = new Config()
    {
        Branches = new Dictionary<string, BranchConfig>()
        {
            { "release", new BranchConfig() { TracksReleaseBranches = true } }
        }
    };
    fixture.AssertFullSemver("1.2.0-just-a-test.1+3", configuration); // 1.1.0 expected
}

That produces the following repository:

    * 8df9b9f 52 minutes ago  (HEAD -> feature/just-a-test)
    *   999e22a 54 minutes ago 
    |\  
    | * 26d759f 55 minutes ago  (develop)
    * | b5addb0 57 minutes ago  (release/1.1.0)
    |/  
    * 065c6eb 59 minutes ago 
    * 73835c8 61 minutes ago  (tag: 1.0.0, main)

with following commit messages:

Expected Behavior

Git Version should generate the semantic version 1.1.0 on feature branch just-a-test in this scenario.

Actual Behavior

The semantic version 1.2.0 will be generated.

Possible Fix

I'm not sure if it is a bug or I have misconfigured something.

Steps to Reproduce

Please take look to the test steps above

Context

I'm using the latest version of GitVersion 5.10.1. My build and deployment pipelines are in AzureDevOps but I can reproduce it locally.

Your Environment

• Version Used: 5.10.1
• Operating System is Windows 10
• Link to your project: n/a
• Link to your CI build (if appropriate): n/a

[asbjornu]

feature branches are not supposed to be branched off of release branches. What happens if your feature branches are branches off of develop instead?

[HHobeck]

Okay I see it's not a bug it's a feature. ;) If I take the diagram of the git flow example you are correct: Direct committing to the release branch is permitted (like you can force a commit in a develop branch). But the meaning of a release branch is the same like a development branch if you ask me with the different that the time of the releasing is not the same. Thus all commits in the release branch should be reviewed and ensured via a gate (Pull Request and/or CI-Pipeline) as well right!? How would you do this?

The problem is not that GitVersion doesn't recognize the feature branch when it is based on a release branch (Therefor I set the TracksReleaseBranches to true). The problem is the rebase commit from the develop branch in my opinion.

OR maybe I missused the TracksReleaseBranches option. Anyway if I test this from develop branch it works of course. But that is not I want to point out here.

[asbjornu]

Thus all commits in the release branch should be reviewed and ensured via a gate (Pull Request and/or CI-Pipeline) as well right!? How would you do this?

You create a pull request for merging the release branch into main and review that pull request?

The problem is the rebase commit from the develop branch in my opinion.

Rewriting or not recording history with rebase, squash, fast-forward, etc., is always going to mess up versioning. Try to avoid it.

[HHobeck]

You create a pull request for merging the release branch into main and review that pull request?

If it is like you have pointed out: Why would you than create a pull request when making changes to the develop branch? I think it is the same reason.

Rewriting or not recording history with rebase, squash, fast-forward, etc., is always going to mess up versioning. Try to avoid it.

I think I can work around this scenario and make a rebase from the feature branch (before merging it to develop and before deleting it).

[HHobeck]

Thank you anyway for your input.

[HHobeck]

@asbjornu I have found the following code in the ConfigurationBuilder:

    AddBranchConfig(Config.FeatureBranchKey,
        new BranchConfig
        {
            Regex = Config.FeatureBranchRegex,
            SourceBranches = new HashSet<string> { Config.DevelopBranchKey, Config.MainBranchKey, Config.ReleaseBranchKey, Config.FeatureBranchKey, Config.SupportBranchKey, Config.HotfixBranchKey },
            Increment = IncrementStrategy.Inherit,
            PreReleaseWeight = 30000
        });

Here you can see that a feature branch can have a release branch as a source. I don't understand your argument. Do you can explain it why do you think a release branch is isolated?

[asbjornu]

Yes. The source branches of feature is documented as well:

    source-branches: [ 'develop', 'main', 'release', 'feature', 'support', 'hotfix' ]

But as you can see, tracks-release-branches is also set to false for feature. It may help if you set that to true.

[HHobeck]

[Test]
public void __Just_A_Test__()
{
    using var fixture = new EmptyRepositoryFixture();
    fixture.Repository.MakeATaggedCommit("1.0.0");

    // create branch from main to develop
    fixture.BranchTo("develop");
    fixture.Repository.MakeACommit();
    fixture.Checkout("main");

    // create branch from develop to release/1.1.0
    fixture.Checkout("develop");
    fixture.BranchTo("release/1.1.0");
    fixture.Repository.MakeACommit();
    fixture.Checkout("main");

    // create branch from develop to release/1.2.0
    fixture.Checkout("develop");
    fixture.BranchTo("release/1.2.0");
    fixture.Repository.MakeACommit();
    fixture.Checkout("main");

    fixture.Checkout("release/1.1.0");
    fixture.Repository.MakeACommit();

    var configuration = new Config()
    {
        Branches = new Dictionary<string, BranchConfig>()
        {
            { "release", new BranchConfig() { TracksReleaseBranches = true } }
        }
    };
    fixture.AssertFullSemver("1.2.0-beta.1+2", configuration); // 1.1.0 expected
}

I have tested it and when you set the property TracksReleaseBranches to true than you have side effects like this scenario above. I think GitVersion dosn't support it (feature/pull branches with release branch as source) at this moment.

[asbjornu]

Why would you set tracks-release-branches to true for release branches? Shouldn't the configuration rather be something like this?

var configuration = new Config
{
    Branches = new Dictionary<string, BranchConfig>()
    {
        {
            "feature",
            new BranchConfig
            {
                TracksReleaseBranches = true,
                SourceBranches = new HashSet<string>
                {
                    Config.DevelopBranchKey,
                    Config.MainBranchKey,
                    Config.ReleaseBranchKey,
                    Config.FeatureBranchKey,
                    Config.SupportBranchKey,
                    Config.HotfixBranchKey
                },
            }            
        }
    }
};

[HHobeck]

The result is the following (same with pull-request branch):

[Test]
public void __Just_A_Test___()
{
    using var fixture = new EmptyRepositoryFixture();
    fixture.Repository.MakeATaggedCommit("1.0.0");

    // create branch from main to develop
    fixture.BranchTo("develop");
    fixture.Repository.MakeACommit();
    fixture.Checkout("main");

    // create branch from develop to release/1.1.0
    fixture.Checkout("develop");
    fixture.BranchTo("release/1.1.0");
    fixture.Repository.MakeACommit();
    fixture.Checkout("main");

    // create branch from release/1.1.0 to feature/just-a-test
    fixture.Checkout("release/1.1.0");
    fixture.BranchTo("feature/just-a-test");
    fixture.Repository.MakeACommit();
    fixture.Checkout("main");

    // create branch from develop to release/1.2.0
    fixture.Checkout("develop");
    fixture.BranchTo("release/1.2.0");
    fixture.Repository.MakeACommit();
    fixture.Checkout("main");

    // checkout feature/just-a-test
    fixture.Checkout("feature/just-a-test");
    fixture.Repository.MakeACommit();

    var configuration = new Config()
    {
        Branches = new Dictionary<string, BranchConfig>()
        {
            { "feature", new BranchConfig() {
                TracksReleaseBranches = true,
                SourceBranches = new HashSet<string>
                {
                    Config.DevelopBranchKey,
                    Config.MainBranchKey,
                    Config.ReleaseBranchKey,
                    Config.FeatureBranchKey,
                    Config.SupportBranchKey,
                    Config.HotfixBranchKey
                }
            } }
        }
    };
    fixture.AssertFullSemver("1.0.0-just-a-test.1+4", configuration); // 1.1.0 expected
}

[asbjornu]

I haven't investigated why, but I managed to get something close to what you're expecting with the following configuration:

var configuration = new Config
{
    Branches = new Dictionary<string, BranchConfig>
    {
        { Config.ReleaseBranchKey, new BranchConfig  {  TracksReleaseBranches = true } }
    }
};

That is, by configuring release/* to track release branches, feature/* branches branched from release/* derives theeir version number from their parent release/* branch. I've also added a few more version assertions that are now successful, which you can see below:

[Test]
public void __Just_A_Test___()
{
    var configuration = new Config
    {
        Branches = new Dictionary<string, BranchConfig>
        {
            { Config.ReleaseBranchKey, new BranchConfig  {  TracksReleaseBranches = true } }
        }
    };

    using var fixture = new EmptyRepositoryFixture();
    fixture.Repository.MakeATaggedCommit("1.0.0");

    // create branch from main to develop
    fixture.BranchTo("develop");
    fixture.Repository.MakeACommit();
    fixture.Checkout("main");
    fixture.AssertFullSemver("1.0.0", configuration);

    // create branch from develop to release/1.1.0
    fixture.Checkout("develop");
    fixture.AssertFullSemver("1.1.0-alpha.1", configuration);

    fixture.BranchTo("release/1.1.0");
    fixture.Repository.MakeACommit();
    fixture.Checkout("main");
    fixture.AssertFullSemver("1.0.0", configuration);

    // create branch from release/1.1.0 to feature/just-a-test
    fixture.Checkout("release/1.1.0");
    fixture.BranchTo("feature/just-a-test");
    fixture.Repository.MakeACommit();
    fixture.AssertFullSemver("1.1.0-just-a-test.1+1", configuration);

    fixture.Checkout("main");

    // create branch from develop to release/1.2.0
    fixture.Checkout("develop");
    fixture.BranchTo("release/1.2.0");
    fixture.Repository.MakeACommit();
    fixture.Checkout("main");

    // checkout feature/just-a-test
    fixture.Checkout("feature/just-a-test");
    fixture.Repository.MakeACommit();

    fixture.AssertFullSemver("1.2.0-just-a-test.1+3", configuration);
}

[HHobeck]

Yep that's true it brings me a little bit nearer. :) But still I would expect the version 1.1.0 in your last scenario. It's working when you have two target releases (develop and one release branch) but if you have more than one release branch than it's not working. To figure this out it takes me some hours of debugging. But thank you that you try to find a solution for my problem.

I'm also not sure if it is worth to make a proposal for a change. I'm a sophisticated .net developer and would help to make this happened. But this seems to me a fundamental change and not so easy to implement... Because the behavior of IncrementStrategy.Inherit should be changed in my opinion (Or I have to create a new one!??). In my opinion this flag should not only inherit the configuration properties... it should behave like the current branch (feature/just-a-test in this scenario) will be not existing and the commits have been made on the source branch (exactly the first source branch where the inherit flag is not set)... in this scenario the latest source branch which doesn't has the inherit flag is the release/1.1.0 branch.

Hope my thoughts are understandable.

[asbjornu]

#3151 seems to be a duplicate of this, or at least very relevant. So I think there is enough interest to fix this somehow. Changing the behavior of increment: Inherit is out of the question for v5, but may be something we can discuss for v6. I would prefer if we could figure out a solution that didn't require breaking changes or additions to the configuration, though.

As creating feature/* branches from release/* to me feels like a rather uncommon and specialized use-case, I think that changing the current behavior when it is detected should be okay. The question is whether solving this is going to break a great number of existing tests or not and whether detecting it is trivial enough to do in the current codebase.

@HHobeck, it would be great if you could discuss with @ni-fgenois and @ni-jsuchocki and figure out a solution that may be suitable to all of you. If that doesn't break a great number of existing tests and the code is reasonable, I will be happy to merge it.

[HHobeck]

I have taken a look into it and also invested some hours of my time. I came to the following solution:

public abstract class VersionStrategyBaseWithInheritSupport : VersionStrategyBase
{
    private readonly IRepositoryStore _repositoryStore;

    protected IRepositoryStore RepositoryStore => _repositoryStore;

    protected VersionStrategyBaseWithInheritSupport(IRepositoryStore repositoryStore, Lazy<GitVersionContext> context)
        : base(context) => _repositoryStore = repositoryStore.NotNull();

    public override IEnumerable<BaseVersion> GetVersions() => GetVersionsRecursive(Context.CurrentBranch);

    private IEnumerable<BaseVersion> GetVersionsRecursive(IBranch branch)
    {
        EffectiveConfiguration configuration = Context.GetEffectiveConfiguration(branch);
        if (configuration.Increment != IncrementStrategy.Inherit)
        {
            return GetVersions(branch, configuration);
        }
        else
        {
            BranchCommit branchCommit = _repositoryStore.FindCommitBranchWasBranchedFrom(branch, Context.FullConfiguration);
            return GetVersionsRecursive(branchCommit.Branch);
        }
    }

    public abstract IEnumerable<BaseVersion> GetVersions(IBranch branch, EffectiveConfiguration configuration);
}

The key point here is that we need to refactor the approach of determining the EffectiveConfiguration of a branch. If you ask me it should be dynamic dependent of the VersionStrategy implementation. Thus we need to remove the static logic from the GitVersionContextFactory class. If you ask me it should be a really simple logic and can be done by calling the function GetEffectiveConfiguration:

    public EffectiveConfiguration GetEffectiveConfiguration(IBranch branch)
    {
        BranchConfig? branchConfiguration = FullConfiguration.GetConfigForBranch(branch.Name.WithoutRemote);
        branchConfiguration ??= BranchConfig.CreateDefaultBranchConfig("Fallback").Apply(new BranchConfig
        {
            Regex = "",
            VersioningMode = FullConfiguration.VersioningMode,
            Increment = FullConfiguration.Increment ?? IncrementStrategy.Inherit
        });
        return new EffectiveConfiguration(FullConfiguration, branchConfiguration);
    }

The hole logic in BranchConfigurationCalculator needs to be removed because it makes every thing so complex. Also the comment in this class gives me the assurance that this is a good idea:

    // TODO I think we need to take a fresh approach to this.. it's getting really complex with heaps of edge cases
    private BranchConfig InheritBranchConfiguration(int recursions, IBranch targetBranch, BranchConfig branchConfiguration, ICommit? currentCommit, Config configuration, IList<IBranch>? excludedInheritBranches)
    {
   ...

Last but not least the implementation of VersionInBranchNameVersionStrategy:

public sealed class VersionInBranchNameVersionStrategy : VersionStrategyBaseWithInheritSupport
{
    public VersionInBranchNameVersionStrategy(IRepositoryStore repositoryStore, Lazy<GitVersionContext> context)
        : base(repositoryStore, context)
    {
    }

    public override IEnumerable<BaseVersion> GetVersions(IBranch branch, EffectiveConfiguration configuration)
    {
        string nameWithoutOrigin = NameWithoutOrigin(branch);
        if (Context.FullConfiguration.IsReleaseBranch(nameWithoutOrigin))
        {
            var versionInBranch = GetVersionInBranch(branch.Name.Friendly, Context.Configuration.GitTagPrefix);
            if (versionInBranch != null)
            {
                var commitBranchWasBranchedFrom = RepositoryStore.FindCommitBranchWasBranchedFrom(branch, Context.FullConfiguration);
                var branchNameOverride = Context.CurrentBranch.Name.Friendly.RegexReplace("[-/]" + versionInBranch.Item1, string.Empty);
                yield return new BaseVersion("Version in branch name", false, versionInBranch.Item2, commitBranchWasBranchedFrom.Commit, branchNameOverride);
            }
        }
    }

    private static Tuple<string, SemanticVersion>? GetVersionInBranch(string branchName, string? tagPrefixRegex)
    {
        var branchParts = branchName.Split('/', '-');
        foreach (var part in branchParts)
        {
            if (SemanticVersion.TryParse(part, tagPrefixRegex, out var semanticVersion))
            {
                return Tuple.Create(part, semanticVersion);
            }
        }
        return null;
    }

    private static string NameWithoutOrigin(IBranch branch) => branch.IsRemote && branch.Name.Friendly.StartsWith("origin/")
        ? branch.Name.Friendly.Substring("origin/".Length)
        : branch.Name.Friendly;
}

[HHobeck]

Of course I'm going to create a feature branch for that. But I would like to here your opinion about this first. @ni-fgenois and @ni-jsuchocki and @asbjornu

[asbjornu]

I like your observations and suggestions, @HHobeck. That part of the code is indeed in need of some serious love and refactoring. The only big question is how many existing tests we break by doing it. Please give it a go and submit a PR so we can run the tests and see what breaks. If we agree the PR solves the problem and is a better approach than what we have today, it will be merged. The only question is whether it will be a part of v5, v6 or v7 depending on how many tests it breaks. 🤞🏼

[HHobeck]

I try to find out a way with minimal breaking changes. I did already make some refactoring but it's not finished yet. At least the semi version should be equals after my change. ;) Anyway. I see a lot of bugs coming in and are existing which are in my opinion related to my issue and should be also resolved after this refactoring.

e.g.
#3020
#3187

I hope I'm the only one try to make this happened. Don't want to waste my time and running into a merge hell. I took the latest tagged version 5.10.3.

[HHobeck]

I have some problems pushing the changes to the server:

Pushing feature/3101
Error encountered while pushing branch to the remote repository: Git failed with a fatal error.
Git failed with a fatal error.
unable to access 'https://github.com/GitTools/GitVersion.git/': The requested URL returned error: 403

Could someone help me please?

[asbjornu]

You have to fork the repository and push to a branch in your fork. You then create a pull request from your fork.

[HHobeck]

Okay thank you for the link. :) I have created a PullRequest to the support/5.x branch. I'm not sure if it is mergeable because I have taken the version from the tag 5.10.3.

[HHobeck]

The way I'm calculating the effective configuration for each base version strategy implementation is highly recursive and works with inheritance from one branch configuration to another. This makes it highly configurable and we can support every workflow on the world and all workflows which are created in the future. Please notice that the BranchConfigurationCalculator has been removed in the current version. This was really a huge act and I invested much more time than I have planed. But anyway it would be really good and I would appreciate it to get help to bring this to production.

The important logic looks as following:

public abstract class VersionStrategyBaseWithInheritSupport : IVersionStrategy
{
    private readonly Lazy<GitVersionContext> contextLazy;

    protected GitVersionContext Context => contextLazy.Value;

    protected IRepositoryStore RepositoryStore { get; }

    protected VersionStrategyBaseWithInheritSupport(IRepositoryStore repositoryStore, Lazy<GitVersionContext> contextLazy)
    {
        this.contextLazy = contextLazy.NotNull();
        RepositoryStore = repositoryStore.NotNull();
    }

    IEnumerable<(SemanticVersion IncrementedVersion, BaseVersion Version)> IVersionStrategy.GetVersions()
    {
        foreach (var item in GetVersionsRecursive(Context.CurrentBranch, null, new()))
        {
            ////
            // Has been moved from BaseVersionCalculator because the effected configuration is only available in this class.
            Context.Configuration = item.Configuration;
            var incrementedVersion = RepositoryStore.MaybeIncrement(item.Version, Context);
            //

            if (Context.FullConfiguration.VersioningMode == VersioningMode.Mainline)
            {
                if (!(incrementedVersion.PreReleaseTag?.HasTag() != true))
                {
                    continue;
                }
            }

            yield return new(incrementedVersion, item.Version);
        }
    }

    private IEnumerable<(EffectiveConfiguration Configuration, BaseVersion Version)> GetVersionsRecursive(IBranch currentBranch,
        BranchConfig? childBranchConfiguration, HashSet<IBranch> traversedBranches)
    {
        if (!traversedBranches.Add(currentBranch)) yield break;

        var branchConfiguration = Context.FullConfiguration.GetBranchConfiguration(currentBranch.Name.WithoutRemote);
        if (childBranchConfiguration != null)
        {
            branchConfiguration = childBranchConfiguration.Inherit(branchConfiguration);
        }

        var branches = Array.Empty<IBranch>();
        if (branchConfiguration.Increment == IncrementStrategy.Inherit)
        {
            branches = RepositoryStore.GetTargetBranches(currentBranch, Context.FullConfiguration, traversedBranches).ToArray();

            if (branches.Length == 0)
            {
                var fallbackBranchConfiguration = Context.FullConfiguration.GetFallbackBranchConfiguration();
                if (fallbackBranchConfiguration.Increment == IncrementStrategy.Inherit)
                {
                    fallbackBranchConfiguration.Increment = IncrementStrategy.None;
                }
                branchConfiguration = branchConfiguration.Inherit(fallbackBranchConfiguration);
            }
        }

        if (branchConfiguration.Increment == IncrementStrategy.Inherit)
        {
            foreach (var branch in branches)
            {
                foreach (var item in GetVersionsRecursive(branch, branchConfiguration, traversedBranches))
                {
                    yield return item;
                }
            }
        }
        else
        {
            var effectiveConfiguration = new EffectiveConfiguration(Context.FullConfiguration, branchConfiguration);
            foreach (var baseVersion in GetVersions(currentBranch, effectiveConfiguration))
            {
                yield return new(effectiveConfiguration, baseVersion);
            }
        }
    }

    public abstract IEnumerable<BaseVersion> GetVersions(IBranch branch, EffectiveConfiguration configuration);
}

[arturcic]

🎉 This issue has been resolved in version 6.0.0 🎉
The release is available on:

• GitHub Release
• Chocolatey Package
• .Net Global Tool

Your GitReleaseManager bot 📦🚀