-
Notifications
You must be signed in to change notification settings - Fork 21
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
otherDeploymentBranchPattern doesn't work with multi-module builds and ${project.version} #105
Comments
Yikes. I've tried to avoid imposing persistent POM manipulations as a consequence of building projects. Any chance you'd be willing to create a test case that clearly duplicates this (and obviously fails during the integration tests)? |
Yes, no problem, I will write a test case and offer a PR. |
@bvarner How would you like to continue with this issue? It's blocking feature branch deployment. My proposal is to just let |
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. |
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. This would include all the projects being built as part of the current session. |
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. 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. |
@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: It looks like this works pretty well. |
@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:
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! |
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. 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:
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. |
After integration with Jenkins I got following warning from Jenkins' Maven probe:
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. |
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. |
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. |
Any updates regarding this? I'm running into the same issue |
@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:
And the following dependencies between the child modules:
Building the project from the 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. |
@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. |
@Rdefreitas I guess the feature toggle is already present, if you configure 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. |
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. |
No worries! I appreciate the work you've already put into this!
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. |
@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 |
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. |
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 |
@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? |
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. |
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. |
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. |
When a branch matches
otherDeploymentBranchPattern
, the normalized branch name will be appended to the POM version. Under the hood, the plugin usesproject.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:
For example, a (parent) POM with dependency management of its modules:
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 #83), or automatically by the plugin.
What do you think?
The text was updated successfully, but these errors were encountered: