Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 15 minutes and 39 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds JDK 26 across CI, build, and Docker artifacts: new Azure Pipelines and CircleCI jobs for JDK 26 (Ubuntu and Rocky Linux variants) wired to depend on corresponding JDK25 jobs; extended job-generation macros to include Java 26; new Dockerfiles for Ubuntu and Rocky Linux (regular and “plus”) installing OpenJDK 26 and CI/tooling; Makefile updates to detect JAVA26 and adjust javac target selection and error messages; CHANGELOG updated to list Java 8–26 support. Minor whitespace/comment edits in a few scripts. Possibly related PRs
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 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 |
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
No description provided.