otherDeploymentBranchPattern doesn't work with multi-module builds and ${project.version}

[robth]

When a branch matches otherDeploymentBranchPattern, the normalized branch name will be appended to the POM version. Under the hood, the plugin uses project.setVersion() to set the version. It does not change the POM.

For multi-module builds and builds that use ${project.version}, this causes problems since parent versions and ${project.version} are not set to the correct version (i.e. the version including the branch name).

For example, a module referencing its parent:

<parent>
  <groupId>test</groupId>
  <artifactId>parent</artifactId>
  <version>1.0.0-SNAPSHOT</version> <!-- Note that the branch name is missing -->
</parent>

For example, a (parent) POM with dependency management of its modules:

<dependencyManagement>
  <dependencies>
    <dependency>
      <groupId>test</groupId>
      <artifactId>module1</artifactId>
      <version>${project.version}</version> <!-- Note that project.version won't contain the branch name -->
    </dependency>
  </dependencies>
</dependencyManagement>

I don't see a simple way to solve this, or am I missing something? The only solution I see is to change the POM file. That can be done either by the user (see https://github.com/egineering-llc/gitflow-helper-maven-plugin/pull/83), or automatically by the plugin.

What do you think?

[bvarner]

Yikes.

I've tried to avoid imposing persistent POM manipulations as a consequence of building projects. I'd like to treat the POM as an immutable file, and I'd like the build tools to be able to do that.

Any chance you'd be willing to create a test case that clearly duplicates this (and obviously fails during the integration tests)?

[robth]

Yes, no problem, I will write a test case and offer a PR.

[robth]

@bvarner How would you like to continue with this issue? It's blocking feature branch deployment. My proposal is to just let enforce-versions check whether the branch name is part of the artifact version. Then it's up to the user to set the correct version. It's not ideal, but it should work. What do you think?

[bvarner]

Sorry for the ridiculously slow response -- I may need to draft another maintainer. :-)

So you'd have enforce-versions check if the branch name is part of the artifact version and then... do what? I'm not sure I follow here.

I'm currently working on a project where we're just talking about pushing feature branch artifacts to the snapshots repo so that other projects can use poc's deployed there. Our plan at this point was to have that dependency managed by the devs, in the pom, with the expectation that there would have to be a non-snapshot release of that feature before the dependent project could continue.

We've so far avoided having a parent pom -- although that's been part of our discussions as well.

[bvarner]

I ran into a similar case the other day, using the quarkus maven plugin to build a quarkus project.

Since then, I've been mulling over what to do.

Currently, the version setting happens in the retarget deploy mojo -- very very late in the build cycle.

The version manipulation should probably happen in an extension. In this case, we could manipulate the project models in memory before any goal execution begins.

This would include all the projects being built as part of the current session. I think this would actually make the intended features work as expected.

[bvarner]

So...

I've been playing around with this idea in a branch, and what I come down to is:

All the projects and their metadata can be updated before the reactor actually kicks off -- if you manipulate versions in an extension. This allows parents, inter-dependencies, plugins, and multi-module projects etc. to all work quite well.

The problem then becomes with the resultant pom that gets attached / deployed. It's not reflecting those changes, because it's based upon the File contents of the originating pom.

The extension manipulations I've been doing are all on the in-memory Project model.... which is nice, in that it's transient and leaves the original POM the reactor was built from in place and unaltered.

What I'm thinking about: is having the RetargetDeployMojo continue to set the snapshot repository, but also write the massaged copy of the project POM to the target directory, then set that as the active pom file on the in-memory project. This should make 'everything just work'.

Other outside projects could use the version as a dependency. Multi-modules and parents could manage versions explicitly.

The only thing I haven't tried out locally yet is the writing of the massaged pom. I'll get that to this week, if things allow.

[bvarner]

@robth, I've pushed an in-progress branch that's currently missing the automated test cases, but if you have any interest in testing this out, here's where I'm currently at:

https://github.com/egineering-llc/gitflow-helper-maven-plugin/tree/feature/other-branch-reversioning-extension

It looks like this works pretty well.

[glimmerveen]

@bvarner as I am interested in this as well I had a look at the branch and tried it out myself. It looks pretty good IMO. I have also seen it play nice with other maven plugins that use/rely on the project version, and correctly 'see' the re-written version.

Some remarks:

• The 'massaged' pom is put next to the regular pom.xml file. Personally I am not too keen on plugins creating files outside the build directory. Would it be possible to create this pom file in the build directory (target folder)?
• When looking at a multi-module project, I noticed a small difference between my pom.xml file and the massaged pom.xml file. The former inherited the version from the parent declaration, whilst the latter would also include an explicit declaration of the version of the child as well. The end result (the version of the child) is the same, so I guess it is not an issue.

