jakartaee / transactions

Welcome to the Jakarta EE Transactions API Project (formerly JTA)
https://jakartaee.github.io/transactions/
Other
28 stars 29 forks source link

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

Open starksm64 opened 1 year ago

starksm64 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

Good point, that is right

arjantijms commented 1 year ago

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 commented 6 months ago

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

yrodiere commented 6 months ago

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: https://github.com/jakartaee/transactions/pull/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 commented 5 months ago

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 commented 5 months ago

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 commented 2 months ago

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 https://github.com/spring-projects/spring-boot/issues/41203

xenoterracide commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

@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... https://github.com/spring-projects/spring-boot/issues/41203#issuecomment-2209210263

arjantijms commented 2 months ago

Spring Boot has decided not to follow the specification @philwebb says they think it's a bug here... https://github.com/spring-projects/spring-boot/issues/41203#issuecomment-2209210263

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 https://github.com/jakartaee/transactions/issues/214#issuecomment-1709024069

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 commented 2 months ago

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 commented 1 month ago

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 commented 1 month ago

The Jakarta Team should provide a Maven BOM

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