Spdx-Java-Library icon indicating copy to clipboard operation
Spdx-Java-Library copied to clipboard

Consider adding a module descriptor

Open rfscholte opened this issue 10 months ago • 8 comments

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 avatar Feb 21 '25 12:02 rfscholte

@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.

goneall avatar Feb 21 '25 18:02 goneall

(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.

bact avatar Feb 22 '25 00:02 bact

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:

      <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.

rfscholte avatar Feb 22 '25 09:02 rfscholte

Thanks @rfscholte @bact - I'll work on a PR based on the suggestions

goneall avatar Feb 23 '25 06:02 goneall

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.

goneall avatar Feb 25 '25 03:02 goneall

@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]

goneall avatar Feb 25 '25 04:02 goneall

Please review the module name addition in org.spdx.v3jsonldstore

  • https://github.com/spdx/spdx-java-v3jsonld-store/pull/17

bact avatar Feb 26 '25 00:02 bact

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.

goneall avatar Feb 27 '25 02:02 goneall

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.

goneall avatar Apr 14 '25 18:04 goneall