graalvm*: add CE JDK 17 and fix CE JDK 21 checkver#592
Conversation
✅ Actions performedFull review triggered. |
✅ Actions performedReviews resumed. |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
bucket/graalvm-ce-17jdk.json (1)
20-24: checkver config LGTM; optional regex tightening.Mirrors the pattern now used in
graalvm21-jdk21.json;jsonpath: $[*].tag_nameagainstreleases?per_page=100correctly yields tag strings for the regex filter.Optional nit (same as the sibling manifest):
jdk-(17[\d.]+)would match a tag likejdk-170.xif one ever appeared. Usingjdk-(17\.[\d.]+)is a touch stricter. Purely defensive — safe to ignore.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bucket/graalvm-ce-17jdk.json` around lines 20 - 24, The checkver block's regex "jdk-(17[\\d.]+)" is permissive and could match unintended tags; update the regex in the checkver section to the stricter pattern "jdk-(17\\.[\\d.]+)" (i.e., modify the regex value in the checkver object) so it requires a dot after "17" and avoids matching things like "jdk-170.x".bucket/graalvm21-jdk21.json (1)
21-23: checkver switch to Releases API LGTM.Querying
/releases?per_page=100and extracting tag names viajsonpathcorrectly avoids the/releases/latestHTML endpoint that was pinned to the 25.x train, so the existingjdk-(21[\d.]+)regex can now pick up the latestjdk-21.xtag among the paged results. Current release cadence ongraalvm/graalvm-ce-buildskeeps the relevant 21.x tags well within the first 100 entries.Optional nit: the regex could be tightened to
jdk-(21\.[\d.]+)so it doesn't match a hypotheticaljdk-21X...tag (e.g.jdk-210.0.0). Not a real-world risk today, purely defensive.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bucket/graalvm21-jdk21.json` around lines 21 - 23, Tighten the release-tag regex used for checkver: replace the current "regex" value jdk-(21[\d.]+) with a more defensive pattern that requires a dot after 21 (e.g. jdk-(21\.[\d.]+)); update the "regex" field in the JSON (the entry with keys "url", "jsonpath", "regex") so it only matches tags like jdk-21.x and not hypothetical jdk-210... tags.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@bucket/graalvm-ce-17jdk.json`:
- Around line 20-24: The checkver block's regex "jdk-(17[\\d.]+)" is permissive
and could match unintended tags; update the regex in the checkver section to the
stricter pattern "jdk-(17\\.[\\d.]+)" (i.e., modify the regex value in the
checkver object) so it requires a dot after "17" and avoids matching things like
"jdk-170.x".
In `@bucket/graalvm21-jdk21.json`:
- Around line 21-23: Tighten the release-tag regex used for checkver: replace
the current "regex" value jdk-(21[\d.]+) with a more defensive pattern that
requires a dot after 21 (e.g. jdk-(21\.[\d.]+)); update the "regex" field in the
JSON (the entry with keys "url", "jsonpath", "regex") so it only matches tags
like jdk-21.x and not hypothetical jdk-210... tags.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 28b087af-1347-4a0b-ae88-be1e829ff9c8
📒 Files selected for processing (2)
bucket/graalvm-ce-17jdk.jsonbucket/graalvm21-jdk21.json
✅ Actions performedReview triggered.
|
|
This is the CE side of the long-running GraalVM request in #472. It adds graalvm-ce-17jdk using the naming discussed there, and fixes graalvm21-jdk21 checkver so it can still resolve the 21 line from the official GraalVM CE releases source. CI and CodeRabbit are clean now, so I thought I’d flag it in case it helps with review. |
|
/verify |
2 participants
This handles the still-useful Community Edition part of #508 without reusing its stale manifest shape directly.
Changes:
graalvm-ce-17jdkfor GraalVM Community JDK 17.0.9, using the naming discussed in [Request] New Oracle GraalVM and GraalVM Community for JDK 17.0.7 and JDK 20.0.1 #472.graalvm21-jdk21manifest for Community JDK 21, but change itscheckversource away fromreleases/latestso it can still see the latest 21.x release instead of only the latest 25.x release.Design decisions:
graalvm-ce-17jdkuses the sameextract_toplus installer move pattern as the existing newer GraalVM manifests, so it does not depend on the internal extracted directory name.graalvm21-jdk21already covers the Community JDK 21 package requested in graalvm-ce-jdk: add version 17.0.9 and 21.0.2 #508, so this PR fixes its version detection rather than adding a duplicate manifest under another name.graalvm/graalvm-ce-buildsGitHub releases API, not a third-party source.Validation:
checkver.ps1 graalvm-ce-17jdk->17.0.9checkver.ps1 graalvm21-jdk21->21.0.2checkurls.ps1 graalvm-ce-17jdk->[1][1][0]checkurls.ps1 graalvm21-jdk21->[1][1][0]I have read the Contributing Guide.
Summary by CodeRabbit
New Features
Improvements