-
Notifications
You must be signed in to change notification settings - Fork 36
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
Feature request: Separate compilation of module-info.java #72
Comments
necessary for further commits MAIN: 1) renamed TestModuleOptions.isRunOnClasspath() to getRunOnClasspath() (otherwise won't work with Kotlin DSL) + added a Kotlin DSL example for "runOnClasspath = true" to README.md 2) introduced JavaProjectHelper and applied it to CompileTask 3) added comments for all anonymous classes that should not be removed 4) minor improvements in ModuleSystemPlugin 5) minor fixes in test-project-kotlin/README.md TEST: 1) bumped smoke-test Gradle version to 5.0 (for improved Kotlin DSL) 2) introduced a no-op "moduleOptions" access in greeter.api (for testing DSL) 3) introduced SmokeTestHelper for ModulePluginSmokeTest
Thanks for the great overview. Definitely a good usecase to support! |
It seems no one is adopting JPMS... Some believe it has no future. |
@tlinkowski The badass-jar plugin is probably what you're looking for. |
@danieldietrich I guess it's because the adoption of JDK 9+ seems very small yet. But if it was easy to bundle a |
@siordache Thanks! I forgot to mention your plugin (I think I stumbled upon it before). I'll edit the overview. |
Hey, thanks for the mention! On this one:
This is only one mode of operation in ModiTect. You also can give the contents of a module-info.java file verbatim in your pom.xml or build.gradle file; alternatively, its contents can also be obtained from a given file in the project. This will be compiled as is to the class file, using ASM. This is also the mode that works on JDK < 9. So it seems it already pretty much does what you are after (IIUC)? |
It depends on the specific implementation of scanning. But we had definitely issues with that, e.g. with older versions of Jetty stumbling upon the file. Note that scanning often isn't done using reflection on the classpath but by "manually" reading class files using ASM or similar. Older tools doing that will usually not be aware of classfiles under META-INF/versions. |
It actually uses the org.javamodularity.moduleplugin, as mentioned in the README: "If the plugin detects that at least one of sourceCompatibility and targetCompatibility has an effective value >= 9, it automatically applies the gradle-modules-plugin, which adds support for the JPMS."
You can prevent the creation of a multi-release jar by setting multiRelease to false in the jar section of your build script:
However, you are right about the plugin not using the |
@gunnarmorling Thanks, I didn't know ModiTect can do it! Still, I'm afraid that using it that way would be a bit like using a sledgehammer to crack a nut 😉 Plus, I'd feel safer to have my
As far as I understand, we're talking about tools that scan the classpath for To sum up, if I understand correctly:
|
necessary for further commits MAIN: 1) renamed TestModuleOptions.isRunOnClasspath() to getRunOnClasspath() (otherwise won't work with Kotlin DSL) + added a Kotlin DSL example for "runOnClasspath = true" to README.md 2) introduced JavaProjectHelper and applied it to CompileTask 3) added comments for all anonymous classes that should not be removed 4) minor improvements in ModuleSystemPlugin 5) minor fixes in test-project-kotlin/README.md TEST: 1) bumped smoke-test Gradle version to 5.0 (for improved Kotlin DSL) 2) introduced a no-op "moduleOptions" access in greeter.api (for testing DSL) 3) introduced SmokeTestHelper for ModulePluginSmokeTest 4) disabled stackTraceFilters in all tests 5) updated Kotlin to 1.3.20
…rately" added CompileModuleOptions and CompileModuleInfoTask NOTE: potentially breaking change for "compileJava" in Kotlin DSL (see modified "build.gradle.kts")
via ModularityExtension interface
tests the "modularity" extension (incl. verifying generated class file formats)
With #73, I provided a complete implementation of this proposal (with tests & documentation). Of course, this issue is still open for discussion, and I'm ready to adapt the provided PR as needed. |
I think there's nothing to be worried about, ASM works just fine. The reasoning for using it btw. was to enable the build itself running on older JDKs, which naturally can't be done when using javac to compile the descriptor. Another thing I'm curious about: how does the suggest approach work exactly, will the classes be compiled twice, once with module descriptor and once without?
That's interesting, because it's actually intended to reduce complexity. E.g. my thinking is that specifying exported packages via wildcards can help to reduce efforts, similar to "requires" clauses derived from dependencies. So I'm curious which complexity you actually perceive? Not I'm welcoming this effort and obviously I've no stake in deciding whether this should be included here or not; I'm just curious to see what could be improved in ModiTect to further improve it. |
Thanks for your response, Gunnar!
Agreed that ASM works fine 🙂 What I'm worried about is that Java compiler (javac) is a really complex tool. Here, I simply propose a thin wrapper over this tool. ModiTect, on the other hand, is another complex tool that (in this use case) duplicates what javac does. Still, ModiTect is a great tool, and I'd use it if the benefits for me:
outweighed the costs for me:
This is certainly an advantage of ModiTect! Only, I don't need to run the build on an older JDK. I only need to target users who might run on an older JDK.
A good question! Actually, every class is compiled only once:
Yes, these are cool features that reduce complexity of usage (if we don't count lack of IDE support). The complexity I perceive is primarily the complexity of the tool itself (another thing to learn, higher risk of issues).
ModiTect is a great tool but:
I'd probably decide against using it. |
@tlinkowski This is great! Thanks for the detailed writeup and PR. I'm fully onboard with supporting this, seems like a great feature to further increase JPMS adoption. |
necessary for further commits MAIN: 1) renamed TestModuleOptions.isRunOnClasspath() to getRunOnClasspath() (otherwise won't work with Kotlin DSL) + added a Kotlin DSL example for "runOnClasspath = true" to README.md 2) introduced JavaProjectHelper and applied it to CompileTask 3) added comments for all anonymous classes that should not be removed 4) minor improvements in ModuleSystemPlugin 5) minor fixes in test-project-kotlin/README.md TEST: 1) bumped smoke-test Gradle version to 5.0 (for improved Kotlin DSL) 2) introduced a no-op "moduleOptions" access in greeter.api (for testing DSL) 3) introduced SmokeTestHelper for ModulePluginSmokeTest 4) disabled stackTraceFilters in all tests 5) updated Kotlin to 1.3.20
…rately" added CompileModuleOptions and CompileModuleInfoTask NOTE: potentially breaking change for "compileJava" in Kotlin DSL (see modified "build.gradle.kts")
via ModularityExtension interface
tests the "modularity" extension (incl. verifying generated class file formats)
necessary for further commits MAIN: 1) renamed TestModuleOptions.isRunOnClasspath() to getRunOnClasspath() (otherwise won't work with Kotlin DSL) + added a Kotlin DSL example for "runOnClasspath = true" to README.md 2) introduced JavaProjectHelper and applied it to CompileTask 3) added comments for all anonymous classes that should not be removed 4) minor improvements in ModuleSystemPlugin 5) minor fixes in test-project-kotlin/README.md TEST: 1) bumped smoke-test Gradle version to 5.0 (for improved Kotlin DSL) 2) introduced a no-op "moduleOptions" access in greeter.api (for testing DSL) 3) introduced SmokeTestHelper for ModulePluginSmokeTest 4) disabled stackTraceFilters in all tests 5) updated Kotlin to 1.3.20
added CompileModuleOptions and CompileModuleInfoTask NOTE: potentially breaking change for "compileJava" in Kotlin DSL (see modified "build.gradle.kts")
tests the "modularity" extension (incl. verifying generated class file formats)
Merged the work done by @tlinkowski! Will be in the next release (which I plan to publish today) |
Overview
I'd like to request a feature for library developers who need to target JDK 6-8 but who also want to provide a
module-info.class
(as per Option 3 by @jodastephen).The proposed feature has two parts:
module-info.java
.--release
option, separately for main Java code and formodule-info.java
.Justification
General
Library maintainers need to target JDK 6-8 because these JDKs (especially JDK 8) are still very popular (taking ~95% share in 2018, and predicted to take ~90% in 2019).
Still, those maintainers could make the most of JPMS by:
module-info.class
for consumers who put the library on module path,module-info.java
against the remaining classes of this module and against other modules (which provides better encapsulation and prevents split packages).So providing a
module-info.class
is like saying:To sum up, I see this feature request as great means of promoting JPMS! Would you agree, @paulbakker, @sandermak?
Examples
Here are some popular libraries that provide
module-info.class
while targeting JDK 6-8, e.g.:module-info.class
in root dir)module-info.class
in root dir)module-info.class
inMETA-INF/versions/9
- Multi-Release JAR)module-info.class
in root dir)There are also some libraries that:
Vavr /ˈveɪ.vɚ/
Let's have a closer look at vavr's case:
module-info.class
while still targeting JDK 8.Proposed DSL
Note: Everything in this section (especially naming) is TBD.
Low-level API
Extra property on
moduleOptions
extension:build.gradle
build.gradle.kts
High-level API
Special
modularity
extension with two functions:mixedJavaRelease(int mainJavaRelease, int moduleInfoJavaRelease = 9)
standardJavaRelease(int mainJavaRelease)
For example, the most common configuration (JDK 8 + JDK 9
module-info.class
) would be a one-liner:build.gradle
modularity.mixedJavaRelease 8
build.gradle.kts
modularity.mixedJavaRelease(8)
Proposed behavior
module-info.java
locationI propose to leave
module-info.java
insrc/main/java
(I don't see any need to put it in a separate source set directory).module-info.class
locationThere are two places where
module-info.class
can end up:module-info.java
location)META-INF/versions/9
(Multi-Release JAR, AKA MRJAR)Having read a post on MRJARs by @melix, I'm rather suspicious of MRJARs, and so I prefer option 1.
On the other hand, @gunnarmorling claims that it's better to use option 2 because
module-info.class
"will be out of the way in most cases when being used on an older Java version". But I don't really know why, because as far as I understand, any tool that scans the classpath (like Guava'sClassPath
) will returnmodule-info.class
no matter where it is (based on its.class
extension). @gunnarmorling, would you care to give me a pointer here?Alternatives
ModiTect
It's fair to mention ModiTect (by @gunnarmorling) and its Gradle plugin (by @siordache) here.
However, ModiTect does much more than I request here: ModiTect actually generates
module-info.class
from a special notation (there's even noedit: it can actually parsemodule-info.java
theremodule-info.java
). Personally, I find it too complex for my needs.Badass-Jar plugin
@siordache also created a Gradle plugin that lets one "seamlessly create modular jars that target a Java release before 9".
It looks quite nice, however:
I'm not sure how it plays along withorg.javamodularity.moduleplugin
(and I'd like to use it for patching modules, testing on module-path, etc.)module-info.java
is correct, you actually have to run the build twice (./gradlew jar
and./gradlew -PjavaCompatibility=9 jar
) - too complex--release
option, which (as opposed to using-source
and-target
) guarantees that only the right APIs are referenced (e.g. it throws when compilingoptional.isEmpty()
with--release 8
)it produces only Multi-Release JARs, and I'm somewhat skeptical about them (as I mentioned before)Related StackOverflow questions
The text was updated successfully, but these errors were encountered: