Wrong insignificant tag generated when using maintenance branches with patch releases

[jnehlmeier]

Consider the following commit graph which I have used to test a workflow using reckon:

The reckon configuration is very basic:

reckon {
    stages('dev', 'final')
    scopeCalc = calcScopeFromProp().or(calcScopeFromCommitMessages())
    stageCalc = calcStageFromProp()
}

The workflow can be described as:

• development takes place in master branch
• occasionally a version from master branch will be released as dev release to a small number of customers.
• At some point a dev release is considered releasable and a final release is done. This can result in a dev and final tag on the same commit.
• For final releases a maintenance/<major>.<minor>.x branch will be created which is used for patch releases
• Patches for final releases are either done in the maintenance branch and cherry picked into master or the other way around.
• maintenance branches and it's tags will be deleted if they are old enough and nobody uses that version anymore

Now as you can see in the above image I have started with a 1.0.0 release as a base point for reckon. Then some dev releases has been made for some new features until 1.1.0-dev.3 becomes a final release 1.1.0. Development continues and the next 1.2.0-dev.1 release has been published.
Now a critical bug has been discovered in 1.1.0. A patch has been done on its maintenance/1.1.x branch, a new release 1.1.1 has been published and the patch has been cherry picked into master. A new 1.2.0-dev.2 release has been done and that one also becomes the final release 1.2.0.

So this worked pretty well because we have a new feature on master branch after the 1.1.0 release has been cut. Thus reckon switches to 1.2.0-.... tags.

Then I checked which tag reckon generates when only patches are on the master branch after release 1.2.0 has been cut. These patches are cherry picked from 1.2.1, 1.2.2 and 1.2.3 (maintenance branch).

What irritates me is that reckon now chooses version 1.2.2-dev.0.3+3c73188 for a ./gradlew build on the latest master commit which seems like a bug to me, as it is based on a hotfix tag between other hotfix tags.

I would have expected either

• 1.2.1-dev.0.3+<hash> because it would be the next tag if only respect master branch tags. However this version would cause confusion as it conflicts with hotfix releases
• 1.2.4-dev.0.3+<hash> if reckon chooses to respect tags from other branches. However commit number is tough to choose in this scenario.

I think the only real solution to that second scenario is to check if something like a release/maintenance/hotfix branch exists, and if it does, do a minor version bump even if only patch commits are on the master branch. This prevents overlapping versions and the minor version bump will happen anyways at some point once the first new feature has been committed to master.

To help implement such a check, reckon would need a configuration property telling it the prefix of such a release branch, e.g. maintenance/.

[ajoberstar]

Yeah, that definitely seems wrong for multiple reasons. Feels like it should be at least 1.2.4-dev instead of 1.2.2-dev if 1.2.3 is already out. But you're saying you'd really want it to be 1.3.0-dev in that case?

I've tried to avoid baking branch names into resolution, so I'll want to see if there's a way around it in this situation.

EDIT: Misread, I see the ones you called out as expected.

Will make some tests to reproduce and also see if the commit message inference plays any role in the issue.

[jnehlmeier]

But you're saying you'd really want it to be 1.3.0-dev in that case?

Yeah my thinking was:

If only patches follow a final release on the master branch, then these patches must be hotfixes of that final release. As a developer I don't see a reason why I would do a significant dev release from master branch with these hotfixes. Instead I will release them from a maintenance branch, either directly as a final release or with some testing period by making a significant version on the maintenance branch before making the final hotfix version on the maintenance branch. And I have to do it in a separate branch, because on master branch a new feature can land at any time.

As a developer I would only start a new significant dev release from master branch if I have committed at least one new feature.

Thus I think reckon could switch to insignificant 1.3.0-dev version if only patches are found since the last final release because reckon could assume that a new feature will land soon anyways. This has the benefit of escaping the 1.2.x maintenance release tags early and IMHO it would be fine for developers.

[ajoberstar]

FYI, I have a test case reproducing this now. Will look into a fix.

[ajoberstar]

