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

track-merge-target in branch config not working #1789

Closed
rose-a opened this issue Aug 21, 2019 · 21 comments · Fixed by #3406
Closed

track-merge-target in branch config not working #1789

rose-a opened this issue Aug 21, 2019 · 21 comments · Fixed by #3406
Labels
Milestone

Comments

@rose-a
Copy link
Contributor

rose-a commented Aug 21, 2019

I'm trying to create a GitVersion config for GitLab Flow.

In GitLab Flow master is effectively the development branch and the production branch resembles master in Git Flow.

Since GitLab allows the automatic creation of issue branches from within its GUI (branch name starting with the GitLab issue number), I created a new branch type for them borrowing the feature branch config.

The resulting config looks like this:

mode: ContinuousDeployment
branches:
  master:   
    tag: alpha
    regex: ^master$
    increment: Minor
    prevent-increment-of-merged-branch-version: false
    track-merge-target: true
    tracks-release-branches: true
    is-release-branch: false
    is-mainline: false
    source-branches: []
  production:
    tag: ''
    increment: Patch
    prevent-increment-of-merged-branch-version: true
    track-merge-target: false
    regex: ^production$
    source-branches:
    - master
    - develop
    - release
    tracks-release-branches: false
    is-release-branch: false
    is-mainline: true
  issue:
    tag: issue-{BranchName}
    increment: Inherit
    prevent-increment-of-merged-branch-version: false
    regex: ^(?=\d+-)
    source-branches:
    - master
    - develop
    - feature
    - hotfix
    - support
    - issue
    - production
ignore:
  sha: []

The one thing which is currently not working is track-merge-target: true on the master branch config, which, according to the docs should do the following:

track-merge-target

Strategy which will look for tagged merge commits directly off the current branch. For example develop → release/1.0.0 → merge into master and tag 1.0.0. The tag is not on develop, but develop should be version 1.0.0 now.

So with the provided config I would expect master → merge into production and tag 1.0.0 to result in master incrementing to 1.1.0 on the next commit.

Do I understand the intention of the track-merge-target setting correctly?

PS: Trying to understand the behaviour from the source code I stumbled upon the following:

grafik

So basically the setting is read but never used! Is this feature actually implemented?

@stale
Copy link

stale bot commented Nov 19, 2019

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 Nov 19, 2019
@rose-a
Copy link
Contributor Author

rose-a commented Nov 19, 2019

Can anybody help with this?

@stale stale bot removed the stale label Nov 19, 2019
@asbjornu
Copy link
Member

Sorry, but you may have stumbled across a bug here. If you are able to submit a pull request that fixes the bug, we will of course merge it, but I don't have time to look at this anytime soon, I'm afraid.

@stale
Copy link

stale bot commented Feb 17, 2020

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 Feb 17, 2020
@rose-a
Copy link
Contributor Author

rose-a commented Feb 25, 2020

Maybe don't let it auto-close and track this as a bug so it might get implemented someday in the future?

@stale stale bot removed the stale label Feb 25, 2020
@arturcic
Copy link
Member

@rose-a The only way we can get this one fixed is when someone will send us a PR we can review and merge, unfortunately we don't have too much time to fix it so a PR from your side it welcomed.

@rose-a
Copy link
Contributor Author

rose-a commented Feb 25, 2020

@arturcic yeah I get that, no pressure... but just letting it go stale and auto-close won't fix things... don't know how you can tell your stale bot to leave it alone, maybe by adding a bug label or something 😉

@asbjornu asbjornu added the bug label Feb 25, 2020
@asbjornu
Copy link
Member

@rose-a, yes, I've been thinking about doing that for a while. Thanks for reminding me. #2135 created to do just that. :)

@L-U-C-K-Y
Copy link

Thanks for this issue, I have also been looking for quite some time for a tool that supports GitLab Flow.

Can someone point us in the right direction where we have to take a look?

Strategy which will look for tagged merge commits directly off the current branch.

I'm not experienced in C#, but are we looking at the GetTaggedVersions function that should take the current branch with the track-merge-target option set?

internal IEnumerable<BaseVersion> GetTaggedVersions(Branch currentBranch, DateTimeOffset? olderThan)

@dxynnez
Copy link

dxynnez commented Sep 15, 2021

I think I am hitting the same problem...

I am testing the behavior locally with:

next-version: 3.0.0
branches
  TestRelease:
    regex: ^TestRelease$
    is-mainline: true
    source-branches: [ 'TestDevelop' ]
    mode: ContinuousDelivery
    tag: ''
    increment: Minor
    prevent-increment-of-merged-branch-version: true
    track-merge-target: false
    tracks-release-branches: false
    is-release-branch: true
  TestDevelop:
    regex: ^TestDevelop$
    is-mainline: false
    source-branches: [ ]
    mode: ContinuousDelivery
    tag: beta
    increment: Patch
    prevent-increment-of-merged-branch-version: false
    track-merge-target: true
    tracks-release-branches: true
    is-release-branch: false

TestDevelop & TestRelease are on 3.0.0 and I then merge a new commit from TestDevelop into TestRelease branch with a merge commit (no-ff). Then I tag the merge commit with a new version number 3.1.0

According to the doc:

track-merge-target
Strategy which will look for tagged merge commits directly off the current branch. For example develop → release/1.0.0 → merge into main and tag 1.0.0. The tag is not on develop, but develop should be version 1.0.0 now.

I was expecting TestDevelop to start from 3.1.0 at this point, but adding another new commit to TestDevelop and run gitversion still give me 3.0.1, which, didn't seem to honor the tag on the merge commit.

Did I just misunderstand the doc or it's indeed a bug? @asbjornu is there any insight you can share on this? I am more than happy to submit a PR to fix it if you can point me to a start point.

@asbjornu
Copy link
Member

Does the behaviour change if you remove next-version, @dxynnez? I'm suspecting its presence may infer a different versioning strategy that is possibly ignoring track-merge-target.

@asbjornu
Copy link
Member

This may be related to #3190. It may also be worth trying to change the configuration so production is identified as main and main is identified as develop.

@HHobeck
Copy link
Contributor

HHobeck commented Sep 10, 2022

Please see the discussion in #3052. I would like to fix this issue and know for sure (like you already said) that the track-merge-target is without function. This feature has replaced with an implicit logic wish full-fill the requirement at least for the gitflow workflow. But I agree it would be nice to support all workflows and give you the possibility to make it configurable like you want.

@HHobeck
Copy link
Contributor

HHobeck commented Sep 10, 2022

In TrackReleaseBranchesVersionStrategy.cs you find the following logic:

    private IEnumerable<BaseVersion> MainTagsVersions()
    {
        var configuration = this.context.Value.Configuration.Configuration;
        var main = this.repositoryStore.FindMainBranch(configuration);

        return main != null
            ? this.taggedCommitVersionStrategy.GetTaggedVersions(main, null)
            : Array.Empty<BaseVersion>();
    }

That means with this strategy the tagged version will be determined from the main branch. The question is how the code detects the main branch?

    public IBranch? FindMainBranch(Config configuration)
    {
        var mainBranchRegex = configuration.Branches[Config.MainBranchKey]?.Regex
            ?? configuration.Branches[Config.MasterBranchKey]?.Regex;

        if (mainBranchRegex == null)
        {
            return FindBranch(Config.MainBranchKey) ?? FindBranch(Config.MasterBranchKey);
        }

        return this.repository.Branches.FirstOrDefault(b =>
            Regex.IsMatch(b.Name.Friendly, mainBranchRegex, RegexOptions.IgnoreCase));
    }

The answer is it is hard coded with the magic string main and master. If we change the logic that we detect the main branch using the existing BranchConfiguration.IsMainline property it would give you the possibility to make it configurable. I'm just wondering if it is possible to have more than one main branches? What about support branch?

@HHobeck
Copy link
Contributor

HHobeck commented Sep 10, 2022

This may be related to #3190. It may also be worth trying to change the configuration so production is identified as main and main is identified as develop.

Actually I like the idea to have a pre-defined workflow template which can be versioned and changed without affecting other workflows.

@asbjornu
Copy link
Member

