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

Maybe CDI dependency in module-info.java should be static #214

Open
starksm64 opened this issue Sep 4, 2023 · 15 comments · May be fixed by #212
Open

Maybe CDI dependency in module-info.java should be static #214

starksm64 opened this issue Sep 4, 2023 · 15 comments · May be fixed by #212

Comments

@starksm64
Copy link
Contributor

It has been brought up on the CDI dev list that the EE 10 release of Transactions has a hard dependency on CDI. This is due to the jakarta.transaction.Transactional and jakarta.transaction.TransactionScoped annotations. In the past it was possible to support Transactions without including CDI if a user did not make use of those annotations. Some runtimes do not support CDI, so perhaps the CDI dependency should be made optional as in:

module jakarta.transaction {
    requires java.rmi;
    requires transitive java.transaction.xa;
    requires static transitive jakarta.cdi;
    requires jakarta.interceptor;

    exports jakarta.transaction;
}
@Ladicek
Copy link

Ladicek commented Sep 5, 2023

I guess this also applies to the dependency on Interceptors (which probably doesn't need to be there, since the dependency on CDI is transitive).

@starksm64
Copy link
Contributor Author

Good point, that is right

@arjantijms
Copy link
Contributor

arjantijms commented Sep 6, 2023

I fully support this, but we do have the somewhat nasty technicality that in the Jakarta Transactions specification document the usage of CDI is not said to be optional at all.

At the very least I think we should change this in an update.

It's in chapter 3 now, as one of the many Jakarta Transactions features: https://jakarta.ee/specifications/transactions/2.0/jakarta-transactions-spec-2.0#transactional-annotation

We probably need a new chapter, like we have in the CDI spec as well that details things that are specific for usage within an EE/CDI environment.

@tomjenkinson
Copy link
Contributor

I have added a message on the mailing list about this topic (https://www.eclipse.org/lists/jta-dev/msg00314.html)

@yrodiere
Copy link

FWIW this hard dependency is making it hard to use Hibernate ORM in the modulepath in a Java SE environment. I submitted a PR to try to address that a while ago: #212

It seems to me this change would just correct something that is present in APIs but is otherwise unspecified anyway (I see no mention of Java modules in the spec), so it would be fair to merge the change now and clarify the spec later?

Let's also keep in mind that implementations are free to declare inter-module dependencies of their own. If an implementation does have a hard dependency on CDI, their own module-info should say so, and changing the dependencies of Jakarta Transactions shouldn't impact users at runtime.

@edburns
Copy link

edburns commented Mar 12, 2024

I am glad to see activity on transactions, however I am unaware of any plan to include an update on transactions in EE 11.

https://jakartaee.github.io/platform/jakartaee11/JakartaEE11ReleasePlan

Please confirm.

@tomjenkinson
Copy link
Contributor

At this stage there is no update planned of Jakarta Transactions for EE11.

However if the Jakarta Transactions project is able to include this change, and if the community would like an API jar to be released for EE11, and if there is enough capacity in the community to pursue that goal, and if the platform can accept a potential proposal for a Jakarta Transactions updated API release in EE 11 then it is possible a plan to update Jakarta Transactions for EE 11 could be made.

@xenoterracide
Copy link

xenoterracide commented Jun 24, 2024

Is there some reason why maven doesn't declare the dependency as optional? why does the module info (arguably) disagree with the pom?

could the pom be fixed in older version if the module info can't be?

related in spring boot spring-projects/spring-boot#41203

@xenoterracide
Copy link

Spring boot has apparently decided that although they are they are willing to force Jakarta transaction upon us in certain circumstances that they are not willing to include these extra jars. I don't know if that violates the specification or not but it certainly violates the code.

@tomjenkinson
Copy link
Contributor

Thank you @xenoterracide . The main point here is that CDI is not optional from a specification point of view and so the code currently seems more consistent with that position. That's not to say that we can't as a community work on changing the expectations of the specification.

@xenoterracide
Copy link

@tomjenkinson well... 🤷🏻‍♂️ all I can say is what I said. Spring Boot has decided not to follow the specification @philwebb says they think it's a bug here... spring-projects/spring-boot#41203 (comment)

@arjantijms
Copy link
Contributor

Spring Boot has decided not to follow the specification @philwebb says they think it's a bug here... spring-projects/spring-boot#41203 (comment)

It's not really a bug. The module-info follows the spec, where the features that happen to use CDI are simply listed among the features that don't as mentioned in #214 (comment)

But you could think of it as a design-bug in the spec, and we probably should have put the features that required CDI in a separate module.

@xenoterracide out of curiousity how Spring is designed; do you also put everything that requires Spring Beans in a separate module, and make sure the features that don't require Spring Beans are usable with say CDI etc without using a Spring Beans dependency?

@xenoterracide
Copy link

Not a good question for me.... I'm not affiliated with the spring boot team. I simply reported the issue and have been told That it works for them since they don't use JPMS, And they aren't willing to add additional dependencies since it appears optional except in jpms. I mean you can read the thread. I would suggest chatting them up.

@xenoterracide
Copy link

Partially copied from other thread. I'm not certain where to otherwise message the Jarkarta Team as this is for all the specs.

The Jakarta Team should provide a Maven BOM so that Spring (Boot) can depend on it and that all versions of dependencies for that release, e.g. 9 or 10, are provided. This would have in part solved my problem.

Also, the maven dependency here should be marked as optional along with the module-info defining them as static, and maybe transient.

@pzygielo
Copy link
Contributor

The Jakarta Team should provide a Maven BOM

Perhaps https://repo.maven.apache.org/maven2/jakarta/platform/jakarta.jakartaee-bom/ would do.

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.

8 participants