andygrove opened a new pull request, #6285:
URL: https://github.com/apache/datafusion-comet/pull/6285

   ## Which issue does this PR close?
   
   Closes #6283.
   
   ## Rationale for this change
   
   Every release from 0.11.0 to 1.0.0 published Spark 3.4 and 3.5 jars that 
don't load on Java 11, although the docs list Java 11 as supported for both. 
Loading one fails with `UnsupportedClassVersionError ... class file version 
61.0`. With this change, 1.0.1 ships those jars for Java 11. `main` drops Java 
11 in 1.1.0, so the change is for `branch-1.0` only.
   
   ## What changes are included in this PR?
   
   The `jdk11` and `jdk17` profiles are activated by the JDK that runs Maven, 
and they're declared after the Spark profiles, so their `java.version` won over 
the `java.version=11` of `spark-3.4` and `spark-3.5`. The release script 
requires JDK 17, so the Spark 3.x jars came out targeting Java 17.
   
   - `pom.xml` removes both JDK profiles, so each Spark profile decides the 
target, and raises the default `java.version` from 11 to 17 to match the 
default Spark version, 4.1.
   - `dev/release/check-class-versions.py` is new. `build-release-comet.sh` 
runs it after the Spark 3.x installs, and it fails the release build if any 
class in a Spark 3.x jar, shaded dependencies included, has a class file 
version above 55 (Java 11).
   - The contributor guide no longer suggests `-Pjdk17`.
   
   `maven.compiler.target` now depends only on the Spark profile:
   
   | JDK running Maven | no profile | `spark-3.4` | `spark-3.5` | `spark-4.x` |
   | ----------------- | ---------- | ----------- | ----------- | ----------- |
   | 11                | 11 → 17    | 11          | 11          | 11 → 17     |
   | 17                | 17         | 17 → 11     | 17 → 11     | 17          |
   | 21                | 11 → 17    | 11          | 11          | 17          |
   
   Building Spark 4.x, or the default profile, on JDK 11 fails either way; it 
now fails straight away rather than at `java.lang.Record`. Every CI job that 
runs on JDK 11 passes `-Pspark-3.4`.
   
   With a target of 11, scala-maven-plugin passes scalac `-release 11`, so the 
Scala code is checked against the Java 11 API even on JDK 17. javac gets 
`-source 11 -target 11`, which only sets the bytecode version and prints a 
"system modules path not set" warning. The Java sources are all shared across 
Spark versions, and the Spark 3.4 JDK 11 jobs already compile them on JDK 11. 
The comparison below shows the JDK 17 build references the same JDK API.
   
   ## How are these changes tested?
   
   I built Spark 3.4 and 3.5, each with Scala 2.12 and 2.13, on JDK 17 with 
`-DskipTests package`, which also compiles the tests. All four builds pass. In 
the resulting jars:
   
   - Every javac-compiled Comet class is class file version 55, where the 
published 1.0.0 jars have 61. The scalac-compiled classes are 55 with Scala 
2.13 and 52 with Scala 2.12.
   - `check-class-versions.py --max 55` passes all four jars. It fails the 
published 1.0.0, 0.17.1 and 0.11.0 jars and a Spark 4.1 jar, and passes 0.10.0, 
the last release built on JDK 11. I ran it with the release script's glob 
against a Maven repository of published jars, where it matched only the Spark 
3.4 and 3.5 ones.
   - On Java 11, `NativeBase` and `CometPlugin` load from the new jars. Loading 
them from the published jars throws `UnsupportedClassVersionError`.
   
   A Spark 3.4 / Scala 2.12 jar built on JDK 11 and one built on JDK 17 
reference exactly the same JDK methods and fields: 1,866 references from the 
666 javac-compiled classes and 2,165 from the 1,055 scalac-compiled ones, with 
no differences.
   
   The Spark 4.1 build still targets Java 17. The Lint Java job's command for 
Spark 3.5 on JDK 17 (`package -DskipTests scalafix:scalafix 
-Dscalafix.mode=CHECK -Psemanticdb -Pspark-3.5 -Pscala-2.12`) passes, and so 
does RAT with the new script.
   
   I didn't run the release script end to end, because it needs the Docker 
cross-builds. I also didn't run a Spark suite on Java 11 against a JDK 17 
build; the reference comparison above stands in for that.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to