eclipse-ee4j / parsson

Parsson Project
Other
10 stars 21 forks source link

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

Open e6c31d opened 1 year ago

e6c31d commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

aalmiray commented 1 year ago

Please be aware that 10b8d107f3b3b1d80798d9733904f70e50ba170e 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 commented 1 year ago

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 commented 1 year ago

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

The "bug" in Maven is the _how_tosolve it, I'm asking the _what_it_is_supposedto be.

aalmiray commented 1 year ago

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 https://github.com/eclipse-ee4j/parsson/blob/10b8d107f3b3b1d80798d9733904f70e50ba170e/pom.xml#L192-L214, 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 https://github.com/eclipse-ee4j/parsson/blob/10b8d107f3b3b1d80798d9733904f70e50ba170e/pom.xml#L99

jbescos commented 1 year ago

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_tosolve it, I'm asking the _what_it_is_supposedto 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 commented 1 year ago

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 commented 1 year ago

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:

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