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

Including both MP POM and Jakarta EE 10 API JAR leads to duplicate classes (JPMS split packages) #297

Closed
lprimak opened this issue Dec 4, 2022 · 19 comments

Comments

@lprimak
Copy link

lprimak commented Dec 4, 2022

Describe the bug
As described in the title, including both MP and Jakarta results in JPMS failures (see below)
The leads to duplicate classes on the classpath and problems
if an application uses has JPMS module-info anywhere (split package issues)

Perhaps another POM can be built without the transitive dependencies,
having them be optional, or exclude them entirely.

% mvnd dependency:tree
[INFO] --- maven-dependency-plugin:3.4.0:tree (default-cli) @ hope-website ---
[INFO] +- org.eclipse.microprofile:microprofile:pom:5.0:provided
[INFO] |  +- jakarta.enterprise:jakarta.enterprise.cdi-api:jar:3.0.0:provided
[INFO] |  +- jakarta.ws.rs:jakarta.ws.rs-api:jar:3.0.0:provided
[INFO] |  +- jakarta.json.bind:jakarta.json.bind-api:jar:2.0.0:provided
[INFO] |  +- jakarta.json:jakarta.json-api:jar:2.0.1:provided
[INFO] |  +- jakarta.annotation:jakarta.annotation-api:jar:2.0.0:provided

Related Issues
#44
#134

To Reproduce
Steps to reproduce the behavior:

        <dependency>
            <groupId>org.eclipse.microprofile</groupId>
            <artifactId>microprofile</artifactId>
            <version>5.0</version>
            <type>pom</type>
            <scope>provided</scope>
        </dependency>

Workaround
The issue can be worked around by excluding transitive dependencies:

        <dependency>
            <groupId>org.eclipse.microprofile</groupId>
            <artifactId>microprofile</artifactId>
            <version>5.0</version>
            <type>pom</type>
            <scope>provided</scope>
            <exclusions>
                <exclusion>
                    <groupId>jakarta.enterprise</groupId>
                    <artifactId>*</artifactId>
                </exclusion>
                <exclusion>
                    <groupId>jakarta.ws.rs</groupId>
                    <artifactId>*</artifactId>
                </exclusion>
                <exclusion>
                    <groupId>jakarta.json</groupId>
                    <artifactId>*</artifactId>
                </exclusion>
                <exclusion>
                    <groupId>jakarta.json.bind</groupId>
                    <artifactId>*</artifactId>
                </exclusion>
                <exclusion>
                    <groupId>jakarta.annotation</groupId>
                    <artifactId>*</artifactId>
                </exclusion>
            </exclusions>
        </dependency>

See similar issue for Jakarta EE:
jakartaee/platform#583

@JanWesterkamp-iJUG
Copy link
Contributor

Hi @lprimak, this is the expected behaviour, as MP 5.0 is compatible to Jakarta EE 9.1 and MP 6.0 (comming soon) will be based on the Jakarta EE 10 Core Profile.

If you include both, you will receive two versions of the Jakarta EE 10 component specs and you need to choose, as you did in your workaround.

@lprimak
Copy link
Author

lprimak commented Dec 5, 2022

Hi, @JanWesterkamp-iJUG
Even if you include Jakarta EE 9 you will have the same issue with JPMS split packages, because dependencies will be included for both individual SPECs and the Umbrella JAR.

I can clearly see that it's the expected behavior, but I am challenging the assumption that this is good and should be left as is.

There is tension here between just having "the one" dependency on MicroProfile and not having to include Jakarta EE explicitly (wih all issues associated) and having to require Jakarta EE dependencies.
I believe that part of the current thinking is that MP does't need all of Jakarta EE so it incudes the minimum requirement as transitive dependencies (Spec JARs)

While those are all valid points, in my opinion, it would be better not to include any Jakarta EE dependencies (and require the MP user to include those explicitly) to avoid all the above issues associated with current dependency tree.

@JanWesterkamp-iJUG
Copy link
Contributor

Hi @lprimak, I am not an JPMS expert, as far as I know, it can have only one dependency with one version in it's tree (in fact, the version dimension is missing at all) - as Maven will have as it's single dependency version tree result from the dependency management (while there is the possibility to manage different versions).

Jakarta EE 9 does not fit to MP 5.0, while Jakarta EE 9.1 should match.
We declare the specs as provided, so the implementor can replace them.

What is not well defined currently, is having one single version in all the dependencies of MP or Jakarta EE specs, but a defined set in the umbrella spec (Profiles/Platform).

Skipping the dependencies simply, omitting required transitive dependencies or make all optional in Maven seems not a valid solution to me. May be, making it fully JPMS compatible might require some action, as this feature is still experimantal here.

How does your module-info file look like? In Maven the mentioned provided dependencies are defined in the runtime. What runtime you use that offers JPMS support for your app?

