Conversation
WalkthroughRemove Windows ARM64 cross-compilation wrappers and toolchain special-casing; treat Windows ARM64 as native aarch64 across CI, CMake, and scripts, add ARM64-aware bootstrap/install flows, simplify codegen/packaging logic, and add small zlib patch support for Windows ARM64. Changes
🚥 Pre-merge checks | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
cmake/scripts/DownloadZig.cmake (1)
23-29:⚠️ Potential issue | 🔴 CriticalThe Windows ARM64 Zig bootstrap binary does not exist yet.
The change correctly maps ARM64 processors to the
aarch64architecture, constructing the download URLbootstrap-aarch64-windows-gnu.zipfor Windows ARM64 hosts. However, this binary does not currently exist in the oven-sh/zig releases for commitc1423ff3fc7064635773a4a4616c5bf986eb00fe(HTTP 404).The DownloadZig.cmake script will fail at runtime on Windows ARM64 with a FATAL_ERROR at line 58-59 if the download returns a non-200 status code. Before merging this change, ensure that:
- The
bootstrap-aarch64-windows-gnu.zipbinary is built and available in the oven-sh/zig autobuild releases for the specified ZIG_COMMIT- The binary has been uploaded and is accessible from the release before this PR is merged
scripts/build.mjs (1)
88-91: 🧹 Nitpick | 🔵 TrivialConsider if
isWindowsARM64should influence build configuration.The
isWindowsARM64variable is currently only used for logging. If any build configuration should differ on Windows ARM64 (beyond what CMake handles), this would be the place to add it. If purely informational logging is the intent, this is fine.cmake/Options.cmake (1)
61-71:⚠️ Potential issue | 🟡 MinorRemove the non-standard
CMAKE_MSVC_CMP0197variable.The CMP0197 policy was introduced in CMake 4.1 (not 3.30). The variable
CMAKE_MSVC_CMP0197is not a standard CMake variable. The correct approach is to set the policy viacmake_policy(SET CMP0197 NEW)at the beginning of the relevant scope, or viaCMAKE_POLICY_DEFAULT_CMP0197as already done. However, setting CMP0197 to NEW instructs CMake to not inject the-machine:flags automatically, so manually appending/machine:ARM64to the linker flags (as done here) may be necessary to compensate. Verify that this is the intended behavior and either remove theset(CMAKE_MSVC_CMP0197 NEW)line or clarify the policy's purpose if this manual flag-setting approach is intentional.
🤖 Fix all issues with AI agents
In @.buildkite/ci.mjs:
- Line 117: The Windows ARM64 matrix entry { os: "windows", arch: "aarch64",
release: "11" } is ambiguous—change it to explicitly indicate Windows 11 (not a
year like Server 2019) by adding a concise inline comment or clarifying text
(e.g., annotate release with "Windows 11" or add a trailing comment) next to the
{ os: "windows", arch: "aarch64", release: "11" } object so readers know this
refers to Windows 11 rather than a calendar year/server release.
In `@cmake/Globals.cmake`:
- Line 459: Update the fatal error message emitted by message(FATAL_ERROR
"SKIP_CODEGEN: Cannot skip ${CMD_TARGET} - missing outputs.") to include a brief
hint on how to resolve the issue (for example, instructing the user to ensure
the target's outputs are defined, run the codegen task, or unset SKIP_CODEGEN),
so change the string to append actionable guidance such as "Ensure the target
defines outputs or run the codegen step; unset SKIP_CODEGEN to proceed." Keep
the reference to ${CMD_TARGET} in the message so the user knows which target is
affected.
| { os: "windows", arch: "x64", baseline: true, release: "2019" }, | ||
| // TODO: Re-enable when Windows ARM64 VS component installation is resolved on Buildkite runners | ||
| // { os: "windows", arch: "aarch64", release: "2019" }, | ||
| { os: "windows", arch: "aarch64", release: "11" }, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Clarify Windows ARM64 release version.
The release changed from "2019" to "11". This appears to reference Windows 11 (the first Windows version with native ARM64 support at consumer level) rather than a server release year like other Windows entries ("2019" for Server 2019). Consider adding a comment to clarify this is the Windows version number, not a year.
🤖 Prompt for AI Agents
In @.buildkite/ci.mjs at line 117, The Windows ARM64 matrix entry { os:
"windows", arch: "aarch64", release: "11" } is ambiguous—change it to explicitly
indicate Windows 11 (not a year like Server 2019) by adding a concise inline
comment or clarifying text (e.g., annotate release with "Windows 11" or add a
trailing comment) next to the { os: "windows", arch: "aarch64", release: "11" }
object so readers know this refers to Windows 11 rather than a calendar
year/server release.
| return() | ||
| else() | ||
| message(FATAL_ERROR "SKIP_CODEGEN: Cannot skip ${CMD_TARGET} - missing outputs. Run codegen on x64 first.") | ||
| message(FATAL_ERROR "SKIP_CODEGEN: Cannot skip ${CMD_TARGET} - missing outputs.") |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding guidance to the error message.
The simplified error message tells users that outputs are missing but doesn't explain how to resolve it. Consider adding a hint about what action to take.
💡 Suggested improvement
- message(FATAL_ERROR "SKIP_CODEGEN: Cannot skip ${CMD_TARGET} - missing outputs.")
+ message(FATAL_ERROR "SKIP_CODEGEN: Cannot skip ${CMD_TARGET} - missing outputs. Run a full build with codegen first, then copy the generated outputs.")📝 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.
| message(FATAL_ERROR "SKIP_CODEGEN: Cannot skip ${CMD_TARGET} - missing outputs.") | |
| message(FATAL_ERROR "SKIP_CODEGEN: Cannot skip ${CMD_TARGET} - missing outputs. Run a full build with codegen first, then copy the generated outputs.") |
🤖 Prompt for AI Agents
In `@cmake/Globals.cmake` at line 459, Update the fatal error message emitted by
message(FATAL_ERROR "SKIP_CODEGEN: Cannot skip ${CMD_TARGET} - missing
outputs.") to include a brief hint on how to resolve the issue (for example,
instructing the user to ensure the target's outputs are defined, run the codegen
task, or unset SKIP_CODEGEN), so change the string to append actionable guidance
such as "Ensure the target defines outputs or run the codegen step; unset
SKIP_CODEGEN to proceed." Keep the reference to ${CMD_TARGET} in the message so
the user knows which target is affected.
VS dev shell with -HostArch amd64 sets PROCESSOR_ARCHITECTURE=AMD64, causing CMake to misdetect the system as x64 and use -march=haswell on ARM64. Restore PROCESSOR_ARCHITECTURE=ARM64 after VS dev shell.
cmake.exe is an x64 binary, so the OS sets PROCESSOR_ARCHITECTURE=AMD64 when it runs on ARM64 Windows. This causes cmake to misdetect as x64 and use -march=haswell. Explicitly set CMAKE_SYSTEM_PROCESSOR=ARM64.
Code ReviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
2 similar comments
Code ReviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
Code ReviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
Code ReviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
On ARM64 Windows, Chocolatey installs x64 cmake and LLVM. x64 cmake reads PROCESSOR_ARCHITECTURE=AMD64 (overridden by OS for x64 processes), causing CMAKE_SYSTEM_PROCESSOR=AMD64 and -march=haswell to be passed to ARM64 clang-cl. - Add ARM64 detection to bootstrap.ps1 - Install cmake ARM64 MSI directly from GitHub on ARM64 - Install LLVM ARM64 (woa64) installer directly from GitHub on ARM64 - Skip nasm (x86 assembler) and mingw on ARM64 - Warn about bun ARM64 not being available via Chocolatey
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@scripts/bootstrap.ps1`:
- Around line 341-351: The script currently starts the msiexec process with
$process.Start() but doesn't check its boolean return value before calling
$process.WaitForExit(), which can fail if the start failed; update the block
around New-Object System.Diagnostics.Process ($process) to capture the result of
$process.Start(), and if it returns $false immediately throw or log a failure
(including context like the $msi path), skipping WaitForExit(); only call
$process.WaitForExit() and check $process.ExitCode when Start() returned $true
to ensure robust error handling.
- Around line 434-444: The code calls $process.Start() without checking its
boolean return value before calling $process.WaitForExit(), so update the
install flow (same pattern used in Install-CMake) to capture and verify the
result of $process.Start() for the LLVM installer: after invoking
$process.Start() check if it returned $true and if not throw a descriptive error
(e.g., "Failed to start LLVM installer") instead of directly calling
$process.WaitForExit(); keep the existing ExitCode check on $process.ExitCode
for non-zero exit codes.
| $startInfo = New-Object System.Diagnostics.ProcessStartInfo | ||
| $startInfo.FileName = "msiexec" | ||
| $startInfo.Arguments = "/i `"$msi`" /quiet /norestart ADD_CMAKE_TO_PATH=System" | ||
| $startInfo.CreateNoWindow = $true | ||
| $process = New-Object System.Diagnostics.Process | ||
| $process.StartInfo = $startInfo | ||
| $process.Start() | ||
| $process.WaitForExit() | ||
| if ($process.ExitCode -ne 0) { | ||
| throw "Failed to install CMake: code $($process.ExitCode)" | ||
| } |
There was a problem hiding this comment.
Check Process.Start() return value before waiting.
Process.Start() returns a boolean indicating success. If it returns $false, calling WaitForExit() may throw or behave unexpectedly.
🛡️ Proposed fix
$process.StartInfo = $startInfo
- $process.Start()
+ if (-not $process.Start()) {
+ throw "Failed to start CMake installer"
+ }
$process.WaitForExit()📝 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.
| $startInfo = New-Object System.Diagnostics.ProcessStartInfo | |
| $startInfo.FileName = "msiexec" | |
| $startInfo.Arguments = "/i `"$msi`" /quiet /norestart ADD_CMAKE_TO_PATH=System" | |
| $startInfo.CreateNoWindow = $true | |
| $process = New-Object System.Diagnostics.Process | |
| $process.StartInfo = $startInfo | |
| $process.Start() | |
| $process.WaitForExit() | |
| if ($process.ExitCode -ne 0) { | |
| throw "Failed to install CMake: code $($process.ExitCode)" | |
| } | |
| $startInfo = New-Object System.Diagnostics.ProcessStartInfo | |
| $startInfo.FileName = "msiexec" | |
| $startInfo.Arguments = "/i `"$msi`" /quiet /norestart ADD_CMAKE_TO_PATH=System" | |
| $startInfo.CreateNoWindow = $true | |
| $process = New-Object System.Diagnostics.Process | |
| $process.StartInfo = $startInfo | |
| if (-not $process.Start()) { | |
| throw "Failed to start CMake installer" | |
| } | |
| $process.WaitForExit() | |
| if ($process.ExitCode -ne 0) { | |
| throw "Failed to install CMake: code $($process.ExitCode)" | |
| } |
🤖 Prompt for AI Agents
In `@scripts/bootstrap.ps1` around lines 341 - 351, The script currently starts
the msiexec process with $process.Start() but doesn't check its boolean return
value before calling $process.WaitForExit(), which can fail if the start failed;
update the block around New-Object System.Diagnostics.Process ($process) to
capture the result of $process.Start(), and if it returns $false immediately
throw or log a failure (including context like the $msi path), skipping
WaitForExit(); only call $process.WaitForExit() and check $process.ExitCode when
Start() returned $true to ensure robust error handling.
| $startInfo = New-Object System.Diagnostics.ProcessStartInfo | ||
| $startInfo.FileName = $installer | ||
| $startInfo.Arguments = "/S" | ||
| $startInfo.CreateNoWindow = $true | ||
| $process = New-Object System.Diagnostics.Process | ||
| $process.StartInfo = $startInfo | ||
| $process.Start() | ||
| $process.WaitForExit() | ||
| if ($process.ExitCode -ne 0) { | ||
| throw "Failed to install LLVM: code $($process.ExitCode)" | ||
| } |
There was a problem hiding this comment.
Check Process.Start() return value before waiting.
Same issue as in Install-CMake — the return value of Process.Start() should be checked.
🛡️ Proposed fix
$process.StartInfo = $startInfo
- $process.Start()
+ if (-not $process.Start()) {
+ throw "Failed to start LLVM installer"
+ }
$process.WaitForExit()🤖 Prompt for AI Agents
In `@scripts/bootstrap.ps1` around lines 434 - 444, The code calls
$process.Start() without checking its boolean return value before calling
$process.WaitForExit(), so update the install flow (same pattern used in
Install-CMake) to capture and verify the result of $process.Start() for the LLVM
installer: after invoking $process.Start() check if it returned $true and if not
throw a descriptive error (e.g., "Failed to start LLVM installer") instead of
directly calling $process.WaitForExit(); keep the existing ExitCode check on
$process.ExitCode for non-zero exit codes.
Chocolatey nodejs installs x64, causing esbuild postinstall scripts to misdetect the platform and look for @esbuild/win32-x64 instead of win32-arm64.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@scripts/bootstrap.ps1`:
- Around line 245-272: The Install-NodeJs function currently returns for ARM64
if any Node.js binary matches the version, but it doesn't verify the binary's
architecture; update the early-return logic to also validate that the existing
node is ARM64 before returning. Specifically, where it currently uses Which node
and (& node --version) inside the $script:IsARM64 branch, replace/augment that
check with an architecture probe (e.g., invoke node -p process.arch or
equivalent) and only return if node exists, its version matches $version, and
its architecture is "arm64" so the ARM64 installer isn't skipped when an x64
binary is present.
- Around line 449-475: The ARM64 branch of Install-Llvm returns early if Which
clang-cl exists without validating its version; change that behavior to mirror
the x64 branch by detecting the existing clang-cl (Which clang-cl/Get-Command),
invoking it to obtain its version string (e.g., run clang-cl --version or
equivalent) and comparing/parsing against $version, and only return early when
the installed LLVM matches $version; if the version is incompatible, proceed
with the ARM64 installer flow (download/run the LLVM-$version-woa64.exe, check
ExitCode, remove installer, Refresh-Path) so the toolchain is guaranteed to be
the requested version.
| function Install-NodeJs { | ||
| Install-Package nodejs -Command node -Version "24.3.0" | ||
| $version = "24.3.0" | ||
| if ($script:IsARM64) { | ||
| if ((Which node) -and (& node --version) -like "*$version*") { | ||
| return | ||
| } | ||
| # Chocolatey nodejs package only installs x64; install ARM64 natively. | ||
| # x64 Node.js causes postinstall scripts to misdetect architecture. | ||
| Write-Output "Installing Node.js $version (ARM64)..." | ||
| $url = "https://nodejs.org/dist/v$version/node-v$version-arm64.msi" | ||
| $msi = Download-File $url -Name "node-arm64.msi" | ||
| $startInfo = New-Object System.Diagnostics.ProcessStartInfo | ||
| $startInfo.FileName = "msiexec" | ||
| $startInfo.Arguments = "/i `"$msi`" /quiet /norestart" | ||
| $startInfo.CreateNoWindow = $true | ||
| $process = New-Object System.Diagnostics.Process | ||
| $process.StartInfo = $startInfo | ||
| $process.Start() | ||
| $process.WaitForExit() | ||
| if ($process.ExitCode -ne 0) { | ||
| throw "Failed to install Node.js: code $($process.ExitCode)" | ||
| } | ||
| Remove-Item $msi -ErrorAction SilentlyContinue | ||
| Refresh-Path | ||
| } else { | ||
| Install-Package nodejs -Command node -Version $version | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the bootstrap.ps1 file to understand the full context
head -n 100 scripts/bootstrap.ps1Repository: oven-sh/bun
Length of output: 2724
🏁 Script executed:
# Search for where $script:IsARM64 is defined
rg "IsARM64" scripts/bootstrap.ps1 -B 3 -A 3Repository: oven-sh/bun
Length of output: 1133
🏁 Script executed:
# Look for the complete Install-NodeJs function and surrounding code
sed -n '240,280p' scripts/bootstrap.ps1Repository: oven-sh/bun
Length of output: 1493
🏁 Script executed:
# Check if there are any other platform/architecture checks in the bootstrap script
rg "arm64|ARM64|x64|process\.arch" scripts/bootstrap.ps1Repository: oven-sh/bun
Length of output: 1521
Avoid skipping ARM64 Node.js install when an x64 build with matching version exists.
The early return only checks version; if x64 Node.js v24.3.0 is already installed (e.g., via Chocolatey), this branch returns without installing the required ARM64 binary. The x64 binary will then trigger the postinstall script misdetection you're trying to prevent. Validate architecture before returning.
🐛 Proposed fix
if ($script:IsARM64) {
- if ((Which node) -and (& node --version) -like "*$version*") {
- return
- }
+ if (Which node) {
+ $nodeVersion = & node --version 2>$null
+ $nodeArch = & node -p "process.arch" 2>$null
+ if ($nodeVersion -like "*$version*" -and $nodeArch -eq "arm64") {
+ return
+ }
+ }
# Chocolatey nodejs package only installs x64; install ARM64 natively.🤖 Prompt for AI Agents
In `@scripts/bootstrap.ps1` around lines 245 - 272, The Install-NodeJs function
currently returns for ARM64 if any Node.js binary matches the version, but it
doesn't verify the binary's architecture; update the early-return logic to also
validate that the existing node is ARM64 before returning. Specifically, where
it currently uses Which node and (& node --version) inside the $script:IsARM64
branch, replace/augment that check with an architecture probe (e.g., invoke node
-p process.arch or equivalent) and only return if node exists, its version
matches $version, and its architecture is "arm64" so the ARM64 installer isn't
skipped when an x64 binary is present.
| function Install-Llvm { | ||
| Install-Package llvm ` | ||
| -Command clang-cl ` | ||
| -Version "21.1.8" | ||
| $version = "21.1.8" | ||
| if ($script:IsARM64) { | ||
| if (Which clang-cl) { | ||
| return | ||
| } | ||
| # Chocolatey LLVM package only downloads x64; install ARM64 natively. | ||
| Write-Output "Installing LLVM $version (ARM64)..." | ||
| $url = "https://github.com/llvm/llvm-project/releases/download/llvmorg-$version/LLVM-$version-woa64.exe" | ||
| $installer = Download-File $url -Name "llvm-arm64.exe" | ||
| $startInfo = New-Object System.Diagnostics.ProcessStartInfo | ||
| $startInfo.FileName = $installer | ||
| $startInfo.Arguments = "/S" | ||
| $startInfo.CreateNoWindow = $true | ||
| $process = New-Object System.Diagnostics.Process | ||
| $process.StartInfo = $startInfo | ||
| $process.Start() | ||
| $process.WaitForExit() | ||
| if ($process.ExitCode -ne 0) { | ||
| throw "Failed to install LLVM: code $($process.ExitCode)" | ||
| } | ||
| Remove-Item $installer -ErrorAction SilentlyContinue | ||
| Refresh-Path | ||
| } else { | ||
| Install-Package llvm -Command clang-cl -Version $version | ||
| } | ||
| Add-To-Path "$env:ProgramFiles\LLVM\bin" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
head -n 475 scripts/bootstrap.ps1 | tail -n 50Repository: oven-sh/bun
Length of output: 1731
🏁 Script executed:
sed -n '449,480p' scripts/bootstrap.ps1Repository: oven-sh/bun
Length of output: 1166
🏁 Script executed:
rg -A 15 "function Install-Package" scripts/bootstrap.ps1Repository: oven-sh/bun
Length of output: 834
🏁 Script executed:
rg -A 10 "function Which" scripts/bootstrap.ps1Repository: oven-sh/bun
Length of output: 299
🏁 Script executed:
rg -A 30 "function Install-Package {" scripts/bootstrap.ps1 | head -n 40Repository: oven-sh/bun
Length of output: 170
🏁 Script executed:
grep -n "choco install" scripts/bootstrap.ps1Repository: oven-sh/bun
Length of output: 114
🏁 Script executed:
sed -n '100,250p' scripts/bootstrap.ps1 | grep -A 40 "function Install-Package"Repository: oven-sh/bun
Length of output: 1710
🏁 Script executed:
sed -n '100,250p' scripts/bootstrap.ps1Repository: oven-sh/bun
Length of output: 4025
ARM64 installation path should validate LLVM version before returning early.
The ARM64 branch currently returns immediately if clang-cl exists, without checking version. The x64 branch calls Install-Package llvm -Command clang-cl -Version $version, which validates the version before early return. This inconsistency can leave ARM64 builds with an incompatible toolchain—for example, if x64 LLVM is already installed, the ARM64 path would skip installation and use the x64 clang-cl binary. The Install-NodeJs function already establishes the pattern of validating version in the ARM64 path, and Install-Llvm should follow the same approach.
Suggested fix
if ($script:IsARM64) {
- if (Which clang-cl) {
- return
- }
+ if (Which clang-cl) {
+ $clangInfo = & clang-cl --version 2>$null
+ if ($clangInfo -like "*$version*" -and $clangInfo -match '(?i)arm64|aarch64') {
+ return
+ }
+ }
# Chocolatey LLVM package only downloads x64; install ARM64 natively.🤖 Prompt for AI Agents
In `@scripts/bootstrap.ps1` around lines 449 - 475, The ARM64 branch of
Install-Llvm returns early if Which clang-cl exists without validating its
version; change that behavior to mirror the x64 branch by detecting the existing
clang-cl (Which clang-cl/Get-Command), invoking it to obtain its version string
(e.g., run clang-cl --version or equivalent) and comparing/parsing against
$version, and only return early when the installed LLVM matches $version; if the
version is incompatible, proceed with the ARM64 installer flow (download/run the
LLVM-$version-woa64.exe, check ExitCode, remove installer, Refresh-Path) so the
toolchain is guaranteed to be the requested version.
Code ReviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
Code ReviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@cmake/tools/SetupZig.cmake`:
- Around line 58-65: The logic currently sets DEFAULT_ZIG_COMPILER_SAFE and the
optionx ZIG_COMPILER_SAFE but does not warn if a user explicitly forces
ZIG_COMPILER_SAFE=ON on Windows ARM64; add a runtime check after optionx that
if(WIN32 AND DEFAULT_ZIG_ARCH STREQUAL "aarch64" AND ZIG_COMPILER_SAFE) invoke
message(WARNING "...") describing that enabling ReleaseSafe on Windows ARM64
reintroduces the LLVM SEH epilogue bug and recommend leaving it off; reference
the existing variables DEFAULT_ZIG_COMPILER_SAFE, DEFAULT_ZIG_ARCH and the
option ZIG_COMPILER_SAFE so the check is colocated with the current
DEFAULT_ZIG_COMPILER_SAFE/optionx block.
| # The ReleaseSafe Zig compiler for Windows ARM64 has an LLVM SEH epilogue bug | ||
| # (incorrect size for compiler_rt.rem_pio2_large epilogue). Use the default build instead. | ||
| if(CI AND WIN32 AND DEFAULT_ZIG_ARCH STREQUAL "aarch64") | ||
| set(DEFAULT_ZIG_COMPILER_SAFE OFF) | ||
| else() | ||
| set(DEFAULT_ZIG_COMPILER_SAFE ${CI}) | ||
| endif() | ||
| optionx(ZIG_COMPILER_SAFE BOOL "Download a ReleaseSafe build of the Zig compiler." DEFAULT ${DEFAULT_ZIG_COMPILER_SAFE}) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add a warning when ReleaseSafe is explicitly re-enabled on Windows ARM64.
The default is safe, but an explicit -DZIG_COMPILER_SAFE=ON on Windows ARM64 will reintroduce the known LLVM SEH epilogue bug. A warning here helps prevent accidental overrides.
Suggested guard
optionx(ZIG_COMPILER_SAFE BOOL "Download a ReleaseSafe build of the Zig compiler." DEFAULT ${DEFAULT_ZIG_COMPILER_SAFE})
+if(WIN32 AND DEFAULT_ZIG_ARCH STREQUAL "aarch64" AND ZIG_COMPILER_SAFE)
+ message(WARNING "ZIG_COMPILER_SAFE is known to be broken on Windows ARM64 (LLVM SEH epilogue bug). Consider -DZIG_COMPILER_SAFE=OFF.")
+endif()🤖 Prompt for AI Agents
In `@cmake/tools/SetupZig.cmake` around lines 58 - 65, The logic currently sets
DEFAULT_ZIG_COMPILER_SAFE and the optionx ZIG_COMPILER_SAFE but does not warn if
a user explicitly forces ZIG_COMPILER_SAFE=ON on Windows ARM64; add a runtime
check after optionx that if(WIN32 AND DEFAULT_ZIG_ARCH STREQUAL "aarch64" AND
ZIG_COMPILER_SAFE) invoke message(WARNING "...") describing that enabling
ReleaseSafe on Windows ARM64 reintroduces the LLVM SEH epilogue bug and
recommend leaving it off; reference the existing variables
DEFAULT_ZIG_COMPILER_SAFE, DEFAULT_ZIG_ARCH and the option ZIG_COMPILER_SAFE so
the check is colocated with the current DEFAULT_ZIG_COMPILER_SAFE/optionx block.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmake/Options.cmake (1)
203-211:⚠️ Potential issue | 🔴 CriticalCreate missing
cmake/toolchains/windows-aarch64.cmaketoolchain file to support Windows ARM64 builds.The TinyCC disable logic in lines 203-211 correctly checks for
ARCH STREQUAL "aarch64"on Windows. However, there is nowindows-aarch64.cmaketoolchain file to enable Windows ARM64 builds to setCMAKE_SYSTEM_PROCESSORcorrectly. Without this toolchain file, Windows ARM64 builds will not haveARCHset to"aarch64"and TinyCC will not be properly disabled.Additionally, no Windows ARM64 CI workflows are currently configured, so this build configuration is not tested. To support Windows ARM64, create the missing toolchain file (following the pattern of existing toolchain files like
windows-x64.cmakeanddarwin-aarch64.cmake) withset(CMAKE_SYSTEM_PROCESSOR aarch64).
2 participants
What does this PR do?
Sets up ci for windows arm64
How did you verify your code works?