How close do you see this to being finished? Any aspects you'd like to see tested? I am willing to help anywhere I can!

[glimmerveen]

I have done some additional experiments and noticed that the re-written version is visible for plugins when they obtain that version from the injected MavenProject. If the version is supplied to the plugin through its configuration as the property ${project.version}, then it is resolved to the original value.

For example see this use of the maven-antrun-plugin:

            <plugin>
                <artifactId>maven-antrun-plugin</artifactId>
                <version>3.0.0</version>
                <executions>
                    <execution>
                        <phase>process-classes</phase>
                        <configuration>
                            <propertyPrefix>mvn.</propertyPrefix>
                            <target>
                                <echo>${project.version}</echo>
                                <echo>${mvn.project.version}</echo>
                            </target>
                        </configuration>
                        <goals>
                            <goal>run</goal>
                        </goals>
                    </execution>
                </executions>
            </plugin>

Will result in:

[INFO] --- maven-antrun-plugin:3.0.0:run (default) @ child2 ---
[INFO] Executing tasks
[WARNING]      [echo] 1.0-SNAPSHOT
[WARNING]      [echo] 1.0-feature-test-SNAPSHOT
[INFO] Executed tasks

The ${project.version} returns the original project version value, whilst the ${mvn.project.version} returns the expected re-written project version value.

I noticed while debugging these from one of the Extensions's afterProjectRead callbacks, that the configuration of the maven-antrun-plugin already had the property project.version expanded to the original value of the project version.

<?xml version="1.0" encoding="UTF-8"?>
<configuration>
  <propertyPrefix>mvn.</propertyPrefix>
  <target>
    <echo>1.0-SNAPSHOT</echo>
    <echo>${mvn.project.version}</echo>
  </target>
</configuration>

That explains why ${mvn.project.version} expands to the expected (re-written) version, as that value is obtained by the maven-antrun-plugin from the MavenProject.

Note sure if this can be addressed or not, but is at least something to be aware of.

[glimmerveen]

After integration with Jenkins I got following warning from Jenkins' Maven probe:

[WARNING] [jenkins-event-spy] Unexpected Maven project file name 'pom-gitflow-massaged.xml', problems may occur

This probe is aware of the fact that Maven plugins could re-configure/alter the pom file and tries to find 'the right one'. In some cases it uses the alternative one, and in others it uses the original one. To address this warning we could see if the Maven probe can be extended to include an specific branch for this plugin as well (it already has some hardcoded branches to deal with various other maven plugins), or the location and name of the massaged pom files should be made configurable. The latter would have my preference, as it should allow the user to also relocate this generated file towards the target folder, for example towards target/gitflow/pom.xml. In that case the Maven probe should stay silent (as the path ends with /pom.xml) and the generated file would be in the target folder. What do you think of this approach @bvarner ?

[glimmerveen]

I have a final observation and that is related to the use of profiles. It appears that if a pom contains a profile that is activated with a file (exists/missing), that the 'massaged' pom will contain the resolved absolute path rather than the path defined in the source pom file. This may be an issue in Maven itself, as I think the MavenProject.getOriginalModel() should have returned a Model completely unresolved. I have posted a question on the Maven mailinglist to see if this is indeed an issue with Maven itself, or that the plugin needs to address.

[glimmerveen]

