Skip to content

Add GitHub release workflow for multi-platform builds#35

Merged
lookbusy1344 merged 1 commit intomainfrom
claude/create-release-workflow-011CUKukJF7We9LEKMTueJXg
Oct 21, 2025
Merged

Add GitHub release workflow for multi-platform builds#35
lookbusy1344 merged 1 commit intomainfrom
claude/create-release-workflow-011CUKukJF7We9LEKMTueJXg

Conversation

@lookbusy1344
Copy link
Owner

Creates release binaries for Mac ARM64, Windows AMD64/ARM64, and Linux AMD64 when tags are pushed.

🤖 Generated with Claude Code

Creates release binaries for Mac ARM64, Windows AMD64/ARM64, and Linux AMD64 when tags are pushed.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@claude
Copy link
Contributor

claude bot commented Oct 21, 2025

Pull Request Review: GitHub Release Workflow

I've reviewed the proposed GitHub Actions workflow for multi-platform releases. Overall, this is a solid implementation with good cross-platform coverage. Here are my findings:

✅ Strengths

  1. Good Platform Coverage: Targets all major platforms (macOS ARM64, Windows AMD64/ARM64, Linux AMD64)
  2. Clean Architecture: Two-job design (build → release) with proper dependency chain
  3. Security: Appropriate contents: write permission scope
  4. Artifact Management: Uses v4 actions (upload/download-artifact) which are current best practices
  5. Consistent Naming: Binary names follow clear platform conventions

🔍 Issues & Recommendations

1. Missing macOS x86_64 Target (Medium Priority)

Your workflow only includes aarch64-apple-darwin (ARM64). Consider adding:

- target: x86_64-apple-darwin
  os: macos-latest
  name: hashrust-macos-amd64

Many Mac users still use Intel-based systems. You could also create universal binaries using lipo if desired.

2. Cargo Build Optimization (Low Priority)

The workflow doesn't leverage the project's optimized release profile settings from Cargo.toml:

[profile.release]
panic = 'abort'
codegen-units = 1
lto = 'fat'
strip = true

These settings are already applied when you run cargo build --release, so this is actually working correctly. However, consider adding a cache step to speed up builds:

- name: Cache cargo registry
  uses: actions/cache@v4
  with:
    path: |
      ~/.cargo/bin/
      ~/.cargo/registry/index/
      ~/.cargo/registry/cache/
      ~/.cargo/git/db/
      target/
    key: ${{ runner.os }}-cargo-${{ matrix.target }}-${{ hashFiles('**/Cargo.lock') }}

3. Cross-Compilation on Windows (High Priority - Potential Bug)

The ARM64 Windows build runs on windows-latest (x86_64 runner) but targets aarch64-pc-windows-msvc. This will likely fail without cross-compilation setup.

Fix: Either:

  • Use GitHub's ARM64 Windows runners (if available to you)
  • Add cross-compilation toolchain setup
  • Use cross tool instead of cargo directly

Example fix:

- name: Build (cross-compile)
  if: matrix.target == 'aarch64-pc-windows-msvc'
  run: |
    cargo install cross
    cross build --release --target ${{ matrix.target }}

- name: Build (native)
  if: matrix.target != 'aarch64-pc-windows-msvc'
  run: cargo build --release --target ${{ matrix.target }}

4. No Checksum Generation (Medium Priority)

Release binaries should include SHA256 checksums for verification. Consider adding:

- name: Generate checksum
  run: |
    sha256sum ${{ matrix.name }} > ${{ matrix.name }}.sha256

5. Missing Test Step (Medium Priority)

Your existing CI runs tests (rust.yml), but the release workflow doesn't verify the binary works before publishing. Consider adding basic smoke tests:

- name: Test binary
  if: matrix.os != 'windows-latest'
  run: ./${{ matrix.name }} --help

- name: Test binary (Windows)
  if: matrix.os == 'windows-latest'
  run: .\${{ matrix.name }} --help

6. Binary Naming Consistency (Low Priority - Style)

Windows binaries use .exe extension in artifact names, but Unix binaries don't include their extension (none). This is fine, but consider whether you want the artifact names to match exactly what users download, or use consistent naming without extensions and let GitHub/users rename as needed.

🔒 Security Assessment

  • ✅ No hardcoded secrets
  • ✅ Uses GITHUB_TOKEN (automatically provided, scoped)
  • ✅ Minimal permission scope (contents: write only)
  • ✅ Uses pinned action versions (v4, v1)
  • ⚠️ Consider pinning actions to SHA instead of tags for maximum security

🚀 Performance Considerations

  • Parallel builds across platforms: Excellent
  • No caching: Builds will be slower than necessary (see recommendation Bump anyhow from 1.0.86 to 1.0.87 #2)
  • Artifact download overhead: Minimal, acceptable ✅

🧪 Test Coverage

  • No automated testing of release binaries before publishing
  • Relies on main branch CI (rust.yml) to catch issues
  • Recommendation: Add smoke tests (see Bump anyhow from 1.0.91 to 1.0.92 #5 above)

📋 Summary

Must Fix Before Merge:

Should Fix:

Nice to Have:

The workflow is well-structured and shows good understanding of GitHub Actions patterns. With the cross-compilation fix, this will work well for your release process.

@lookbusy1344 lookbusy1344 merged commit fcd6991 into main Oct 21, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants