commons-lang
commons-lang copied to clipboard
apache
Add module-info for Java 9
Awesome! Would be create if you could create and reference a JIRA ticket as described in CONTRIBUTING.md, so this will show up in our release notes.
Coverage increased (+0.01%) to 95.199% when pulling cb5a17949bf941118a80ef05ccd5a77717779792 on jodastephen:module-info into 1f0dfc31b51a445eb2cfbee5321800cf51e10b67 on apache:master.
Coverage increased (+0.01%) to 95.199% when pulling 07c0c408e72836231ef262f565761778e3e9e103 on jodastephen:module-info into 1f0dfc31b51a445eb2cfbee5321800cf51e10b67 on apache:master.
Coverage increased (+0.01%) to 95.199% when pulling 886b26aa09efc1a4bfca3470e33b952399f18f6d on jodastephen:module-info into 1f0dfc31b51a445eb2cfbee5321800cf51e10b67 on apache:master.
Is there any way to rewrite AbstractCircuitBreaker to eliminate the dependency on java.desktop? Java.desktop is quite a large dependency to have for the sake of one class.
@michaelsavich I have a proposal in a branch, submitted some months ago https://github.com/apache/commons-lang/pull/275
Waiting for feedback to prepare deprecated annotations in place, and merge in a major release (I believe it breaks BC)
@michaelsavich , this PR does not create a full dependency on java.desktop. The dependency is expressed as requires static, which means that it will not be pulled in by default. Users that want to use the two broken classes on Java 9 in module mode (a very small percentage of commons-lang users) will need to manually include the java.desktop dependency. As such, there is no urgency to get AbstractCircuitBreaker removed.
Coverage decreased (-0.02%) to 95.163% when pulling 8362e24c2edd5a7cb88fd9b74c71475462e2fb64 on jodastephen:module-info into 1f0dfc31b51a445eb2cfbee5321800cf51e10b67 on apache:master.
Coverage decreased (-0.02%) to 95.163% when pulling 8362e24c2edd5a7cb88fd9b74c71475462e2fb64 on jodastephen:module-info into 1f0dfc31b51a445eb2cfbee5321800cf51e10b67 on apache:master.
I'll bring this up a last time on the ML to make sure nobody has objections against merging this.
mvn install works fine on each JDK version AFAICT. mvn site still breaks on cobertura on Java 9 and I'm not sure I can work around it without removing it from the parent pom.
@jodastephen What's the error that you get with site? Any links to the build?
Coverage decreased (-0.02%) to 95.163% when pulling 313723da37a87a683683ff5edab139cf10612573 on jodastephen:module-info into 1f0dfc31b51a445eb2cfbee5321800cf51e10b67 on apache:master.
@namannigam You need to check it out and run mvn clean install site on Java 9.
Adding module-info causes some older/Android projects to fail as they can't handle the class format. So, projects are left with the choice of moving forwards or breaking these older/Android projects.
This issue is also tied up in the need to modernise the commons parent pom.xml for Java 9 and later.
@jodastephen have you checked out https://github.com/moditect/moditect ?
Moditect uses Objweb-ASM to compile the module-info.class which means you can support Java 1.7 / 1.8 for Android.
Its not just compiling, there are issues at runtime still AFAIK, especially with Java EE and older bytecode libraries.
There is a lot of complexity in commons-parent and commons generally. Getting a build to work for Joda was hard enough, and there I am willing to insist on releases being done on Java 9 or later. There needs to be a lot more work on the commons build before module-info should be considered, however at the moment its not clear as to whether doing that work would be positive (due to Android, Jakarta EE issues etc)
@jodastephen moditect can be configured to generate module-info.class into META-INF/versions/9/module-info.class instead, maintaining compatibility with Java8 and earlier runtimes.
@jodastephen @jfallows I have successfully managed to get moditect on lang3, io, fileupload and text -
https://github.com/GedMarc/commons-lang https://github.com/GedMarc/commons-io https://github.com/GedMarc/commons-fileupload
Using source and target of 1.6 is still supported to build for that target JDK (I have done this successfully with the org.json project), however the actual build has to happen in JDK 8 for the plugin to work.
When building the module-info, no import statements are allowed as yet
I place them directly in the root, @ClassGraph (Duke Choice Winner), places the module info files in the meta-inf/versions/ correctly with moditect. He also participates in the moditect development.
https://github.com/GedMarc/commons-lang/blob/master/src/jre11/java/module-info.java https://github.com/GedMarc/commons-fileupload/blob/master/src/jre11/java/module-info.java
@GedMarc if you add the following to <configuration> section of moditect-maven-plugin version 1.0.0.Beta2, it will place module-info.class into META-INF/versions/9/module-info.class instead.
<jvmVersion>9</jvmVersion>
@jfallows Yeah I've gone through a couple iterations and different mechanisms, about a year and half ago there was an issue in a few of the older ones which I'm sure this project will care about, in things like Wildfly 10/EAP 7 using Jandex the scanner would warning out on the class structure and items like that, so "hiding" the class there seemed like a good idea, or the older reflections etc,
For me though I'm planning on discontinuing JDK 8 soon though, so for my purposes and to build cleanly with a full distribution cycle jdk 12 13 and up, the root definitely seems like the best idea and 190 odd projects later (Assisting Jackson JSON, having to use local builds of Guice no replies on the pull requests, stopped making pull requests because of that actually, commons).
JLink, and JPMS, has it's requirements though, but the results are spectacular.
@jfallows for interests sake though - https://github.com/GedMarc/JPMSPendingParent/tree/80383124a9c0f804dc1d1518aae54581d7c00521 is a listing of all the libraries that I'm expecting full JPMS compliance with in the short term, they are deployed on maven central under group com.jwemp.jpms
The list of modules that are either finalized/closed/owner's non-contactable, or the repositories abandoned are here https://github.com/GedMarc/JPMSThirdPartyParent/tree/86537e0f18de56e10c3d9da4cb5a68f9273b7e87
If you can help in getting a hold of these guys, specially javax.inject, man that would be fantastic
I've seen error reports from using multi-version jar files as well as from just placing the module-info.class in the root. It is perfectly possible to create a fully modularized commons-lang provided the team are willing to exclude some users who can't handle module-info.class or multi-version jar files.
@GedMarc thanks for the links, will help however I can.
I'm intrigued by the italics of "seemed like a good idea". 😄
Libraries designed with a base version of Java9 can handle module-info.class at the root, whereas older libraries designed with base version of Java8 or lower might invalidate assumptions about class file contents when placing module-info.class at root, but that should be largely mitigated by placing module-info.class under META-INF/versions/9 instead for runtime discovery by Java9 and higher.
Did you find that there were remaining issues where libraries scanned for all class files inside a JAR, even if they were under META-INF/versions/9 and still tripped up on module-info.class due to their historical assumptions about class file structure? If so, is the proposed solution for those libraries to make them resilient to the presence of module-info.class in general, whether at the root or under META-INF/versions/9, i.e. treat like a bug and fix them?
@jodastephen please share details about those error reports if possible, just in case there are additional relevant pitfalls to navigate 👍
The last time I tried to use a jar with a Java 9 class file properly embedded in META-INF it blew up the Android toolchain. It's been a while so I am not sure what the current state is...