@jnehlmeier Would appreciate any feedback on the solution described in #181 and released as 0.17.0-beta.1.

[jnehlmeier]

@ajoberstar I tried 0.17.0-beta.2 with some cases and it seems like it won't generate any versions that have already been taken. So it is definitely better than before.

However it is kind of annoying that the behavior of reckon isn't consistent now. The behavior changes depending on the number of tags I made in a parallel branch. So sometimes it does a patch increment and sometimes if will do a minor increment. Someone (co-workers) not knowing reckon well will have questions about it.

I feel like behavior should be consistent. If I see it correctly you do not want to make a loop to calculate a free version but instead you try it twice and if both are already taken you give up and increment the next higher scope. That is basically why behavior isn't consistent.

Maybe I am thinking too much in my workflow but I have a really hard time to imagine having a calculated version on master branch that "belongs" to a parallel maintenance branch. In my example above reckon will generate 1.3.0-dev since I have three tags on the maintenance/1.2.x branch. But if I delete tags 1.2.[1-3] and only create 1.2.1 tag on maintenance/1.2.x branch reckon will generate 1.2.2-dev. While technically ok, given I have only done patch commits on master branch, it still has a strong naming connection to the maintenance/1.2.x branch even though we are on master.

Maybe providing a configuration option that tells reckon to always increment the higher scope would be a solution to gain back some consistency. So when this option is active you only try to calculate a version once instead of twice before falling back to increment the next higher scope. The result is that as soon as a single tag has been made on a parallel branch, the next higher scope will be incremented.
Then there is only the situation having a parallel branch without any tags. In that case master branch and parallel branch will generate the same base version (but with different commit counts and hashes). Given that there is no information other than the branch existence and its name, I feel like that would be fine. Maybe one day reckon will also know about branch naming conventions and use that information for tag calculation as well.

[ajoberstar]

However it is kind of annoying that the behavior of reckon isn't consistent now. The behavior changes depending on the number of tags I made in a parallel branch. So sometimes it does a patch increment and sometimes if will do a minor increment. Someone (co-workers) not knowing reckon well will have questions about it.

Yeah that was my concern with this approach.

If I see it correctly you do not want to make a loop to calculate a free version but instead you try it twice and if both are already taken you give up and increment the next higher scope. That is basically why behavior isn't consistent.

What do you mean by a loop? Are you suggesting: first increment patch, then increment minor, then increment major until you find a free one? Might be misunderstanding you.

Maybe providing a configuration option that tells reckon to always increment the higher scope would be a solution to gain back some consistency. So when this option is active you only try to calculate a version once instead of twice before falling back to increment the next higher scope. The result is that as soon as a single tag has been made on a parallel branch, the next higher scope will be incremented.

So maybe an option around parallelBranchScope that you set to the scope of what your maintenance branches cover. e.g. In your case set parallelBranchScope=MINOR and if we find 1.2.1 on a parallel branch, we assume it claims all of 1.2.x, meaning, in your scenario, master would increment as a MINOR.

Similarly a parallelBranchScope=MAJOR would cover scenarios where a you only do maintenance branches for 2.x or 3.x.

parallelBranchScope=PATCH which would probably be the default would just assume parallels are targeting an exact version (e.g. branching off from mainline before all final releases).

Then there is only the situation having a parallel branch without any tags. In that case master branch and parallel branch will generate the same base version (but with different commit counts and hashes). Given that there is no information other than the branch existence and its name, I feel like that would be fine.

Yeah, for now this limitation would stay.

[jnehlmeier]

If I see it correctly you do not want to make a loop to calculate a free version but instead you try it twice and if both are already taken you give up and increment the next higher scope. That is basically why behavior isn't consistent.

What do you mean by a loop? Are you suggesting: first increment patch, then increment minor, then increment major until you find a free one? Might be misunderstanding you.

