Repository navigation
[CALCITE-4553] Use Gradle toolchain to compile Calcite with JDK 25 and --release 8 - #5278
Conversation
4416b15 to
04fa96b
Compare
…d --release 8 Previously javac came from the JDK that ran Gradle, so the compiler version varied with each CI job and each developer's JAVA_HOME, and only sourceCompatibility/targetCompatibility 1.8 kept the bytecode at Java 8 without checking the JDK 8 API. Older javac versions also have known bugs with type annotations, which Calcite uses for nullability. JavaCompile tasks now take their compiler from a toolchain, jdkBuildVersion (default 25), and pass --release 8, which also rejects calls to APIs newer than Java 8. Tests, javadoc, and JavaExec tasks still run on the JDK that runs Gradle. The foojay resolver downloads the toolchain when no matching JDK is installed. It stays at 0.10.0 because 1.0.0 requires Gradle to run on Java 17, and CI still runs Gradle on Java 8 and 11. When Gradle runs on Java 8, the download fails anyway: api.foojay.io answers the Java 8 User-Agent with "error code: 1010", so the build JDK must be installed. CI installs JDK 25 next to the JDK each job tests with, and sets org.gradle.java.installations.auto-download=false, so a build fails instead of silently compiling with a JDK that foojay downloaded. The ErrorProne and Checker Framework jobs pass -PjdkBuildVersion=11: ErrorProne 2.5.1 fails on JDK 25 with "IllegalAccessError: class com.google.errorprone.JavacErrorDescriptionListener ... cannot access class com.sun.tools.javac.tree.JCTree$JCCompilationUnit". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
04fa96b to
3f06c09
Compare
zabetak
left a comment
There was a problem hiding this comment.
Changes LGTM! I tested the patch locally and seems to work as expected. I don't feel strongly about having the resolver or not; personally I can leave without it.
| } | ||
|
|
||
| plugins { | ||
| id("org.gradle.toolchains.foojay-resolver-convention") |
There was a problem hiding this comment.
Not strictly opposed to introducing the resolver but personally I like having control on what is installed on my machine. I don't mind getting an error if JDK 25 is not installed.
There was a problem hiding this comment.
It is documented in PR description and at https://docs.gradle.org/current/userguide/toolchains.html#sub:disable_auto_provision
Does that fix the case for you?
There was a problem hiding this comment.
Yes, I saw it so for me its fine. I guess most people will simply run the build as is and may never really realise which JDK they are using :)
|
Does this change mean we'd need to adapt our instructions to make a release candidate?
|
The build compiles with the jdkBuildVersion toolchain (JDK 25 by default) and --release 8, but howto.md listed only the JDKs that run Gradle, and develop/index.md still asked for JDK 8u220 because of javac 8 bugs with type annotations. Neither page said that Gradle on Java 8 cannot download the build JDK, and the Error Prone and Checker Framework commands failed on JDK 25 as written. Add a howto.md section on the build JDK with the error Gradle prints when it is missing, point the build prerequisites and the release steps to it, and pass -PjdkBuildVersion=11 in the Error Prone and Checker Framework commands. Drop the JDK 8u202 and CALCITE-6616 notes: both were about javac 8 or 21 compiling the code. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Yes, and the PR now updates that step. The CALCITE-6616 note no longer applies: the code is compiled by JDK 25 with So after this change, release managers don't really need to run on Java 8. The step still asks for JDK 8 only because I wanted a minimal change that gets us past the compiler bugs. It now also says that JDK 25 must be installed, since Gradle on Java 8 cannot download it (see The JDK that compiles Calcite). I can lift the JDK 8 requirement if you want. |
|
…R needs A pull request can be reviewed along many axes (tests, user docs, doc comments, the change description, performance, security, compatibility, and more), and a focused reviewer per axis on every pull request costs too much. In apache/calcite#5278 a JDK toolchain bump reached review with the docs still naming JDK 8, and only a reviewer's question caught it. The review needs to know, per pull request and cheaply, which axes owe a focused check and whether to split the work across subagents. research/review-axis-classifier holds the experiment: 37 pull requests from pgjdbc, NullAway and Calcite pinned at their as-submitted commits, a 16-axis rubric, ground truth labeled with hindsight by Claude Opus 5.5 agents (kappa 0.86 against a rerun), and 21 classifier variants scored on the same input. Jev with a yes/no question per axis and a short project profile ranks best among the cheap ones (AUC 0.94, about $0.0002 per pull request); Sonnet 5 and GPT-6 Luna reach 0.91 at 7 to 290 times the cost, Haiku 4.5 0.87, zero-shot CLM-v0.1-8B 0.72. Opus 5.5 reaches 0.98 but is the labeler's own model. The predictions, labels and scripts are committed so that a new classifier is added by writing preds/<id>.jsonl, registering it in classifiers.json, and running scripts/report.py, which regenerates results.md and dataset.md. Fetched diffs stay in an ignored cache/. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>



Jira: CALCITE-4553
Previously javac came from whichever JDK ran Gradle, so the compiler differed between CI jobs and between developers' machines. Only
sourceCompatibility/targetCompatibility1.8 kept the bytecode at Java 8, and nothing checked that the code uses only the Java 8 API. Older javac versions also have known bugs with type annotations, which Calcite uses for nullability.JavaCompiletasks now take their compiler from a Gradle toolchain whose version is set by thejdkBuildVersionproperty (default25), and pass--release 8. Tests, javadoc, andJavaExectasks still run on the JDK that runs Gradle, so the existing JDK 8/11/17/21/24 test coverage is unchanged.The foojay toolchain resolver downloads the build JDK when it is not installed. It stays at 0.10.0, because 1.0.0 requires Gradle to run on Java 17 and CI still runs Gradle on Java 8 and 11. When Gradle runs on Java 8, the download fails anyway (
api.foojay.ioanswers the Java 8 User-Agent witherror code: 1010), so the build JDK must already be installed; on Java 11 and later the download works.CI: every job installs JDK 25 next to the JDK it tests with, and the test JDK stays the default.
org.gradle.java.installations.auto-download=falsemakes a job fail rather than compile with a JDK foojay downloaded instead of the onesetup-javainstalled. The ErrorProne and Checker Framework jobs pass-PjdkBuildVersion=11, because ErrorProne 2.5.1 fails on JDK 25 withIllegalAccessError: class com.google.errorprone.JavacErrorDescriptionListener ... cannot access class com.sun.tools.javac.tree.JCTree$JCCompilationUnit.Verification: locally,
build -x test javadocpasses, classes have major version 52, and with Gradle on JDK 8 the compiler is JDK 25 while:linq4j:testruns on JDK 8. Thejandextask is part ofcheck, so every CIbuildjob parses the produced bytecode.🤖 Generated with Claude Code