Open wheene opened 5 years ago
:warning: PR #86 broke certain dependency setups:
I am using cxf-codegen-plugin
to generate java classes from wsdl files. For this plugin, I defined the following jaxb dependencies:
<plugin>
<groupId>org.apache.cxf</groupId>
<artifactId>cxf-codegen-plugin</artifactId>
<version>3.0.16</version>
<configuration>
<!-- ... -->
</configuration>
<dependencies>
<dependency>
<groupId>org.glassfish.jaxb</groupId>
<artifactId>jaxb-runtime</artifactId>
<version>2.2.11</version>
</dependency>
<dependency>
<groupId>org.glassfish.jaxb</groupId>
<artifactId>jaxb-xjc</artifactId>
<version>2.2.11</version>
</dependency>
<!-- ... -->
</dependencies>
</plugin>
When buliding the respective module with the current 1.2.0-SNAPSHOT version of flatten-plugin (which incorporates PR #86), everything works as expected.
But if other modules (without cxf-codegen-plugin
) are built before this module, the binding process fails with:
java.lang.ClassNotFoundException: com.sun.xml.bind.v2.model.impl.ModelBuilderI
Running mvn -X ...
revealed the following:
[INFO] --- cxf-codegen-plugin:3.0.16:wsdl2java (VidyoServices) @ middleware-common-wsdl ---
[WARNING] The POM for org.glassfish.jaxb:jaxb-runtime:jar:2.2.11 is invalid, transitive dependencies (if any) will not be available: 1 problem was encountered while building the effective model for org.glassfish.jaxb:jaxb-runtime:2.2.11
[ERROR] 'dependencyManagement.dependencies.dependency.systemPath' for com.sun:tools:jar must specify an absolute path but is ${tools.jar} @
[WARNING] The POM for org.glassfish.jaxb:jaxb-xjc:jar:2.2.11 is invalid, transitive dependencies (if any) will not be available: 1 problem was encountered while building the effective model for org.glassfish.jaxb:jaxb-xjc:2.2.11
[ERROR] 'dependencyManagement.dependencies.dependency.systemPath' for com.sun:tools:jar must specify an absolute path but is ${tools.jar} @
Both jaxb artifacts share the same root parent jaxb-parent which contains:
<!-- ... -->
<dependencyManagement>
<dependencies>
<!-- ... -->
<!-- JDK dependencies -->
<dependency>
<groupId>com.sun</groupId>
<artifactId>tools</artifactId>
<version>1.6</version>
<scope>system</scope>
<systemPath>${tools.jar}</systemPath>
</dependency>
</dependencies>
</dependencyManagement>
<!-- ... -->
<profile>
<id>default-tools.jar</id>
<activation>
<file>
<exists>${java.home}/../lib/tools.jar</exists>
</file>
</activation>
<properties>
<tools.jar>${java.home}/../lib/tools.jar</tools.jar>
</properties>
</profile>
<profile>
<id>default-tools.jar-mac</id>
<activation>
<file>
<exists>${java.home}/../Classes/classes.jar</exists>
</file>
</activation>
<properties>
<tools.jar>${java.home}/../Classes/classes.jar</tools.jar>
</properties>
</profile>
<!-- ... -->
This default-tools.jar
profile is (obviously) not activated anymore and I am pretty sure that this is caused by the following block: https://github.com/mojohaus/flatten-maven-plugin/blob/master/src/main/java/org/codehaus/mojo/flatten/FlattenMojo.java#L737-L773
As there is no new DefaultModelBuilder
instance anymore per execution/module, the injector/selector installation now has a negative global effect.
I strongly recommend to revert df25d03a4d6c06c4de5cfd9f52dfbe72e823e403 for now and to rethink the approach.
You are right it would have this effect as before there was one builder per invocation while with #86 there is one per session. I would dare to say though that if you depend on there being dedicated instance per invocation the problem goes deeper than PR 86 which just exposed the problem.
I'm talking about this line https://github.com/mojohaus/flatten-maven-plugin/blob/master/src/main/java/org/codehaus/mojo/flatten/FlattenMojo.java#L188 where the whole MOJO is declared as executionStrategy = "once-per-session"
whereas you'd need it to be "always"
.
@hohwille what is the reason for the Mojo to be "once-per-session"
? Would something else break if it were changed to "always"
?
@famod could you try if using "always"
strategy would solve your problem?
@wheene
could you try if using
"always"
strategy would solve your problem?
Unfortunately, this doesn't solve the problem.
PS:
Would something else break if it were changed to
"always"
?
At least all tests (including integration tests) are passing.
@famod @wheene @hohwille
Can someone states what's next on that issue?
Is changing the strategy to "always"
enough to make it work and have a release with #86 included?
@McFoggy
Is changing the strategy to
"always"
enough to make it work
No, this doesn't work for me. I just re-checked with Maven 3.6.2 with and without "always"
.
I created a PR that resets ProfileSelector
and ProfileInjector
so that the flatten-plugin specific selector and injector do not leak into the rest of the build process.
I am asking myself whether the mutation of the apparently singleton (?) DefaultModelBuilder
poses a thread safety issue as well.
One flatten-plugin execution might be in the process of building the model while another concurrently running execution might have already reset selector and injector.
This has to checked! Edit: As expected, concurrently running executions operate on the very same DefaultModelBuilder
instance!
A simple synchronized
would help in this scenario but what about aother plugins and components mutating DefaultModelBuilder
?
At this point I am asking myself whether it would be more safe and enough for jgitver
to switch back to the old solution of building a dedicated DefaultModelBuilder
, but with the currently "active" ModelProcessor
(via injection)?
@McFoggy & @wheene WDYT?
Apart from the newly found concurrency issue that is caused by this issue (its PR respectively): This already went into 1.2.0 and should be closed, no? @olamy @lasselindqvist
FlattenMojo
obtainsModelBuilder
instance by usingnew
directly, making it impossible to use custom impl of this interface provided by extensions. I hit this problem when combined with jgitver - tool for dynamically synthesizing project version from git, implemented as maven extension. Flatten plugin does not see versions produced by the extension leading to build failures.Problem occurs in versions
1.0.1
,1.1.0
and1.2.0-SNAPSHOT
from current master (ac9eddb349691d91ad96188dbf01272b57b07efb)Pull request which suggests solution exists here #86. Reproducer case can be found here https://github.com/wheene/jgitver-flatten.
cc @McFoggy