Add support for changelist property per branch for maven ci friendly versioning - closes #305 #314

[mmusenbr]

No description provided.

[mmusenbr]

Changed visibility to make sure the wrapper checkoutAndSetConfigForBranch is used in the future.

[mmusenbr]

Changed visibility to make sure the wrapper createAndCheckoutAndSetConfigForBranch is used in the future.

[mmusenbr]

Could you give a hint for the code-format/formatter you are using? I tried to resemble the current style as much as possible.

[aleksandr-m]

Could you give a hint for the code-format/formatter you are using? I tried to resemble the current style as much as possible.

The formatting looks good, great job! There is .editorconfig with some general rules.

Of course it is needed, so that the info is passed to the maven calls. But it may be more beautiful to append args in executeMavenCommand instead of manipulating the argLine.

Have you tried that? Why there is need for the property replacing in session.getProjectBuildingRequest().getUserProperties()?

[mmusenbr]

Of course it is needed, so that the info is passed to the maven calls. But it may be more beautiful to append args in executeMavenCommand instead of manipulating the argLine.

Have you tried that? Why there is need for the property replacing in session.getProjectBuildingRequest().getUserProperties()?

We need to tackle two use-case:

• the 'external' calls via executeMavenCommand, here we could use the additional args to pass, and get rid of the argLine-manipulation.
• the calls using reloadProject, for those there is the need for manipulating the properties from session.getProjectBuildingRequest().getUserProperties(). So the projectBuilder.build also makes use of the settings.

[mmusenbr]

... But it may be more beautiful to append args in executeMavenCommand instead of manipulating the argLine.

Have you tried that?

@aleksandr-m I have created a feature/add_maven_ci_friendly_changelist_handling__via_mvn_execute-branch which uses the approach to append the args in the executeMavenCommand (see commit ed83d03f for the diff to master) The property replacement is still necessary.
Please let me know, which approach do you prefer. I think the new implementation is a little bit more straight-forward, which I would prefer.

[mmusenbr]

@aleksandr-m this would be the diff between those two versions (argLine-manipulation vs append mvn args)

[aleksandr-m]

I tried to run first implementation and it always appended development changelist property for me. Maybe I need to configure something differently. Can you share configuration, goals needed to be run and expected results?

[mmusenbr]

I tried to run first implementation and it always appended development changelist property for me. Maybe I need to configure something differently. Can you share configuration, goals needed to be run and expected results?

Hi @aleksandr-m, thank for having a look.
For testing I had following config present:

<configuration>
  <verbose>true</verbose>
  <pushRemote>true</pushRemote>
  <useSnapshotInHotfix>false</useSnapshotInHotfix>
  <allowSnapshots>true</allowSnapshots>
  <versionProperty>revision</versionProperty>
  <skipUpdateVersion>true</skipUpdateVersion>
  <versionDigitToIncrement>1</versionDigitToIncrement>
  <gitFlowConfig>
    <productionBranch>master</productionBranch>
    <developmentBranch>dev</developmentBranch>
  </gitFlowConfig>
</configuration>

And then on eg a hotfix-branch I run: mvn -X gitflow:hotfix-finish -DhotfixVersion=1.23.1 -DproductionChangelistValue='' -DhotfixChangelistValue='' -DdevelopmentChangelistValue='-SNAPSHOT'

[mmusenbr]

@aleksandr-m excpected result would be, that dependent on which branch is currently checked out by the plugin, the changelist property gets set to whatever configured via productionChangelistValue, hotfixChangelistValue, releaseChangelistValue, developmentChangelistValue, featureChangelistValue, supportChangelistValue. If the value-property is not set, none will be configured for that branch.
You would see the property in the debug output for the mvn-calls. What you do not see in the debug-output is for the internal ProjectBuilder.build-calls.

[mmusenbr]

For example if I run mvn -X gitflow:hotfix-finish -DhotfixVersion=1.23.1 -DinstallProject=true -DproductionChangelistValue='' -DhotfixChangelistValue='' -DdevelopmentChangelistValue='-SNAPSHOT' | grep DEBUG | grep -e "git checkout" -e mvn, I see:

[DEBUG] git checkout hotfix/1.23.1
[DEBUG] mvn clean test -Dchangelist=
[DEBUG] git checkout master
[DEBUG] git checkout dev
[DEBUG] mvn -DgenerateBackupPoms=false -DnewVersion=1.23.1 org.codehaus.mojo:versions-maven-plugin:set-property -Dproperty=revision -Dchangelist=-SNAPSHOT
[DEBUG] mvn -DgenerateBackupPoms=false -DnewVersion=1.24.0-SNAPSHOT org.codehaus.mojo:versions-maven-plugin:set-property -Dproperty=revision -Dchangelist=-SNAPSHOT
[DEBUG] mvn clean install -Dchangelist=-SNAPSHOT

[mmusenbr]

@aleksandr-m I additionally updated the README.md to reflect the change. Please let me know if this is fine, or if you think more explanation is needed.

[mmusenbr]

