-
Notifications
You must be signed in to change notification settings - Fork 654
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
[Bug] track-merge-changes
produces unexpected result when combining hotfix
and support
branches
#3052
Comments
track-merge-changes
produces unexpected result when combining hotfix
and support
branches
I'm not sure, but I think the problem may be that the tag |
@asbjornu yes, but main is not on version 1.7.1, it is still on version 1.7.0 in our case, so that is not an option. Can merge targets be configured? |
You can configure |
I'm not sure what your branching and merging strategy is exactly but it seems to be that the gitflow strategy maybe a good solution for you (see gitflow example here). Anyway if you ask me you should on the one hand create a dedicated hotfix branch from the source branch you want to place the bug fix and on the other hand create a hotfix branch with the target semantic version number (e.g. hotfix/1.7.1). At this moment you have tested your hotfix and ensure the stability of the release candidate (RC), you are going to merge the hotfix to the support branch (support/1.7) and delete the hotfix branch. That’s mean you declare this fix as resolved and released it to manufacturing (RTM) (e.g. tagging this commit with 1.7.1). Now if you have another bug (which needs to be hot fixed) you create an additional branch from the support branch with the name hotfix/1.7.2. In my opinion is this the same procedure like you would fix bugs in the main branch. |
I have taken a look in to the source code of GitVersion in version 5.10.1 and cannot find any usage of the TrackMergeTarget settings. In my opinion this property is not used and should be marked as deprecated. @asbjornu: Do you know if I have missed something? |
Hm, good question, @HHobeck. I'm not using that feature myself, did not implement it, and am not sure exactly how it's supposed to work. It may be that we didn't have tests covering that feature and the affected code has just been refactored away. Perhaps the best thing to do is to deprecate it entirely in v6. |
I have found the following logic changed by Jake Ginnivan in 2015 (see commit [1]): public class GitFlowDevelopBranchBaseVersionStrategy : HighestTagBaseVersionStrategy
{
protected override bool IsValidTag(GitVersionContext context, string branchName, Tag tag, Commit commit)
{
if (!string.IsNullOrWhiteSpace(branchName))
{
if (context.Configuration.TrackMergeTarget)
{
return IsDirectMergeFromCommit(tag, commit);
}
}
return base.IsValidTag(context, branchName, tag, commit);
}
static bool IsDirectMergeFromCommit(Tag tag, Commit commit)
{
var targetCommit = tag.Target as Commit;
if (targetCommit != null)
{
var parents = targetCommit.Parents;
if (parents != null)
{
return parents
.Where(parent => parent != null)
.Any(parent => string.Equals(parent.Id.Sha, commit.Id.Sha, StringComparison.OrdinalIgnoreCase));
}
}
return false;
}
} The configuration properties TrackMergeTarget was used to determine base versions in the scenario where a branch has been merged into the current branch or vice versa. In the latest code base I cannot find any usage. commit [1]:
|
I would like to understand the intention of the TrackMergeTarget property. @asbjornu May be you can give me your opinion about the following scenario: [Test]
public void __Just_A_Test__()
{
var configuration = new Config()
{
VersioningMode = VersioningMode.ContinuousDeployment,
Branches = new Dictionary<string, BranchConfig>()
{
{
"develop", new BranchConfig() {
TrackMergeTarget = true
}
}
}
};
using var fixture = new EmptyRepositoryFixture();
fixture.MakeACommit();
fixture.AssertFullSemver("0.1.0-ci.0", configuration);
fixture.BranchTo("develop");
fixture.AssertFullSemver("0.1.0-alpha.0", configuration);
fixture.MakeACommit();
fixture.AssertFullSemver("0.1.0-alpha.1", configuration);
fixture.BranchTo("release/1.0.0");
fixture.AssertFullSemver("1.0.0-beta.0", configuration);
fixture.MakeACommit();
fixture.AssertFullSemver("1.0.0-beta.1", configuration);
fixture.MakeACommit();
fixture.AssertFullSemver("1.0.0-beta.2", configuration);
fixture.Checkout("develop");
fixture.AssertFullSemver("0.1.0-alpha.1", configuration); // 1.1.0 expected
fixture.MakeACommit();
fixture.AssertFullSemver("1.1.0-alpha.1", configuration);
fixture.MergeNoFF("release/1.0.0");
fixture.AssertFullSemver("1.1.0-alpha.4", configuration);
fixture.Repository.Branches.Remove("release/1.0.0");
fixture.AssertFullSemver("1.1.0-alpha.4", configuration); // okay because TrackMergeTarget=true ??
configuration.Branches["develop"].TrackMergeTarget = false;
fixture.AssertFullSemver("1.1.0-alpha.4", configuration); // 0.1.0 expected because TrackMergeTarget=false ??
} From the release management point of view this means the following: You create a release 1.0.0 branch and want to stabilize the release candidate to bring this one to production. So what happened if your product manager says to cancel the release and go back to the develop phase? The release candidate 1.0.0 has not been merged and tagged on master. But it will be properly merged to develop and deleted. Thus the next version on the develop branch is still the next version of the previous version (that means in my example the version 0.1.0). Maybe it is a good idea to reuse this property and make the following change: /// <summary>
/// Version is extracted from older commits' merge messages.
/// BaseVersionSource is the commit where the message was found.
/// Increments if PreventIncrementForMergedBranchVersion (from the branch config) is false.
/// </summary>
public class MergeMessageVersionStrategy : VersionStrategyBase
{
private readonly ILog log;
public MergeMessageVersionStrategy(ILog log, Lazy<GitVersionContext> versionContext) : base(versionContext) => this.log = log.NotNull();
public override IEnumerable<BaseVersion> GetVersions()
{
if (Context.CurrentBranch.Commits == null || Context.CurrentCommit == null)
return Enumerable.Empty<BaseVersion>();
// new
if (!Context.Configuration.TrackMergeTarget)
return Enumerable.Empty<BaseVersion>();
... |
I'm also thinking about it in which circumstances the MergeMessageVersionStrategy is used in the gitflow worflow. Cannot find any scenario. Maybe it is good for the github workflow with continues delivery? |
Why would a |
I would recommend against its use for any other workflow than trunk based development aka |
I think it is quite common to merge from release to develop and not to master (at least not at the same time). Think about it if you finalize or stabilize a beta version and find issues. You will fix it on release and merge it back to the develop branch. Another scenario could be if you cancel the hole release or you decide to go back to the alpha phase. Please see my comment here |
Okay and why is the TrackMergeTarget property be true per default in the develop branch configuration? (Please see ConfigurationBuilder.cs) |
I don't see how |
I suppose that's a valid use-case, but it's going to be strange selective merges because I wouldn't want everything in a
As long as the |
What do you mean with the changelogs, version number increments, etc.? I cannot see any problems with that. A merge from release to develop is just a normal commit: [Test]
public void __Just_A_Test__()
{
var config = new Config() { NextVersion = "1.0.0" };
using EmptyRepositoryFixture fixture = new("develop"); // starting with develop branch
fixture.MakeACommit();
fixture.GetVersion(config).FullSemVer.ShouldBe("1.0.0-alpha.1");
fixture.BranchTo("release/1.0.0");
fixture.GetVersion(config).FullSemVer.ShouldBe("1.0.0-beta.1+0");
fixture.MakeACommit(); // <<-- first
fixture.GetVersion(config).FullSemVer.ShouldBe("1.0.0-beta.1+1");
fixture.MakeACommit(); // <<-- second
fixture.GetVersion(config).FullSemVer.ShouldBe("1.0.0-beta.1+2");
fixture.Checkout("develop");
fixture.GetVersion(config).FullSemVer.ShouldBe("1.1.0-alpha.0");
fixture.MakeACommit();
fixture.GetVersion(config).FullSemVer.ShouldBe("1.1.0-alpha.1");
fixture.MergeNoFF("release/1.0.0"); // <<-- third
fixture.GetVersion(config).FullSemVer.ShouldBe("1.1.0-alpha.4"); // +3 one merge and two commits from release
} |
Yep I agree with you. But when this scenario happens GitVersion should also know how to handle this. |
Yep that's the big question actually. ;) If you ask me MergeMessageVersionStrategy is the logic which should be called only when the TrackMergeTarget is set to true and this is only good for the main line mode. |
Here is an example which illustrates the behavior: [Test]
public void __Just_A_Test__()
{
var config = new Config() { NextVersion = "1.0.0" };
var configWithoutTrackMergeTarget = new Config()
{
NextVersion = "1.0.0",
Branches = new Dictionary<string, BranchConfig>()
{
{ "develop", new BranchConfig() { TrackMergeTarget = false } }
}
};
using EmptyRepositoryFixture fixture = new("develop"); // starting with develop
fixture.MakeACommit();
fixture.GetVersion(config).FullSemVer.ShouldBe("1.0.0-alpha.1");
fixture.BranchTo("release/1.0.0");
fixture.GetVersion(config).FullSemVer.ShouldBe("1.0.0-beta.1+0");
fixture.MakeACommit();
fixture.GetVersion(config).FullSemVer.ShouldBe("1.0.0-beta.1+1");
fixture.MakeACommit();
fixture.GetVersion(config).FullSemVer.ShouldBe("1.0.0-beta.1+2");
fixture.Checkout("develop");
fixture.GetVersion(config).FullSemVer.ShouldBe("1.1.0-alpha.0");
fixture.MakeACommit();
fixture.GetVersion(config).FullSemVer.ShouldBe("1.1.0-alpha.1");
fixture.MergeNoFF("release/1.0.0");
fixture.Repository.Branches.Remove("release/1.0.0");
fixture.GetVersion(config).FullSemVer.ShouldBe("1.1.0-alpha.4");
fixture.GetVersion(configWithoutTrackMergeTarget).FullSemVer.ShouldBe("1.0.0-alpha.5");
} |
Here is a better example: [Test]
public void __Just_A_Test__()
{
var config = new Config()
{
VersioningMode = VersioningMode.ContinuousDelivery,
Branches = new Dictionary<string, BranchConfig>()
{
{ "main", new BranchConfig() { TrackMergeTarget = true } }
}
};
var configWithoutTrackMergeTarget = new Config()
{
VersioningMode = VersioningMode.ContinuousDelivery,
Branches = new Dictionary<string, BranchConfig>()
{
{ "main", new BranchConfig() { TrackMergeTarget = false } }
}
};
using EmptyRepositoryFixture fixture = new("main"); // starting with main
fixture.MakeACommit();
fixture.ApplyTag("1.0.0");
fixture.GetVersion(config).FullSemVer.ShouldBe("1.0.0");
fixture.BranchTo("release/2.0.0");
fixture.GetVersion(config).FullSemVer.ShouldBe("2.0.0-beta.1+0");
fixture.MakeACommit();
fixture.GetVersion(config).FullSemVer.ShouldBe("2.0.0-beta.1+1");
fixture.MakeACommit();
fixture.GetVersion(config).FullSemVer.ShouldBe("2.0.0-beta.1+2");
fixture.Checkout("main");
fixture.MergeNoFF("release/2.0.0");
fixture.GetVersion(config).FullSemVer.ShouldBe("2.0.0+0");
fixture.Repository.Branches.Remove("release/2.0.0");
fixture.GetVersion(config).FullSemVer.ShouldBe("2.0.0+3");
// until you are not tagging it this is a hotfix
fixture.GetVersion(configWithoutTrackMergeTarget).FullSemVer.ShouldBe("1.0.1+3");
} |
What I mean is that in a
I see. Yes, that's certainly a valid way to look at it. I don't think that expectation holds in GitVersion today, though. I believe |
Good point. I agree with you that this is even more complicated as expected. But I think still that GitVersion can also track or even not track the bump message when it comes from a merged branch. I mean GitVersion generates a minor or major version change on master but it is not
One thing may be an idea to separate this and make it configurable. This would be easier to implement and better for the understanding. Concrete my proposal is to separate this two features and introducing a new configuration property like track-merge-message. Last but not least we need to either remove the feature track-merge-target or re-implement it in this way like it was build in the origin intention. |
But still could some one explain me for which scenario the feature track-merge-target is good for or give me a business use case please?
@Crown0815 Your example is not clear for me what you are trying to do. Which workflow applies in your example? |
@HHobeck we are using a custom workflow where we try to reduce dependencies between different branches and prevent "back-merges". It looks like this: You can see that the Simply put, All I want is to be able to merge a |
Okay thank you very much for outlining your branching and merging strategy. If I compare your custom workflow with the gitflow workflow then the difference is only how you treat the main, hotfix and support branch right!? The main branch represents the initial release (tagged as RTM) and the support branch contains the hotfix releases (also tagged as RTM of the same lifecycle). I don't understand exactly what your motivation behind it is and what advantages you have compared to the gitflow workflow. But anyway I don't want to challenge your motivation behind it. If I read the documentation about the track-merge-changes feature then I agree this might help you detecting the version in the hotfix branch which have been done in the support branch. But I'm not sure if it is the ideal way to detect the version in all circumstances because this feature is only restricted on the current branch. |
In my opinion this issue here is related to the issue #1789 and vice versa and both can be solved with the track-merge-targets feature. But like I have in the other discussion already pointed out the problem might be located in the TrackReleaseBranchesVersionStrategy that this class not detecting the tagged version on branches which are marked as IsMainlin=true. The irony of my suggestion is that not only the track-merge-target feature is broken the feature detecting the tagged version on mainline branches is broken as well. Another point is that the track-merge-target feature is from the performance perspective maybe not so optimal. TLDR: You should maybe define the support branch with IsMainline=true and implement the fix for TrackReleaseBranchesVersionStrategy detecting not only the hardcoded branch with the name master or main. |
🎉 This issue has been resolved in version 6.0.0-beta.1 🎉 Your GitReleaseManager bot 📦🚀 |
Describe the bug
It appears that
track-merge-changes
does not work when combininghotfix
andsupport
branches. I have the following testthat produces the following repository:
Expected Behavior
hotfix
is set totrack-merge-targets = true
; hence, I would expect its next version to be1.7.2-beta.1
Actual Behavior
Running the test produces:
should be
1.7.2-beta.1
but was
1.7.1-beta.2+2
Possible Fix
I suspect the merge target tracking is incomplete, or I am not configuring GitVersion correctly.
Steps to Reproduce
See test above
Context
I am attempting to merge
hotfix
branches intosupport
branches and have the version of thehotfix
branches bumped without requiring a merge back fromsupport
intohotfix
.Your Environment
Standard git repository
The text was updated successfully, but these errors were encountered: