[tonygermano]

Allow building with a Java 21 toolchain while targeting Java 17

Branch: tony:build/java21-toolchain → main

Summary

This PR makes it possible to compile and test with a newer JDK while still shipping Java 17 bytecode and staying within the Java 17 API. The default build is unchanged: it still uses a Java 17 toolchain.

./gradlew build -PjavaToolchain=21

To make that the default on one machine, add javaToolchain=21 to ~/.gradle/gradle.properties.

Java 21 works with the current Gradle (8.14) and test libraries. Java 25 isn't covered here because it needs Gradle 9.1+ and a newer Byte Buddy for Mockito.

The PR has three commits:

1. Upgrade JaCoCo to 0.8.15. JaCoCo 0.8.8 can't read class files newer than Java 18. On JDK 21 it logs a stack trace for each JDK class it fails to instrument, such as CLDRLocaleDataMetaInfo. 0.8.15 officially supports up to Java 26 and has no API changes since 0.8.8. Its ASM 9.10.1 (replacing 9.2) is only on JaCoCo's own classpath and doesn't change the ASM we ship. gradle/verification-metadata.xml was regenerated with a cold cache, and the entries for the old versions were removed.
2. Compile server tests without JDK internal exports. javac doesn't allow --add-exports of system modules together with --release. KeyEncryptorTest only imported com.sun.crypto.provider.SunJCE to get its class name, which the encryption settings take as a string. It now uses the name as a constant. The other exported packages only appeared in string literals, so the compileTestJava block in server/build.gradle is removed.
3. Allow building with a newer JDK while targeting Java 17. All JavaCompile tasks now use options.release = 17, so the output targets Java 17 whichever JDK compiles it. The toolchain defaults to the release version and can be overridden with -PjavaToolchain. The tests run on the same JDK. userApiJavadoc now runs the toolchain's javadoc instead of the one from the JDK running Gradle, and the javadoc version is now a task input, so switching toolchains regenerates the docs. CONTRIBUTING.md describes the new option.

Closes #215.

Testing

Claude checked

• ./gradlew clean build -Pcoverage=true -PdisableSigning=true passed before each commit. At the tip it passes with both the 17 and 21 toolchains, and all 768 tests pass.
• With JaCoCo 0.8.15, neither toolchain logs any instrumentation errors.
• Bytecode from the JDK 17 and JDK 21 toolchains (both built from the tip):
  • Every class file from the 21 build is major version 61 (Java 17).
  • Of the 3,640 classes, 1,487 are byte-identical. The 6,228 vocab classes are also byte-identical.
  • 2,118 classes differ only in metadata from the newer javac: constant pool order, MethodParameters attributes for the mandated and synthetic parameters of inner class constructors, and argN names in the LocalVariableTable of anonymous class constructors.
  • In 25 classes, Object methods (toString, equals, getClass) called on an interface type are emitted as invokeinterface on the interface instead of invokevirtual on Object. The JVM resolves both to the same method.
  • In 8 classes, a switch on an enum inside that enum's own class uses ordinal() directly instead of a $SwitchMap. This drops two synthetic classes (MetaDataColumnType$1 and ContextType$1) and three switch map fields from other synthetic classes.

---

🤖 Drafted with Claude Code

[github-actions]

Test Results

130 files  ±0  130 suites  ±0   4m 20s ⏱️ + 1m 9s
789 tests ±0  789 ✅ ±0  0 💤 ±0  0 ❌ ±0 
915 runs  ±0  909 ✅ ±0  6 💤 ±0  0 ❌ ±0 

Results for commit de69da8. ± Comparison against base commit 4589238.

♻️ This comment has been updated with latest results.

[NicoPiel]

LGTM. A few non-blocking notes:

1. userApiJavadoc up-to-date check: the JDK used isn't a task input, so after switching between 17 and 21 the task can be marked up to date and keep javadoc from the other version. Maybe add inputs.property('javaToolchain', rootProject.javaToolchain). Release builds use 17, so this is minor.
2. CONTRIBUTING wording: "the version pinned in .sdkmanrc" suggests the build uses that exact JDK, but the toolchain only sets the version, not the vendor. Gradle can pick any installed 17, not necessarily the pinned zulu-fx. That was true before this PR; just a wording nit.

[mgaffigan]

But why? Can we just take the plunge? Is there a reason to run parallel? (And for that matter, why 21 instead of 25)?

