-
Notifications
You must be signed in to change notification settings - Fork 6
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
Enhancements: failOnAutomaticModules, Real Module Patching, and Module Specs Generation #75
Conversation
52a1534
to
b23e05b
Compare
4e9e4de
to
3e8eb69
Compare
3e8eb69
to
2900689
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is awesome @iherasymenko! I 100% would like to have the main use case of the plugin to create fully modular applications (and not to do automatic modules or any other in between things). This moves it further into that direction. Thanks so much.
For the first two features, I only have some comments about wording. They are both great improvements and I have been thinking about adding things like that myself.
The recommendation task also looks good and very helpful. I am happy to have it as part of the plugin. The only larger comment is that the current solution is not configuration cache compatible. Which is a common problem with tasks that take dependency analysis results as input. I think I worked something out that solves it (see comment). If you could integrate that change or something similar, we can integrate the PR.
src/main/java/org/gradlex/javamodule/moduleinfo/ExtraJavaModuleInfoTransform.java
Outdated
Show resolved
Hide resolved
src/main/java/org/gradlex/javamodule/moduleinfo/ExtraJavaModuleInfoTransform.java
Outdated
Show resolved
Hide resolved
src/main/java/org/gradlex/javamodule/moduleinfo/ExtraJavaModuleInfoTransform.java
Outdated
Show resolved
Hide resolved
src/main/java/org/gradlex/javamodule/moduleinfo/ModuleDescriptorRecommendation.java
Outdated
Show resolved
Hide resolved
src/main/java/org/gradlex/javamodule/moduleinfo/ExtraJavaModuleInfoPlugin.java
Outdated
Show resolved
Hide resolved
src/main/java/org/gradlex/javamodule/moduleinfo/ExtraJavaModuleInfoPlugin.java
Outdated
Show resolved
Hide resolved
src/main/java/org/gradlex/javamodule/moduleinfo/ModuleDescriptorRecommendation.java
Outdated
Show resolved
Hide resolved
d743c78
to
bbe947c
Compare
Thank you very much for the thorough review, @jjohannes! I applied the suggestions in the three new separate commits (for the ease of review). Please let me know if I missed anything and if I can squash the fix commits with the feature commits. |
Released as |
Hi @jjohannes . I apologize in advance if this is a too big of a PR. I'm happy to split it into a few smaller ones; however, each commit represents a different piece of functionality so it should be easy to review/merge only portions of it.
This PR add the three new features to the plugin.
failOnAutomaticModules configuration
This is useful when dealing with big (10+ elements) classpath's with the goal to migrate 100% of them to be real modules. Similar to
failOnMissingModuleInfo
, this setting will fail the build if a configuration contains a JAR with an automatic module defined via the JAR Manifest.Allow patching real modules
Even modular JARs may carry invalid module descriptors. For example,
org.apache.tomcat.embed:tomcat-embed-core:10.1.13
exports
a package that does not exist (org.apache.catalina.ssi
) in the JAR and as a result, this module cannot be used withjlink
.A new task to generate module specs (DSL?) using jdeps.
This is similar to
requireAllDefinedDependencies()
andexportAllPackages()
. This task usesjdeps
under the hood so that it can infer usages of the JDK modules (e.g,java.sql
,java.xml
, etc) compared to just looking at the dependency metadata. It also operates on the classpath of a configuration, i.e. users will not need to defineknownModule
for the already modularized JARs.This task requires Gradle to be powered by JDK 11+ but I made sure the other functionality of the plugin can still be used on JDK 8.
I can open a follow-up PR to make this task compatible with Gradle JVM Toolchains if needed.