IO-747 Make commons-io a proper JPMS module

[martin-g]

No description provided.

[martin-g]

This PR is similar to apache/commons-fileupload#107
If commons team likes the approach I could work on a PR that moves the logic to commons-parent and is reused for all commons-** projects.

[coveralls]

Coverage decreased (-0.05%) to 89.202% when pulling 37d5e7d on martin-g:io-747-make-commons-io-proper-jpms-module into 70f3ce0 on apache:master.

[garydgregory]

That's not good. I build releases with Java 8 since we want to bind to the JRE 8 APIs and not 9 especially with problems due to the NIO Buffer class changes. I think we have other Commons components that are OSGi and JPMS modules, so you might want to look around. Why is there not a module-info.java somewhere?

[martin-g]

What are the issues with NIO Buffer classes if you build with Java 9 and --release=8 ?

[jochenw]

@garydgregory I think, that release building with Java 9+ is a more than reasonable requirement, if we want to fit into the Java 9 ecosphere. (There is no question, that we want that, isn't there?)

@martin-g I'd like to see two improvements:

1. Move the plugin definition into a profile, that enables the moditect-maven-plugin only when building with Java 9+. Alternatively, set the property moditect.skip for building with Java 8.
2. Make sure, that the apache-release profile enforces building with Java 9+.

Regarding commons-parent: My personal stance is, that we have covered commons-io, and commons-fileupload now (Perhaps, cover a few others, in case of demand.) Let's wait, and see, until we did a release with that, then approach parent.

[martin-g]

@jochenw Done with 37d5e7d

[emiliofernandes]

Thank you @martin-g for raising this! We also need JPMS-zed version of commons-io and commons-collections4.
There are some forks like guicedee and jwebmp at Maven Central but it is time the originals to evolve too!

[garydgregory]

Why does this patch not follow the recommeded pattern from https://maven.apache.org/plugins/maven-compiler-plugin/examples/module-info.html ?

[martin-g]

I used Moditect because it generates the module-info.java. With maven-compiler-plugin module-info.java must be in src/main/java, i.e. require JDK 9+ for development.
Also I am not sure how to setup toolchains in GitHub Actions.

I don't mind closing this PR if someone provides different way to achieve the end goal!

[jochenw]

One minor nitpick: As I understand it, there is now a hardcoded version 8 for the Java compiler. IIRC, there is a property in commons-parent for that. Please, change that to use that property. Thanks!

[martin-g]

@jochenw I guess you mean https://github.com/apache/commons-parent/blob/dd3e4bc64628c0e1ae15cf16a48a04bdfd8dbcc9/pom.xml#L109-L110

Those are overwritten here -

commons-io/pom.xml

Lines 297 to 298 in 319a540

	<maven.compiler.source>1.8</maven.compiler.source>
	<maven.compiler.target>1.8</maven.compiler.target>

But there is no such for maven.compiler.release which uses 8 as a value vs. 1.8 used by source and target.

[jochenw]

But there is no such for maven.compiler.release which uses 8 as a value vs. 1.8 used by source and target.

If so, you can override the property maven.compiler.release with the value "8". Should work fine, as far as I know.

[garydgregory]

FYI: This is how we use toolchains over at Log4j, see the master and release-2.x branches: https://github.com/apache/logging-log4j2/tree/release-2.x/.github/workflows

[martin-g]

If so, you can override the property maven.compiler.release with the value "8". Should work fine, as far as I know.

@jochenw I don't follow you! There is no mention of maven.compiler.release in the parent pom, so there is nothing to override.
In this PR I set <maven.compiler.release>8</maven.compiler.release> in the JDK 9+ profile. I cannot move it to the common (no-profile) properties because javac 8 will complain that it has not a parameter --release.