Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 7 additions & 9 deletions .github/workflows/build-reusable.yml
Original file line number Diff line number Diff line change
Expand Up @@ -254,17 +254,15 @@ jobs:
runs-on: ${{ matrix.runner }}
timeout-minutes: 90
steps:
- name: Install Scoop
run: |
Set-ExecutionPolicy -ExecutionPolicy RemoteSigned -Scope CurrentUser
Invoke-RestMethod -Uri https://get.scoop.sh | Invoke-Expression
Join-Path (Resolve-Path ~).Path "scoop\shims" >> $Env:GITHUB_PATH
- name: Install LLVM and Ninja (ARM64)
# No scoop: `irm get.scoop.sh | iex` installs nothing since
# ScoopInstaller/Install@a6210927 (the installer skips Install-Scoop
# when $MyInvocation.InvocationName is '.', and the runner's pwsh
# shell dot-sources every `run:` script). Nothing needed it anyway:
# cmake and ninja come from the Visual Studio install, see the PATH
# reorder in windows-release.ps1.
- 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"
Comment on lines +263 to 268

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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…

Expand Down
1 change: 1 addition & 0 deletions windows-release.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ $env:PATH = ($SplitPath | Where-Object { $_ -notlike "*strawberry*" }) -join ';'
Write-Host $env:PATH

(Get-Command link).Path
(Get-Command ninja).Path
clang-cl.exe --version

$env:CC = "clang-cl"
Expand Down
Loading