@lprimak
Copy link
Author

lprimak commented Dec 6, 2022

as Maven will have as it's single dependency version tree

That's will not be true in the most common use cases.
Most applications will include the Jakarta EE umbrella JAR, instead of the individual SPEC JARs, and thus will include two versions (one from the individual SPEC and one from the umbrella JAR) for the same Jakarta EE classes, leading to failures.

Skipping the dependencies simply, omitting required transitive dependencies or make all optional in Maven seems not a valid solution to me.

The issue is that the Jakarta EE transitive dependencies are not required by Microprofile APIs in a lot of cases (take MP-Config or MP HTTP Client for example). They may be required by the implementation of the SPECs, but, in keeping with best practices from Maven and Jakarta EE, only dependencies that are required by the APIs should be brought in.

MP does get a bit muddled here because some API specs do require the Jakarta EE classes at compile time, but most of them do not though,

I understand this is not a clear-cut issue and the resolution is not that obvious, that's why I am soliciting comments like yours and other opinions on the matter.

Thank you!

@lprimak
Copy link
Author

lprimak commented Dec 6, 2022

I have added some existed related issues that have already relate to this (see edited description)

@lprimak
Copy link
Author

lprimak commented Dec 6, 2022

I actually see a simple solution, just add this to all the Jakarta dependencies:

<properties>
<jakarta-deps-exclude>false</jakarta-deps-exclude>
</properties>
...
<dependency>
...
<artifactId>jakarta.servlet</artficactID>
<optional>${jakarta-deps-exclude}</optional>
</dependency>

@lprimak
Copy link
Author

lprimak commented Dec 6, 2022

Also, I see there is a lot of confusion here as well, as evidenced here:
#293

@JanWesterkamp-iJUG
Copy link
Contributor

I totally agree with you regarding the current use of different versions in the current Jakarta EE component and umbrella specs dependencies need to be clean up - I tried to address that issue and we made some progress in Jakarta EE 10 but is not finished yet: jakartaee/jakartaee-api#125

