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

Add module descriptor #4559

Closed
XakepSDK opened this issue Apr 14, 2021 · 8 comments
Closed

Add module descriptor #4559

XakepSDK opened this issue Apr 14, 2021 · 8 comments
Assignees

Comments

@XakepSDK
Copy link

Add module descriptor (module-info.java)
It's possible to use multi-release jar to support both Java 8 and have module descriptor
Compiled module descriptor should be here META-INF/versions/9/module-info.class

@cowwoc
Copy link

cowwoc commented Oct 26, 2021

@smillst Any update on this issue?

@mtf90
Copy link
Contributor

mtf90 commented Nov 28, 2023

I am currently in the process of modularizing some projects in order to use jlink (and later jpackage) for distributing software and came across the issue that checkerframework only provides module support via the Automatic-Module-Name: org.checkerframework.checker.qual entry in the MANIFEST.MF. This causes problems with jlink:

[ERROR] Error: automatic module cannot be used with jlink: org.checkerframework.checker.qual from file:///home/frohme/.m2/repository/org/checkerframework/checker-qual/3.40.0/checker-qual-3.40.0.jar

Are there any plans for (or is there at least any interest in) adding proper modulization support to checkerframework (at least for the client-facing artifacts such as checker-qual)?

Since I am in a similar boat of wanting to add modulization support without breaking Java 8 compatibility, I can report that this can be relatively cleanly implemented with Multi-Release JARs indeed. There are only small "drawbacks" to this approach:

  • You now need a Java 9+ JDK to build checkerframework
    • Between Java 9-11 there is some wonkiness in the Javadoc generation so that it may happen that people on Java 8 cannot properly reference, e.g., JDK11 generated Javadoc (I had a similar issue with recent guava releases). However, given that you seem to build on JDK17 anyway (see the links to java.lang here), you are already affected by this problem or the fix thereof.
  • The build process gets a little bit bloated since you would now need three compilation steps.

If wanted, I can try to sketch an initial PR draft for this feature. However, since I am more used to Maven than Gradle and I am rather unfarmiliar with all the intrinsics of the checkerframework build process, I'll probably need some hand holding later.

@mernst
Copy link
Member

mernst commented Nov 28, 2023

@mtf90 Thanks for bringing this up. Modularization isn't high on our priority list, but we would welcome it. If you can take the lead, we would gladly help. Thank you!

Requiring a Java 9+ JDK to build the Checker Framework is not a problem.

@mtf90
Copy link
Contributor

mtf90 commented Nov 30, 2023

Dear @mernst, I have good news and bad news:

The good news is that adding a module-info.java is even easier than OP and I previously suggested. You don't even need a multi-release JAR setup (which may cause some headaches later during testing) but you can simply add the module-info.java to the root directory of the sources and classes folder. Since module-info is an invalid Java identifier, you can't accidentally reference it in Java 8 code and only JDKs newer than 8 know about the special semantics of this file (of course you could still programmatically scan a .jar file for *.classes and try to load them with an UnsupportedClassVersionError but this would also be possible with MRJARs). This is particularly nice for development because the file can reside next to the regular code and is correctly picked up by most IDEs. (Credit goes to https://beryx.org/blog/2018-11-21-/modular-jars-targeted-at-pre-java-9/)

The bad news is that Gradle broke me. I have drafted a PR (#6326) which ... works. It creates a .jar file whose .classes have major version 52 with the exception of the root module-info.class which has major version 53. I have verified with my internal projects that the checker-qual module can be correctly jlinked and still compiles on JDK8.

The changes itself could potentially be implemented more elegantly. I think sourceSets are probably the way to go but I couldn't get them to share folders while having separate compile executions. Also, the dependencies between the different tasks are a little bit overwhelming so the config looks a little bit patched together.

There is one big workaround that I am still unhappy with. Initially, the javadoc generation failed because in a module-senstive environment the module-info.java determines the dependencies. However, checker-qual has a (javadoc) compile dependency on the checker module while simultaneously checker requires the compiled checker-qual annotations. I believe Gradle allows one to workaround such cyclic dependencies by creating one big classpath. However, I can't see how this could be solved for modules. Typically you would add another require static dependency in the module-info.java (like the commented one) but the referenced module could only be read after the checker.jar has been built. Maybe this can be solved by isolating the javadoc process a little bit more, maybe this requires some actual refactoring. For now, I have disabled the module-semantics during javadoc generation. Unfortunately, this means that (compile-time) dependencies could be missed.

I am happy to discuss further actions or handing things over to the Checkerframework team. Do you want to continue on this issue or the PR?

@mernst
Copy link
Member

mernst commented Nov 30, 2023

@smillst Could you please take over?

@mtf90
Copy link
Contributor

mtf90 commented Jan 25, 2024

Just a heads-up for everyone that faces similar jlink-/jpackage-related problems as mentioned in #4559 (comment): A potential workaround to this issue is to declare the dependency on org.checkerframework.checker.qual as static so that it won't be included in the jlinked/jpackaged bundle. This should not cause any problems, as JLS 13.5.8 states that

Adding or deleting annotations has no effect on the correct linkage of the binary representations of programs in the Java programming language.

In case of a distributed application the missing annotations should not matter, because the user of the application typically does not care about the application's internals (e.g., which classes it uses, etc.). In case of a library-like artifact, any code that would make use of the annotations (e.g., for nullness analysis) would add its own dependency on checker-qual making the annotations available again when loading (and analyzing) the code even with the static dependency declaration.

This should take some pressure off this issue, as I mentioned in #6326 (comment) that it may take some effort to support modules cleanly. For now, an Automatic-Module-Name may be the best we can get.

@sgammon
Copy link

sgammon commented Mar 8, 2024

#6326 was merged; I think this can be closed @mernst 😄

@mernst
Copy link
Member

mernst commented Mar 8, 2024

@sgammon Thanks for the ping.

@mernst mernst closed this as completed Mar 8, 2024
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

6 participants