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

Separate artefacts for CDI lite and CDI full #640

Closed
rdebusscher opened this issue Dec 12, 2022 · 19 comments
Closed

Separate artefacts for CDI lite and CDI full #640

rdebusscher opened this issue Dec 12, 2022 · 19 comments

Comments

@rdebusscher
Copy link

Since there is only 1 artifact, containing both CDI lite and CDI full, it is not possible safely compile a program against CDI lite only, or build a CDI lite implementaton.

Developer viewpoint

Having the CDI full classes available in the API artifact means that a library/application compiled against this code might fail when running on a CDI lite implementation. Since the CDI full classes are available in the API, a developer can make use of it but the CDI lite implementation is not required to support them/have them available at runtime.

This is a bad user experience since a successfully compiled program using the API fails at runtime.

Implementation viewpoint

When developing an implementation, there is no API that can be added that contains only those classes that must be supported by the implementation.

@manovotn
Copy link
Contributor

This was discussed repeatedly in CDI calls and from the top of my head I recall these two major reasons why there is only one JAR present:

  1. CDI Lite defines the minimal set of features that it supports but doesn't prevent implementations from adding anything else from CDI Full - this is intentional design choice. For example, you can have Lite impl that also supports decorators and that's perfectly fine with CDI spec. Single JAR allows to support this without requiring user to declare additional dependencies.
  2. When we initially discussed splitting it, it was turned down when we realized that it would break pretty much 100% of existing applications. This is because there are packages (such as jakarta.enterprise.context) in which some classes belong only to CDI Lite while others are used in CDI Full. To avoid package split which is not valid on newer JDKs, we'd have to break compatibility by moving classes into new packages altogether.

Now, I am by no means saying that the current state is ideal, just listing what I recall were the reasons for this choice.

@Ladicek
Copy link
Contributor

Ladicek commented Jan 2, 2023

@manovotn is right on the money. I don't see how we could possibly "fix" this without breaking the world.

@rdebusscher
Copy link
Author

But as described in my initial comment, CDI is now a bit of a mess.

Which apparently can't be solved/untangled according to the replies. Nice.

@Ladicek
Copy link
Contributor

Ladicek commented Jan 2, 2023

I just recalled there's an alternative option that I think we also discussed: ship 2 overlapping API JARs. I believe we rejected this because it would very easily lead to non-deterministic classpath situations. It's something we could reconsider, perhaps.

@m-reza-rahman
Copy link

I would keep an eye on this and see how many folks run into issues Rudy pointed out. It’s certainly a possible problem. It’s also possible in practice it’s OK the way it is.

@rdebusscher
Copy link
Author

If users/developers report this kind of issues or just abandon Jakarta EE (most people are not very communicative about issues and just go find better alternatives)

@struberg
Copy link
Contributor

For the record: there was even a PR with tons of experts speaking out in favour of a split. It just got ignored and slammed. #582

@manovotn
Copy link
Contributor

tons of experts speaking out in favour of a split.

That's curious, up until today, I didn't take tons to mean three :)
But joked aside, the PR was closed as it was left there forgotten long after final release and after the CDI group has settled on a different approach. This topic has been repeatedly discussed on mailing list and in meetings and rest of the group agreed on what you see published today. The reasons can be fathomed from those discussions and they spanned from compatibility between Lite, Full to not breaking every existing app out there by moving packages to usability concerns because even as we drafted the Lite spec, we saw emerging implementations that wanted to go beyond what was mandatory to support in Lite and hence needed Full API anyway.

Last but not least, like I stated earlier in this thread, the solution isn't perfect but that's what we arrived at and agreed on.

@struberg
Copy link
Contributor

  • 3 expert group members is not nothing!
  • my PR was submitted BEFORE the final Release afair! This was way back in 2021.

@Ladicek
Copy link
Contributor

Ladicek commented Jan 26, 2023

I'd just like to point out that #582 does not split CDI Lite from CDI Full. It only splits the Build Compatible Extensions API, which achieves nothing with respect to the stated goals of CDI Lite, namely that CDI Lite is a strict subset of CDI Full.

@struberg
Copy link
Contributor

Again: CDI-Lite is not a subset of CDI-Full. CDI-Lite technically is all of the CDI-full API minus core CDI features, but plus build-time functionality.
Let's rather stick with

  • CDI-core (old CDI)
  • CDI-light (old CDI minus Extensions, but plus build-time support)
  • CDI-full (full old CDI plus the new build-time support)

This is imo much more where we are right now, isn't?

@manovotn
Copy link
Contributor

3 expert group members is not nothing!

Indeed and I did not say that.
I merely wanted to point out, for future readers, what does a 'ton' stand for in this case :)

my PR was submitted BEFORE the final Release afair! This was way back in 2021.

That is correct and I never indicated that it would be otherwise either. I simply tried to explain that a different decision was agreed upon and the PR in question hanged open even after release.

Again: CDI-Lite is not a subset of CDI-Full.

OK, I guess. Let's agree to disagree. Again.

@Ladicek
Copy link
Contributor

Ladicek commented Jan 27, 2023

Again: CDI-Lite is not a subset of CDI-Full.

CDI Lite is what the specification says it is.

The manifestation of CDI Lite and Full APIs in a single JAR is an unfortunate effect of us actually caring about compatibility to the greatest extent practically possible.

I'll reiterate the options we have considered, to my best recollection:

  1. Split the CDI API into 2 JARs: one with only the CDI Lite API, the other with only the CDI Full additions on top of CDI Lite. This would either require moving classes to different packages (huge breaking change), or split packages (not allowed in the JPMS).
  2. Ship 2 CDI API JARs: one with only the CDI Lite API, the other with the entire CDI Full API (including CDI Lite API). This would very likely lead to nondeterministic classpath situations. That might be OK, but only in case of full version convergence.
  3. Just keep it all in a single JAR and add a warning to the API documentation. This is bad in theory, because it might create an illusion that some APIs can be used when they in fact can't. In practice, the true nature of that illusion is revealed very quickly, so I don't really think this is a big deal. It is not pretty, for sure.

As this issue points out, we chose option 3. Maybe there are other options we should have considered but didn't -- I'd still like to hear about them.

@rdebusscher
Copy link
Author

When you start with the requirement, we need a CDI lite that relies less reflection and doesn't break anything, the current solution is the only correct one. But introduces an incompatibility between future CDI 'extensions' since if an extension is not restricted to CDI lite functionality, it might break on Core Profile Products (and documentation and warnings are key here but not ideal)

But more importantly, what about the user and if this technical requirement is something that benefits them and is easy and clear for the user? I have the impression that CDI lite is made just to allow some certification requirements but actually results in a not optimal user experience (my first reaction was to write 'bad user experience') for those who wants to use CDI lite - Core Profile - Quarkus (and potentially other products)

The solution does not allow any compile time dependencies for your project that will result in a compiled binary that is guaranteed to run. And yes, user will see it very quickly that it fails, maybe even when Quarkus compilation already refuses to create your program.

But if there is no good technical solution for a requirement, maybe it was better to drop CDI lite altogether and not push something that potentially puts Jakarta in a negative perspective. (Jakarta is useless since you can successfully
compile your application but it doesn't run - by design not due to a bug)

This as feedback, I don't expect any solution in a future version.

@Ladicek
Copy link
Contributor

Ladicek commented Jan 27, 2023

There are literally millions of ways how to create a program that compiles but doesn't run. As far as I'm concerned, this doesn't qualify as an argument against trying to shed some baggage and adapt to the ever-changing technology landscape. That said, I think there's at least one good idea to distill from this: we could/should require CDI implementations that implement CDI Lite but do not implement CDI Full to detect (as much as they practically can) usages of CDI features they do not implement and treat (some of) them as deployment problems. Failing early is always better than failing late. (In build-time implementations like ArC or ODI, "deployment time" is build time, so this would eliminate "compiles but doesn't run".)

@arjantijms
Copy link

@Emily-Jiang @edburns This is issue is about what we discussed in the platform meeting, regarding CDI Lite not having its own jar split out.

@arjantijms
Copy link

See also jakartaee/jakartaee-api#139

@arjantijms
Copy link

in which some classes belong only to CDI Lite while others are used in CDI Full. To avoid package split which is not valid on newer JDKs, we'd have to break compatibility by moving classes into new packages altogether.

As CDI lite is new, could it not have used a new package? That would not have broken anything, would it?

@manovotn
Copy link
Contributor

in which some classes belong only to CDI Lite while others are used in CDI Full. To avoid package split which is not valid on newer JDKs, we'd have to break compatibility by moving classes into new packages altogether.

As CDI lite is new, could it not have used a new package? That would not have broken anything, would it?

@arjantijms not really. CDI Lite is a subset of Full and as such uses many/most CDI functionalities but not all of them.
First example coming to mind is jakarta.enterprise.inject.spi package - https://github.com/jakartaee/cdi/tree/master/api/src/main/java/jakarta/enterprise/inject/spi
That already existed prior to Lite, but Lite only uses some parts - you can't split that package without breaking basically all CDI apps out there because every CDI Full app might have been using those parts that are in Lite.

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

No branches or pull requests

6 participants