Use Zig for Windows and ARM64 Linux #1713
Conversation
WalkthroughThe pull request modifies the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Taskfile.yml (1)
165-165: Consider handling other architectures explicitly.The current implementation assumes all non-AMD64 architectures should use the ARM64 target. While this works for the immediate use case, it might cause issues if other architectures need to be supported in the future.
Consider using explicit architecture checks:
- sh: echo "{{if eq "amd64" ARCH}}CC=\"zig cc -target x86_64-linux-gnu.2.28\"{{else}}CC=\"zig cc -target aarch64-linux-gnu.2.28\"{{end}}" + sh: echo "{{if eq "amd64" ARCH}}CC=\"zig cc -target x86_64-linux-gnu.2.28\"{{else if eq "arm64" ARCH}}CC=\"zig cc -target aarch64-linux-gnu.2.28\"{{else}}CC=\"gcc\"{{end}}"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Taskfile.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: build-app (windows, windows-latest)
- GitHub Check: build-app (linux, ubuntu-24.04-arm64-16core)
- GitHub Check: build-app (linux, ubuntu-latest)
- GitHub Check: build-app (darwin, macos-latest)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Build Docsite
- GitHub Check: Build for TestDriver.ai
- GitHub Check: Analyze (go)
- GitHub Check: merge-gatekeeper
🔇 Additional comments (1)
Taskfile.yml (1)
165-165: LGTM! Verify ARM64 builds work as expected.The change correctly extends Zig compiler support to ARM64 Linux builds, maintaining consistency with the existing x64 builds. This simplifies the setup by eliminating the need for build-essentials.
Run the following script to verify ARM64 builds:
✅ Verification successful
Verified: ARM64 Zig target configuration is correct
The
aarch64-linux-gnu.2.28target string follows the correct format and aligns with:
- Documented platform support for Linux ARM64 in README.md
- Required glibc-2.28 baseline for Linux builds
- Existing build system patterns for multi-architecture support
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the Zig compiler is correctly configured for ARM64 builds # Test 1: Check if Zig supports the target zig targets | grep 'aarch64-linux-gnu.2.28' # Test 2: Check if there are any existing ARM64 builds that we can compare against fd -t f 'wavesrv.*arm64' dist/bin/Length of output: 271
Script:
#!/bin/bash # Search for ARM64/aarch64 related configurations and documentation echo "=== Searching for ARM64/aarch64 configurations ===" rg -i "aarch64|arm64" --type yaml --type md echo -e "\n=== Checking for build documentation ===" fd -e md -e txt . -x grep -l -i "build|zig" {} echo -e "\n=== Looking for CI configurations ===" fd -g ".github/workflows/*.yml" . fd -g "*ci*.yml" .Length of output: 2086
esimkowitz
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/build-helper.yml (1)
41-43: Consider adding Zig version input for better control.The action supports specifying a Zig version, which would be beneficial for ensuring consistent builds across all platforms.
if: matrix.platform != 'darwin' - uses: mlugg/setup-zig@v1 + uses: mlugg/setup-zig@v1.2.0 + with: + version: 0.11.0
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/build-helper.yml(1 hunks).github/workflows/testdriver-build.yml(1 hunks)BUILD.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- BUILD.md
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Build for TestDriver.ai
- GitHub Check: Analyze (go)
- GitHub Check: merge-gatekeeper
- GitHub Check: Build Docsite
🔇 Additional comments (3)
.github/workflows/testdriver-build.yml (1)
60-61: 🛠️ Refactor suggestionPin the Zig setup action version for stability.
While adding Zig installation is good, the action version should be pinned for reproducible builds.
- uses: mlugg/setup-zig@v1 + uses: mlugg/setup-zig@v1.2.0Let's verify if this action supports Windows builds:
✅ Verification successful
Pin the Zig setup action version for stability - Windows compatibility verified
The action supports Windows builds as evidenced by Windows-specific discussions in the project issues. The version pinning recommendation remains valid for build stability.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if the action supports Windows curl -s https://raw.githubusercontent.com/mlugg/setup-zig/v1/action.yml | grep -A 5 "runs-on:"Length of output: 94
Script:
#!/bin/bash # Try multiple approaches to verify Windows support # Check action.yml from main branch curl -s https://raw.githubusercontent.com/mlugg/setup-zig/main/action.yml # Search for Windows-related issues or discussions gh api repos/mlugg/setup-zig/issues?state=all | jq -r '.[] | select(.title | test("(?i)windows")) | "Issue #\(.number): \(.title)"' # Check if this repository has other Windows workflows using this action fd -e yml -e yaml . .github/workflows/ --exec grep -l "mlugg/setup-zig"Length of output: 1121
.github/workflows/build-helper.yml (2)
Line range hint
24-26: Consider enabling Windows ARM64 builds.Since this PR aims to improve ARM64 support and simplify Windows builds using Zig, it might be worth uncommenting and testing the Windows ARM64 configuration.
Let's check if there are any related discussions or issues:
41-43: 🛠️ Refactor suggestionEnsure consistent Zig setup across workflows.
The Zig installation step should:
- Use a pinned version for stability
- Be verified for ARM64 compatibility
- uses: mlugg/setup-zig@v1 + uses: mlugg/setup-zig@v1.2.0Let's verify ARM64 compatibility:
✅ Verification successful
Version pinning recommended, ARM64 compatibility verified
The action fully supports ARM64 platforms. Recommend pinning to v1.2.0 for stability:
- uses: mlugg/setup-zig@v1 + uses: mlugg/setup-zig@v1.2.0🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if the action supports ARM64 curl -s https://raw.githubusercontent.com/mlugg/setup-zig/v1/action.yml | grep -A 10 "inputs:" # Verify if there are any known issues with ARM64 gh api graphql -f query=' { repository(owner: "mlugg", name: "setup-zig") { issues(first: 5, states: OPEN) { nodes { title body labels(first: 5) { nodes { name } } } } } }'Length of output: 7720
I'm making steps to simplify our build dependencies, consolidating our C compiler requirements so we only depend on Zig.
Before, we used Zig for x64 but not for arm64. This meant that users using an ARM dev machine would need to install
build-essentialsand Zig. We also required MinGW-w64 on Windows, which is a pain to install since it can be provided by a bunch of different tools, the smallest of which is like a 2GB install.