fix(release): build musl via cargo-zigbuild + make release resilient#46
Conversation
The v0.1.1 release built 5/6 targets; the musl leg failed because esaxx-rs (C++, via tokenizers -> model2vec-rs) needs a musl C++ cross compiler and musl-tools only ships musl-gcc, not musl-g++. A dependency audit found the native build surface is bounded (esaxx-rs C++, onig/zstd C, ring) with NO onnxruntime, so a single C/C++ cross toolchain covers it. - musl target now builds with cargo-zigbuild (zig = clang + musl + libc++), installed from the official zig tarball (no extra pinned action). Other targets keep plain cargo build. - Release resilience: upload-release-assets, publish-npm, and the Homebrew job now run with always(), so a single failed build leg no longer blocks the whole release — the targets that built still publish. The npm generator is already partial-matrix-tolerant; upload fails loudly only if nothing built.
|
Note Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughTwo release workflow files are updated to tolerate partial matrix build failures. The Rust workflow switches musl target builds from ChangesRelease Workflow Resilience
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Up to standards ✅🟢 Issues
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/release-please.yml:
- Around line 155-157: The `always()` condition in the Homebrew job allows it to
proceed even with partial build failures, but the current asset download
mechanism using curl without the `-f` flag silently succeeds even when assets
are missing, causing corrupt formula generation. Add the `-f` flag to all curl
commands downloading checksum assets to make them fail on missing resources, and
add explicit validation logic before the formula generation step to verify that
all required core assets (darwin and linux-gnu checksums) are present and
hard-fail the job if any are missing, while keeping the `always()` condition to
permit musl build failures that shouldn't block the release.
In @.github/workflows/release-rust.yml:
- Around line 85-89: The workflow downloads and extracts a Zig tarball without
verifying its integrity, which is a security risk. Modify the Zig installation
step to download the minisign signature file for the tarball (typically with a
.sig extension), retrieve the official Zig public key from
ziglang.org/download/, and use minisign to verify the tarball signature before
extracting it. Only proceed with the tar extraction if the minisign verification
succeeds. Reference the ZIG_VERSION variable in the download URLs for both the
tarball and signature file to ensure consistency.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6d7e9380-a555-4621-b113-49fc883689e6
📒 Files selected for processing (2)
.github/workflows/release-please.yml.github/workflows/release-rust.yml
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
- homebrew: use 'curl -fLsS' for checksum downloads so a missing core asset hard-fails the job under always() instead of baking a corrupt sha256 into the formula (coderabbit) - musl zig setup: verify the Zig tarball with minisign against the official ziglang.org public key before extracting/executing it (coderabbit)
1 participant
Problem
v0.1.1 built 5/6 targets; the musl leg failed:
esaxx-rsis C++ (viatokenizers→model2vec-rs);musl-toolsshipsmusl-gccbut notmusl-g++. Becauserelease-rust.ymlrequired all builds before upload, this one failure blocked the entire release (no assets, no npm publish).Dependency audit (musl-relevant native deps)
No
onnxruntime/ort— model2vec uses pure-Rust static embeddings. The native surface is bounded, so one C/C++ cross toolchain (zig) covers all of it.Fix
cargo build.upload-release-assets,publish-npm, and the Homebrew job now run withalways(), so a single failed build leg no longer blocks the whole release — the targets that built still publish (the npm generator is already partial-matrix-tolerant). Upload fails loudly only if nothing built.Recovery
Merging this
fix:cuts 0.1.2; with zigbuild all 6 targets should build and publish cleanly via OIDC. Even if musl regresses, resilience lets the other 5 ship.Test plan
Summary by cubic
Switch
*-muslbuilds tocargo-zigbuildwithzigand harden the release so successful targets still publish. Adds signature verification and strict checksum downloads for safer releases.*-muslwithcargo zigbuild; installzig0.13.0 andcargo-zigbuild^0.19from the official tarball, verifying the Zig download with minisign before use.upload-release-assets,publish-npm, and Homebrew withalways(); the uploader fails only if no artifacts are staged, and Homebrew checksum downloads usecurl -fLsSto hard-fail when a core asset is missing.Written for commit c681d35. Summary will update on new commits.
Summary by CodeRabbit