Add module info as per platform wiki #146
Conversation
…form/wiki/Modularized-Jars Signed-off-by: Scott M Stark <[email protected]>
| @@ -50,7 +48,8 @@ | |||
| <maven-enforcer-plugin.version>3.0.0-M3</maven-enforcer-plugin.version> | |||
| <build-helper-maven-plugin.version>3.1.0</build-helper-maven-plugin.version> | |||
| <maven-project-info-reports-plugin.version>3.0.0</maven-project-info-reports-plugin.version> | |||
| <jakarta.transaction-api.version>2.0.0</jakarta.transaction-api.version> | |||
| <jakarta.transaction-api.version>2.0.1-RC1</jakarta.transaction-api.version> | |||
| <jakarta.cdi-api.version>3.0.1</jakarta.cdi-api.version> | |||
There was a problem hiding this comment.
There are no changes to the EJB API for Jakarta EE 10, so why is a dependency on CDI being added?
There was a problem hiding this comment.
Also, I would assume this would need to be for EJB 4.0.1-RC1 until transactions releases 2.0.1.
There was a problem hiding this comment.
JTA has a transitive dependency on CDI. Building without the dependency produces:
[INFO] Compiling 82 source files to /Users/starksm/Dev/JBoss/Jakarta/rh-ejb/api/target/classes
[INFO] -------------------------------------------------------------
[ERROR] COMPILATION ERROR :
[INFO] -------------------------------------------------------------
[ERROR] module not found: jakarta.cdi
There was a problem hiding this comment.
JTA has a transitive dependency on CDI. Building without the dependency produces:
[INFO] Compiling 82 source files to /Users/starksm/Dev/JBoss/Jakarta/rh-ejb/api/target/classes [INFO] ------------------------------------------------------------- [ERROR] COMPILATION ERROR : [INFO] ------------------------------------------------------------- [ERROR] module not found: jakarta.cdi
Already approved
- Add module-info.java #141
compiles fine, and does not add CDI dependency.
There was a problem hiding this comment.
I was going to comment on the dependency on cdi as well. I think it will automatically pull in the transitive dependencies.
There was a problem hiding this comment.
It is not provided to the compiler because the pom level dependency of transactions on CDI is provided scope, and maven does not include this in the dependency graph:
[INFO] --- maven-dependency-plugin:2.8:tree (default-cli) @ jakarta.ejb-api ---
[INFO] jakarta.ejb:jakarta.ejb-api:jar:4.0.1-SNAPSHOT
[INFO] \- jakarta.transaction:jakarta.transaction-api:jar:2.0.1-RC1:compile
[INFO] ------------------------------------------------------------------------
| <arg>-Xlint:all</arg> | ||
| </compilerArgs> | ||
| <showDeprecation>true</showDeprecation> | ||
| <showWarnings>true</showWarnings> |
There was a problem hiding this comment.
The PR #141 has a review comment from Emily about it needing to be an MR JAR. I don't really see why that is needed. If that is required then I would expect something like this to be configured for the release 11 execution:
<outputDirectory>${project.build.outputDirectory}/META-INF/versions/9</outputDirectory>
If no MR JAR is required then we might as well use PR #141
There was a problem hiding this comment.
I agree; I don't see why a MR JAR would be needed here. The guidelines referenced in the description indicate the following:
If your spec is not updating the API and producing a service release that needs to support Java SE 8,
run two passes of the compiler-plugin to produce the SE 8 based content, followed by a pass to
produce the SE 9 based module-info.class.
PR #141 appears to follow the guidelines exactly, producing Java 8 content, except for a Java 9 module-info.class.
The guidelines then provide an example of another project which seems to produce a MR JAR, but I don't understand what advantage that would have for enterprise beans, and would just double the size of the API JAR.
There was a problem hiding this comment.
#141 is also an MR jar, but it does not depend on the correctly modularizied transactions artifact which is seen when compiling that module-info:
[WARNING] /Users/starksm/Dev/JBoss/Jakarta/rh-ejb/api/src/main/java/module-info.java:[22,32] requires transitive directive for an automatic module
That is also why the jakarta.transactions transitive dependency on CDI is not seen and thus required to compile.
There was a problem hiding this comment.
I commented on the change not following the same style as other specs: see here for the other spec doing
|
So we can go with #141 for EE10, but it is not a complete solution to the JPMS integration. The reality is that enterprise-beans has a provided dependency on several other specifications that are not listed in its API artifact pom: annotations, interceptors, DI, CDI. |
Add the module-info using an MR jar as per the guidelines at:
https://github.com/eclipse-ee4j/jakartaee-platform/wiki/Modularized-Jars
Signed-off-by: Scott M Stark [email protected]