Hi @aleksandr-m, do you already have some input to share about this PR? Or is it still not behaving properly on your side and you need more information?

[mmusenbr]

Hi @aleksandr-m, I prefer the property addition variant, so I decided to switch the content of the PR to that version. Both branches still exists in my repo for comparison and to use one or the other.

• Variant: argLine manipulation
• Variant: mvn execute properties (same as current PR content)
• Diff between the two versions

Testing and behavior in the end is the same for both.

[simontunnat]

I would also prefer the solution using the additional properties.
This should solve my use case as well.

I did not have any time to follow up on my related issue so I'm very grateful that you started working on it. :)

[mmusenbr]

Hi @aleksandr-m, I just wanted to let you know, that I published a fork of your repo based on 1.16.0 including the changes of this PR.
It was the easiest way for us to make the support for the changeset accessible without publishing it on various internal maven repositories. I do not intent to keep the fork available for long, just until we have a chance to integrate this in the official version.
I tried to make it visible in all the Readme, etc files, that this is just a fork and you are the originator, kept the copyright notices, ... Feel free to review the changes needed to fork the plugin and give some input, if you are not satisfied with the references. I will change that fast if requested and publish an updated version.

I will just keep the releases streamlined with yours and only add this changes. We only use the plugin internally without advertising the existence of the fork.

I hope that is fine, thank you very much!

[aleksandr-m]

Are you building dependency projects separately? Why do you call mvn install in CI/CD environment?
Also mvn test can be probably moved to separate stage in CI/CD build and skipped in plugin.

[mmusenbr]

Yes, dependency projects are build separately. We have multiple dependent modules,
mvn install is called by the plugin for my given example, just to make it more visible, what the change does. In our CI we do not have the installProject set.
Even if mvn test is moved out and mvn install does not run (which in our case is not), there is still the call to mvnSetVersion call, and the call via the ProjectBuilder which needs to correct changelist-property set.

And I know, that for full support of Maven-CI-friendly versioning, there would still be changes needed, that no -SNAPSHOT is set into the revision field, we currently run a replacer after the gitlfow-maven-plugin, but I just see, if there is a nice way to support that as well in the plugin.

[aleksandr-m]

Yes, dependency projects are build separately. We have multiple dependent modules,

Not sure how you can use project.version for the dependencies in that case, without some pom which builds all projects.

the call via the ProjectBuilder

What do you mean?

Calling versions-maven-plugin:set doesn't need existing dependencies.

that no -SNAPSHOT is set into the revision field, we currently run a replacer after the gitlfow-maven-plugin,

How do you end up with snapshot when you use useSnapshotInHotfix=false?

[mmusenbr]

Yes, dependency projects are build separately. We have multiple dependent modules,

Not sure how you can use project.version for the dependencies in that case, without some pom which builds all projects.

Why not with a streamlined/unified version number between the modules? But I simplified that a little bit for the example. We more or less push the version number of all modules on every release, but in practice we use properties like <module1.version>X.Y.Z${revision}</module1.version> which imho boils down to project.version if X.Y.Z is is the same for all modules always.

the call via the ProjectBuilder

What do you mean?

ProjectBuilder.build is used in reloadProject and this in eg getCurrentProjectVersion. And this build command resolved dependencies, where then the problem occurs.

Calling versions-maven-plugin:set doesn't need existing dependencies.
Did not check that, thanks for the info.

that no -SNAPSHOT is set into the revision field, we currently run a replacer after the gitlfow-maven-plugin,

How do you end up with snapshot when you use useSnapshotInHotfix=false?

Because if the hotfix is merged to dev as well in GitFlowHotfixFinishMojo.java, -SNAPSHOT is appended.

if (supportBranchName == null) {
    // SNIP
    // if release branch exists merge hotfix changes into it
    if (StringUtils.isNotBlank(releaseBranch)) {
        // SNIP
    } else if (!skipMergeDevBranch) {
        GitFlowVersionInfo developVersionInfo = new GitFlowVersionInfo(currentVersion);
        if (notSameProdDevName()) {
            // git checkout develop
            // SNIP

            // get next snapshot version
            final String nextSnapshotVersion = developVersionInfo.getSnapshotVersionString();

            // SNIP
            mvnSetVersions(nextSnapshotVersion);

            Map<String, String> properties = new HashMap<String, String>();
            properties.put("version", nextSnapshotVersion);

            // git commit -a -m updating for next development version
            gitCommit(commitMessages.getHotfixFinishMessage(), properties);
        }
    }
}

[simontunnat]

@aleksandr-m is there any chance you will merge this soon? :)

[aleksandr-m]

If I got it right, then the whole thing is needed for correct dependencies resolution during plugin run and main issue with that is the usage of changelist property which holds SNAPSHOT qualifier.

Personally, I don't see much benefit in using changelist property at all, especially when releases are not done manually.
Secondly, updating versions and all the git commands should work, even if dependencies cannot be resolved.

The more I dig into this the more it seems like workaround for very specific edge use case.