Zig15 windows ci #8
Conversation
…cross-compiled testing
There was a problem hiding this comment.
Pull request overview
This PR updates the codebase to work with Zig 0.15.2 and adds Windows CI support by replacing std.heap.page_allocator with GeneralPurposeAllocator across Zig files and implementing a cross-compilation workflow.
Key Changes:
- Replaced
std.heap.page_allocatorwithGeneralPurposeAllocatorthroughout Zig codebase to address Zig 0.15.2 compatibility requirements - Added GitHub Actions workflow for Windows builds that cross-compiles from macOS due to known Zig 0.15.2 bugs on Windows
- Added
--skip-zig-buildflag to build script for using pre-built binaries
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/zig/lib.zig | Replaces page_allocator with a global GeneralPurposeAllocator instance used throughout the module |
| packages/core/src/zig/terminal.zig | Adds local GeneralPurposeAllocator in checkEnvironmentOverrides function |
| packages/core/src/zig/bench-utils.zig | Adds local GeneralPurposeAllocator in printResults function |
| packages/core/scripts/build.ts | Adds --skip-zig-build flag to optionally skip Zig compilation and use existing binaries |
| .github/workflows/build-windows.yml | Implements Windows CI workflow that cross-compiles from macOS and tests on Windows |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var gpa = std.heap.GeneralPurposeAllocator(.{}){}; | ||
| const gpa_allocator = gpa.allocator(); |
There was a problem hiding this comment.
The GeneralPurposeAllocator is declared as a global variable but is never deinitialized. This will cause memory leak detection to fail when the allocator is cleaned up. The allocator should be properly cleaned up at program termination, or the leak detection should be checked. Consider adding cleanup logic or declaring the allocator with leak checking disabled if intentional.
| var gpa = std.heap.GeneralPurposeAllocator(.{}){}; | ||
| defer _ = gpa.deinit(); | ||
| var env_map = std.process.getEnvMap(gpa.allocator()) catch return; |
There was a problem hiding this comment.
The local GeneralPurposeAllocator is created and deinitialized within this function scope. However, the env_map allocated from this allocator outlives the allocator itself due to the early return on line 228. If getEnvMap succeeds, env_map.deinit() on line 229 will be called after the GPA has been deinitialized on line 227, which could lead to use-after-free or undefined behavior. The allocator must remain valid for the lifetime of all allocations.
| var gpa = std.heap.GeneralPurposeAllocator(.{}){}; | ||
| defer _ = gpa.deinit(); | ||
| var arena = std.heap.ArenaAllocator.init(gpa.allocator()); |
There was a problem hiding this comment.
The local GeneralPurposeAllocator is created and deinitialized within this function scope. However, the arena allocator initialized from this GPA outlives the GPA itself. The arena.deinit() on line 67 will attempt to free memory after the GPA has been deinitialized on line 65, which could lead to use-after-free or undefined behavior. The GPA must remain valid for the lifetime of the arena allocator.
No description provided.