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

shipping all platform JARs as empty JARs #137

Merged
merged 1 commit into from
May 30, 2023

Conversation

lprimak
Copy link
Contributor

@lprimak lprimak commented May 3, 2023

Avoids duplicate class files and fixes OSGi and JPMS issues
fixes #133

@lprimak lprimak changed the title shipping all platform JARs as empty JARs, fixes #133 shipping all platform JARs as empty JARs May 3, 2023
@lprimak lprimak marked this pull request as draft May 3, 2023 05:39
@starksm64
Copy link
Contributor

Look good to me. I did some basic compile testing of the generated empty jars and it works.

@lprimak lprimak marked this pull request as ready for review May 3, 2023 06:31
@lprimak
Copy link
Contributor Author

lprimak commented May 3, 2023

Thanks! I just tested in my environment and it also works!
I think it's important enough to release 10.0.1 IMHO

@lprimak
Copy link
Contributor Author

lprimak commented May 10, 2023

Time to merge (and release?)

@OndroMih
Copy link

I think this will break javadoc generation for Jakarta EE platform and profiles.

I assume that javadocs are generated with mvn javadoc:aggregate. If I run it before this PR, this goal runs everything until the compile phase, including glassfishbuild-maven-plugin:3.2.28:unpack-sources and maven-resources-plugin:2.4.3:copy-resources. Sources and resources from dependencies are used to generate the aggregate javadocs in target/site/apidocs. With this PR, this doesn't happen and aggregate Javadocs in target/site/apidocs are empty.

@OndroMih
Copy link

OndroMih commented May 16, 2023

Instead of the changes in this PR, it should be enough to modify the parent pom with the following changes: OndroMih@9b56052

I tested it and all works - the platform JARs don't contain classes and running mvn javadoc:aggregate inside platform dir generates javadoc with all APIs.

@lprimak
Copy link
Contributor Author

lprimak commented May 16, 2023

Can you make the PR then I can close mine?

@lprimak
Copy link
Contributor Author

lprimak commented May 16, 2023

Or I can just port your changes into mine if you prefer

@lprimak
Copy link
Contributor Author

lprimak commented May 16, 2023

I found an issue with javadoc, I will fix it and combine our solutions into a the existing PR

@lprimak
Copy link
Contributor Author

lprimak commented May 16, 2023

@OndroMih I incorporated your changes, plus additional source dependencies for platform API javadocs so they include core and web APIs, just like they did in the past.

Let me know what you think, thank you!

@OndroMih
Copy link

Thanks, @lprimak. Now it looks good to me.

@Emily-Jiang
Copy link

Would this break the current apps that compile against the platform jars but now it is empty?

@lprimak
Copy link
Contributor Author

lprimak commented May 23, 2023

Would this break the current apps that compile against the platform jars but now it is empty?

Absolutely not, since they will still have maven dependencies on the spec JARs.
The only way I can see anything breaking is if the umbrella JARs are manually copied and used on the CLASSPATH, i.e. copied from ~/.m2/repository/... into tomcat/lib or something similar.

@edburns edburns merged commit e8fcd8c into jakartaee:master May 30, 2023
@lprimak lprimak deleted the empty-platform-jars branch May 30, 2023 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants