-
Notifications
You must be signed in to change notification settings - Fork 40.9k
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
JPMS: missing jakarta dependencies #41203
Comments
We have a long-standing policy of only managing the versions of dependencies upon which we depend directly. This limits the scope of We don't declare a direct dependency on I don't think we should change our policy to accommodate the JPMS. AIUI, the logical conclusion of such a policy change would be that we have to manage the versions of the entire transitive dependency graph. I don't think that would be sustainable and we should decline this. In addition to not wanting the scope of our dependency management to be unmanageable, as far as I can tell this is only a problem due to the oddities in the pom and module info of the transaction API. The pom for |
Possibly... I'm going to try to create a minimal reproducer on Monday. This results in additional problems. To be honest I actually don't have any direct dependency on the Jakarta stuff at all. Which is why I'm going to create a reproducer. I speculate, but I'm not certain that there may be an incorrect dependency on jakarta.transaction... I'm going to speculate that the reason That they Don't provide it is that they want to be able to match multiple versions. It requires some version not a specific one. If that is the rationale then I don't think Jakarta transaction should be in the starters which is I assume where I'm pulling it in from since I have no direct dependency on it. Either that or the starter should also contain the full dependencies for the module. If it truly is optional code then the Java module is wrong and those should be declared as static. Right now I only work here but this feels kind of messy and yes I should reach out to the Jakarta people I haven't had time yet. Anyways let's talk more about this once I have the reproducer I hope. |
here's the reproducer. You'll note that I'm not using jakarta transaction directly and my module info doesn't depend on it. I've commented out the required fix in the build.gradle.kts
to be fair this module doesn't define it as optional, although it might be that way for the pom, there might be other reasons for that. Maven and Gradle aren't really great at distinguishing required but alternative dependencies, and optional dependencies respectively. I need to investigate that bit further. I don't think ignoring JPMS required but not static dependencies is a good precedence as it arguably breaks the jvms expectations and will break in the future if spring decides to add its own module-info. now that I have the reproducer, I'm off to figure out why jakarta module seems to be different from it's maven pom. That's an explicit for this case though, not the general policy. |
I found an existing issue jakartaee/transactions#214 There appears that at this time there are no plans to make this optional, but that the community can use the community process to request the module-info change.
My understanding of maven is that provided mean required by the consuming environment at runtime, but that will not depend on a specific version. Without the optional flag it doesn't mean optional. So the starter providing it without providing the required dependencies seems highly inappropriate since it's kind of spring boot's starter "promise" that it'll give you everything you "need". I shouldn't need to provide a dependency for a dependency that I don't have any need of. I do see correct 2 paths on this specific issue based on that ticket without a response and update to the pom to my comment there
If there's any more leg work I can do, let me know. I would ask that you not make a general policy of ignoring module-info requirements. |
We discussed this today and we feel that this is a bug with the module-info in |
Then please file a request for change the specification with Jakarta. It's their standard and they are very clearly stating that they will not be making a change to the standard in this version and that a request for change via the community process would be required to make such a change If you're not going to conform to the standards then you should remove the dependency from the starter |
I guess I'd love to know why you think it's a mistake on their end when they're saying it's not jakartaee/transactions#214 (comment) |
I don't think there's much to say beyond the information Andy has already provided. We thought it was a bug since things appear work just fine without Unfortunately we need to prioritize the little time we have on the issues that have the most impact. If you're going to choose to use the JPMS then I'm afraid you're going to need to take on the extra burden of managing a few more dependencies yourself. |
[partially copied from the other thread] But you could think of it as a design-bug in the spec, and we probably should have put the features that required CDI and Interceptors in a separate module. As you probably know, Spring is often a concern of big importance for a number of Jakarta specs (e.g. Servlet and Persistence), ahead of the concerns of the Jakarta platform itself. Transactions also should have been thinking about Spring first, but it wasn't detected since JPMS wasn't out then. @philwebb 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? |
Hi @arjantijms, Currently the extent of JPMS support is setting There's an open Spring Framework issue that has been looking into this (spring-projects/spring-framework#18079), but it's not got very far and given this comment by @jhoeller may not get traction anytime soon. I suspect if we even did add complete |
personally I think a single module is fine, but the pom should define these as optional dependencies (It doesn't and spring boot is choosing to ignore that, and instead supplying incomplete dependencies). Also, then the |
@philwebb @wilkinsona I'm kind of making an assumption here; it's based on the fact that I didn't get versions when trying to add these myself. Is there a reason that Spring Boot doesn't use the Jakarta EE BOM? which would provide the necessary versions even if you don't allow substitution. |
@xenoterracide I think the main reasons are:
|
Right, but you can force upgrade a dependency? Shouldn't you be asking them to do an upgrade? As far as the first point goes. What is the negative for having a version of something that you don't otherwise intend to use? Like a starter doesn't have to contain that dependency. You don't have to add any additional support for it yourself aside from the fact that it's in the list and if someone were to add it for whatever reason they would already have a version. |
Our current process is working quite well for the majority of our users and we don't really have the bandwidth to switch to the Jakarta BOM then immediately start to override versions or submit requests to them to do so. Dependency management is hard, and we really don't want to start including more than we need in our own BOM. I would suggest using the Jakarta BOM directly in your own project if it solves your problem. |
So I currently have code that doesn't directly use
jakarta.transaction
but a test is failing to start on missing it and modules for it and those modules for it. Some of these modules rely on modules I can't lock using the spring bom.So this is at least a request to add those modules to the bom so I can match versions properly.
This is
jakarta.transaction
module info butjakarta.enterprise:jakarta.cdi-api
is not available in the bom.jakarta.interceptor:jakarta.interceptor-api
is also missing.this is the maven pom from jakarta, which means these are expected to be provided by the consuming environment, and they are not marked optional. The version here is only for the consumer.
edit: done with additional details in ticket
repro.tar.gz
The text was updated successfully, but these errors were encountered: