Thanks!

How do I reproduce this issue? I started a Docker container with openjdk:9 image, created a dummy Maven project with:

mvn archetype:generate -DgroupId=com.mycompany.app -DartifactId=my-app -DarchetypeArtifactId=maven-archetype-quickstart -DarchetypeVersion=1.4 -DinteractiveMode=false

I edited pom.xml and modified both <maven.compiler.source> and <maven.compiler.target> to 9. I added pecoff4j as a dependency. Finally I ran mvn package and didn't get that warning.

Your previous version had module-info.java in the diff. The latest one doesn't. Where does it come from? Does Maven generate it?

My apologies, I made a mistake last night when I tried to fix something and messed up some commits. module-info.java is meant to be written by developers. I have pushed the relevant module-info.java now.

Give me a moment and I will create a reproduction repository

Do you think adding a manifest entry with the future module name will solve it too?

http://branchandbound.net/blog/java/2017/12/automatic-module-name/

It seems much simpler and doesn't require JDK 9.

I have created a temporary repository over at concision/pecoff4j-warning that will log the following line:

[INFO] Required filename-based automodules detected: [pecoff4j-0.3.1.jar]. Please don't publish this project to a public artifact repository!

I am not exactly sure why it is different from one of my other builds, but there is a lot more at play within that build. Using Docker inside of (Git) Bash, you can execute the following command in the root directory of the cloned project to reproduce it:

MSYS_NO_PATHCONV=1 docker run --rm -v "$(pwd):/usr/src/pecoff4j" -w "/usr/src/pecoff4j" maven mvn package

If you would prefer it, you might be able to use a manifest entry instead. I am not 100% sure how that plays out with module support (e.g. such as with JDK tools like jlink). If you do plan on adding any Maven project dependencies, using module-info would be preferred to explicitly declare all module dependencies. JDK 9 has been released for a few years now, so I do not think it is unreasonable to use in projects; furthermore, this project seems to leverage CI for building the application anyways.

edit: on second thought, both could be used - a manifest entry and module-info.java. I do think this ultimately comes down to what decision you want to make - as it stands, I think either should work

Thanks, I was able to reproduce with your code. Looks like the inclusion of module-info.java was the missing part. I was also able to confirm adding that manifest line resolves the issue.

I am leaning towards a manifest line mostly because it's a simpler solution. I don't plan on adding any dependencies and it seems like we export everything anyway. The idea of including .class files of multiple Java versions also seems like something that might break some tools. All that said, I have zero experience with Java modules before today. I have no idea what this means for jlink. How about we start with the manifest as the simpler solution and if it causes issues, we can reevaluate?

Also, do you have any input on the module name? I think com.kichik.pecoff4j will leave room for people to publish their own versions more easily.

A quick tl;dr on jlink is that it produces a minimal JRE based on the Java modules you require - this is useful for bundling small JREs in Docker images (I personally use jlink in almost all of my Java docker builds). I did some preliminary testing with jlink with using the manifest entry versus module-info.java. I put both compiled .jar files (pecoff4j-xxx.jar and pecoff4j-warning-xxx.jar) in a target folder and executed the following command:

jlink --module-path target --add-modules me.concision.pecoff4j.warning --output jre

With the manifest entry, I received the following error:

Error: automatic module cannot be used with jlink: com.kichik.pecoff4j from file:///C:/.../target/pecoff4j-1.0-SNAPSHOT.jar

With module-info.java, a JRE about ~39MB in size is successfully created.

I did not expect this behavior from jlink, but it is worth noting that there might be some incompatibilities with JDK tools without a fully qualified module-info. This seems to be a bit of an annoying decision - some tooling could potentially break with class level 53+/Java 9+, and some tooling might still break/be broken without module-info. In this case, this puts forth a stronger recommendation for module-info. There might be a way to circumvent this jlink error, but I am not sure at the moment.

Let me know how you wish to proceed, and I can update pom.xml accordingly.

As for the module name, I think com.kichik.pecoff4j is perfectly suitable since all of the packages are under this name.

If that's the case, then I think we should just go ahead and drop Java 8 support. Whoever needs it can use 0.3.1.

To clarify - do you mean dropping <= jdk 8 Maven compilation support, or do you mean dropping <= java 8 library support?

In the case of the latter, I do not think that is necessary - a jar can have a module-info.class and still be backwards compatible and run on JRE 7 fine, for example.

The latter. Dropping 8 altogether. I don't feel comfortable with including multiple class versions in the same JAR.

I would not recommend unnecessarily dropping support - multi-release JARs are pretty commonplace in the Java/Maven ecosystem.

Generally, the META-INF directory on <= java 8 is not scanned for classes. On Java 9+, class resolution will scan for classes in META-INF/versions/9 (or any other relevant version). It is not uncommon to place a module-info.class in there, which I propose as a possible alternative.
Take a quick look at the top of this article: https://www.logicbig.com/tutorials/core-java-tutorial/java-9-changes/multi-release-jars.html

That's a cool trick. If it's truly as simple as described here, I'm up for it. Otherwise all the added complexity just to have Java 8 support is not quite worth it. This library doesn't get enough updates to worry about it. People who really need it will be able to use 0.3.1.

I have updated this pull request to favor the multi-release route. Here is an overview of the changes:

• enforced JDK 9+ JAVA_HOME requirement with the plugin org.apache.maven.plugin:maven-enforcer-plugin when compiling the project in Maven; however, the built artifact is still compatible with >= Java 7 JREs
• moved module-info.java from src/main/java to src/main/java9; any additional classes in the java9 directory are properly handled as a multi-release as well
• added a dummy phase with the plugin org.codehaus.mojo:build-helper-maven-plugin to indicate src/main/java9 is a source directory to IDEs during Maven auto-import (tested with JetBrain's IntelliJ IDE)
• set the default compilation execution to Java 9 and as no-op; this indicates to IDEs that the project language level should be at least Java 9 during Maven auto-import (tested with JetBrain's IntelliJ IDE)
• set the Java 7 compilation execution (java-7-compile) to use the --release flag added in JDK 9; this ensures that no classes, methods, or fields from any JDKs > 7 are accidentally used (otherwise, the build will fail)
• add a Java 9 compilation stage (java-9-compile) with the source directory src/main/java9 that is a multi-release; all compiled classes are properly relocated to META-INF/versions/9 in the final JAR
• set the built JAR as a multi-release in the manifest with the plugin org.apache.maven.plugins:maven-jar-plugin
• ci: removed JDK 7/8 compilations in the GitHub actions CI; only JDK 9+ compilation is supported

These changes are relatively straightforward without any strange Maven hacks that are hard to maintain. I think these changes are ultimately better than the originally pitched changes and is a proper multi-release JAR with module support. Testing with the CLI tool jlink was successful with these changes.

Thanks for your patience and timely feedback on this PR. Let me know what your thoughts are from here.

Thanks! This works great. It has been a real pleasure working with you on this PR.

I'm not 100% sure it's related yet, but some JAR signing issue popped up when trying to release 0.3.2 with this change.
https://github.com/kichik/pecoff4j/runs/1185269314?check_suite_focus=true

Maven 🤷‍♂️ d31fc0a

Thank you for going through the steps with me on this pull request! I appreciate the timely responses and open mind to these changes.

Were you able to isolate the issue? Was it related to these changes in any way?

Maybe? If you look into the logs of failed builds, you can see it recompiling the source code after building the JAR and signing it because "something changed". It then builds another JAR and submits that.