ci: install clang-22 in setup-llvm22 and assert the pair - #7388
Merged
Conversation
added 2 commits
August 4, 2026 18:37
apt ships opt and clang as separate packages, so llvm-22-dev alone leaves /usr/lib/llvm-22/bin/opt with no clang beside it -- the state ubuntu-24.04-arm runners land in. Consumers needing a matched opt+clang pair (the RS4GC arm in gc-native-roots.yml) then fail the pair-check, or fall back to hand-rolled discovery and install the distro's unversioned llvm clang, which on Ubuntu 24.04 is LLVM 18 -- running opt 18 over IR emitted by Perry's linked LLVM 22. Install clang-22, symlink it under the llvm-config-22 prefix so directory-based resolution finds a co-located pair, and assert clang's major alongside the existing llvm-config check. A green setup step that leaves a mismatched clang is the failure this action exists to prevent. Fixes the root cause of #7384 for all 18 workflows rather than one.
6 tasks
📝 WalkthroughWalkthroughThe LLVM 22 setup action now installs Clang 22, co-locates it with LLVM 22 tools, validates both versions, and exports the LLVM prefix. A changelog entry documents the updated setup. ChangesLLVM 22 toolchain setup
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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: 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 `@changelog.d/7388-setup-llvm22-clang.md`:
- Line 18: Revise the changelog entry’s “Fixes the root cause of `#7384` for all
18 workflows” statement to describe only the setup-llvm22 fix. Remove the claim
that all 18 workflows or the follow-up gc-native-roots.yml discovery work are
complete, while preserving the reference to `#7384` if appropriate.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 72838eb8-2fe6-4f92-b79e-5d2e05d4ab1b
📒 Files selected for processing (2)
.github/actions/setup-llvm22/action.ymlchangelog.d/7388-setup-llvm22-clang.md
| setup step that leaves a mismatched clang is precisely the failure this action | ||
| exists to prevent. | ||
|
|
||
| Fixes the root cause of #7384 for all 18 workflows rather than one. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Qualify the #7384 claim.
The PR scope leaves gc-native-roots.yml discovery changes for follow-up issue #7384. Line 18 says the root cause is fixed for all 18 workflows. This overstates the documented scope. Reword the line to describe the setup-llvm22 fix without claiming that the follow-up discovery work is complete.
Based on the PR objectives, gc-native-roots.yml discovery remains follow-up work.
Proposed changelog wording
-Fixes the root cause of `#7384` for all 18 workflows rather than one.
+Ensures all 18 workflows receive a co-located LLVM 22 `clang` and prevents
+fallback to Ubuntu 24.04's unversioned LLVM 18 toolchain. Discovery changes
+for `#7384` remain follow-up work.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Fixes the root cause of #7384 for all 18 workflows rather than one. | |
| Ensures all 18 workflows receive a co-located LLVM 22 `clang` and prevents | |
| fallback to Ubuntu 24.04's unversioned LLVM 18 toolchain. Discovery changes | |
| for `#7384` remain follow-up work. |
🤖 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 `@changelog.d/7388-setup-llvm22-clang.md` at line 18, Revise the changelog
entry’s “Fixes the root cause of `#7384` for all 18 workflows” statement to
describe only the setup-llvm22 fix. Remove the claim that all 18 workflows or
the follow-up gc-native-roots.yml discovery work are complete, while preserving
the reference to `#7384` if appropriate.
jdalton
added a commit
to jdalton/perry
that referenced
this pull request
Aug 4, 2026
The native-roots-rs4gc job re-discovered opt by hand and could resolve an unversioned /usr/bin/opt, then install the distro's LLVM 18 — both binaries present, the same-directory pair-check passing, and RS4GC running opt 18 over IR from Perry's linked LLVM 22 (a green gate on the wrong LLVM). Use the LLVM 22 prefix the setup-llvm22 action pins instead, drop the generic llvm/clang fallback (a non-22 clang would skew the pair), and install the matching clang-22 the action does not yet ship as a bridge until PerryTS#7388 folds it into the action.
proggeramlug
added a commit
that referenced
this pull request
Aug 4, 2026
* docs(plan): fold in the 2026-08-04 findings Two things this plan treated as measured were not. Statepoints could not compile on aarch64-ELF at all -- a hard failure on a default-on path, from two stacked bugs (#7390: the compact stack-map parser did not model GNU-as `sym = expr`, emitted only at -O3 and only on ELF; and the assembler was not told the -mcpu the code generator was told, so Graviton-emitted SVE was rejected) behind two toolchain ones (#7384, #7388). And three of the four RS4GC matrix arms had NEVER executed, in any run, for want of a concurrency group (#7393). Every "the ELF arm is the only one red" conclusion rested on arms that never reached a runner. That is a fifth way a gate cannot fail, and it is now written down. Also folded in: nine Layer 3 rooting fixes and the rule they share (ordering, not missing roots; a fault that MOVES is a real fix, one that does not move by a byte was already dead before you rooted it); #7380's type confusion and the `gc_type == GC_TYPE_OBJECT` generalisation; RSS -69% (#7377); and the first honest performance measurement -- two benchmarks that measure nothing (#7395) and the array-store guard's siting cost (#7396). The Layer 1 framing is corrected: lower_exprs_rooted already implements the RFC's proposal for codegen operands, gated on any_later_ref_may_trigger_gc, and all four arms of func_ref.rs use it. So the gap is Layer 3, where #7389 supplies the first structural answer. * docs: name the fragment for its real PR (#7397) --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>
proggeramlug
added a commit
that referenced
this pull request
Aug 5, 2026
…#7417) * fix(ci): stop interpolating the LLVM version into the pwsh setup step zizmor has been red on every `main` commit since #7353 created `.github/actions/setup-llvm22/action.yml` -- roughly 40 consecutive commits. #7388 and #7393 only shifted the reported line numbers, which is what made them look implicated; neither introduced a finding. Four high-severity findings, one of which is properly fixed here. The Windows arm interpolated a composite-action input straight into a PowerShell script body (`$ver = "${{ inputs.version }}"`), which `template-injection` flags at High confidence: the expansion is substituted as raw text before pwsh parses the line, so an input carrying a quote plus a statement separator would execute as code with the runner's privileges. The input now arrives through an `env:` block and is read as `$env:LLVM_VERSION`, a plain string load. The other three are `github-env` at Low confidence -- the single `LLVM_SYS_221_PREFIX=<prefix>` line the action exists to write, once per platform arm -- and are suppressed with reasoning in `.github/zizmor.yml`. Measured: the audit is satisfiable only by not writing the environment file at all, and the clean alternative ($GITHUB_OUTPUT plus composite outputs) costs 44 jobs and 140 downstream steps, recreating the duplication the action exists to remove. The carve-out is a dated ratchet with an explicit delete-condition. Verified with the repo's SRI-pinned zizmor 1.28.0: pristine config plus this fix reports 3 high and exits 14; with the carve-out it exits 0 and `ignored` rises 119 -> 122, matching the three suppressed findings exactly. Claude-Session: https://claude.ai/code/session_019EHcmXKArA7m42SihYCcgH * docs: name the fragment for its real PR (#7417) --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>
proggeramlug
added a commit
that referenced
this pull request
Aug 5, 2026
…ds (#7418) * fix(ci): satisfy the phantom xml2s.lib the LLVM Windows tarball demands LLVM's official Windows release script builds a static libxml2 into a scratch directory and points cmake at it with -DLLVM_ENABLE_LIBXML2=FORCE_ON -DLIBXML2_LIBRARIES=%libxmldir%/lib/libxml2s.lib. %libxmldir% is never installed, so the published clang+llvm-*-pc-windows-msvc tarball carries the dependency but not the library. llvm-config --system-libs --link-static reports xml2s.lib, llvm-sys forwards every system lib verbatim with no knob to filter one out, and link.exe dies with LNK1181 before resolving a symbol. Synthesize an empty archive at the LLVM libdir when llvm-config reports xml2s.lib AND the libdir lacks it. It is a name dependency, not a symbol dependency: libxml2 is reachable only from LLVMWindowsManifest, which the LLVM-C surface inkwell drives never touches, and rustc bundles the component archives into libllvm_sys.rlib where link.exe pulls members lazily. If that stops being true the link fails loudly with LNK2019 rather than silently dropping manifest support. Checking both conditions makes the workaround self-deleting once a release ships or stops reporting the library. Latent since #7353 made the in-process LLVM backend the default and statically linked, not caused by #7388 (which touches only the Linux arm). It became visible when #7393's concurrency group let gc-native-roots.yml's windows-latest arm reach a runner for the first time. Fixing it in the composite action also unblocks test.yml's windows-build. Claude-Session: https://claude.ai/code/session_019EHcmXKArA7m42SihYCcgH * docs: name the fragment for its real PR (#7418) --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>
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
1 participant
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.
Root cause behind #7384, fixed in one place instead of one workflow.
What's wrong
setup-llvm22installsllvm-22-dev libpolly-22-dev libzstd-dev— noclang-22. apt ships opt and clang as separate packages, so this leaves/usr/lib/llvm-22/bin/optwith no clang beside it. That is exactly theubuntu-24.04-armstate #7384 is working around.Why it matters beyond a missing binary
gc-native-roots.ymlalready doesuses: ./.github/actions/setup-llvm22at line 167 — then throws the pinned prefix away at line 209 and re-discovers a toolchain by hand:If
command -v optresolves to an unversioned/usr/bin/opt, that installs the distro's LLVM 18. Both binaries exist, the same-dir pair-check passes, and RS4GC runsopt18 over IR emitted by Perry's linked LLVM 22. The gate is green and its subject is wrong — CLAUDE.md's fourth hazard. The action's own comment warns about this ("Ubuntu 24.04 ships 18") and the workflow bypasses it forty lines later.This PR
clang-22llvm-config-22prefix, because consumers resolve a toolchain by directory —PERRY_LLVM_OPTandPERRY_LLVM_CLANGmust come from the same bin dir or they can skew across majorsllvm-configcheckFollow-up, deliberately not bundled
With a guaranteed co-located pair under
$LLVM_SYS_221_PREFIX/bin,gc-native-roots.yml's hand-rolled Linux discovery can be deleted outright and the skew hazard with it. I've left that to #7384's author rather than conflict with their open diff on that file.Summary by CodeRabbit
Bug Fixes
Documentation