Support Java9 modules while still Java7+ compatible

jfallows commented 5 years ago

Uses moditect to generate module-info.class without requiring Java9 as minimum source version. Successful local builds with Java7, Java8, Java9, Java10.

Note: moditect requires Java8, so while Java7 build is successful, no module-info.class is included in the final JAR when built with Java7 toolchain.

Recommend performing releases with Java8 to include module-info.class.

Now enforcing Java8 toolchain in release profile while still targeting 1.7 class format for runtime compatibility.

coveralls commented 5 years ago

Pull Request Test Coverage Report for Build 475

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-0.1%) to 70.895%

Totals
Change from base Build 458:	-0.1%
Covered Lines:	5203
Relevant Lines:	7339

Pull Request Test Coverage Report for Build 484

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-0.1%) to 70.895%

Totals
Change from base Build 458:	-0.1%
Covered Lines:	5203
Relevant Lines:	7339

💛 - Coveralls

jfallows commented 5 years ago

Do you want to include the POM changes to set up the release profile to require/target JDK 8 at a minimum?

No problem, I'll take a look.

Did you produce these module-info.java by hand or did you use the plugins generate-module-info goal to generate them? If the latter it might be useful to also make that available in a Maven profile so if dependencies change in the future we can regenerate the module info's appropriately

Each module-info.java is authored by hand for the moment, but I'll investigate generation options.

jfallows commented 5 years ago

Added generation of candidate module-info.java via generate-module-info maven profile.

However, latest SNAPSHOT of maven-moditect-plugin fixes issue when attempting to generate-module-info in project of type pom such that jar artifact cannot be found, hence the -Dplugin.moditect=1.0.0-SNAPSHOT below, which can be removed after upgrading plugin.moditect to next released version.

export JAVA_HOME=`/usr/libexec/java_home -v 9`
mvn clean install -P-moditect-jdk8-plus
mvn generate-sources -Pgenerate-module-info -Dplugin.moditect=1.0.0-SNAPSHOT
cat airline-core/target/generated-sources/modules/com.github.rvesse.airline/module-info.java
cat airline-io/target/generated-sources/modules/com.github.rvesse.airline.io/module-info.java

Note: it is necessary to first build without module-info.class in the resulting JARs, hence the -P-moditect-jdk8-plus above, otherwise jdeps complains during -Pgenerate-module-info that the project artifact JAR is already modular.

jfallows commented 5 years ago

The generated candidate module-info.java for com.github.rvesse.airline module previously included incorrect required module name airline-io instead of com.github.rvesse.airline.io.

Now added profile to include Automatic-Module-Name matching actual module name when moditect-maven-plugin is skipped via moditect.skip property. This capability requires latest SNAPSHOT build of moditect-maven-plugin, hence -Dplugin.moditect=1.0.0-SNAPSHOT below.

export JAVA_HOME=`/usr/libexec/java_home -v 9`
mvn clean install -Dmoditect.skip -Dplugin.moditect=1.0.0-SNAPSHOT
mvn generate-sources -Pgenerate-module-info -Dplugin.moditect=1.0.0-SNAPSHOT
cat airline-core/target/generated-sources/modules/com.github.rvesse.airline/module-info.java
cat airline-io/target/generated-sources/modules/com.github.rvesse.airline.io/module-info.java

After upgrading to next moditect-maven-plugin release, this can be simplified as follows.

export JAVA_HOME=`/usr/libexec/java_home -v 9`
mvn clean install -Dmoditect.skip
mvn generate-sources -Pgenerate-module-info
cat airline-core/target/generated-sources/modules/com.github.rvesse.airline/module-info.java
cat airline-io/target/generated-sources/modules/com.github.rvesse.airline.io/module-info.java

Note: the current 1.0.0.Beta2 release of moditect-maven-plugin is sufficient for the airline build to generate module-info.class from the checked in module-info.java. The latest SNAPSHOT version is only required for the manual build steps outlined above to generate candidate module-info.java. I have explicitly requested a new moditect release so that you can avoid the need to use the local SNAPSHOT version at all, see moditect/moditect#98.

jfallows commented 5 years ago

Generated module-info.class to META-INF/versions/9 as described by JEP-238.

A multi-release modular need not have a module descriptor at the located root. In this respect a module descriptor would be treated no differently to any other class or resource file. This can ensure that, for example, only Java 8 versioned classes are present in the root area while Java 9 versioned classes (including the module descriptor) are present in the 9 versioned area.

jfallows commented 5 years ago

@rvesse I have no further changes planned for this PR, please review and let me know if you need anything else before merge.

rvesse commented 5 years ago

@jfallows Thanks. It’s a holiday weekend here in the UK so will be Tuesday before I’m able to review properly

jfallows commented 5 years ago

No worries @rvesse. Hope that you had a great Bank Holiday Monday. 😄

rvesse commented 5 years ago

@jfallows Merged, thanks for the great contribution

rvesse commented 5 years ago

@jfallows I made some notes in the docs about this change in c1d3c22, please take a look and feel free to suggest any changes

jfallows commented 5 years ago

@rvesse suggest removing all references to Automatic-Module-Name in the docs as that is only present during the build when moditect.skip is set and a candidate module-info.java is generated into target, but normal clean builds and the release build itself should contain only module-info.class and no Automatic-Module-Name in the JAR manifest for the airline and airline-io artifacts.

rvesse commented 5 years ago

@jfallows Thanks, like I said I'm pretty ignorant of JPMS! Removed references to Automatic-Module-Name as suggested

jfallows commented 5 years ago

Looks perfect @rvesse 👍

Any chance of a new release that includes these changes soon please? 🙏

rvesse commented 5 years ago

@jfallows Sure, I'll try and push that out today

rvesse commented 5 years ago

Release done, might take a few hours before the artifacts are visible on Maven central

jfallows commented 5 years ago

Much appreciated, thanks @rvesse !

rvesse / airline