fix(build-libkrun-macos-arm64): build libkrun using cargo - #659
Conversation
📝 WalkthroughWalkthroughThe release workflow's macOS libkrun build job is rewritten to use a direct Changeslibkrun macOS Build Pipeline
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/release.yml (1)
295-295: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winBuild the release dylib with Cargo’s lockfile enforced.
Add
--lockedso the release artifact can’t silently resolve dependencies differently from the pinned libkrun ref.Proposed change
- cargo build --release -p libkrun --no-default-features --features blk,net + cargo build --release -p libkrun --locked --no-default-features --features blk,net🤖 Prompt for 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. In @.github/workflows/release.yml at line 295, The libkrun release build command is not enforcing Cargo’s lockfile, so update the cargo build invocation in the release workflow to include --locked. Modify the existing libkrun build step so it keeps the pinned dependency resolution from the referenced libkrun ref and doesn’t silently resolve newer versions.
🤖 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.yml:
- Around line 296-310: The self-contained dylib validation in the release
workflow only rejects Homebrew paths, so it can miss other non-system
dependencies reported by otool -L as `@rpath` or bare names. Tighten the check in
the “Stage the dylib and assert it is self-contained” step by explicitly
allowlisting only expected system library locations and failing on any
dependency outside that set, using the existing dylib/otool validation logic as
the place to update.
---
Nitpick comments:
In @.github/workflows/release.yml:
- Line 295: The libkrun release build command is not enforcing Cargo’s lockfile,
so update the cargo build invocation in the release workflow to include
--locked. Modify the existing libkrun build step so it keeps the pinned
dependency resolution from the referenced libkrun ref and doesn’t silently
resolve newer versions.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: aa759090-606f-4d72-bf5d-a51514635e3c
📒 Files selected for processing (1)
.github/workflows/release.yml
Summary by CodeRabbit