With regards to the massaged pom file containing resolved (absolute) paths for file-based profile activation: this is indeed an issue in Maven itself. There is a PR open at Maven (apache/maven#347) to solve this issue.

[rondefreitas]

Any updates regarding this? I'm running into the same issue

[glimmerveen]

@rdefreitas : as far as I can tell, we are still waiting for @bvarner to have a look at the provided feedback.

From the last couple of months using the proposed changes I found one additional limitation that can be quite annoying. Suppose you have the following multi-module structure:

parent
|- child1
|- child2
`- child3

And the following dependencies between the child modules:

child3 -> child2
child2 -> child1

Building the project from the parent's basedir works as expected: all the dependencies between the child modules are properly rewritten and have the feature suffix added. However, if, after building the entire project locally using mvn clean install, you want to build just child3 again for instance, it will fail to recognise that it should rewrite the dependency to child2, and will fallback to using the version in the pom file (so without the feature suffix).

In case of large multi-module projects and/or projects with long build times times, having to build the entire multi-module project can become very time consuming.

[rondefreitas]

@glimmerveen @bvarner this sounds like a case in which it might be worth allowing the new functionality to have a configuration flag and marked as an experimental feature. (unless of course the change is so pervasive that's impossible).

These issues sound a bit too large in scope for one individual to handle and may require multiple iterations based on user feedback. It looks like the branch has been stagnant for 8 months.

I might be able to take a look myself later in the week. We run our builds inside of Azure Devops, so I can provide feedback on any oddities I see on that platform once I get this branch published to our internal maven snapshot feed.

[glimmerveen]

@rdefreitas I guess the feature toggle is already present, if you configure <otherDeployBranchPattern/>, this functionality is disabled. To my knowledge this functionality has never worked for multi-module projects, and the last limitation I mentioned is not really an issue for a project without modules.

I agree it is a tricky piece of functionality to get right, so additional testing is greatly appreciated.

Together we may be able to extend the proposed changes with support for building a partial set of modules in a multi-module project.

[bvarner]

My apologies for not giving this the attention it deserves -- it sounds like this solves some problems (while potentially creating plenty more corner cases) for some of you.

If I've skimmed through this and am reading (and recalling my own) notes properly -- (January seems so long ago) the branch was in a 'workable' state.

If everyone on this thread could do me a favor and thumbs-up this comment if you're OK with me pushing a release of that branch, I'll work to make that happen -- if there's unresolved issues that will be huge blockers or cause things to fail, please thumbs-down and comment specifically what the issue is, and we'll start addressing those more... directly.

[glimmerveen]

My apologies for not giving this the attention it deserves -- it sounds like this solves some problems (while potentially creating plenty more corner cases) for some of you.

No worries! I appreciate the work you've already put into this!

If I've skimmed through this and am reading (and recalling my own) notes properly -- (January seems so long ago) the branch was in a 'workable' state.

If everyone on this thread could do me a favor and thumbs-up this comment if you're OK with me pushing a release of that branch, I'll work to make that happen -- if there's unresolved issues that will be huge blockers or cause things to fail, please thumbs-down and comment specifically what the issue is, and we'll start addressing those more... directly.

I think it will help to have a release of the current state. I agree it is in a workable state, and having the feature available in a release will allow more people to test and provide feedback. I am looking at the limitation I mentioned a couple of days ago, to see if there is a way to solve this, but I don't mind that solution being part of a future release.

[glimmerveen]

@bvarner I think I have been able to address the limitation I mentioned about partial building of multi-module project. I opened PR #120. Could you take a look at it, and determine whether this can be merged into the feature you are working on?

[rondefreitas]

@bvarner a release of this will be helpful... I've been able to play with this a bit locally but the real catch is getting it to work on our build server environment, and unfortunately I'm having some issues dealing with our current environment and trying to publish and consume custom builds of plugins

[glimmerveen]

Hi @bvarner, any progress on getting a 3.1.0 release of gitflow with the proposed changes?

Also I'd like to bring to your attention PR #120 that removes an, in my opinion, big limitation related to partial building of multi-module projects. I believe including this PR makes the feature more user friendly, so I would prefer to have it included in the upcoming release as well, but I will leave it up to your discretion whether this is feasible and desirable.

[rondefreitas]

Speaking of which, I discovered an issue with the otherBranch functionality that can allow the branch name to be applied to the snapshot more than once in certain scenarios. I've added a fix here #122

[glimmerveen]

@bvarner do you agree that with the suggested deltas from #120 and #122 , we can move forward to merging branch feature/other-branch-reversioning-extension and cutting a 3.1.0 release?

[rondefreitas]

Haven't had the chance to come back to this. Is this issue considered fully resolved now with the current release? I'm still running our custom build based on the changes proposed here last here.

[glimmerveen]

Hi @rondefreitas , sorry for this (very) late reply. I have been using a locally forked version of this relatively successfully. The main issue I encounter with this functionality is related to IntelliJ import of a project when it is on a branch matching the othterDeploymentBranchPattern. In case of a multi-module project, some (a bit older) IntelliJ versions would allow inter-module dependencies to resolve properly to the IDEA modules in the same project, however the later versions (2021.x) unfortunately does not work as desired: dependencies are always resolved towards the local Maven repository. This is ofcourse very inconvenient, as the linked aspect of a multi-module project is no longer directly linked inside your IDE. There is a way around this, by configuring the following as VM options for importer: -Dgitflow.skip.extension. This disables the magic and ensures that IntelliJ understands the project again. This does mean that IntelliJ no longer 'sees' the rewritten version, but is able to properly link the modules inside the IDE based on the inter-module dependencies.

[glimmerveen]

Hi @rondefreitas and @bvarner , while using this feature for a longer period, there are some changes I think are needed, in order for the OtherBranchVersionExtension to run more reliably, especially in combination with IntelliJ IDEA's Maven integration. I opened PR #128 with the proposed changes.

[glimmerveen]

I guess that with the merging of PR #126 and #128 this issue can be closed as well. There is an open PR #134 with some improvements, but I think the requested capability is now present in development.