[NicoPiel]

True. If we're getting rid of JFX too, why not upgrade to 25 directly?

[tonygermano]

@mgaffigan It says in the PR description

Java 25 isn't covered here because it needs Gradle 9.1+ and a newer Byte Buddy for Mockito.

I was just trying to have a targeted PR that allows for different toolchain and release versions. Upgrading Gradle to 9.x was more than I wanted to handle in a single PR. For the next release I'd like to get to allowing a 25 toolchain to build for release 21, but if for some reason we don't, at least the framework is in place.

[mgaffigan]

Jumping to Java 25 is not substantially larger. I suggest we take the leap rather than adding more build options. See https://github.com/OpenIntegrationEngine/engine/pull/536/changes

[tonygermano]

Jumping to Java 25 is not substantially larger. I suggest we take the leap rather than adding more build options. See https://github.com/OpenIntegrationEngine/engine/pull/536/changes

@mgaffigan I don't think the two PRs are as far apart as they look. #536 also splits the release and toolchain versions (toolchain 25, --release 21), which is what #215 asks for and what this PR sets up. Both PRs also make the same preparatory changes: a JaCoCo upgrade, and dropping the --add-exports from the server test compile by changing KeyEncryptorTest.

As far as I can tell, the real difference is whether the toolchain can be overridden with a build property. If that's the point of contention, let's work it out here rather than replacing one PR with the other. My reason for wanting it: with --release fixed, an overridable toolchain lets us run the unit tests on the minimum supported JDK as well as the one we build with. With the toolchain fixed at 25 and --release 21, the unit tests never run on 21. If that's not important, I won't require it.

I'd suggest building #536 on top of this PR instead, as the fix for #306. After this merges, moving to 21/25 is a change to javaRelease and the default toolchain, and the rest of #536 (Gradle 9.8.2, the newer Byte Buddy and Mockito, removing JavaFX, the JRE 21 smoke test image) stays as separate commits.

---

🤖 Drafted with Claude Code

[tonygermano]

LGTM. A few non-blocking notes:

1. userApiJavadoc up-to-date check: the JDK used isn't a task input, so after switching between 17 and 21 the task can be marked up to date and keep javadoc from the other version. Maybe add inputs.property('javaToolchain', rootProject.javaToolchain). Release builds use 17, so this is minor.
2. CONTRIBUTING wording: "the version pinned in .sdkmanrc" suggests the build uses that exact JDK, but the toolchain only sets the version, not the vendor. Gradle can pick any installed 17, not necessarily the pinned zulu-fx. That was true before this PR; just a wording nit.

@NicoPiel I pushed a change addressing both of these items.

[mgaffigan]

As far as I can tell, the real difference is whether the toolchain can be overridden with a build property. If that's the point of contention, let's work it out here rather than replacing one PR with the other.

Yes - it is my position that we take too few requirements. Options = risk, and most of the time YANGI.

Honestly: I kept the 21 backcompat on the 25 tree by influence from this PR, but if I were the reviewer instead of the author, I'd be arguing against allowing 21 after the 25 upgrade. If you want to run 21, use 4.7.0. If you want 4.7.1, it requires 25.

[mgaffigan]

I will also say that 4x PR's for a 250 line change is a lot of approvals. I still think the entire jump can be a single PR. I'm not seeing a lot of work required here.

[tonygermano]

So wanting to remove the build property has more to do with the fact that you don't think we should support more than one LTS release of Java? It's true that in that case we wouldn't need the build property. We could also add the higher toolchain version to gradle.properties as the default and allow overriding it down to the release version. Why don't we get this merged, and then hash that out on #306 with input from more than just you and me?

I think several of the PRs lately have been too big. I really like the occasional ones that I open and quickly can tell what it does without asking an AI to explain it to me. I'm not sure specifically what you are referring to with 4x PRs for 250 lines. I already have 3 different but loosely related items in this PR as separate commits.

[tonygermano]

I honestly don't understand how javafx relates to this either. It's not required to build because we have client/lib/openjfx.jar in the classpath. It's only currently needed in the build environment if you want to launch ./gradlew :client:devClient. I intentionally left any mention of javafx out of the update to CONTRIBUTING.md because I knew it was going away soon. The commit to remove openjfx shouldn't be wrapped up with a PR that upgrades gradle and changes the java versions. It's not a build system change.

[gibson9583]

Looks safe to merge.