provided support for windows arm64#92
Conversation
There was a problem hiding this comment.
Pull request overview
Adds Windows ARM64 support to the project’s release build pipeline and improves portability of artifact packaging/checksum generation across environments (e.g., Git Bash).
Changes:
- Add
windows/arm64to the cross-compilation build matrix and binary verification list. - Extend Windows artifact packaging to include ARM64 and fall back to
.tar.gzwhenzipis unavailable. - Make checksum generation tool selection more robust (
shasumpreferred,sha256sumfallback, otherwise warn).
| (cd "$BUILD_DIR" && shasum -a 256 * > "${PACKAGE_NAME}-${VERSION}-checksums.txt") | ||
| elif command -v sha256sum >/dev/null 2>&1; then | ||
| (cd "$BUILD_DIR" && sha256sum * > "${PACKAGE_NAME}-${VERSION}-checksums.txt") |
There was a problem hiding this comment.
The checksum generation uses *, which will include the raw binaries in dist/ (and possibly an old *-checksums.txt from a previous run). However, the release workflow only uploads archives (dist/*.tar.gz, dist/*.zip), so the generated checksums won’t correspond 1:1 with the released artifacts. Consider generating checksums from an explicit artifact glob/list (e.g., only *.tar.gz and *.zip, excluding the checksum file itself) to keep checksums accurate and reproducible across reruns.
| (cd "$BUILD_DIR" && shasum -a 256 * > "${PACKAGE_NAME}-${VERSION}-checksums.txt") | |
| elif command -v sha256sum >/dev/null 2>&1; then | |
| (cd "$BUILD_DIR" && sha256sum * > "${PACKAGE_NAME}-${VERSION}-checksums.txt") | |
| ( | |
| cd "$BUILD_DIR" || exit 1 | |
| artifacts=() | |
| for pattern in "${PACKAGE_NAME}-${VERSION}-"*.tar.gz "${PACKAGE_NAME}-${VERSION}-"*.zip; do | |
| [ -e "$pattern" ] || continue | |
| artifacts+=("$pattern") | |
| done | |
| if [ ${#artifacts[@]} -gt 0 ]; then | |
| shasum -a 256 "${artifacts[@]}" > "${PACKAGE_NAME}-${VERSION}-checksums.txt" | |
| else | |
| echo "Warning: No release archives found, skipping checksums" | |
| fi | |
| ) | |
| elif command -v sha256sum >/dev/null 2>&1; then | |
| ( | |
| cd "$BUILD_DIR" || exit 1 | |
| artifacts=() | |
| for pattern in "${PACKAGE_NAME}-${VERSION}-"*.tar.gz "${PACKAGE_NAME}-${VERSION}-"*.zip; do | |
| [ -e "$pattern" ] || continue | |
| artifacts+=("$pattern") | |
| done | |
| if [ ${#artifacts[@]} -gt 0 ]; then | |
| sha256sum "${artifacts[@]}" > "${PACKAGE_NAME}-${VERSION}-checksums.txt" | |
| else | |
| echo "Warning: No release archives found, skipping checksums" | |
| fi | |
| ) |
| (cd "$BUILD_DIR" && zip "${PACKAGE_NAME}-${VERSION}-windows-arm64.zip" "${BINARY_NAME}-${VERSION}-windows-arm64.exe") | ||
| else | ||
| echo "Warning: zip command not found, skipping Windows archives" | ||
| echo "Warning: zip command not found, falling back to tar.gz for Windows" |
There was a problem hiding this comment.
In the zip-missing branch, the script assumes tar is available but doesn’t verify it. Since the script also doesn’t use set -e, a missing tar would result in failed archive creation while the script still reaches the final “Build complete” message. Consider adding a command -v tar check (and exiting non-zero if unavailable) before attempting the Windows tar.gz fallback so failures are explicit.
| echo "Warning: zip command not found, falling back to tar.gz for Windows" | |
| echo "Warning: zip command not found, falling back to tar.gz for Windows" | |
| if ! command -v tar >/dev/null 2>&1; then | |
| echo "Error: tar command not found, cannot create Windows fallback archives" >&2 | |
| exit 1 | |
| fi |
alexrinass
left a comment
There was a problem hiding this comment.
The core change (adding windows/arm64) is straightforward and correct — it follows the existing build pattern exactly. However, there are a few items to address before merge.
Must Fix
- Commit message format:
provided support for windows arm64doesn't follow project conventions (COMMIT_GUIDELINES.md) — missing type prefix, uses past tense instead of imperative mood. Should be e.g.build: Add windows/arm64 build target
Should Fix
- Mixed concerns: The PR bundles three unrelated changes (arm64 target, zip→tar.gz fallback, sha256sum fallback). These should be separate commits with appropriate commit messages (e.g.,
build: Fall back to tar.gz when zip unavailable,build: Add sha256sum fallback for checksum generation).
Nit
- The tar.gz fallback for Windows is unconventional — Windows users generally expect
.ziparchives. The fallback is better than nothing, but worth noting.
🤖 Review generated with Claude Code
| echo "Warning: Neither shasum nor sha256sum found, skipping checksums" | ||
| fi | ||
|
|
||
| echo "Build complete! Artifacts are in the $BUILD_DIR directory" No newline at end of file |
There was a problem hiding this comment.
Must Fix: Missing newline at end of file. The PR removes the trailing newline — \ No newline at end of file appears in the diff. POSIX requires a trailing newline for well-formed text files, and many tools will warn about this.
| echo "Generating checksums..." | ||
| (cd "$BUILD_DIR" && shasum -a 256 * > "${PACKAGE_NAME}-${VERSION}-checksums.txt") | ||
| if command -v shasum >/dev/null 2>&1; then | ||
| (cd "$BUILD_DIR" && shasum -a 256 * > "${PACKAGE_NAME}-${VERSION}-checksums.txt") |
There was a problem hiding this comment.
Should Fix: The * glob checksums everything in dist/ — raw binaries (which aren't released, only archives are uploaded) and potentially a stale checksums file from a previous run. Should target only release archives:
shasum -a 256 *.tar.gz *.zip > "${PACKAGE_NAME}-${VERSION}-checksums.txt"(Same applies to the sha256sum branch below.)
Note: this is a pre-existing issue on main, but since this code block is already being modified, it's the right time to fix it.
| (cd "$BUILD_DIR" && zip "${PACKAGE_NAME}-${VERSION}-windows-arm64.zip" "${BINARY_NAME}-${VERSION}-windows-arm64.exe") | ||
| else | ||
| echo "Warning: zip command not found, skipping Windows archives" | ||
| echo "Warning: zip command not found, falling back to tar.gz for Windows" |
There was a problem hiding this comment.
Nit: The fallback branch uses tar without verifying it exists via command -v, while zip gets that check. In practice tar is a POSIX standard utility and will always be present on systems that can run this script, so this is a consistency nit rather than a real risk.
Adds support for Windows ARM64 builds and enhances build script robustness with utility fallbacks.
The scripts/build.sh script now includes windows/arm64 in the cross-compilation targets. Additionally, it dynamically detects available checksum tools (preferring shasum but falling back to sha256sum) and archiving tools. For Windows builds, it now falls back to tar.gz if the zip command is unavailable, ensuring
the build process completes successfully in environments like Git Bash.
Relates to multi-platform support and build environment compatibility
Remarks
Review focus:
Author Checklist (Verified)