ci: drop scoop from the Windows arm64 job - #523
Conversation
|
Warning Review limit reached
On-demand reviews are free for the next 24 days. After that, they cost $0.25 per reviewed file. Or wait 2 minutes for your next included review. View limit detailsLimit details: You’ve used all 5 included reviews currently available. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — straightforward CI fix that swaps a fragile remote installer for a pinned, checksummed download.
What was reviewed:
- Download/verify/extract logic:
Get-FileHashresult is lower-cased before comparison,throwfails the step on mismatch,Expand-Archive -Forceand>> $Env:GITHUB_PATHare correct. - Removed
"C:\Program Files\7-Zip" >> $Env:GITHUB_PATHis safe — the LLVM extraction on line 280 already uses the absolute7z.exepath, and no other step relied on it. - No remaining
scoopreferences in any workflow;Set-ExecutionPolicyremoval is fine since nothing else needed it.
Extended reasoning...
Overview
This PR touches a single file, .github/workflows/build-reusable.yml, modifying only the Windows ARM64 job's dependency-install step. It removes the Install Scoop step (which piped get.scoop.sh into Invoke-Expression) and the scoop install ninja lines, replacing them with a direct download of ninja-winarm64.zip v1.13.2 from the official ninja-build/ninja GitHub release, a SHA-256 check, extraction to C:\ninja, and a GITHUB_PATH append. The LLVM 21 ARM64 install portion is untouched. An inline comment documents why Scoop was dropped (the upstream installer now no-ops when dot-sourced, which is how the runner's default pwsh shell invokes run: scripts).
Security risks
This is a net security improvement: the old flow executed a remote script fetched at runtime (irm get.scoop.sh | iex) with no integrity check on the installer itself; the new flow downloads a fixed asset over HTTPS from github.com and verifies it against a pinned SHA-256 before use. A wrong or tampered archive fails the step via throw. There is no injection surface (no user/PR-controlled input flows into the URL, hash, or paths — all are literals), no credential handling, and no change to permissions or artifact publishing. The removed Set-ExecutionPolicy RemoteSigned and 7-Zip GITHUB_PATH append were only needed for Scoop and are no longer referenced anywhere in .github/workflows/.
Level of scrutiny
Low. This is a small, mechanical CI infrastructure fix responding to an upstream breakage, confined to one job in a reusable workflow. No product code (JSC/WTF/bmalloc) is touched. Correctness is directly verifiable by the PR's own Preview Build run, and a wrong hash or URL would fail loudly rather than silently. The .github/CODEOWNERS entry for /.github is inherited from upstream Apple WebKit and the file itself states it does not denote ownership; it is not a meaningful gate for Bun-fork CI changes.
Other factors
I confirmed the only remaining 7-Zip usage (LLVM extraction) invokes 7z.exe by absolute path, so dropping the PATH append is safe. Get-FileHash returns uppercase hex, and the code normalizes with .ToLower() before comparing to the lowercase literal. Expand-Archive -Force avoids failures if C:\ninja pre-exists on the runner image. The bug-hunt exited on dry_streak with no findings. There are no prior reviews or outstanding objections in the timeline.
Preview Builds
|
The get.scoop.sh installer no longer runs Install-Scoop when the script is dot-sourced (ScoopInstaller/Install@a6210927, 2026-08-27). The runner's pwsh shell dot-sources every `run:` script, and Invoke-Expression inherits that invocation, so the "Install Scoop" step exited 0 without installing anything. The next step then failed on the missing `scoop` command, and no preview release was published. The job only used scoop to install ninja, and that ninja was never the one the build ran: windows-release.ps1 moves every Visual Studio PATH entry to the front, so cmake picks up VS's bundled ninja (the build log shows Common7\IDE\CommonExtensions\Microsoft\CMake\Ninja\ninja.exe). Remove the scoop steps and the 7-Zip PATH entry that only served scoop. The LLVM download is unchanged. windows-release.ps1 now prints the ninja it resolves next to the link and clang-cl it already prints, so a missing ninja fails there with the path in the log.
7aed464 to
9877148
Compare
robobun
changed the title
| - name: Install LLVM (ARM64) | ||
| if: matrix.platform == 'ARM64' | ||
| run: | | ||
| "C:\Program Files\7-Zip" >> $Env:GITHUB_PATH | ||
| scoop config use_external_7zip true | ||
| scoop install ninja | ||
| # Install LLVM ARM64 from official LLVM releases | ||
| # Use LLVM 21 for ARM64 - has better Windows ARM64 support and fixes SEH unwind bugs | ||
| $llvmVersion = "21.1.8" |
There was a problem hiding this comment.
🟡 PR title/description say ninja is downloaded from the GitHub release (ninja-winarm64.zip 1.13.2, sha256-checked, extracted to C:\ninja, added to GITHUB_PATH), but the diff only deletes the scoop steps — no ninja install is added. The new comment claims ninja comes from the Visual Studio PATH reorder in windows-release.ps1, yet the PR body itself says "this change does not start to" rely on the runner-preinstalled ninja, so the intended install step appears to have been left out of the commit.
Extended reasoning...
After merge the Windows arm64 job depends on whatever ninja happens to be on PATH (Launch-VsDevShell.ps1 does not add VS's bundled CMake/Ninja dir, so this is really the runner-image ninja the author explicitly said not to rely on). If that preinstalled ninja is removed or changes in a future windows-11-arm image, (Get-Command ninja).Path at windows-release.ps1:39 throws under $ErrorActionPreference = "Stop" and the build fails again — the pinned, hash-verified download the PR promises would prevent that but is absent; a correct fix adds the described Invoke-WebRequest + Get-FileHash check + Expand-Archive to C:\ninja + >> $Env:GITHUB_PATH to this step.
Verification: normal — The PR title ("install ninja for the Windows arm64 job from its GitHub release") and the Fix section of the description ("Download ninja-winarm64.zip 1.13.2 from the ninja GitHub release, check its sha256, extract it to C:\ninja, and add that to GITHUB_PATH") describe a ninja install step that is not present in the diff. The post-change workflow at… | nit — The mismatch is…
2 participants
Problem
Build / Windows (windows-11-arm, Debug, arm64, bun-webkit-windows-arm64-debug, ARM64):Install LLVM and Ninja (ARM64)exits 1 at once, and thereleasejob needs that artifact.Install Scooptakes 2 s (24 s before) and installs nothing:ScoopInstaller/Install@a6210927(07:15 UTC) skips the install flow when$MyInvocation.InvocationNameis., which is whatirm get.scoop.sh | iexsees inside a dot-sourced step script. The next step fails on the missingscoopcommand.Fix
Install Scoopstep and the three scoop lines (scoop config,scoop install ninja, the 7-ZipGITHUB_PATHentry that only served scoop). Rename the step toInstall LLVM (ARM64). The LLVM download is unchanged.windows-release.ps1:30-34moves every Visual Studio PATH entry to the front, socmake -G Ninjaresolves VS's bundledCMake\Ninja\ninja.exe. Every passing run's log shows it as the build command.windows-release.ps1now prints(Get-Command ninja).Pathnext to thelinkandclang-clit already prints, so a missing ninja fails there with the reason in the log.Background
releasejob needs all 42 artifacts.bun-webkit-windows-arm64-debugis the only one built on a native Windows runner.pwshshell runspwsh -command ". '<script>'", so$MyInvocation.InvocationNameis.in a step.Invoke-Expressionruns in the caller's scope with the caller's$MyInvocation.Notes
Failing runs: 33060265924 (and its rerun), 33060247998, 33060250463, 33060284775, 33060269980.
Timing from the Actions API for the Windows arm64 job:
The cause was confirmed by reproducing the invocation on a Windows 11 arm64 machine:
With the current installer,
irm get.scoop.sh | iexin such a step prints nothing (not evenInitializing...).iex "& {$(irm get.scoop.sh)}"or running the downloaded file with&does run the install flow. Neither is used here because nothing in the job needs scoop: the installer changed behavior three times in 2026 (#124/#125 in January, #134 on 2026-08-20, #135/#136 on 2026-08-27).Which ninja the build uses, from the log of run 33065119524 (first revision of this PR, image 20260823.149.1): the PATH printed after the reorder in
windows-release.ps1has...\CommonExtensions\Microsoft\CMake\Ninjaat position 15,C:\LLVM\binandC:\ninjaafter it, the image'sC:\Tools\Ninja(Ninja 1.13.2) further back. Line 13781:Run Build Command(s): C:/PROGRA~1/MICROS~2/2022/ENTERP~1/Common7/IDE/COMMON~1/MICROS~1/CMake/Ninja/ninja.exe -v jsc testFFI. cmake itself also comes from VS (...\CMake\CMake\bin\cmake.exe). Run 33047663455 (last pass with scoop) shows the same:scoop\shimswas behind VS's ninja after the reorder.The first revision of this PR downloaded
ninja-winarm64.zip1.13.2 with a sha256 check. That was dropped after review because it was dead code: the build does not use it, and the job already floats on the image's cmake, VS, Windows SDK and 7-Zip.#524 and the workflow commits in #485 and #517 address the same break.
Not changed here: whether the
releasejob should require the Windows arm64 debug artifact at all. Bun's CI consumes only the release Windows arm64 tarball.