Skip to content
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

Dependency on outdated JTA API #608

Closed
manovotn opened this issue May 16, 2022 · 6 comments · Fixed by #609
Closed

Dependency on outdated JTA API #608

manovotn opened this issue May 16, 2022 · 6 comments · Fixed by #609
Assignees

Comments

@manovotn
Copy link
Contributor

The latest release of CDI 4 has a dependency on jakarta.transaction-api in version 2.0.0.
That artifact is actually not Jakarta EE 10 (there is no such artifact as of now it seems?) and it in turn depends on CDI 3.0, see this POM.
The dependency is not even optional.

This causes a cyclic dependency that has been mentioned in the jakarta-platform mailing list - link to the archive. This is nothing new and the dep has existed prior to our release.

As far as I can tell, we could completely remove the JTA API dependency as it is only used in javadoc inside TransactionPhase enum.
Granted, the javadoc won't be as nice but we'll get rid of the dep altogether. OTOH, JTA certainly cannot remove dep on CDI API, so we are the only ones who can break that cycle.

So we have two choices:

  • Remove the dependency
  • Upgrade the dependency (once there is EE 10 version)

Personally, I think we can easily remove the dependency.

Either way, we should then have an SP release and I am not sure what would that take- cc @starksm64.

@Ladicek
Copy link
Contributor

Ladicek commented May 16, 2022

+1 to remove.

@starksm64
Copy link
Contributor

The EE10 version of JTA is 2.0.1. There is no upgrade in EE10.

We cannot build the current javadoc without this dependency:

[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-javadoc-plugin:3.3.0:jar (attach-javadocs) on project jakarta.enterprise.cdi-api: MavenReportException: Error while generating Javadoc: 
[ERROR] Exit code: 1 - /Users/starksm/Dev/JBoss/Jakarta/cdi/api/src/main/java/jakarta/enterprise/event/TransactionPhase.java:28: error: reference not found
[ERROR]  * If the transaction is in progress, but {@link jakarta.transaction.Synchronization} callback cannot be registered due to the transaction being already
[ERROR]                                                  ^
[ERROR] /Users/starksm/Dev/JBoss/Jakarta/cdi/api/src/main/java/jakarta/enterprise/event/TransactionPhase.java:29: error: reference not found
[ERROR]  * marked for rollback or in state where {@link jakarta.transaction.Synchronization} callbacks cannot be registered, the {@link #BEFORE_COMPLETION},
[ERROR]                                                 ^
[ERROR] /Users/starksm/Dev/JBoss/Jakarta/cdi/api/src/main/java/jakarta/enterprise/event/TransactionPhase.java:54: error: reference not found
[ERROR]      * but {@link jakarta.transaction.Synchronization} callback cannot be registered due to the transaction being already
[ERROR]                   ^
[ERROR] /Users/starksm/Dev/JBoss/Jakarta/cdi/api/src/main/java/jakarta/enterprise/event/TransactionPhase.java:55: error: reference not found
[ERROR]      * marked for rollback or in state where {@link jakarta.transaction.Synchronization} callbacks cannot be registered.
[ERROR]                                                     ^
[ERROR] /Users/starksm/Dev/JBoss/Jakarta/cdi/api/src/main/java/jakarta/enterprise/event/TransactionPhase.java:66: error: reference not found
[ERROR]      * but {@link jakarta.transaction.Synchronization} callback cannot be registered due to the transaction being already
[ERROR]                   ^
[ERROR] /Users/starksm/Dev/JBoss/Jakarta/cdi/api/src/main/java/jakarta/enterprise/event/TransactionPhase.java:67: error: reference not found
[ERROR]      * marked for rollback or in state where {@link jakarta.transaction.Synchronization} callbacks cannot be registered.
[ERROR]                                                     ^
[ERROR] /Users/starksm/Dev/JBoss/Jakarta/cdi/api/src/main/java/jakarta/enterprise/event/TransactionPhase.java:79: error: reference not found
[ERROR]      * but {@link jakarta.transaction.Synchronization} callback cannot be registered due to the transaction being already
[ERROR]                   ^
[ERROR] /Users/starksm/Dev/JBoss/Jakarta/cdi/api/src/main/java/jakarta/enterprise/event/TransactionPhase.java:80: error: reference not found
[ERROR]      * marked for rollback or in state where {@link jakarta.transaction.Synchronization} callbacks cannot be registered.
[ERROR]                                                     ^
[ERROR] 
[ERROR] Command line was: /Library/Java/JavaVirtualMachines/jdk-11.0.13.jdk/Contents/Home/bin/javadoc @options @packages @argfile
[ERROR] 
[ERROR] Refer to the generated Javadoc files in '/Users/starksm/Dev/JBoss/Jakarta/cdi/api/target/apidocs' dir.
[ERROR] 
[ERROR] -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
[ERROR] 
[ERROR] After correcting the problems, you can resume the build with the command
[ERROR]   mvn <args> -rf :jakarta.enterprise.cdi-api 

This class really should be a cdi additional library produced by the JTA project, but we are where we are. There also should be a javadoc scope for dependencies to make it clear the nature of the dependency. It is not a dependency of the API in any hard sense.

We can discuss this in the CDI meeting and platform group call before doing anything about it.

@manovotn
Copy link
Contributor Author

@starksm64 I agree and I have sent a PR where I changed the {@link } notation to {@code } at which point we can remove the dependency. But I do intend to keep that open until the meeting tomorrow.

@manovotn
Copy link
Contributor Author

In the meeting it was agreed that we would also want to remove EJB dependency and rewrite any code links to just {@code }.
I will update the PR accordingly today or later tomorrow.

@starksm64
Copy link
Contributor

Sorry, I already merged the JTA PR. Add a new one for EJB removal.

@manovotn
Copy link
Contributor Author

I was away from PC yesterday and didn't get to it earlier.
Here it is #612

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants