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

Regression: EE 10 Platform API JARs include all classes, and transitive dependencies to Spec API JARs leading to duplicate classes and JPMS split package failures and OSGi issues #133

Closed
lprimak opened this issue Dec 6, 2022 · 21 comments · Fixed by #137
Assignees

Comments

@lprimak
Copy link
Contributor

lprimak commented Dec 6, 2022

Describe the bug
This is a regression from Jakarta EE 8 (possibly 9)

Platform API JARs now include both all the API .class files as well as all Spec JARs as transitive dependencies
This is seen below.
The leads to duplicate classes on the classpath and problems
if an application uses has JPMS module-info anywhere (split package issues)
This also leads to OSGi deployment issues.

% mvnd dependency:tree
[INFO] --- maven-dependency-plugin:3.4.0:tree (default-cli) @ hope-website ---
[INFO] +- jakarta.platform:jakarta.jakartaee-api:jar:10.0.0:provided
[INFO] |  +- jakarta.platform:jakarta.jakartaee-web-api:jar:10.0.0:provided
[INFO] |  |  +- jakarta.servlet:jakarta.servlet-api:jar:6.0.0:provided
[INFO] |  |  +- jakarta.servlet.jsp:jakarta.servlet.jsp-api:jar:3.1.0:provided
[INFO] |  |  +- jakarta.el:jakarta.el-api:jar:5.0.1:provided
[INFO] |  |  +- jakarta.servlet.jsp.jstl:jakarta.servlet.jsp.jstl-api:jar:3.0.0:provided
[INFO] |  |  +- jakarta.faces:jakarta.faces-api:jar:4.0.1:provided
[INFO] |  |  +- jakarta.websocket:jakarta.websocket-api:jar:2.1.0:provided
[INFO] |  |  +- jakarta.websocket:jakarta.websocket-client-api:jar:2.1.0:provided
[INFO] |  |  +- jakarta.ejb:jakarta.ejb-api:jar:4.0.1:provided
[INFO] |  |  +- jakarta.transaction:jakarta.transaction-api:jar:2.0.1:provided
[INFO] |  |  +- jakarta.persistence:jakarta.persistence-api:jar:3.1.0:provided
[INFO] |  |  +- jakarta.validation:jakarta.validation-api:jar:3.0.2:provided
[INFO] |  |  +- jakarta.interceptor:jakarta.interceptor-api:jar:2.1.0:provided
[INFO] |  |  +- jakarta.enterprise:jakarta.enterprise.lang-model:jar:4.0.1:provided
[INFO] |  |  +- jakarta.inject:jakarta.inject-api:jar:2.0.1:provided
[INFO] |  |  +- jakarta.authentication:jakarta.authentication-api:jar:3.0.0:provided
[INFO] |  |  \- jakarta.security.enterprise:jakarta.security.enterprise-api:jar:3.0.0:provided
[INFO] |  +- jakarta.jms:jakarta.jms-api:jar:3.1.0:provided
[INFO] |  +- jakarta.activation:jakarta.activation-api:jar:1.2.2:provided
[INFO] |  +- jakarta.mail:jakarta.mail-api:jar:2.1.0:provided
[INFO] |  +- jakarta.resource:jakarta.resource-api:jar:2.1.0:provided
[INFO] |  +- jakarta.authorization:jakarta.authorization-api:jar:2.1.0:provided
[INFO] |  \- jakarta.batch:jakarta.batch-api:jar:2.1.1:provided

To Reproduce
Steps to reproduce the behavior:

        <dependency>
            <groupId>jakarta.platform</groupId>
            <artifactId>jakarta.jakartaee-api</artifactId>
            <version>10.0.0</version>
            <scope>provided</scope>
        </dependency>

Workaround
The issue can be worked around by using it as pom dependency to exclude the platform JAR file from the classpath:

        <dependency>
            <groupId>jakarta.platform</groupId>
            <artifactId>jakarta.jakartaee-api</artifactId>
            <version>10.0.0</version>
            <type>pom</type>
            <scope>provided</scope>
        </dependency>

This is the offending commit: 8ff41dc / #127
Related to #125

Possible Solution
Provide an empty platform JAR file instead of including all Spec .class files in it.
Spec JARs as transitive dependencies will make the classpath correct and will work perfectly.

@lprimak
Copy link
Contributor Author

lprimak commented Dec 6, 2022

@starksm64 @ivargrimstad Do you remember what was the reason for this change?
It seems to be somewhat-unrelated to the PR it's in...thanks!

@lprimak lprimak changed the title Regression: EE 10 Platform API JARs include transitive dependencies to Spec API JARs leading to duplicate classes Regression: EE 10 Platform API JARs include transitive dependencies to Spec API JARs leading to duplicate classes and JPMS split package failures Dec 8, 2022
@ivargrimstad
Copy link
Member

The reason is to explicitly state the top-level dependencies, and also make sure that the JavaDoc for these is generated for the Platform JavaDoc. See the mail thread here: https://www.eclipse.org/lists/jakartaee-platform-dev/msg03436.html

There may be other ways of achieving this, so feel free to suggest a PR.
But also keep in mind that these API artifacts are not normative deliverables of the specifications, so it may not have the highest priority.

@lprimak
Copy link
Contributor Author

lprimak commented Dec 19, 2022

Thanks Ivar,

perhaps the way to go is to have "empty" JARs in jakarta.platform (full/web), that way there are no duplicates and the "new" way forward with transitive dependencies would work

@OndroMih
Copy link

I agree that platform API JARs shouldn't contain anything that is already in spec API JARs brought as dependencies. So, in the end, they should be more or less empty and just an aggregator for all related spec API JARs. That's how usually Maven dependencies work and it works well.

@lprimak
Copy link
Contributor Author

lprimak commented Dec 19, 2022

More thoughts: Platform APIs should be of type pom instead of jar so they just bring in the appropriate spec JARs as transitive dependencies. I believe this is how MicroProfile does it as well.
This would also fix eclipse/microprofile#297
This would remove the shade legacy altogether and would be a better overall

@OndroMih
Copy link

How would POM type help? I really don't like that MP platform API is a POM type, the type always has to be specified in the dependency in pom.xml. The JAR type is implicit, so it doesn't have to be explicitly specified. If the umbrella artifact is an empty JAR, it should be enough, am I wrong?

@lprimak
Copy link
Contributor Author

lprimak commented Dec 21, 2022

How would POM type help?

Just for it not to have a JAR at all, as an empty JAR may (or may not!) cause confusion.
However, I do see your point about the JAR being default and either way works for me just the same.

@lprimak lprimak changed the title Regression: EE 10 Platform API JARs include transitive dependencies to Spec API JARs leading to duplicate classes and JPMS split package failures Regression: EE 10 Platform API JARs include all classes, and transitive dependencies to Spec API JARs leading to duplicate classes and JPMS split package failures Jan 7, 2023
@lprimak
Copy link
Contributor Author

lprimak commented Jan 23, 2023

@edburns What do you think? Should I submit a PR? Should be a simple matter

@lprimak lprimak changed the title Regression: EE 10 Platform API JARs include all classes, and transitive dependencies to Spec API JARs leading to duplicate classes and JPMS split package failures Regression: EE 10 Platform API JARs include all classes, and transitive dependencies to Spec API JARs leading to duplicate classes and JPMS split package failures and OSGi issues Jan 28, 2023
@lprimak
Copy link
Contributor Author

lprimak commented Jan 28, 2023

@edburns @OndroMih @ivargrimstad I have changed the description of the ticket to simplify and align with the current problem status and ways to fix it.

Thank you

@starksm64
Copy link
Contributor

It seems to be an unexpected side effect of trying to clean up the dependency graph of the profile/platform jars. As @ivargrimstad says we are not using these jars as normative artifacts that are validated in any TCK, but it might as well be fixed.

@lprimak
Copy link
Contributor Author

lprimak commented Feb 14, 2023

@starksm64 So do you agree with the approach to ship empty JARs for umbrella (platform and web profile) JARs? Or do you have something else in mind?

@JanWesterkamp-iJUG
Copy link
Contributor

@lprimak, I agree that the umbralla specs should not add APIs, they should consist of the components specs API only, as provided specs.

I am unsure I understand what is your exact solution for the problem - mark the umbrella spec as provided or component specs as optional (with the last, I would disagree)?
Or package the umbrella spec dependency as type POM?
This may indeed bring Maven to not adding/creating a JAR file consting the compoent spec API artefacts JARs...

Your linked commit is refering to the Jakarta Core Profile API - which is not a real subset of Jakarta Web Profile API (only an equal subset) at the moment - personally, I would like to change that, but this is under discussion...
A change in the Core Profile API can not have a direct impact on the Web or Full Profile aka Platform - but of course when adding both it can lead to dublicates too, if the implementations dependency management does not remove doublets.

I think it is very important to understand the system environment behaviour with modularisation technics like JPMS, Maven, OSGi here to solve this.

May be you can give some details about the environment you used to reproduce and test potential solutions with it?

@lprimak
Copy link
Contributor Author

lprimak commented Mar 1, 2023

@JanWesterkamp-iJUG My solution is simple:
Ship empty JARs for all three umbrella JARs: jakarta.jakartaee-api-10.0.0.jar , jakarta.jakartaee-web-api-10.0.0.jar , and jakarta.jakartaee-core-api-10.0.0.jar

By 'empty' I mean a JAR that contains no files at all.

This way, all classes will be loaded from the transitive dependencies and no duplicates will be loaded.

@OndroMih
Copy link

OndroMih commented Mar 1, 2023

@JanWesterkamp-iJUG My solution is simple: Ship empty JARs for all three umbrella JARs: jakarta.jakartaee-api-10.0.0.jar , jakarta.jakartaee-web-api-10.0.0.jar , and jakarta.jakartaee-core-api-10.0.0.jar

By 'empty' I mean a JAR that contains no files at all.

This way, all classes will be loaded from the transitive dependencies and no duplicates will be loaded.

I fully support this.

This will also solve the problem when adding multiple profiles as dependencies. If they have the same version, they would pull the same dependencies without duplicates.

@JanWesterkamp-iJUG
Copy link
Contributor

@OndroMih and I think this should work even when there are different versions in the profiles (in Maven, then the order and depth defines which version is choosen), but having the Core Profile to be a true subset of the Web Profile (as this is a true subset of the Platform/Full Profile) can solve this too - my favorite solution ;-)

@lprimak so which options we have to achieve this (in Maven, so it is fixed in JPMS and OSGi too)? Make the component specs optional in umbrella specs might solve this, but creates other issues...
Can you list the potential options please, as you did some experiments already?

I would like to add this to the CN4J call preparation slides then. A new version is available here: jakartaee/platform#607).

@lprimak
Copy link
Contributor Author

lprimak commented Mar 1, 2023

I think there is only one viable option, which is the empty JAR option outlines above.

The more I think about the other possibilities the more problems I see with them

@lprimak
Copy link
Contributor Author

lprimak commented Mar 16, 2023

@starksm64 ping :) Any feedback is appreciate, thank you!

@starksm64
Copy link
Contributor

Yes, let's move ahead with the empty jar approach.

@lprimak
Copy link
Contributor Author

lprimak commented May 3, 2023

I think this is important enough to release 10.0.1 IMHO
@ivargrimstad

@lprimak
Copy link
Contributor Author

lprimak commented May 15, 2023

@OndroMih what do you think of the fix?

@lprimak
Copy link
Contributor Author

lprimak commented May 16, 2023

As a "side-effect" of the fix of this issue, it'll be much easier to update individual Specs on ad-hoc basis, as noted in the Jakarta EE Contribution Guide @m-reza-rahman

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.

5 participants