The current situation is (as descibed in #293) is, that vendors have different views how to design a Jakarta EE implementation - some are by architecture very flexible to handle different versions of specs (i.e. OSGi based implemetations). Some wanted to be as backward compatible as possible (including a mixture of Jakarta EE and MP versions in one implementation). This results in the try to define dependencies as ranges instead of defining a specific version and it might look like this is an issue in Maven (here you can define ranges, but best practice is not to do it) and more important JPMS.

Of course, there are simply issues to solve regarding things like unnecessary or even forbidden use of dependencies on Jakarta component spec APIs (i.e. test dependencies or defining the hole Core Profile as dependecy in MP).

Do you attend the JakartaOne (https://jakartaone.org/2022/) currently? It looks like Piranha Cloud is making heavy use of JPMS - it would be interesting getting their view on this topic.

@lprimak
Copy link
Author

lprimak commented Dec 6, 2022

I think the discussion scope has gotten too wide :)
This issue is about the "consumer / app developer" point of view, not the vendor's point of view.
To put it simply, there are duplicate classes with the "as-documented" way to create MP applications,
when mixed with "as-documented" way to create Jakarta EE applications,
which makes it not work with JPMS out-of-the box (as a sample issue, not the "whole" issue)
The whole issue being that API classes are duplicated from Spec APIs and Jakarta EE umbrella APIs.
Same issue would exist if the app would have a dependency on Jakarta EE Web Profile Umbrella API JAR, and MP would have a dependency on Jakarta EE Core Profile Umbrella API JAR (for example)

Now that I had some time to think about solutions, I have a couple of proposed solutions:

  • Remove dependencies on Jakarta EE Spec APIs from MP API (as consumed by apps)
    Obviously that's less compatible, but since MP increments it's version # frequently, that would not be a huge issue.
    Since it's documented which MP is compatible with which Jakarta EE, the "version problem" would not be an issue
  • Implement <optional> for Jakarta EE dependency as I mentioned Including both MP POM and Jakarta EE 10 API JAR leads to duplicate classes (JPMS split packages) #297 (comment)
  • Add another microprofile POM microprofile-standalone without Jakarta EE dependencies

@rdebusscher
Copy link
Member

rdebusscher commented Dec 6, 2022

This also depends on what the goal of MicroProfile is

1- Provide an API to implement a MicroProfile Application (without the need for other APIs)
2- Provide a set of libraries that work on top of Jakarta EE.

It was always the idea of providing solution 1 (although MicroProfile is using some of the Jakarta EE specs), since the only dependency that you need was always

<dependency>
            <groupId>org.eclipse.microprofile</groupId>
            <artifactId>microprofile</artifactId>
            <version>${mp-version}</version>
            <type>pom</type>
            <scope>provided</scope>
 </dependency>

So if you as a user want to combine it with Jakarta EE, you as a user should do something additional now in the case of JPMS. Which is a sad effect of JPMS.

If we want to switch to solution 2, this is a different goal for MicroProfile for which I don't think there is a lot of support. (as it gives up the independent status of MicroProfile)

@lprimak
Copy link
Author

lprimak commented Dec 6, 2022

All great points Rudy! (very nice talk today and great work in general BTW)

However, it's not quite the reality that MP is independent today (it does depend on Jakarta EE specs), nor is it a forgone conclusion that, if the Jakarta EE dependencies would be required, MP would lose its independence.

I am guessing that more data is needed resolve how many applications (and future work) really would use MP with Jakarta EE APIs that are included (jax-rs, jsonp) without including them explicitly from Jakarta EE.

Dare I say it that currently MP currently "tricks" the user into using Jakarta EE without knowing it (well, so does Spring)
Is a good thing? Debatable.

I am definitely on the side of not including the Jakarta EE dependencies by default, but that's just my opinion and take it for what it's worth. I would be happy with either solution as the status quo isn't so good.

@lprimak
Copy link
Author

lprimak commented Dec 6, 2022

Another solution:
provide a standalone MP POM file that doesn't include the Jakarta EE dependencies (perhaps a parent POM to the current POM?)

@lprimak
Copy link
Author

lprimak commented Dec 6, 2022

new Jakarta EE issue: jakartaee/jakartaee-api#133

@JanWesterkamp-iJUG
Copy link
Contributor

I think breaking the existing working solution in one module management system (Maven) to simplify support in another one (JPMS) is not a solution that is acceptable for the MPWG, especially when the last is very limited and the first is wildly in use...

May be generating a module-info from the reactor of Maven, where propper dependency management is avaliable already and multiple, even deviating definitions of the same module can be normalized to a single one might be a solution in the future.
Meanwhile, as @rdebusscher said, doing it manually is the way to go from my point of view too, when there is no JPMS tooling to automate this task is available yet.

Automated transitve dependency managment is a design feature of Maven, that makes it successful for now two decades - why should we disable this? These dependencies are required (most of them) and prevent us from declaring them again and again manually.

@lprimak
Copy link
Author

lprimak commented Dec 6, 2022

@JanWesterkamp-iJUG I think you are concentrating on the JPMS aspect too much. JPMS failure is result of the duplicate classes on the classpath. The only thing JPMS does is clearly illustrates the problem because with JPMS, execution fails, but without it, it succeeds. Also it doesn't matter what the module-info.java contains, how it's generated, etc., if only it exists (empty) will break the build, since that will trigger the JDK to check for split packages (like beans.xml, sort-of).
See https://nipafx.dev/java-9-migration-guide/#split-packages for a great explanation.

None my the solutions I proposed break maven, or MP, in any way, unless I am missing something significant.
Having transitive dependencies is a choice (not a requirement), and I am simply questioning that choice (at least have an option to easily disable) so combination of MP and Jakarta EE is not painful.

@arjantijms
Copy link

@rdebusscher

Provide an API to implement a MicroProfile Application (without the need for other APIs)
It was always the idea of providing solution 1

I don't think that's really the case. People easily forgot their history, or maybe didn't know it in the first place.

Almost all the MP APIs that are now in the MP Platform were originally targeted for Java EE 8. With all that went down with Java EE at the time, those new APIs were materialised on top of a very minimal Java EE profile (which is now the core profile).

Without the Java EE problems that we had back then, basically everything from the MP Platform would now have been in EE.

@rdebusscher
Copy link
Member

rdebusscher commented Dec 8, 2022 via email

@lprimak
Copy link
Author

lprimak commented Dec 8, 2022

t started differently, but once the group was formed, certain members are
very passionate about it being independent. (Just voicing the sentiment

Understandable.
However, the landscape changed drastically since then, remember that Jakarta EE was not anywhere close to where it is now, I would argue it achieved success under Eclipse thus far.

In any case, I can see this is a very controversial issue.
I want to remove the controversy and resolve the technical issue only.

So, here are my revised two options:

  1. Add a property to exclude Jakarta dependencies as described here: Including both MP POM and Jakarta EE 10 API JAR leads to duplicate classes (JPMS split packages) #297 (comment)
  2. Provide a standalone pom (that microprofile pom inherits from) that doesn't include the transitive dependencies.

Both are small, pragmatic, resolve the actual problem at hand, and do not change the architecture of MP in any way.

What do you think?

@lprimak
Copy link
Author

lprimak commented Jan 23, 2023

Closing this issue as jakartaee/jakartaee-api#133 is the root cause

@lprimak lprimak closed this as not planned Won't fix, can't repro, duplicate, stale Jan 23, 2023
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

4 participants