Test under Java 26#773
Open
mernst wants to merge 22 commits into
Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds JDK 26 support across build and CI: detects JAVA26 and adjusts javac target selection in java/Makefile, extends .azure/.circleci m4 matrices, generates Ubuntu and RockyLinux JDK26 jobs (quick/nonquick/non-txt/misc/kvasir and multipart typecheck parts), wires workflow dependencies so JDK26 jobs require canary and corresponding JDK25 jobs (RockyLinux jobs also require Ubuntu JDK26), and updates the changelog and a small comment and README snippet. Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.circleci/config.yml:
- Around line 1048-1051: The workflow references 16 JDK26 job names (e.g.,
quick-txt-diff-ubuntu-jdk26, nonquick-txt-diff-ubuntu-jdk26,
non-txt-diff-ubuntu-jdk26, misc-ubuntu-jdk26, kvasir-ubuntu-jdk26,
typecheck-latest-part1-ubuntu-jdk26 ... typecheck-bundled-part3-ubuntu-jdk26,
plus the rockylinux variants) that are not defined under the top-level jobs:
map; add corresponding job stanzas for each of these 16 names in the jobs:
section by copying their JDK25 counterparts (e.g., quick-txt-diff-ubuntu-jdk25,
typecheck-bundled-part2-ubuntu-jdk25, quick-txt-diff-rockylinux-jdk25, etc.) and
update JDK-specific settings (Docker image tag, environment variables, any
matrix or executor names) from jdk25 to jdk26 so the workflow references match
defined jobs.
In `@scripts/Dockerfile-rockylinux-jdk26-plus`:
- Around line 68-72: The RUN layer currently calls dnf config-manager
--set-enabled crb before installing dnf-plugins-core, which can fail on the base
image; change the RUN instruction so dnf-plugins-core is installed first
(install dnf-plugins-core in the same or a prior RUN) and then run dnf
config-manager --set-enabled crb, ensuring the plugin is available when running
the config-manager command.
- Around line 91-96: Replace the unpinned Astral installer URL used in the RUN
step with a version-pinned URL and propagate this change across all "-plus"
Dockerfile variants; specifically, introduce a UV_VERSION (e.g., 0.10.11) and
use the installer path https://astral.sh/uv/<UV_VERSION>/install.sh in the RUN
that currently contains "wget -qO- https://astral.sh/uv/install.sh | sh",
updating the RUN block that sets up uv and preserving the subsequent chmod and
ENV PATH lines so builds are reproducible across the Ubuntu and RockyLinux
"-plus" Dockerfiles.
In `@scripts/Dockerfile-ubuntu-jdk26-plus`:
- Around line 87-100: Pin versions and add checksum verification: change the two
pipx installs (PIPX_HOME=/opt/pipx PIPX_BIN_DIR=/usr/local/bin pipx install mypy
and pipx install ruff) to install specific released versions (e.g., pipx install
mypy==<version> and pipx install ruff==<version>) and update any related PIPX_*
env usage accordingly; replace the wget | sh for uv (wget -qO-
https://astral.sh/uv/install.sh | sh) with downloading a specific uv release
artifact (use a fixed release URL), verify its checksum (e.g., sha256) before
executing, and invoke the installer from the verified file so the Dockerfile
uses pinned, reproducible tooling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e3bead74-da5d-4e37-a9f5-5783cf7eab88
📒 Files selected for processing (12)
.azure/azure-pipelines.yml.azure/azure-pipelines.yml.m4.azure/jobs.m4.circleci/config.yml.circleci/config.yml.m4doc/CHANGELOG.mdjava/Makefilescripts/Dockerfile-READMEscripts/Dockerfile-rockylinux-jdk26scripts/Dockerfile-rockylinux-jdk26-plusscripts/Dockerfile-ubuntu-jdk26scripts/Dockerfile-ubuntu-jdk26-plus
There was a problem hiding this comment.
♻️ Duplicate comments (1)
java/Makefile (1)
54-58:⚠️ Potential issue | 🟠 Major
JAVA26_HOMEis still bypassed or fails open depending onJAVA_HOME.This is the same root cause as the earlier JDK 26 toolchain-selection comment:
JAVA25_HOME := ${JAVA_HOME}is expanded before the laterJAVA_HOME ?= ...autodetection block. IfJAVA_HOMEis exported, Line 75 always wins and the newJAVA26_HOMEfallback added forscripts/Dockerfile-ubuntu-jdk26Line 51 /scripts/Dockerfile-rockylinux-jdk26Line 40 is never used. IfJAVA_HOMEis not exported,JAVA25_HOMEbecomes empty and Line 82 aborts beforeJAVA_HOMEcan be derived fromwhich javac. That leaves JDK 26 builds dependent on ambient env vars instead of the selected toolchain.💡 Possible fix
ifeq ($(shell test ${JAVA_RELEASE_NUMBER} -ge 25; echo $$?),0) JAVA25 := 1 - ifndef JAVA25_HOME - JAVA25_HOME := ${JAVA_HOME} + ifeq ($(shell test ${JAVA_RELEASE_NUMBER} -eq 25; echo $$?),0) + JAVA25_HOME ?= ${JAVA_HOME} endif endif ... ifeq (${JAVA25}, 1) ifdef JAVA25_HOME JAVAC_TARGET_FLAGS ?= --source 25 --target 25 --system ${JAVA25_HOME} else ifdef JAVA26_HOME JAVAC_TARGET_FLAGS ?= --source 25 --target 25 --system ${JAVA26_HOME} else - $(error JAVA25_HOME and JAVA26_HOME are not set; JAVA_HOME=${JAVA_HOME}) + JAVAC_TARGET_FLAGS ?= --source 25 --target 25 --system ${JAVA_HOME} endif endif else#!/bin/bash set -euo pipefail echo "Relevant logic from java/Makefile:" sed -n '54,83p' java/Makefile echo echo "Later JAVA_HOME autodetection:" sed -n '132,137p' java/Makefile cat >/tmp/java26-home-repro.mk <<'EOF' JAVA25 := 1 ifndef JAVA25_HOME JAVA25_HOME := ${JAVA_HOME} endif ifeq (${JAVA25}, 1) ifdef JAVA25_HOME TARGET := ${JAVA25_HOME} else ifdef JAVA26_HOME TARGET := ${JAVA26_HOME} else $(error JAVA25_HOME and JAVA26_HOME are not set; JAVA_HOME=${JAVA_HOME}) endif endif endif JAVA_HOME ?= /detected/later/from-path print: ; `@printf` 'TARGET=%s\n' '$(TARGET)' EOF echo echo "Case 1: exported JAVA_HOME keeps the JAVA26_HOME branch unreachable:" JAVA_HOME=/env/jdk26 JAVA26_HOME=/explicit/jdk26 make -f /tmp/java26-home-repro.mk print echo echo "Case 2: with neither variable exported, the file errors before the later JAVA_HOME default is visible:" if make -f /tmp/java26-home-repro.mk print >/tmp/java26-home-repro.err 2>&1; then echo "unexpected success" exit 1 else sed -n '1,5p' /tmp/java26-home-repro.err fiAlso applies to: 74-83
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@java/Makefile` around lines 54 - 58, The Makefile sets JAVA25_HOME from JAVA_HOME too early which prevents the later JAVA_HOME autodetection and blocks the JAVA26_HOME fallback; update the logic so JAVA_HOME is determined first or avoid early unconditional expansion—remove or change "JAVA25_HOME := ${JAVA_HOME}" to a conditional assignment (e.g. use "?="), or better: do not set JAVA25_HOME there and instead compute TARGET after the "JAVA_HOME ?= ..." autodetection using the chain (ifdef JAVA25_HOME -> TARGET=JAVA25_HOME else ifdef JAVA26_HOME -> TARGET=JAVA26_HOME else error), ensuring JAVA25_HOME, JAVA26_HOME and JAVA_HOME references (and TARGET) resolve correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@java/Makefile`:
- Around line 54-58: The Makefile sets JAVA25_HOME from JAVA_HOME too early
which prevents the later JAVA_HOME autodetection and blocks the JAVA26_HOME
fallback; update the logic so JAVA_HOME is determined first or avoid early
unconditional expansion—remove or change "JAVA25_HOME := ${JAVA_HOME}" to a
conditional assignment (e.g. use "?="), or better: do not set JAVA25_HOME there
and instead compute TARGET after the "JAVA_HOME ?= ..." autodetection using the
chain (ifdef JAVA25_HOME -> TARGET=JAVA25_HOME else ifdef JAVA26_HOME ->
TARGET=JAVA26_HOME else error), ensuring JAVA25_HOME, JAVA26_HOME and JAVA_HOME
references (and TARGET) resolve correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 938f3dbd-c45c-466c-b468-706d9bbe43c7
📒 Files selected for processing (1)
java/Makefile
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
java/Makefile (1)
59-66:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix Java 26 build failure caused by
JAVAC_TARGET_FLAGSrequiringJAVA25_HOME/JAVA26_HOMEbeforeJAVA_HOMEis detected
- With
JAVA_RELEASE_NUMBER >= 25,JAVA25 := 1, so theJAVAC_TARGET_FLAGSblock always uses--system ${JAVA25_HOME}or--system ${JAVA26_HOME}and errors out if both are unset.JAVA25_HOMEis only seeded whenJAVA_RELEASE_NUMBER == 25, whileJAVA_HOME ?= ...detection happens later in the file, so local JDK 26 builds can fail unlessJAVA26_HOME(orJAVA25_HOME) is pre-exported (CI masks this viaJAVA26_HOME).- Move the
JAVA_HOMEdiscovery block above theJAVAC_TARGET_FLAGScomputation (or otherwise deriveJAVA26_HOMEfrom the detectedjavac/JAVA_HOMEbefore the error path).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@java/Makefile` around lines 59 - 66, The build fails for Java 26 because JAVAC_TARGET_FLAGS chooses --system ${JAVA25_HOME} or ${JAVA26_HOME} before JAVA_HOME is discovered; move the JAVA_HOME discovery / javac detection logic earlier so JAVA25_HOME/JAVA26_HOME can be derived before computing JAVAC_TARGET_FLAGS, or alternatively detect javac/JAVA_HOME and set JAVA26_HOME (and JAVA25_HOME fallback) immediately after computing JAVA_RELEASE_NUMBER and before the JAVAC_TARGET_FLAGS block so the flags use valid paths; update references to JAVA_RELEASE_NUMBER, JAVA25_HOME, JAVA26_HOME, JAVAC_TARGET_FLAGS and JAVA_HOME accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@java/Makefile`:
- Around line 59-66: The build fails for Java 26 because JAVAC_TARGET_FLAGS
chooses --system ${JAVA25_HOME} or ${JAVA26_HOME} before JAVA_HOME is
discovered; move the JAVA_HOME discovery / javac detection logic earlier so
JAVA25_HOME/JAVA26_HOME can be derived before computing JAVAC_TARGET_FLAGS, or
alternatively detect javac/JAVA_HOME and set JAVA26_HOME (and JAVA25_HOME
fallback) immediately after computing JAVA_RELEASE_NUMBER and before the
JAVAC_TARGET_FLAGS block so the flags use valid paths; update references to
JAVA_RELEASE_NUMBER, JAVA25_HOME, JAVA26_HOME, JAVAC_TARGET_FLAGS and JAVA_HOME
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 64face08-f189-425d-9101-e44f522bcaae
📒 Files selected for processing (7)
.azure/azure-pipelines.yml.azure/jobs.m4.circleci/config.yml.circleci/config.yml.m4doc/CHANGELOG.mdjava/Makefilejava/daikon/chicory/Instrument24.java
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@java/Makefile`:
- Around line 26-35: The outer Darwin conditional ifeq ($(shell uname), Darwin)
currently remains open until much later, which wrongly scopes
JAVA_RELEASE_NUMBER and related setup to non-Darwin systems; close that outer
conditional immediately after setting JAVA_HOME on Darwin by inserting an endif
right after the Darwin branch (right after the line with JAVA_HOME ?=
$(/usr/libexec/java_home)) so that the subsequent non-Darwin javac detection and
the JAVA_RELEASE_NUMBER/JAVA_TARGET_FLAGS logic apply to all platforms; ensure
the symbols referenced (ifeq ($(shell uname), Darwin), JAVA_HOME,
JAVA_RELEASE_NUMBER, JAVA_TARGET_FLAGS) are no longer nested under the Darwin
block.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 41a050c1-a804-4669-9166-76869c4ac06c
📒 Files selected for processing (1)
java/Makefile
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
java/Makefile (3)
26-27:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix Darwin
JAVA_HOMEdetection syntax injava/Makefile(line 27).
JAVA_HOME ?= $(/usr/libexec/java_home)is GNU Make variable expansion (not command execution), so it will expand to empty on macOS. Use$(shell ...)to run the command and capture its output.Suggested fix
ifeq ($(shell uname), Darwin) - JAVA_HOME ?= $(/usr/libexec/java_home) + JAVA_HOME ?= $(shell /usr/libexec/java_home) else🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@java/Makefile` around lines 26 - 27, The Darwin branch in the Makefile uses GNU Make variable expansion instead of executing the macOS helper, so change the JAVA_HOME assignment in the ifeq ($(shell uname), Darwin) block to execute /usr/libexec/java_home and capture its output (i.e., replace the current JAVA_HOME ?= $(/usr/libexec/java_home) line with one that uses $(shell /usr/libexec/java_home) so JAVA_HOME is set correctly).
39-40:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDerive
JAVA_RELEASE_NUMBERfrom the selected compiler (JAVAC/JAVA_HOME), not PATHjavac.
java/MakefileloadsMakefile.userfirst (-include Makefile.user), butJAVA_VERSION_STRING_WITH_EA/JAVA_VERSION_STRINGare still computed via$(shell javac -version ...)(hardcoded). The actual compile uses${JAVAC}(JAVAC_COMMAND := ${JAVAC} ...), so overridingJAVAC/JAVA_HOMEcan desync the computedJAVA_RELEASE_NUMBERand therefore the derivedJAVAC_TARGET_FLAGS(e.g., falling back to-source 8 -target 8based on whateverjavacis on PATH).Suggested fix
+JAVAC_VERSION_CMD := $(if ${JAVA_HOME},${JAVA_HOME}/bin/javac,$(firstword ${JAVAC})) -JAVA_VERSION_STRING_WITH_EA := $(shell javac -version 2>&1 | head -1 | cut "-d " -f2) -JAVA_VERSION_STRING := $(shell javac -version 2>&1 | head -1 | cut "-d " -f2 | sed 's/-ea//') +JAVA_VERSION_STRING_WITH_EA := $(shell ${JAVAC_VERSION_CMD} -version 2>&1 | head -1 | cut "-d " -f2) +JAVA_VERSION_STRING := $(shell ${JAVAC_VERSION_CMD} -version 2>&1 | head -1 | cut "-d " -f2 | sed 's/-ea//')🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@java/Makefile` around lines 39 - 40, The version variables JAVA_VERSION_STRING_WITH_EA and JAVA_VERSION_STRING are being derived from the PATH `javac` instead of the actual compiler chosen by the Makefile (JAVAC/JAVA_HOME), causing JAVA_RELEASE_NUMBER and JACAC_TARGET_FLAGS to drift; change the shell invocations to call the selected compiler variable (use ${JAVAC} or ${JAVA_HOME}/bin/javac if you prefer) when computing JAVA_VERSION_STRING_WITH_EA and JAVA_VERSION_STRING (e.g., run `${JAVAC} -version 2>&1 | ...`) after Makefile.user is included so overrides take effect, and ensure all later logic that computes JAVA_RELEASE_NUMBER and sets JAVAC_TARGET_FLAGS uses those updated variables instead of the hardcoded `javac`.
76-98:⚠️ Potential issue | 🟠 Major | ⚡ Quick winJDK 26 local builds can fail unless
JAVA26_HOMEis set (theJAVA_HOMEdetection doesn’t prevent the$(error ...)).When
JAVA_RELEASE_NUMBER >= 26,JAVA25is also enabled, butJAVA25_HOMEis only defaulted forJAVA_RELEASE_NUMBER == 25andJAVA26_HOMEis never defaulted. As a result, theJAVAC_TARGET_FLAGSblock errors if neitherJAVA25_HOMEnorJAVA26_HOMEis defined, even thoughJAVA_HOMEis detected earlier.Suggested fix
ifeq ($(shell test ${JAVA_RELEASE_NUMBER} -ge 26; echo $$?),0) JAVA26 := 1 + ifndef JAVA26_HOME + JAVA26_HOME := ${JAVA_HOME} + endif endifTest with a local JDK 26 where only
JAVA_HOMEis set (noJAVA26_HOME).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@java/Makefile` around lines 76 - 98, The Makefile errors when JAVA25 is enabled for JDK 26 because the JAVAC_TARGET_FLAGS block requires JAVA25_HOME or JAVA26_HOME even though JAVA_HOME may be set; update the logic around JAVA25/JAVA26 handling so that when JAVA_RELEASE_NUMBER >= 26 you default JAVA26_HOME from JAVA_HOME if JAVA26_HOME is not defined (or accept JAVA_HOME as a valid fallback) before emitting the $(error), and then set JAVAC_TARGET_FLAGS using the resolved JAVA25_HOME or JAVA26_HOME; specifically adjust the conditional that references JAVA25, JAVA25_HOME, JAVA26_HOME, and JAVAC_TARGET_FLAGS to prefer existing JAVA*_HOME values and fallback to JAVA_HOME so local JDK 26 builds succeed without an explicit JAVA26_HOME.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@java/Makefile`:
- Around line 26-27: The Darwin branch in the Makefile uses GNU Make variable
expansion instead of executing the macOS helper, so change the JAVA_HOME
assignment in the ifeq ($(shell uname), Darwin) block to execute
/usr/libexec/java_home and capture its output (i.e., replace the current
JAVA_HOME ?= $(/usr/libexec/java_home) line with one that uses $(shell
/usr/libexec/java_home) so JAVA_HOME is set correctly).
- Around line 39-40: The version variables JAVA_VERSION_STRING_WITH_EA and
JAVA_VERSION_STRING are being derived from the PATH `javac` instead of the
actual compiler chosen by the Makefile (JAVAC/JAVA_HOME), causing
JAVA_RELEASE_NUMBER and JACAC_TARGET_FLAGS to drift; change the shell
invocations to call the selected compiler variable (use ${JAVAC} or
${JAVA_HOME}/bin/javac if you prefer) when computing JAVA_VERSION_STRING_WITH_EA
and JAVA_VERSION_STRING (e.g., run `${JAVAC} -version 2>&1 | ...`) after
Makefile.user is included so overrides take effect, and ensure all later logic
that computes JAVA_RELEASE_NUMBER and sets JAVAC_TARGET_FLAGS uses those updated
variables instead of the hardcoded `javac`.
- Around line 76-98: The Makefile errors when JAVA25 is enabled for JDK 26
because the JAVAC_TARGET_FLAGS block requires JAVA25_HOME or JAVA26_HOME even
though JAVA_HOME may be set; update the logic around JAVA25/JAVA26 handling so
that when JAVA_RELEASE_NUMBER >= 26 you default JAVA26_HOME from JAVA_HOME if
JAVA26_HOME is not defined (or accept JAVA_HOME as a valid fallback) before
emitting the $(error), and then set JAVAC_TARGET_FLAGS using the resolved
JAVA25_HOME or JAVA26_HOME; specifically adjust the conditional that references
JAVA25, JAVA25_HOME, JAVA26_HOME, and JAVAC_TARGET_FLAGS to prefer existing
JAVA*_HOME values and fallback to JAVA_HOME so local JDK 26 builds succeed
without an explicit JAVA26_HOME.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4e8c1efc-583e-423d-bb2b-e3f9f76281a0
📒 Files selected for processing (1)
java/Makefile
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.