No. I was saying your current code tries to increment twice (which could be refactored into a loop with two iterations) before doing something else (incrementing using an incremented scope).
So if refactored into a loop with two iterations you could increase the iterations up to Integer.MAX_VALUE. Then you would find a targetNormal unless you have Integer.MAX_VALUE tags on the parallel branch. Only in that case you would increment the scope and use that new scope to increment the base normal.

Maybe providing a configuration option that tells reckon to always increment the higher scope would be a solution to gain back some consistency. So when this option is active you only try to calculate a version once instead of twice before falling back to increment the next higher scope. The result is that as soon as a single tag has been made on a parallel branch, the next higher scope will be incremented.

So maybe an option around parallelBranchScope that you set to the scope of what your maintenance branches cover. e.g. In your case set parallelBranchScope=MINOR and if we find 1.2.1 on a parallel branch, we assume it claims all of 1.2.x, meaning, in your scenario, master would increment as a MINOR.

Similarly a parallelBranchScope=MAJOR would cover scenarios where a you only do maintenance branches for 2.x or 3.x.

parallelBranchScope=PATCH which would probably be the default would just assume parallels are targeting an exact version (e.g. branching off from mainline before all final releases).

I kind of like that. Assuming I have configured parallelBranchScope=MINOR what would happen if I develop a new feature in a local feature branch that definitely does not have any tag and can live longer in parallel to the master branch until rebased onto / merged into the master?

Given my example image above, assume that maintenance/1.1.x is feature/f1 and maintenance/1.2.x is feature/f2 and both branches do not have any tags. So tags 1.1.1, 1.2.1, 1.2.2, 1.2.3 do not exist. That would be a typical scenario while developing new features locally.

• What would happen if we checkout feature/f1 and build the project?
• What would happen if we checkout feature/f2 and build the project?

[ajoberstar]

Parallel logic will only kick if there are tagged versions that are both:

• Not in the history of HEAD
• HEAD is not in the history of the tag

If no parallel version tags are found, the parallelBranchScope wouldn't kick in (nor would the existing double increment handling). So you would still see feature/f1 as a 1.1.1-dev version and feature/f2 as 1.2.1-dev release.

I'll code this up to make sure it behaves that way, but let me know if you expect something different to happen.

[ajoberstar]

Here's the new approach, which I'll release as 0.17.0-beta.3

e831ac3

[jnehlmeier]

If no parallel version tags are found, the parallelBranchScope wouldn't kick in (nor would the existing double increment handling). So you would still see feature/f1 as a 1.1.1-dev version and feature/f2 as 1.2.1-dev release.

I'll code this up to make sure it behaves that way, but let me know if you expect something different to happen.

That is what I would expect too, given that both these branches are branched off of an older commit. I assume if I would rebase both these feature branches on master (to get the latest master changes into the feature branch), both would have the same 1.2.1-dev version but with different commit hashes in its version, right? At least that is what I would expect.

I will play a bit with 0.17.0-beta.3, to see how it behaves and how it feels.

[ajoberstar]

I assume if I would rebase both these feature branches on master (to get the latest master changes into the feature branch), both would have the same 1.2.1-dev version but with different commit hashes in its version, right? At least that is what I would expect.

I'd expect the same

[jnehlmeier]

Hmm, I am pretty sure that the below example works as expected with the current solution, however it feels a bit weird (using parallelBranchScope: 'MINOR'):

This looks fine. We have two feature branches and version stays on 1.1.0-dev.2 when calculating insignificant versions.

Then I have added an additional commit to master branch and the situation was the same. Then I tagged that new commit on master using 1.1.0-dev.3 and then the version within feature branches change:

It feels weird because feature branches are usually branched off of old commits and are compared against the master branch. But the master branch can only have increasing versions and building an insignificant version on the feature branch can use the same base version as the newest tag found in the history of that feature branch. So in my example above it could very well stay on 1.1.0-dev.2 to compute insignificant versions. Only if I want to make a significant version on the feature branch (for whatever reason I can not think of) tags on master branch need to be checked for potential collisions.

Also if the master branch continues to evolve without rebasing the feature branch you might end up with the following:

Now we have old feature branches that use version 1.2.0-dev.0 but master is on 1.2.0-dev.1 or in the future 1.3.0-dev.1. Luckily insignificant versions on feature branch use .0 as counter and thus do not conflict with the master tag 1.2.0-dev.1 but a significant version on the feature branch creates a conflict and reckon produces error message Reckoned version 1.2.0-dev.1 has already been released..

So for release/maintenance branches parallelBranchScope works well. But for feature branches it doesn't.

[jnehlmeier]

On the other hand the error message tells me to rebase the feature branch because it has become too old to do a significant release from.

[ajoberstar]

For the feature branches, would you expect them to stay as 1.1.0-dev.2.5+abc123 and 1.1.0-dev.2.6+abc123? That was my initial thought too.

Presuming that is what you were thinking, this seems like a long-standing issue with the parallel logic and not really dependent on the new changes. Prior to this version, I think they would have still been bumped, but to 1.1.1-dev.0.5+abc123. We're just bumping to a higher number than previously.

What I'm wondering is maybe the parallel branch behavior shouldn't kick in unless it's a significant release? That way a feature branch, which you would never make a significant release on, would always just stay on the same version instead of trying to avoid the parallel. Though I wonder about main/master vs a release/maintenance branch. Will have to think some more.

[ajoberstar]

Interestingly, it was never documented or tested in a way where an insignificant had to use the same parallel avoidance. I'd still call it out as a breaking change if I went this way.

Two branches MUST NOT create tagged pre-releases for the same targeted final version.

If the branches have a shared merge base the version being inferred must skip the targeted final version and increment a second time.
If no shared merge base the inference should fail.

Here's a PR with what the logic change would be: #183

[jnehlmeier]

For the feature branches, would you expect them to stay as 1.1.0-dev.2.5+abc123 and 1.1.0-dev.2.6+abc123?

Yes that is what I would expect.

What I'm wondering is maybe the parallel branch behavior shouldn't kick in unless it's a significant release? That way a feature branch, which you would never make a significant release on, would always just stay on the same version instead of trying to avoid the parallel.

Yes that is why I said that for insignificant versions on a feature branch only the tag history of that feature branch is relevant because significant and final versions on master branch will always increase. So there is no way that an additional tag on master branch will conflict with insignificant versions on feature branches.

Though I wonder about main/master vs a release/maintenance branch. Will have to think some more.

On release/maintenance branches we continuously create pre-release (to pre-release fixes) or final versions as tags and we want to make sure that future master branch tags do not conflict with these. However during local development of fixes in the release/maintenance branch we also need to create insignificant versions to test fixes locally or on CI. Inference of these insignificant versions only need to check history of that release/maintenance branch (so the same behavior as for feature branches, which only has insignificant versions).

On feature branches we only create insignificant versions based on the tag history of the commit the feature branch is created from. Here it is safe to assume that the insignificant version is safe because the parallel branch will have tags with increasing versions.

So I think it seems fine to disable parallel branch logic for insignificant version inference. I am also thinking that reckon should forbid creating tags for insignificant versions (maybe it already does).

[ajoberstar]

0.17.0-beta.4 released with that change.

Reckon for quite a while has prohibited tagging insignificant releases, so we're good there.

[jnehlmeier]

I just played a bit with beta 4 and below is the result:

You can see

• Good: feature f3 + f4 branches being based on the previous 1.0.0 tag
• Good: maintenance branch produces good tags
• Questionable: feature branch f7 does increment the scope while master branch does not. However both branches share the same starting commit and should see the same in terms of history and parallel branches/tags. Master branch only increases the scope if tagging a significant.
• Questionable: Even though I marked feature branch f7 green, it might be the one that behaves wrong. That is because when building insignificants on master, f3 and f4 no scope increment will be done. Only f7 does so.

[jnehlmeier]

After thinking about it, I feel like feature/f7 should produce 1.1.0-dev.0.2+587b5d6 (1.1.0-..., because it does not have any commit info about patch/minor/major) just like any other feature branch does, simply for consistency reasons. So I would consider the version produced in the image above a bug.

But the master branch, which is the default branch, must honor the parallelBranchScope = MINOR configuration and produce 1.2.0-dev.0.5+4e7a16b if a parallel branch which conflicting tags exist (in the above case the maintenance/1.1.x branch). Currently it uses the new implementation and ignores parallel branch logic for insignificants. But IMHO that undermines the parallelBranchScope configuration and its documentation ("defines what versions a parallel branch owns").

I feel like master branch and feature branches should not behave the same because master branch is the default, working branch. On master branch insignificants must also apply parallel branch logic, otherwise it feels weird that a significant tag has a different base version (x.y.z) than an insignificant (see latest commit).
However I have no idea how you would distinguish features/f7 and master since both have no tags, both share a common commit from which f7 has been branched and both have a parallel branch maintenance/1.1.x "owning" versions up do MINOR scope.

I think reckon needs an option to ask for the default, working branch name.

[ajoberstar]

Sorry for the delay. I believe the inconsistency between master and features/f7 is down to defaultInferredScope (which is MINOR if not overridden). Since the master commits have scopes in the messages, but features/f7 commits don't, reckon's using the default of MINOR. I should clarify the docs that defaultInferredScope should be set to PATCH if you're using the commit message feature, otherwise it gets confusing. A long time ago, the default was patch, which maybe should be the default again before 1.0 of reckon or maybe it should just be a required setting with no default.

I think all that's left then is the question of whether master in this situation should be 1.1.1-dev.0.5+4e7a16b or 1.2.0-dev.0.5+4e7a16b. I think 1.2.0 is a reasonable intuition, but I'd rather have 1.1.1 be the result in this case.

All else equal, reckon should produce a version reflective of the code present in the commit. Given that it's nearest normal version is 1.1.0 and it has three patch commits, it's appropriate that in this insignificant versioning phase it would merely use what it knows from tags and commit history and use 1.1.1-dev.0.5+.4e7a16b. Generally, I would expect an actual minor commit to be merged to master before a significant release was made if you intended it to be 1.2.0. While that expectation should be somewhat relaxed pre-final, the parallelBranchScope would cover the desire once you got to making a significant release on master.

Given the earlier change to make feature branches work more naturally, the parallelBranchScope document should be updated to say "defines what significant versions a parallel branch owns".

Explicit first-class branch support isn't something I'm interested in adding to reckon. The nebula-release-plugin (which is a fork of the deprecated gradle-git release plugin that preceded reckon) does have explicit branch pattern support, so could be more intuitive to some.

[jnehlmeier]

@ajoberstar I have changed reckon to use defaultInferredScope = patch and see how it behaves using the commit graph of my previous post.

With that features/f7 will generate 1.1.1-dev.0.2+587b5d6 because no scope information is found in commit messages. That feels better and also makes it clear that parallelBranchScope does not kick in here. So with that change features/f7 and master behave the same in terms of versioning. f7 because it defaults to patch and master because it only contains patches.

So my only critique is that master produces 1.1.1.. insignificants. Changing the documentation of parallelBranchScope to only own significant versions seems like a bad solution, because there is a hierarchy of insignificant -> significant -> final version. If something owns significants, why doesn't it own insignificants then? So that documentation change wouldn't be logical in itself.

That is why I proposed a single, additional configuration property that asks for a single branch name representing the main development branch. On that branch parallelBranchScope will then also kick in for insignificant releases. Then it would feel significantly better when you build from master branch locally or during CI / nightly builds and no new features haven't landed on master branch yet. Why does it feel better? Because the version has escaped the maintenance/1.1.x branch and parallelBranchScope works logically.

I understand your argument that master should have a feature at some point and then the insignificant version would escape naturally to 1.2.0... but I can also see lots of cherry picked patches on master before that new feature lands and during that time I don't like seeing master version numbers conflict with a release branch in nightly builds created after a release has been cut.