The answer is it is hard coded with the magic string main and master. If we change the logic that we detect the main branch using the existing BranchConfiguration.IsMainline property it would give you the possibility to make it configurable.

Yep, sure.

I'm just wondering if it is possible to have more than one main branches? What about support branch?

I suppose so. I believe a branch being considered main only affects itself and its own versioning strategy. I don't think other branches should be affected by one or more branches being configured as main.

@HHobeck
Copy link
Contributor

HHobeck commented Sep 12, 2022

I suppose so. I believe a branch being considered main only affects itself and its own versioning strategy. I don't think other branches should be affected by one or more branches being configured as main.

Except that the TrackReleaseBranchesVersionStrategy class works not only on the current branch. It tries to get the tagged version explicit from the main or master branch configuration. My suggestion would be each of the following variants:

  1. Either we change the precondition and allow only one branch marked with IsMainline flag and adapt the RepositoryStore::FindMainBranch using this flag
  2. Or we are returning a list of branches marked as IsMainline and doing a foreach

What do you think?

@asbjornu
Copy link
Member

Sorry, I got a bit confused there for a moment. The is-mainline flag is set to indicate which branch is the main branch when mode is set to Mainline. That shuld only be one branch. I don't think there are any valid trunk-based development workflows that have two trunk (main) branches. So I don't think it makes much sense to have is-mainline: true for more than one branch.

Now, being a mainline branch is not the same as being a main branch, as all of the different workflows have a main branch in some form or another. In those, it may make sense to have more than one main branch – I don't really know. Either way, in other workflows than Mainline, which branches are main are only configured for the main key in GitVersion.config, not with is-mainline: true.

I know this is confusing, but that's where we are. It would probably be better to have a typed configuration system that was wholly dependent on mode and that blew up if you tried to add keys that didn't make any sense in the mode chosen.

@HHobeck
Copy link
Contributor

HHobeck commented Sep 22, 2022

Sorry, I got a bit confused there for a moment. The is-mainline flag is set to indicate which branch is the main branch when mode is set to Mainline. That shuld only be one branch. I don't think there are any valid trunk-based development workflows that have two trunk (main) branches. So I don't think it makes much sense to have is-mainline: true for more than one branch.

Now, being a mainline branch is not the same as being a main branch, as all of the different workflows have a main branch in some form or another. In those, it may make sense to have more than one main branch – I don't really know. Either way, in other workflows than Mainline, which branches are main are only configured for the main key in GitVersion.config, not with is-mainline: true.

I know this is confusing, but that's where we are. It would probably be better to have a typed configuration system that was wholly dependent on mode and that blew up if you tried to add keys that didn't make any sense in the mode chosen.

Okay if I understand you correct we have branches with the property IsMainline=true for:

  • exactly one main branch (where the name can be changed in configuration) in the trunkbase workflow (with VersioningMode Mainline)
  • maybe zero one or more branches in other workflows (with Versioning Mode ContinuousDelivery or Deplyoment)

Is this correct? I'm just wonder what a trunkbase workflow makes so special that we need a dedicated VersioningMode for that and if this is really import for this issue. At the end to come back to the problem of the author it is just important to detect the tagged commits on branches which are marked with IsMainline=true independent of its name.

@asbjornu
Copy link
Member

Okay if I understand you correct we have branches with the property IsMainline=true for:

  • exactly one main branch (where the name can be changed in configuration) in the trunkbase workflow (with VersioningMode Mainline)

  • maybe zero one or more branches in other workflows (with Versioning Mode ContinuousDelivery or Deplyoment)

Is this correct?

Afaik, yes.

I'm just wonder what a trunkbase workflow makes so special that we need a dedicated VersioningMode for that and if this is really import for this issue.

Good question. I'm not sure, to be honest. I've never used mode: Mainline myself.

At the end to come back to the problem of the author it is just important to detect the tagged commits on branches which are marked with IsMainline=true independent of its name.

From my understanding, Mainline should not require tags at all. See #950 for its initial implementation in GitVersion.

@arturcic
Copy link
Member

arturcic commented Mar 2, 2023

🎉 This issue has been resolved in version 6.0.0-beta.1 🎉
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
Labels
Projects
None yet
6 participants