Conversation
Bumps the pinned LLVM version across build scripts, bootstrap, Dockerfile, GitHub Actions, nix, and docs. Also removes the asan-dyld-shim workaround: the upstream fix (llvm/llvm-project#188913) is in release/22.x, so 22.1.8 no longer needs the interpose dylib and the workaround registry would fail configure for darwin+asan otherwise. WebKit side: oven-sh/WebKit#296
|
Updated 10:39 AM PT - Jul 21st, 2026
❌ @autofix-ci[bot], your commit e88f16a has 1 failures in
🧪 To try this PR locally: bunx bun-pr 34299That installs a local version of the PR into your bun-34299 --bun |
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 47 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
WalkthroughChangesLLVM references and installation instructions are updated from version 21 to 22. Alpine CI and sysroot versions move to 3.24, the macOS ASAN dyld shim workaround is removed, WebKit selection changes, and Objective-C runtime call sites receive formatting-only updates. LLVM 22 Toolchain
Alpine and Build Cleanup
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
|
Found 2 issues this PR may fix:
🤖 Generated with Claude Code |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
- test/napi/node-napi-tests/harness.ts: linuxClang path llvm-21 -> llvm-22 - test/internal/*-config.test.ts: mock clangVersion/clangResourceDir 21 -> 22 - scripts/build/rules.ts: update stale shim_dylib comment - .github/workflows/CLAUDE.md: LLVM_VERSION_MAJOR=19 example -> 22
![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 `@scripts/bootstrap.ps1`:
- Line 259: Update the version marker at the top of scripts/bootstrap.ps1 from
21 to 22 to match the LLVM_VERSION dependency upgrade and trigger a fresh Azure
image bake; leave the LLVM_VERSION value unchanged.
🪄 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: 33aeccb9-cd85-4cf5-a050-acb07c5484e9
📒 Files selected for processing (19)
.buildkite/Dockerfile.github/workflows/CLAUDE.md.github/workflows/clippy.yml.github/workflows/format.yml.github/workflows/miri.ymlCONTRIBUTING.mddocs/project/building-windows.mdxdocs/project/contributing.mdxflake.nixscripts/bootstrap.ps1scripts/bootstrap.shscripts/build/config.tsscripts/build/shims.tsscripts/build/shims/asan-dyld-shim.cscripts/build/tools.tsscripts/build/workarounds.tsscripts/run-clang-format.shshell.nixsrc/runtime/webview/ObjCRuntime.h
💤 Files with no reviewable changes (3)
- scripts/build/shims/asan-dyld-shim.c
- scripts/build/workarounds.ts
- scripts/build/shims.ts
- workarounds.ts: remove rust-lld-for-crosslang-lto entry (clang 22 ==
rustc's LLVM 22, so clangMajor >= rustMajor trips). The wantRustLld swap
in config.ts is kept for the next time rustc's LLVM outpaces clang's;
it's a no-op when the majors match.
- test/internal/windows-cross-config.test.ts: bump the rust-lld test's
rustLlvmVersion to 23.1.0 so the 'rust > clang' precondition holds.
- test/bundler/compile-sourcemap-internal.test.ts: drop the
DYLD_FALLBACK_LIBRARY_PATH workaround for the deleted asan-dyld-shim.
- bootstrap.{sh,ps1}: bump the Version markers (38->39, 21->22).
…leanup apt.llvm.org's llvm-toolchain-focal-22 has no arm64 binaries (only the Architecture:all packages). llvm-toolchain-bullseye-22 has the full arm64 set and is built against glibc 2.31 (same as focal), so: - .buildkite/Dockerfile: on arm64, add the bullseye repo and install the specific package set; stub the three Debian gcc-10 dev Depends with equivs since we use gcc-13's libstdc++ via the PPA. - scripts/bootstrap.sh: same workaround for focal arm64 hosts. Also in this commit (review cleanup, comment-only): - config.ts / rust.ts / workarounds.ts: drop stale references to the removed rust-lld-for-crosslang-lto workarounds entry.
…macOS (#34508) ## Problem `bun bd` aborts at startup on macOS releases older than 26.4 (seen on 15.6.1, Homebrew llvm@21): ``` AddressSanitizer: CHECK failed: sanitizer_procmaps_mac.cpp:214 "((res)) == ((0))" (0xffffffffffffffff, 0x0) <empty stack> Abort trap: 6 ``` The post-link `bun-debug --revision` smoke test dies with this, so the debug build never completes. Workaround has been `bun run build --asan=off`. ## Cause `scripts/build/shims/asan-dyld-shim.c` interposes `dyld_shared_cache_iterate_text` to keep ASAN init from deadlocking on macOS 26.4 (llvm/llvm-project#182943). It looks up the private `_dyld_get_dyld_header` via `dlsym` to synthesize the one cache entry ASAN needs. Apple's own `dyld_priv.h` marks that symbol ["Added in macOS/iOS 26.4"](https://github.com/apple-oss-distributions/dyld/blob/dyld-1376.6/include/mach-o/dyld_priv.h) (first appears in dyld-1376.6; absent from dyld-1340 and every earlier tag). On any macOS < 26.4, `dlsym` returns `NULL`, the shim returns `-1`, and compiler-rt's `CHECK_EQ(res, 0)` aborts. The shim is linked into every `cfg.darwin && cfg.asan` build with no runtime check, so it breaks every debug build on macOS hosts that predate 26.4. ## Fix When the shim can't synthesize the entry (`dlsym` returned `NULL`, or no shared cache), delegate to the real `dyld_shared_cache_iterate_text` instead of returning `-1`. The deadlock and the getter were introduced together in 26.4, so "getter absent" is exactly "real iterate is safe". This is the same shape as the upstream compiler-rt fix (llvm/llvm-project#188913): weak-import `_dyld_get_dyld_header`, use it when present, otherwise fall through to the existing iterate path. Calling the original by name from inside the interposer does not recurse. dyld explicitly adds an identity-mapping tuple for the defining image; from [DyldRuntimeState.cpp](https://github.com/apple-oss-distributions/dyld/blob/dyld-1378/dyld/DyldRuntimeState.cpp): > `// now add specific interpose so that the generic is not applied to the interposing dylib, so it can call through to old impl` (This is also how Apple's own `DYLD_INTERPOSE` wrapper example in [dyld-interposing.h](https://github.com/apple-oss-distributions/dyld/blob/main/include/mach-o/dyld-interposing.h) works.) ## Verification Shim is only compiled for `darwin && asan` (no CI lane exists for that combination); verified the change compiles clean with `-Wall -Wextra -fblocks` against stubbed darwin headers. Behaviour on 26.4+ is unchanged: `dlsym` finds the symbol there and the same synthesize path runs as before. ## Why this is the right fix The alternative is gating shim emission on host macOS version in `shims.ts`, but the runtime check is strictly better: one binary works on both, and it matches upstream exactly. The shim is temporary regardless (self-obsoletes via `workarounds.ts` once LLVM 22.1.4+ is the floor; #34299 deletes it as part of the LLVM 22 bump), so the goal here is just to stop breaking pre-26.4 macOS until that lands. <!-- robobun:evidence:begin --> --- **no test proof** · iteration 0 · build/CI scripts only; test-proof not applicable <!-- robobun:evidence:end -->
Conflict: scripts/build/shims/asan-dyld-shim.c was modified on main and deleted on this branch. Keep the delete: LLVM 22.1.8 carries the upstream fix (llvm/llvm-project#188913).
…ebKit pin - .buildkite/Dockerfile arm64: install bullseye libz3-4/libz3-dev (sha256-pinned) and drop lldb-22, matching oven-sh/WebKit@5c5e1cdf. libz3.so.4 is a real DT_NEEDED of libLLVM so an equivs dummy can't cover it; the two bullseye .debs only need libc6 (>= 2.30) / libstdc++6 (>= 9), which focal satisfies. - .buildkite/ci.mjs: alpine 3.23 -> 3.24 (3.23 has no clang22). - scripts/bootstrap.sh apk: tag alpine edge for the llvm/clang/lld packages so CI musl images get 22.1.8 instead of 3.24's 22.1.3. musl is 1.2.6-r2 in both, so edge's binaries run on 3.24. - scripts/bootstrap.{sh,ps1}: bump Version markers to 41 / 23 (main caught up to 39 / 22 and another branch is at 40). - scripts/build/deps/webkit.ts: pin to the LLVM-22-built preview release autobuild-preview-pr-296-1614a889 (oven-sh/WebKit#296, synced with WebKit main and building musl with 22.1.8 via edge).
dfc4fd3 to
abdd9ea
Compare
…64 bootstrap branch - scripts/bootstrap.sh apk: use append_file for /etc/apk/repositories (alpine has /bin/sh, not /usr/bin/sh). - scripts/bootstrap.sh apt: replace the incomplete focal-arm64 bullseye branch with a clear error. No CI lane runs bootstrap.sh on focal arm64 (that path goes through .buildkite/Dockerfile, which has the full libz3/no-lldb workaround); keeping a half-working copy here just ships broken code. - scripts/update-test-durations.mjs, test/expected-durations.json: alpine-323 -> alpine-324 step key to match ci.mjs.
There was a problem hiding this comment.
Additional findings (outside current diff — PR may have been updated during review):
-
🔴
scripts/bootstrap.sh:1211-1213— The newapk)branch runsexecute_sudo /usr/bin/sh -c '...', but Alpine has no usr-merge — busybox sh is at/bin/shand/usr/bin/shdoesn't exist on stock images, soexecute()gets "not found",error()fires, and the alpine-3.24 image bake this PR introduces aborts before installing LLVM. The path was copy-pasted from the Debian-13 sequoia workaround at L1177 (where usr-merge makes it valid); usesh="$(require sh)"thenexecute_sudo "$sh" -c '...'(or baresh -c) like every other shell invocation in this file (append_file,execute_as_user,install_docker, etc.).Extended reasoning...
What the bug is
This PR adds an
apk)branch toinstall_llvm()inscripts/bootstrap.shthat tags the Alpine edge repo before installing LLVM 22. Line 1212 reads:execute_sudo /usr/bin/sh -c 'echo "@edge https://dl-cdn.alpinelinux.org/alpine/edge/main" >> /etc/apk/repositories'The absolute path
/usr/bin/shwas copy-pasted from the Debian-13 sequoia workaround this PR touches at ~L1177 (execute_sudo /usr/bin/sh -c "sed ..."). On Debian/Ubuntu that path is valid because of usr-merge (/bin→/usr/binsymlink). Alpine Linux does not implement usr-merge:alpine-baselayoutkeeps/binand/usr/binas separate real directories, busybox installsshat/bin/shonly, and/usr/bin/shdoes not exist on a stock Alpine image (verified through 3.19/3.20/3.21; Alpine has been notably resistant to usr-merge and there is no indication 3.24 changed this).The specific code path that triggers it
execute_sudo→executeruns"$@"directly (bootstrap.sh:35-42). With$1 = /usr/bin/sh, the shell returns 127 ("/usr/bin/sh: not found").execute()then checks[ "$status" -ne 0 ]and callserror(), whichkill -s TERM "$pid"+exit 1— the entire bootstrap aborts.The
if ! grep -q '@edge' /etc/apk/repositoriesguard doesn't help: a fresh Alpine 3.24 image's/etc/apk/repositoriescontains only thev3.24/mainandv3.24/communitylines, no@edgetag, so the grep fails, theifbody runs, and line 1212 executes.Why existing code doesn't prevent it
Nothing earlier in bootstrap creates
/usr/bin/sh.install_common_software()installsbashon apk, but Alpine's bash package places it at/bin/bash. No usr-merge symlink is created anywhere in the script.Every other place bootstrap.sh needs a shell resolves it via the file's own convention —
sh="$(require sh)"thenexecute_sudo "$sh" -c ...: seeappend_file(L182),execute_as_user(L54),install_cmake(L988),install_rust(L1315),install_docker(L1497),install_tailscale(L1553),install_fuse_python(L1597). Line 1212 is the only place in the script that hardcodes/usr/bin/shin a non-Debian context. REVIEW.md: "Match the exact file's local conventions."Step-by-step proof
- This PR bumps
buildPlatformsalpine entries torelease: "3.24"(.buildkite/ci.mjs:144-146) and bumps bootstrap.sh's# Version:marker to 41, so the[build images]commit in this PR triggers a fresh alpine-3.24 image bake. - The bake runs
bootstrap.shon a stockalpine:3.24base.check_package_manager()setspm=apk. main()→install_build_essentials()→install_llvm()→case "$pm" in apk).grep -q '@edge' /etc/apk/repositoriesfinds nothing (fresh image), so theifbody runs.execute_sudo /usr/bin/sh -c '...'→execute /usr/bin/sh -c '...'(root, sosudo=1short-circuits to plain execute)."$@"at execute():37 tries to exec/usr/bin/sh. File doesn't exist → shell returns 127.status=127,[ "$status" -ne 0 ]is true,error "Command failed: /usr/bin/sh -c ..."fires.error()prints to stderr and exits 1. Bootstrap aborts. LLVM 22 is never installed. The image bake fails.
Impact
Concrete CI breakage on the musl image bake path this PR itself introduces: all three alpine-3.24
buildPlatformsentries (aarch64-musl, x64-musl, x64-musl-baseline) fail at bootstrap time. The PR's HEAD commit subject already contains[build images], so this fires on the very next CI run — the author would catch it there rather than post-merge, but it's still a bug introduced by this PR that blocks its stated goal.Fix
Match the file's convention:
if ! grep -q '@edge' /etc/apk/repositories 2>/dev/null; then sh="$(require sh)" execute_sudo "$sh" -c 'echo "@edge https://dl-cdn.alpinelinux.org/alpine/edge/main" >> /etc/apk/repositories' fi
Or, since
/bin/shis guaranteed by POSIX and this script's own shebang is#!/bin/sh, justexecute_sudo sh -c '...'(PATH lookup) also works. - This PR bumps
|
The |
Conflicts resolved: - .buildkite/ci.mjs: take main's (single debian-13 build host); re-apply alpine 3.23 -> 3.24 in testPlatforms and getVerifyBaselineHost. - .buildkite/Dockerfile: revert the focal-arm64 bullseye workaround (no ci.mjs platform uses features:["docker"] anymore), keep only the LLVM_VERSION 21->22 bump. - scripts/build/deps/webkit.ts: take main's comment, keep the preview pin (will update after WebKit PR sync). - scripts/build/workarounds.ts: keep rust-lld-for-crosslang-lto deleted (clang 22 == rustc 2026-07-20's LLVM 22.1.8, so it trips). - test/internal/source-lints/windows-cross-config.test.ts: take main's baseline:false, keep rustLlvmVersion 23.1.0.
- scripts/bootstrap.{sh,ps1}: Version 41->42 / 23->24 (main caught up to
41/22 via #34782).
- scripts/bootstrap.sh: alpine_sysroot_version 3.23 -> 3.24.
- scripts/build/deps/webkit.ts: pin to autobuild-preview-pr-296-6144f510
(oven-sh/WebKit#296 rebased onto main c9296e353e).
- src/jsc/bindings/highway_{json,sourcemap}.cpp: disable scalable
HWY_SVE/HWY_SVE2. clang >= 22 stops marking them HWY_BROKEN in
detect_targets.h, so foreach_target compiles for them, but BitsFromMask
only exists for the fixed-size SVE_256/SVE2_128 variants.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (9)
scripts/bootstrap.sh (3)
1280-1280: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive
llvm_vfrom the pinned LLVM version.
llvm_v="22"duplicatesllvm_version_exact(). A future pin bump could install LLVM 23 while still exporting and symlinkingclang-22,lld-22, and related tools. Usellvm_versionhere instead.🤖 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 `@scripts/bootstrap.sh` at line 1280, Update the llvm_v assignment in the bootstrap configuration to derive its value from the existing llvm_version symbol instead of hardcoding "22". Keep the pinned LLVM version and related tool naming consistent with llvm_version.
1494-1495: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse an explicit completion marker for sysroots.
These checks treat one header directory or
libc.soas a complete installation. An interrupted extraction can leave that path behind, causing later bootstraps to skip cleanup and reuse a partial sysroot. Write a stamp only after all extraction and symlink mutations complete, and make the matchingconfig.tsdetectors require it.Based on coding guidelines, completion markers must be written only after the final mutation.
Also applies to: 1597-1599
🤖 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 `@scripts/bootstrap.sh` around lines 1494 - 1495, Replace the header-directory and libc.so existence checks in the sysroot bootstrap logic with an explicit completion-stamp check, and update the corresponding config.ts detectors to require the same stamp. Write the stamp only after every extraction and symlink mutation has completed, including the flows around the affected checks, so interrupted installations cannot be treated as complete.Source: Coding guidelines
1502-1521: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftPin every sysroot source
The Ubuntu rootfs tag is mutable, the Ubuntu package indexes/debs are fetched without signature or checksum checks, and--allow-untrusteddisables Alpine package verification. A compromised mirror or registry can poison the sysroot and the binaries it produces. Pin the image digest, add verification for the downloaded packages/artifacts, and remove--allow-untrusted.🤖 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 `@scripts/bootstrap.sh` around lines 1502 - 1521, Pin the Ubuntu image reference used by the sysroot bootstrap to a verified digest instead of the mutable 20.04 tag, and validate downloaded package indexes and .deb artifacts against trusted signatures or checksums before extraction. Update the Alpine package installation flow to remove --allow-untrusted and require normal repository signature verification, covering all sysroot sources and downloaded artifacts.Source: Coding guidelines
scripts/bootstrap.ps1 (1)
259-266: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winCheck
clang-cl's version before skipping LLVM.if (Which clang-cl)treats any existingclang-clas sufficient, so an older LLVM or the Visual Studio toolchain can block installation of the pinnedllvm@22.1.8package. Compareclang-cl --versionagainst the pinned version, or let the Scoop install run when it doesn't match.🤖 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 `@scripts/bootstrap.ps1` around lines 259 - 266, Update the clang-cl availability check in the LLVM installation flow to validate that the existing executable reports the pinned $LLVM_VERSION, rather than returning for any result from Which clang-cl. Only skip Scoop installation when the detected clang-cl version matches; otherwise continue through the ARM64 or standard Install-Scoop-Package branch.scripts/build/shims.ts (2)
96-103: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winKeep new comments within the repository’s three-line limit.
Condense these rationale blocks while preserving the invariant or safety justification, especially the post-link compression and CRT-probe explanations.
Also applies to: 109-113, 180-183, 249-252
🤖 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 `@scripts/build/shims.ts` around lines 96 - 103, Condense the newly added rationale comments at the post-link compression block and the referenced CRT-probe sections (including the areas around lines 109-113, 180-183, and 249-252) to no more than three lines each. Preserve the essential invariant and safety justification for post-link debug compression and CRT probing while removing nonessential background and detail.Source: Coding guidelines
253-255: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winHandle compiler-probe failures before reading
stdout.spawnSync()can returnerror/status: nulland nostdout, so.trim()turns a missing compiler into a configure-timeTypeError. Check the result first and throw aBuildErrorwith the probe details.🤖 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 `@scripts/build/shims.ts` around lines 253 - 255, Update the compiler probe around spawnSync to inspect its result before accessing stdout.trim(). When the probe reports an error or status of null, throw a BuildError containing the available probe details; only trim and use stdout for successful results.scripts/build/config.ts (2)
563-588: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReturn only absolute, complete sysroot paths.
The environment override is validated and returned unchanged. For example,
LINUX_GLIBC_SYSROOT=relative/pathcan passexistsSync()relative to the configure working directory, but Ninja later invokes commands frombuildDir, soConfig.sysrootpoints somewhere else. Resolve environment overrides before validation and return; also require the same complete-install sentinel used by bootstrap.Based on coding guidelines, all paths in
Configare required to be absolute.🤖 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 `@scripts/build/config.ts` around lines 563 - 588, Update detectLinuxGlibcSysroot and detectLinuxMuslSysroot so environment overrides are resolved to absolute paths before validation and return. Validate overrides and built-in candidates using the complete-install sentinel shared with bootstrap, and ensure every returned sysroot path is absolute while preserving the existing architecture-specific candidate selection.Source: Coding guidelines
1180-1180: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not fall back to the target ABI when host detection fails.
detectLinuxAbi() ?? abimakescanRunOnHosttrue whenever host ABI detection returnsundefined, including an Android target on a normal Linux host. Consumers relying on this field may attempt to execute an incompatible binary. Cache the host ABI once and require a known equality; Android should not be considered runnable on ordinary Linux.+ const hostAbi = host.os === "linux" ? detectLinuxAbi() : undefined; + const canRunOnHost = + os === host.os && + arch === host.arch && + (!linux || (abi !== "android" && hostAbi !== undefined && abi === hostAbi));🤖 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 `@scripts/build/config.ts` at line 1180, Update the canRunOnHost calculation to cache detectLinuxAbi() once and require the detected host ABI to be defined and equal to the target abi when evaluating Linux compatibility. Remove the fallback to abi so Android targets are not considered runnable when host ABI detection fails, while preserving the existing OS and architecture checks.scripts/build/tools.ts (1)
667-675: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRepair the Rust toolchain before probing
rust-lld.--profile minimalcan narrow an existing rustup install on reinstall; use--force --no-self-updatehere and keep--component rust-srcso a partial toolchain gets repaired instead of leaving the laterrustcprobe to fall back to the host linker.Suggested change
- ["-q", "toolchain", "install", channel, "--no-self-update", "--profile", "minimal", "--component", "rust-src"], + ["-q", "toolchain", "install", channel, "--force", "--no-self-update", "--component", "rust-src"],🤖 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 `@scripts/build/tools.ts` around lines 667 - 675, Update the rustup toolchain installation invocation in the surrounding Rust setup flow to include --force while retaining --no-self-update and --component rust-src, so existing or partial toolchains are repaired before the rust-lld probe. Preserve the current timeout, encoding, and stdio behavior.
🤖 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 `@scripts/bootstrap.ps1`:
- Line 1: Save bootstrap.ps1 using UTF-8 encoding with a BOM, preserving its
existing content and non-ASCII characters so Windows PowerShell tooling decodes
it correctly.
---
Outside diff comments:
In `@scripts/bootstrap.ps1`:
- Around line 259-266: Update the clang-cl availability check in the LLVM
installation flow to validate that the existing executable reports the pinned
$LLVM_VERSION, rather than returning for any result from Which clang-cl. Only
skip Scoop installation when the detected clang-cl version matches; otherwise
continue through the ARM64 or standard Install-Scoop-Package branch.
In `@scripts/bootstrap.sh`:
- Line 1280: Update the llvm_v assignment in the bootstrap configuration to
derive its value from the existing llvm_version symbol instead of hardcoding
"22". Keep the pinned LLVM version and related tool naming consistent with
llvm_version.
- Around line 1494-1495: Replace the header-directory and libc.so existence
checks in the sysroot bootstrap logic with an explicit completion-stamp check,
and update the corresponding config.ts detectors to require the same stamp.
Write the stamp only after every extraction and symlink mutation has completed,
including the flows around the affected checks, so interrupted installations
cannot be treated as complete.
- Around line 1502-1521: Pin the Ubuntu image reference used by the sysroot
bootstrap to a verified digest instead of the mutable 20.04 tag, and validate
downloaded package indexes and .deb artifacts against trusted signatures or
checksums before extraction. Update the Alpine package installation flow to
remove --allow-untrusted and require normal repository signature verification,
covering all sysroot sources and downloaded artifacts.
In `@scripts/build/config.ts`:
- Around line 563-588: Update detectLinuxGlibcSysroot and detectLinuxMuslSysroot
so environment overrides are resolved to absolute paths before validation and
return. Validate overrides and built-in candidates using the complete-install
sentinel shared with bootstrap, and ensure every returned sysroot path is
absolute while preserving the existing architecture-specific candidate
selection.
- Line 1180: Update the canRunOnHost calculation to cache detectLinuxAbi() once
and require the detected host ABI to be defined and equal to the target abi when
evaluating Linux compatibility. Remove the fallback to abi so Android targets
are not considered runnable when host ABI detection fails, while preserving the
existing OS and architecture checks.
In `@scripts/build/shims.ts`:
- Around line 96-103: Condense the newly added rationale comments at the
post-link compression block and the referenced CRT-probe sections (including the
areas around lines 109-113, 180-183, and 249-252) to no more than three lines
each. Preserve the essential invariant and safety justification for post-link
debug compression and CRT probing while removing nonessential background and
detail.
- Around line 253-255: Update the compiler probe around spawnSync to inspect its
result before accessing stdout.trim(). When the probe reports an error or status
of null, throw a BuildError containing the available probe details; only trim
and use stdout for successful results.
In `@scripts/build/tools.ts`:
- Around line 667-675: Update the rustup toolchain installation invocation in
the surrounding Rust setup flow to include --force while retaining
--no-self-update and --component rust-src, so existing or partial toolchains are
repaired before the rust-lld probe. Preserve the current timeout, encoding, and
stdio behavior.
🪄 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: 680ca684-04a4-409c-a2a1-9d60385985cf
📒 Files selected for processing (15)
.buildkite/ci.mjs.github/workflows/CLAUDE.md.github/workflows/clippy.yml.github/workflows/format.yml.github/workflows/miri.ymlscripts/bootstrap.ps1scripts/bootstrap.shscripts/build/config.tsscripts/build/deps/webkit.tsscripts/build/rules.tsscripts/build/rust.tsscripts/build/shims.tsscripts/build/tools.tsscripts/build/workarounds.tsscripts/update-test-durations.mjs
There was a problem hiding this comment.
Beyond the inline findings: checked the other Highway foreach_target TUs (highway_strings.cpp, image_resize.cpp, xxhash3.cpp) — none call BitsFromMask, so the two-file SVE-disable patch covers every affected site. Also, the earlier .buildkite/Dockerfile lldb-symlink note is moot now: the origin/main merge reverted the arm64 branch, so llvm.sh ${LLVM_VERSION} all installs lldb on both arches again.
Extended reasoning...
New since the last pass: the origin/main merge (bringing in the single-arm64-debian-13-build-host change) plus a102903 (highway SVE disable, WebKit preview-pin refresh, version-marker bumps). The merge collapsed .buildkite/Dockerfile's net diff to just the two ARG lines, which both moots my prior dangling-lldb-symlink comment and is what makes the bootstrap.sh error-message reference stale (inline nit). For the highway fix I grepped every foreach_target.h consumer for BitsFromMask — only the two patched files use it. Not re-raising the preview WEBKIT_VERSION pin; the author already acknowledged it stays until oven-sh/WebKit#296 merges.
…llowlist, stale error text - test/internal/macos-cross-config.test.ts: the rust-lld postlink-compress test needs rustLlvmVersion 23.1.0 so wantRustLld (rustLlvmMajor > clangMajor) still fires with clang 22. Sibling of the windows-cross-config.test.ts fix. - scripts/verify-baseline-static/allowlist-aarch64.txt: clang 22 stops marking scalable SVE as HWY_BROKEN, so foreach_target now compiles for N_SVE/N_SVE2/N_SVE_256/N_NEON_BF16 on aarch64. Add the 17 new hwy::SupportedTargets-gated symbols (bun_image x 15, N_SVE_256 JsonIndexImpl, N_NEON_BF16 VisibleLatin1WidthExcludeANSI). - scripts/bootstrap.sh: drop the stale '.buildkite/Dockerfile' reference from the focal-arm64 error (that workaround was reverted in the origin/main merge).
56e2513 to
6fffa5c
Compare
…use prior bakes
Throwaway images from a '[build images]' run were tagged
-build-<buildNumber>, so every subsequent push on the same PR branch
requested -v<N> (which doesn't exist until [publish images]) and died
with 'no agent found'. Tag them -branch-<branchSlug> instead, and make
getImageName() request that tag on PR branches whose changedFiles
include the bootstrap script. This lets a PR iterate after a single
[build images] bake without re-baking on every push.
- scripts/utils.mjs: add getBranchImageSuffix().
- .buildkite/ci.mjs getImageName(): branch tag for [build images] and
for PR pushes that touch bootstrap.{sh,ps1}.
- scripts/machine.mjs: bake with the branch tag instead of buildNumber.
|
Closing as part of a cleanup of stale pull requests. This PR has had no new commits since 2026-07-21, it conflicts with main, and its last CI run failed. This is not a judgment on the fix itself. If the problem still reproduces on a current build, reopen this PR after a rebase or open a new one against main. |
1 participant
Bumps the pinned LLVM version from 21.1.8 to 22.1.8 across build scripts, bootstrap, CI images, GitHub Actions, nix, and docs.
WebKit side: oven-sh/WebKit#296 (needs to merge and produce artifacts first).
Version pins
scripts/build/tools.ts:LLVM_VERSION/LLVM_MAJOR21 -> 22 (LLVM_VERSION_RANGEbecomes>=22.1.0 <22.1.99)scripts/bootstrap.sh/scripts/bootstrap.ps1: 21.1.8 -> 22.1.8.buildkite/Dockerfile:LLVM_VERSION21 -> 22,REPORTED_LLVM_VERSION21.1.8 -> 22.1.8.github/workflows/{format,clippy,miri}.yml:LLVM_VERSION_MAJOR21 -> 22scripts/run-clang-format.sh: default 21 -> 22flake.nix/shell.nix:llvm_21/clang_21/lld_21->*_22scripts/build/config.ts: install-hint textlld-21/llvm-21-> 22CONTRIBUTING.md,docs/project/{contributing,building-windows}.mdx: all install instructions updatedasan-dyld-shim removed
The
asan-dyld-shimworkaround (macOS 26.4 dyld_Block_copydeadlock during ASAN init) hadFIXED_IN_LLVM = "22.1.4"in its self-obsoleting check. The backport llvm/llvm-project#188913 merged torelease/22.xon 2026-03-31, so 22.1.8 carries it. Removedscripts/build/shims/asan-dyld-shim.c, theshim_dylibrule, and theworkarounds.tsentry. Other shims (macho-postlink,cpu_model, musl CRT decompress) are unaffected.Not touched
-stack_sizeinld64.lld(workarounds.ts / shims.ts / macho-postlink.c): that workaround's threshold isFIXED_IN_LLVM = "23.0.0", still applies.scripts/build/tools.ts:660/rust.ts/flags.tscomments referencing "LLVM 22 bitcode" / "LLVM 22 ThinLTO": descriptive, still accurate.Merge order
SetupWebKit/ the WebKit prebuilt pin here to that autobuild tag.[build images]commit bakes LLVM 22 into the CI images.Opened as draft until step 1 completes.
no test proof · iteration 7 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/internal/macos-cross-config.test.ts test/internal/source-lints/windows-cross-config.test.ts