Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

package-info.class files inside org.eclipse.parsson:jakarta.json are compiled with the wrong Java version #73

Open
e6c31d opened this issue Nov 23, 2022 · 13 comments

Comments

@e6c31d
Copy link

e6c31d commented Nov 23, 2022

Jars of org.eclipse.parsson:jakarta.json are compiled for Java version 8 (class version 52). However, the package-info.class files inside versions 1.0.1, 1.0.2, and 1.1.1 are actually compiled for Java version 9 (class version 53).

image

image

This is causing issues with IBM Websphere Application Server. It scans the jars before loading them and refuses to load them because they have classes with version>52:

W com.ibm.ws.ecs.internal.scan.context.impl.ScannerContextImpl scanJAR unable to open input stream for resource jakarta/json/spi/package-info.class in archive WEB-INF/lib/jakarta.json-1.1.1.jar
                                 java.lang.IllegalArgumentException

A workaround is using jakarta.json:jakarta.json-api and org.eclipse.parsson:parsson instead of org.eclipse.parsson:jakarta.json

@jbescos
Copy link
Member

jbescos commented Nov 23, 2022

That happens with the whole bundle, not only package-info.class

Parsson takes the java sources and compiles everything for version 53.

For example with the class JsonNumber:

$:~/workspace/parsson/bundles/jakarta.json$ javap -verbose ./target/classes/jakarta/json/JsonNumber.class
Classfile /home/jbescos/workspace/parsson/bundles/jakarta.json/target/classes/jakarta/json/JsonNumber.class
  Last modified Nov 23, 2022; size 1677 bytes
  MD5 checksum dc9c34247527151bf5bd8fa91878aaa6
  Compiled from "JsonNumber.java"
public interface jakarta.json.JsonNumber extends jakarta.json.JsonValue
  minor version: 0
  major version: 53

I think that is unexpected. It should compile for release 8

@e6c31d
Copy link
Author

e6c31d commented Nov 23, 2022

For the jars on Maven Central, only package-info.class is affected. See for example:

$ javap -verbose -classpath ~/.m2/repository/org/eclipse/parsson/jakarta.json/1.1.1/jakarta.json-1.1.1.jar jakarta.json.JsonNumber | grep version
  minor version: 0
  major version: 52

$ javap -verbose -classpath ~/.m2/repository/org/eclipse/parsson/jakarta.json/1.1.1/jakarta.json-1.1.1.jar jakarta.json.package-info | grep version
  minor version: 0
  major version: 53

@jbescos
Copy link
Member

jbescos commented Nov 23, 2022

I have downloaded also the jakarta.json-api-2.1.1.jar and it has the same issue.

$ javap -verbose -classpath ~/Downloads/jakarta.json-api-2.1.1.jar jakarta.json.package-info | grep version
  minor version: 0
  major version: 53

Parsson seems to be doing it well. Here it compiles with -release 8

[DEBUG] -d /home/jbescos/workspace/parsson/bundles/jakarta.json/target/classes -classpath /home/jbescos/workspace/parsson/bundles/jakarta.json/target/classes:/home/jbescos/.m2/repository/jakarta/json/jakarta.json-api/2.1.1/jakarta.json-api-2.1.1.jar:/home/jbescos/.m2/repository/org/eclipse/parsson/parsson/1.1.2-SNAPSHOT/parsson-1.1.2-SNAPSHOT.jar: -sourcepath /home/jbescos/workspace/parsson/bundles/jakarta.json/src/main/java:/home/jbescos/workspace/parsson/bundles/jakarta.json/target/generated-sources/dependencies:/home/jbescos/workspace/parsson/bundles/jakarta.json/target/generated-sources/annotations:/home/jbescos/workspace/parsson/bundles/jakarta.json/target/generated-sources/annotations: -s /home/jbescos/workspace/parsson/bundles/jakarta.json/target/generated-sources/annotations -g -nowarn --release 8 -encoding UTF-8 -Xlint:all

One of the paths in the sourcespath is bundles/jakarta.json/target/generated-sources and there is where package-info.java exists:

$ find ./ -name "package-info.java"
./bundles/jakarta.json/target/generated-sources/dependencies/jakarta/json/package-info.java
./bundles/jakarta.json/target/generated-sources/dependencies/jakarta/json/spi/package-info.java
./bundles/jakarta.json/target/generated-sources/dependencies/jakarta/json/stream/package-info.java

Maybe this is a bug in the maven-compiler-plugin.

@jbescos
Copy link
Member

jbescos commented Nov 23, 2022

I have created a simple reproducer. It seems it is an issue in the JDK, because we see the maven-compiler-plugin sets the arguments correctly.

The reproducer is:

$ git clone https://github.com/jbescos/Questions.git
$ cd maven-compiler-packageinfo
$ mvn clean install
$ javap -verbose -classpath ./target/maven-compiler-packageinfo-1.0.jar com.github.jbescos.issue.SomeClass | grep version
  minor version: 0
  major version: 52
$ javap -verbose -classpath ~/Downloads/jakarta.json-api-2.1.1.jar jakarta.json.package-info | grep version
  minor version: 0
  major version: 53

@e6c31d
Copy link
Author

e6c31d commented Nov 24, 2022

I found a workaround for the maven-compiler-plugin bug. Set createMissingPackageInfoClass to false at the plugin configuration level, and set it to true at the execution configuration level:

            <plugin>
                <groupId>org.apache.maven.plugins</groupId>
                <artifactId>maven-compiler-plugin</artifactId>
                <version>3.10.1</version>
                <configuration>
                    <release>9</release>
                    <createMissingPackageInfoClass>false</createMissingPackageInfoClass>
                    <compilerArgs>
                        <arg>-Xlint:all</arg>
                    </compilerArgs>
                </configuration>
                <executions>
                    <execution>
                        <id>base-compile</id>
                        <goals>
                            <goal>compile</goal>
                        </goals>
                        <configuration>
                            <release>8</release>
                            <createMissingPackageInfoClass>true</createMissingPackageInfoClass>
                            <excludes>
                                <exclude>module-info.java</exclude>
                            </excludes>
                        </configuration>
                    </execution>
                </executions>
            </plugin>

@jbescos
Copy link
Member

jbescos commented Nov 24, 2022

Thank you for your finding, I am going to use it.

@aalmiray
Copy link

aalmiray commented Feb 8, 2023

Please be aware that 10b8d10 generates JAR files with mixed bytecode versions in the unversioned space. module-info targets bytecode 53 while the rest of the classes target bytecode 52. This will make Maven and Gradle enforcer rule EnforceBytecodeVersion fail a build if the maximum bytecode version is set to 52.

The way to fix this is to turn JARs into Multi Release JARs https://docs.oracle.com/javase/10/docs/specs/jar/jar.html#multi-release-jar-files

$ jarviz bytecode show --file impl/target/parsson-1.1.2-SNAPSHOT.jar 
Unversioned classes. Bytecode version: 52 total: 67
Unversioned classes. Bytecode version: 53 total: 1
  1. Place module-info.class inside /META-INF/versions/9.
$ jarviz manifest show --file impl/target/parsson-1.1.2-SNAPSHOT.jar 
Manifest-Version: 1.0
Bundle-Description: Jakarta JSON Processing provider
Bundle-DocURL: https://www.eclipse.org
Bundle-License: https://projects.eclipse.org/license/epl-2.0, https://pr
 ojects.eclipse.org/license/secondary-gpl-2.0-cp
Bundle-ManifestVersion: 2
Bundle-Name: Eclipse Parsson
Bundle-SymbolicName: org.eclipse.parsson
Bundle-Vendor: Eclipse Foundation
Bundle-Version: 1.1.2.SNAPSHOT
Created-By: Apache Maven Bundle Plugin 5.1.7
Export-Package: org.eclipse.parsson.api;version="1.1.2"
HK2-Bundle-Name: org.eclipse.parsson:parsson
Implementation-Build-Id: 1.1.2-SNAPSHOT - 10b8d10
Import-Package: jakarta.json;version="[2.1,3)",jakarta.json.spi;version=
 "[2.1,3)",jakarta.json.stream;version="[2.1,3)",org.eclipse.parsson.api
Require-Capability: osgi.ee;filter:="(&(osgi.ee=JavaSE)(version=1.8))"
  1. Add Multi-Release: true attribute to manifest.

@lukasj
Copy link
Member

lukasj commented Feb 8, 2023

The way to fix this is to turn JARs into Multi Release JARs

No. The cause is "a bug" in maven-compiler-plugin, or better two issues there:

  1. fix for https://issues.apache.org/jira/browse/MCOMPILER-205 changed the behaviour (see the last comment there)
  2. second round of compilation does not re-compile generated package-info files, while it should - simply because it does so for everything else

@aalmiray
Copy link

aalmiray commented Feb 8, 2023

Well, no. Actually, it depends. Is Parsson's baseline supposed to be Java 8?

  • Yes. Then MR-JAR is needed as module-info.class must be placed inside /META-INF/versions/x.
  • No. The current state is OK.

The "bug" in Maven is the how_to_solve it, I'm asking the what_it_is_supposed_to be.

@aalmiray
Copy link

aalmiray commented Feb 8, 2023

The README does not state what's the Java baseline for the project, nor any of the direct links found in the README. At the moment the only way to tell is by looking at

parsson/pom.xml

Lines 192 to 214 in 10b8d10

<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<configuration>
<release>9</release>
<compilerArgs>
<arg>-Xlint:all</arg>
</compilerArgs>
</configuration>
<executions>
<execution>
<id>base-compile</id>
<goals>
<goal>compile</goal>
</goals>
<configuration>
<release>8</release>
<excludes>
<exclude>module-info.java</exclude>
</excludes>
</configuration>
</execution>
</executions>
</plugin>
, also inspecting all classes found the JAR.

One may infer Java 8 given that json-p 2.x targets JakartaEE 9. But Parsson does not state explicitly (README or project site (at least on the overview page and releases)) which version of json-p is implemented unless one looks again inside the pom.xml file

<jakarta.json-api.version>2.1.1</jakarta.json-api.version>

@jbescos
Copy link
Member

jbescos commented Feb 8, 2023

Well, no. Actually, it depends. Is Parsson's baseline supposed to be Java 8?

* Yes. Then MR-JAR is needed as `module-info.class` must be placed inside `/META-INF/versions/x`.

* No. The current state is OK.

The "bug" in Maven is the how_to_solve it, I'm asking the what_it_is_supposed_to be.

module-info.class seems to be ignored in runtime by jre 8, this is why it is included in the jar compiled with release 9. There are no errors with this behavior. Let me know if you have any reason to modify this.

We use multirelease when we want to modify an existing class for different jre versions.

@aalmiray
Copy link

aalmiray commented Feb 8, 2023

module-info.class seems to be ignored in runtime by jre 8, this is why it is included in the jar compiled with release 9. There are no errors with this behavior. Let me know if you have any reason to modify this.

It depends on the tool. Maven & Gradle enforcers will fail the build if the baseline is set to Java 8 because module-info.class is found in the unversioned space. Other tools that expect similar constraints will fail as well.

Yes, a Java 8 VM will ignore module-info.class when running an application or test, but not these tools as they perform different analysis.

We use multirelease when we want to modify an existing class for different jre versions.

That is but one of the uses of MR-JARs. Another is to "hide" classes from older VMs.

@aalmiray
Copy link

aalmiray commented Feb 8, 2023

Latest spec https://docs.oracle.com/en/java/javase/19/docs/specs/jar/jar.html has this to say:

Modular multi-release JAR files

A modular multi-release JAR file is a multi-release JAR file that has a module descriptor, module-info.class, in the top-level directory (as for a modular JAR file), or directly in a versioned directory.

A public or protected class in a non-exported package (that is not declared as exported in the module descriptor) need not preside over a class of the same fully qualified name and access modifier whose class file is present under the top-level directory.

👇 this is the important bit
A module descriptor is generally treated no differently to any other class or resource file. A module descriptor may be present under a versioned area but not present under the top-level directory. This ensures, for example, only Java 8 versioned classes can be present under the top-level directory while Java 9 versioned classes (including, or perhaps only, the module descriptor) can be present under the 9 versioned directory.

Any versioned module descriptor that presides over a lesser versioned module descriptor or that at the top-level, M say, must be identical to M, with two exceptions:

  • the presiding versioned descriptor can have different non-transitive requires clauses of java.* and jdk.* modules; and
  • the presiding versioned descriptor can have different uses clauses, even of service types defined outside of java.* and jdk.* modules.

👇 this one as well
Tooling, such as the jar tool, should perform such verification of versioned module descriptors but a Java runtime is not required to perform any verification.

lukasj pushed a commit that referenced this issue Apr 21, 2023
…compiled with the wrong Java version #73

Signed-off-by: Jorge Bescos Gascon <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants