Consider adding a module descriptor
Currently state of this library is like this:
mvn org.apache.maven.plugins:maven-dependency-plugin:3.8.1:list -DincludeScope=runtime
[INFO] Scanning for projects...
[INFO]
[INFO] ---------------------< org.spdx:java-spdx-library >---------------------
[INFO] Building java-spdx-library 2.0.0-RC3-SNAPSHOT
[INFO] from pom.xml
[INFO] --------------------------------[ jar ]---------------------------------
[INFO]
[INFO] --- maven-dependency-plugin:3.8.1:list (default-cli) @ java-spdx-library ---
[INFO] Can't extract module name from spdx-java-model-2_X-1.0.0-RC2.jar: spdx.java.model.2.X: Invalid module name: '2' is not a Java identifier
[INFO] Can't extract module name from spdx-java-model-3_0-1.0.0-RC2.jar: spdx.java.model.3.0: Invalid module name: '3' is not a Java identifier
[INFO]
[INFO] The following files have been resolved:
[INFO] org.slf4j:slf4j-api:jar:2.0.7:compile -- module org.slf4j
[INFO] org.apache.commons:commons-lang3:jar:3.5:compile -- module commons.lang3 (auto)
[INFO] org.jsoup:jsoup:jar:1.15.3:compile -- module org.jsoup [auto]
[INFO] com.google.code.gson:gson:jar:2.8.9:compile -- module com.google.gson
[INFO] com.google.code.findbugs:jsr305:jar:3.0.2:compile -- module jsr305 (auto)
[INFO] org.spdx:spdx-java-model-2_X:jar:1.0.0-RC2:compile
[INFO] org.spdx:spdx-java-core:jar:1.0.0-RC2:compile -- module spdx.java.core (auto)
[INFO] org.spdx:spdx-java-model-3_0:jar:1.0.0-RC2:compile
If a line ends with (auto), the module name is based on the filename If a line ends with [auto], the module name is the Automatic-Module-Name entry of the META-INF/MANIFEST.MF All others are explicit modules (containing a module-info.class)
Once it is structured as a valid module (e.g. no split packages with other modules), you could add the Automatic-Module-Name entry in the META-INF/MANIFEST.MF for this project. Once all modules are explicit (no more automatic modules), you can also add the module-info.java to this project.
@rfscholte - can you provide a bit more specifics on a recommended approach?
We would like to continue to support Java 8 which doesn't support modules, at the same time we would like to support proper module naming as introduced with this pull request: https://github.com/spdx/Spdx-Java-Library/pull/192
Is the solution to add a module.info file to the dependencies (e.g. spdx-java-core, spdx-java-model-2_X, and spdx-java-model-3_0)?
I haven't used modules in the past, so any advice is much appreciated.
(based on @rfscholte and @goneall comments and some research: )
In this case, Maven is trying to determine the name of the unnamed module based on the JAR filename (spdx-java-model-2_X-1.0.0-RC2.jar).
spdx-java-model-2_X is being converted to spdx.java.model.2.X
The issue here is, each part of the module name must be a valid Java identifier name -- but 2 is not.
One of the way to fix this particular module naming issue is to change the JAR name to spdx-java-model-v2_X-RC2.jar, so the extracted module name will be spdx.java.model.v2.X -- every single parts are now a valid Java identifier.
--
But ideally, and by convention, the module name should be in a reverse DNS fashion.
#192 agrees that this should be org.spdx.library for Spdx-Java-Library.
Based on the current directory structure/package name, the module name for spdx-java-model-v2_X is likely to be: org.spdx.library.model.v2.
To achieve that, we have to explicitly name the module.
Automatic-Module-Name entry in pom.xml is one of the ways.
Another way is to put the module name inside module-info.java (to be compiled and resulting module-info.class) .
The content of module-info.java may look like something along this line:
module org.spdx.library.model.v2 {
exports org.spdx.library.model.v2;
exports org.spdx.library.referencetype;
exports org.spdx.storage.compatv2;
requires transitive org.spdx.core;
requires transitive org.spdx.storage;
}
If we can do this with all org.spdx.* libraries, I believe it will fix the module naming issues.
--
However, the code base with module-info.java will not be well compiled in Java 8.
I'm not sure if we can use Maven build profiles to include/exclude module-info.java based on a JDK version (include if JDK is [9,), exclude if JDK is (,8]).
@rfscholte please correct me / add anything. I never use modules as well. Thank you.
That's indeed quite a complete and correct analysis.
A view things to add to that: Maven just passes the files to Java, asking for it's module name. https://github.com/openjdk/jdk/blob/jdk-21%2B35/src/java.base/share/classes/jdk/internal/module/ModulePath.java#L509 will transform the filename to a modulename based on a couple of rules. In the end https://github.com/openjdk/jdk/blob/jdk-21%2B35/src/java.base/share/classes/jdk/internal/module/Checks.java#L44 gives the exception (and that message is used by the maven-dependency-plugin)
Profiles are useful for different Java runtimes for Maven. Back in the days of Java 9 that approach would make sense because some code could not be compiled with Java 9 yet. But currently most projects are running Maven with at least Java 11 without any uses, even to create Java 8 compatible code.
What I would do is the following:
- add the requireJavaVersion enforcer rule and set it to 11 (first LTS after 8 with module support)
- configure the maven-compiler-plugin as follows
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<version>3.13.0</version>
<executions>
<execution>
<id>java8-base</id>
<configuration>
<release>8</release>
<excludes>
<exclude>module-info.java</exclude>
</excludes>
</configuration>
</execution>
<execution>
<id>java11</id>
<configuration>
<release>11</release>
<includes>
<include>module-info.java</include>
</includes>
</configuration>
</execution>
</executions>
</plugin>
Looking at model-2_X:
mvn org.apache.maven.plugins:maven-dependency-plugin:3.8.1:list -DincludeScope=runtime
[INFO] Scanning for projects...
[INFO]
[INFO] --------------------< org.spdx:spdx-java-model-2_X >--------------------
[INFO] Building spdx-java-model-2_X 1.0.0-RC3-SNAPSHOT
[INFO] from pom.xml
[INFO] --------------------------------[ jar ]---------------------------------
[INFO]
[INFO] --- maven-dependency-plugin:3.8.1:list (default-cli) @ spdx-java-model-2_X ---
[INFO]
[INFO] The following files have been resolved:
[INFO] org.spdx:spdx-java-core:jar:1.0.0-RC2:compile -- module spdx.java.core (auto)
[INFO] com.google.code.findbugs:jsr305:jar:3.0.2:compile -- module jsr305 (auto)
[INFO] org.slf4j:slf4j-api:jar:2.0.3:compile -- module org.slf4j
[INFO] org.apache.commons:commons-lang3:jar:3.5:compile -- module commons.lang3 (auto)
[INFO] org.jsoup:jsoup:jar:1.15.3:compile -- module org.jsoup [auto]
[INFO] com.google.code.gson:gson:jar:2.8.9:compile -- module com.google.gson
This tells me you should not add the module-info.java yet, because I'm pretty sure jsr350 will not be the modulename. Up until everything has an explicit modulename, stick to providing an Automatic-Module-Name.
Thanks @rfscholte @bact - I'll work on a PR based on the suggestions
From reading this article, it looks like adding the Automatic-Module-Name entry in pom.xml will add it to the Manifest and not impact the ability to use the library in Java 8 whereas adding the manifest file will require the above mentioned changes to the POM file to have separate executions for Java 8 and Java 9+. Since supporting multiple Java JDKs is more involved (e.g., I'd like to have the CI test the different JDK compilations), I'm thinking I'll just add the Automatic-Module-Name entry in the following libraries:
I'd like to leave this issue open to either implement the module descriptor once we have to drop support for Java 8 or we implement a more sophisticated CI.
@bact @rfscholte - Thanks again for the advice and pointers. If you disagree with the approach, I'll need a bit more help in constructing the PR which includes all the necessary tests and changes.
@bact @rfscholte - Please review the following PR's:
- https://github.com/spdx/spdx-java-core/pull/30
- https://github.com/spdx/spdx-java-model-2_X/pull/16
- https://github.com/spdx/spdx-model-to-java/pull/15
This now generates the following dependencies:
[INFO] The following files have been resolved:
[INFO] org.slf4j:slf4j-api:jar:2.0.7:compile -- module org.slf4j
[INFO] org.apache.commons:commons-lang3:jar:3.5:compile -- module commons.lang3 (auto)
[INFO] org.jsoup:jsoup:jar:1.15.3:compile -- module org.jsoup [auto]
[INFO] com.google.code.gson:gson:jar:2.8.9:compile -- module com.google.gson
[INFO] com.google.code.findbugs:jsr305:jar:3.0.2:compile -- module jsr305 (auto)
[INFO] org.spdx:spdx-java-model-2_X:jar:1.0.0-RC3-SNAPSHOT:compile -- module org.spdx.model.v2 [auto]
[INFO] org.spdx:spdx-java-core:jar:1.0.0-RC3-SNAPSHOT:compile -- module org.spdx.core [auto]
[INFO] org.spdx:spdx-java-model-3_0:jar:1.0.0-RC3-SNAPSHOT:compile -- module org.spdx.model.v3 [auto]
Please review the module name addition in org.spdx.v3jsonldstore
- https://github.com/spdx/spdx-java-v3jsonld-store/pull/17
Please review the module name addition in org.spdx.v3jsonldstore
Done.
I only looked at the dependencies on the Java library.
We should look at tools-java and all of its dependencies.
I believe this is now resolved for the SPDX java library - I'll go ahead and close this issue.
@rfscholte - if I missed anything we can re-open or open a new PR.