Skip to content

fix(minvmd): do not require libkrun when used as a rust dep - #668

Merged
twitchyliquid64 merged 1 commit into
mainfrom
tom/minvmd
Jul 8, 2026
Merged

fix(minvmd): do not require libkrun when used as a rust dep#668
twitchyliquid64 merged 1 commit into
mainfrom
tom/minvmd

Conversation

@twitchyliquid64

@twitchyliquid64 twitchyliquid64 commented Jul 8, 2026

Copy link
Copy Markdown
Member

Fixes minimal on MacOS exploding when theres no libkrun, by fixing the issue where all dependents of minvmd crate would inherit that linker requirement.

Summary by CodeRabbit

  • Bug Fixes
    • Tightened release validation for macOS ARM64 builds to catch unexpected non-system library dependencies.
    • Ensured the minimal binary only links system libraries, reducing the risk of accidentally including extra VM-related linkage.
    • Improved build behavior so opting out of default features correctly prevents backend linkage in minimal builds.

@coderabbitai

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 44beca9a-ae9e-420d-8874-7f7265dde480

📥 Commits

Reviewing files that changed from the base of the PR and between c6c63e4 and da63d58.

📒 Files selected for processing (5)
  • .github/workflows/release.yml
  • crates/minimal/Cargo.toml
  • crates/minvmd/Cargo.toml
  • crates/minvmd/build.rs
  • scripts/stage-release.sh

📝 Walkthrough

Walkthrough

Introduces a libkrun Cargo feature in minvmd to gate real VM backend linking, updates minimal's dependency to opt out via default-features = false, adjusts build.rs to compute prefix/rpaths only when the feature is enabled, and strengthens macOS release workflow and staging script validation/comments around @rpath/system-only library linkage.

Changes

Feature-gated libkrun linking

Layer / File(s) Summary
libkrun feature flag and Cargo manifests
crates/minvmd/Cargo.toml, crates/minimal/Cargo.toml
Adds a [features] section defaulting to libkrun in minvmd, and updates minimal's dependency on minvmd to default-features = false with explanatory comments.
build.rs prefix/rpath gating on libkrun feature
crates/minvmd/build.rs
Rewrites module docs on rpath/dyld behavior; main() now computes prefix only when CARGO_FEATURE_LIBKRUN is set, otherwise forcing None (no link-search, rpaths, or cfg); reorders/reformats emit_rpaths() logic and comments.
Release workflow and staging script validation updates
.github/workflows/release.yml, scripts/stage-release.sh
Replaces the simple minvmd linkage grep-check with stricter validation requiring @rpath/libkrun.1.dylib plus system-only other dependencies; adds a new check that minimal links only system libraries; rewords comments in the release workflow and staging script about @rpath/@loader_path resolution.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

Not applicable — changes are primarily build configuration, feature gating, and CI validation logic without multi-component runtime interaction flows.

Possibly related PRs

  • gominimal/minimal#614: Both PRs modify crates/minvmd/build.rs around libkrun linking/rpath handling and the macOS release workflow for minvmd artifacts.
  • gominimal/minimal#655: Both PRs modify the macOS ARM64 release packaging pipeline (.github/workflows/release.yml and scripts/stage-release.sh) around shipping/using libkrun.dylib.
  • gominimal/minimal#659: Both PRs deal with macOS release workflow validation ensuring built artifacts link only expected libkrun/system paths.

Suggested reviewers: norrietaylor, bryan-minimal

Poem

A feature flag hops into place,
libkrun linked with newfound grace,
rpaths line up, system-clean,
minimal skips the VM machine,
this bunny thumps in CI's embrace! 🐇🔧

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: making minvmd stop requiring libkrun when used as a Rust dependency.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Comment @coderabbitai help to get the list of available commands.

@twitchyliquid64
twitchyliquid64 enabled auto-merge (squash) July 8, 2026 01:38
@twitchyliquid64
twitchyliquid64 merged commit b29841d into main Jul 8, 2026
55 of 56 checks passed
@twitchyliquid64
twitchyliquid64 deleted the tom/minvmd branch July 8, 2026 01:48
norrietaylor added a commit that referenced this pull request Jul 9, 2026
…ipt (#685)

* ci: extract shared setup composites for rust, libkrun, and guest artifacts

Four new composite actions dissolve copy-pasted setup blocks:

- setup-rust: free-disk + protoc + rust-cache preamble, previously
  inlined in ci.yml clippy AND duplicated inside core-tests
- setup-libkrun-macos: the brew slp/krun tap install, previously
  copy-pasted across ci-macos.yml and release.yml (the release job's
  separate dylib-existence check is dropped like ci-macos's was in
  #678: a failed brew install already fails the step, and a missing
  libkrun still fails loudly at link/rewrite time)
- setup-libkrun-linux: fetch-libkrun.sh + the LIBKRUN_PREFIX /
  LD_LIBRARY_PATH exports, previously duplicated between
  ci-linux-kvm.yml and release.yml's amd64 build
- guest-artifacts: the kernel + rootfs cache pulls, previously
  repeated in ci-macos.yml, ci-linux-kvm.yml, and release.yml

The two fetch composites retry 3x to absorb transient cache/network
hiccups; commands, arguments, and env are otherwise unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* ci(release): extract minvmd linkage rewrite into a script

The ~60-line inline install_name_tool rewrite + otool verification in
build-release-macos-arm64 (grown across #664/#668/#670) moves to
scripts/rewrite-macos-linkage.sh, command-for-command. A script lets
CI exercise the production @rpath rewrite on a throwaway copy of the
debug binary later, instead of the rewrite only ever running at
release time. Signing stays the caller's job (Developer ID, last
mutation before upload, per #680).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* ci(setup-minimal): drop dead cache-minimal install conditional

The Install Minimal step guarded on
steps.cache-minimal.outputs.cache-hit, but no step with id
cache-minimal exists (leftover from a removed cache step), so the
condition was always true. Remove it; the step runs unconditionally
as it already did in practice.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(scripts): harden rewrite-macos-linkage.sh

Three latent bugs surfaced by review once the previously-inline code
became a parameterized, reusable script:

- otool -L's header line is the binary's own path; a binary living
  under a path containing "libkrun" (e.g. a throwaway CI copy) would
  false-match and silently skip the real rewrite. Skip the header
  (NR>1).
- install_name_tool -rpath errors when the bare @loader_path entry is
  already gone, so a second run on an already-rewritten binary
  hard-failed. Retarget only when the dev rpath is present.
- set -o pipefail aborted the `current=` capture before the annotated
  "no libkrun load command" diagnostic could fire when otool itself
  fails. Guard the capture with || true; the -z check handles both.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* ci: address review findings in shared composites and their callers

- Trigger the VM lanes when their composite actions change:
  ci-macos.yml and ci-linux-kvm.yml path filters now include the
  composites their setup runs through, so a composite-only edit can
  no longer merge unexercised and break the lanes post-merge.
- Restore the libkrun.dylib existence check in setup-libkrun-macos:
  brew exits 0 for an installed-but-unlinked keg, so "a failed
  install fails the step" was unsound on a persistent shared runner;
  without the check a release build dies later with a raw
  `ld: library 'krun' not found` instead of a provisioning pointer.
- Fail fast on deterministic errors: the mip CLI is validated/built
  once, outside the retry loops (guest-artifacts, setup-libkrun-linux),
  so a compile error or bad mip path fails in one attempt instead of
  being re-run into the job timeout. Only the network-bound
  materialize retries.
- One retry implementation: scripts/ci/retry.sh replaces the three
  hand-copied divergent loops, and the previously-unretried gvproxy
  downloads (ci-linux-kvm + release) now use it too.
- release.yml: drop the inline LIBKRUN_PREFIX re-hardcode (the
  composite publishes it); document why the job-wide LD_LIBRARY_PATH
  is safe (the prefix holds only libkrun/libkrunfw by construction);
  fix the linkage-step comment that implied a CI consumer exists.

Not fixed: ci-netns.yml still inlines its rust preamble — its
free-disk config differs (no remove_tool_cache) and the file is
slated for deletion when the networking proofs are mothballed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* ci: drop the fetch retries — lane history shows zero fetch failures

Actions history for both VM lanes (last ~200 runs each, plus the
first attempts of every manually re-run run) records not a single
failure in the retried steps: the kernel/rootfs cache pulls, the
libkrun prefix fetch, and the gvproxy download have never failed.
The retries were insurance without receipts; remove them and the
now-unused scripts/ci/retry.sh, returning the composites to plain
extractions of the original steps (the mip prebuild and input
pre-validation existed only to keep deterministic work out of the
retry loops, so they go too).

The transient failures the history DOES show are apt-get installs —
the only main-branch KVM lane failure in the window and both of its
manual reruns died in "Install build dependencies". Retry belongs
there if anywhere, left for a separate change.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* ci(macos): reap the whole Session-E2E process tree before auto-spawn

The __krun-vmm-only reap from #672 is insufficient: main run
29032763009 — on the very commit that added it — still failed the
cold `minimal ls` with `ssh connect: Disconnected` against a fully
healthy guest (READY emitted, minimald listening on vsock:2222, no
connection ever accepted): the #588 bridge wedge. The remaining
leftover is the host gvproxy switch, which minvmd owns and
Guest::drop never kills; the failing runs' proxy-publish WARN
corroborates a lingering gvproxy. Reap any stray minvmd first (so
nothing respawns), then the VMM and gvproxy.

The proper fix — the session harness reaping its own process group —
stays tracked under #588; this keeps the merged e2e job's steps
isolated in the meantime.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
norrietaylor added a commit that referenced this pull request Jul 10, 2026
* ci: extract shared setup composites for rust, libkrun, and guest artifacts

Four new composite actions dissolve copy-pasted setup blocks:

- setup-rust: free-disk + protoc + rust-cache preamble, previously
  inlined in ci.yml clippy AND duplicated inside core-tests
- setup-libkrun-macos: the brew slp/krun tap install, previously
  copy-pasted across ci-macos.yml and release.yml (the release job's
  separate dylib-existence check is dropped like ci-macos's was in
  #678: a failed brew install already fails the step, and a missing
  libkrun still fails loudly at link/rewrite time)
- setup-libkrun-linux: fetch-libkrun.sh + the LIBKRUN_PREFIX /
  LD_LIBRARY_PATH exports, previously duplicated between
  ci-linux-kvm.yml and release.yml's amd64 build
- guest-artifacts: the kernel + rootfs cache pulls, previously
  repeated in ci-macos.yml, ci-linux-kvm.yml, and release.yml

The two fetch composites retry 3x to absorb transient cache/network
hiccups; commands, arguments, and env are otherwise unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* ci(release): extract minvmd linkage rewrite into a script

The ~60-line inline install_name_tool rewrite + otool verification in
build-release-macos-arm64 (grown across #664/#668/#670) moves to
scripts/rewrite-macos-linkage.sh, command-for-command. A script lets
CI exercise the production @rpath rewrite on a throwaway copy of the
debug binary later, instead of the rewrite only ever running at
release time. Signing stays the caller's job (Developer ID, last
mutation before upload, per #680).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* ci(setup-minimal): drop dead cache-minimal install conditional

The Install Minimal step guarded on
steps.cache-minimal.outputs.cache-hit, but no step with id
cache-minimal exists (leftover from a removed cache step), so the
condition was always true. Remove it; the step runs unconditionally
as it already did in practice.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(scripts): harden rewrite-macos-linkage.sh

Three latent bugs surfaced by review once the previously-inline code
became a parameterized, reusable script:

- otool -L's header line is the binary's own path; a binary living
  under a path containing "libkrun" (e.g. a throwaway CI copy) would
  false-match and silently skip the real rewrite. Skip the header
  (NR>1).
- install_name_tool -rpath errors when the bare @loader_path entry is
  already gone, so a second run on an already-rewritten binary
  hard-failed. Retarget only when the dev rpath is present.
- set -o pipefail aborted the `current=` capture before the annotated
  "no libkrun load command" diagnostic could fire when otool itself
  fails. Guard the capture with || true; the -z check handles both.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* ci: address review findings in shared composites and their callers

- Trigger the VM lanes when their composite actions change:
  ci-macos.yml and ci-linux-kvm.yml path filters now include the
  composites their setup runs through, so a composite-only edit can
  no longer merge unexercised and break the lanes post-merge.
- Restore the libkrun.dylib existence check in setup-libkrun-macos:
  brew exits 0 for an installed-but-unlinked keg, so "a failed
  install fails the step" was unsound on a persistent shared runner;
  without the check a release build dies later with a raw
  `ld: library 'krun' not found` instead of a provisioning pointer.
- Fail fast on deterministic errors: the mip CLI is validated/built
  once, outside the retry loops (guest-artifacts, setup-libkrun-linux),
  so a compile error or bad mip path fails in one attempt instead of
  being re-run into the job timeout. Only the network-bound
  materialize retries.
- One retry implementation: scripts/ci/retry.sh replaces the three
  hand-copied divergent loops, and the previously-unretried gvproxy
  downloads (ci-linux-kvm + release) now use it too.
- release.yml: drop the inline LIBKRUN_PREFIX re-hardcode (the
  composite publishes it); document why the job-wide LD_LIBRARY_PATH
  is safe (the prefix holds only libkrun/libkrunfw by construction);
  fix the linkage-step comment that implied a CI consumer exists.

Not fixed: ci-netns.yml still inlines its rust preamble — its
free-disk config differs (no remove_tool_cache) and the file is
slated for deletion when the networking proofs are mothballed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* ci: drop the fetch retries — lane history shows zero fetch failures

Actions history for both VM lanes (last ~200 runs each, plus the
first attempts of every manually re-run run) records not a single
failure in the retried steps: the kernel/rootfs cache pulls, the
libkrun prefix fetch, and the gvproxy download have never failed.
The retries were insurance without receipts; remove them and the
now-unused scripts/ci/retry.sh, returning the composites to plain
extractions of the original steps (the mip prebuild and input
pre-validation existed only to keep deterministic work out of the
retry loops, so they go too).

The transient failures the history DOES show are apt-get installs —
the only main-branch KVM lane failure in the window and both of its
manual reruns died in "Install build dependencies". Retry belongs
there if anywhere, left for a separate change.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* ci(macos): reap the whole Session-E2E process tree before auto-spawn

The __krun-vmm-only reap from #672 is insufficient: main run
29032763009 — on the very commit that added it — still failed the
cold `minimal ls` with `ssh connect: Disconnected` against a fully
healthy guest (READY emitted, minimald listening on vsock:2222, no
connection ever accepted): the #588 bridge wedge. The remaining
leftover is the host gvproxy switch, which minvmd owns and
Guest::drop never kills; the failing runs' proxy-publish WARN
corroborates a lingering gvproxy. Reap any stray minvmd first (so
nothing respawns), then the VMM and gvproxy.

The proper fix — the session harness reaping its own process group —
stays tracked under #588; this keeps the merged e2e job's steps
isolated in the meantime.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* ci(macos): consolidate libkrun on our own pinned source build

CI previously tested against the slp/krun Homebrew bottle (a
full-feature build, version drifting with the tap) while the release
shipped our trimmed source build (blk,net; no gpu, no init-blob) —
which nothing executed before it reached users. Consolidate both on
one build:

- vendor/libkrun/libkrun.lock pins the containers/libkrun version AND
  its resolved commit (replaces the LIBKRUN_REF env pin; the build
  fetches the commit, so a moved tag cannot change what we build).
- scripts/build-libkrun-macos.sh builds the trimmed dylib at the pin,
  asserts self-containment and the krun_add_disk3 (>= 1.19.0) API
  floor, sets the install name to @rpath/libkrun.1.dylib, ad-hoc
  signs, and stages libkrun.1.dylib + a libkrun.dylib linker symlink
  into a prefix.
- setup-libkrun-macos builds on miss into a commit-keyed prefix
  ($HOME/.cache/minimal-ci/libkrun/<commit>) — actions/cache on
  GitHub-hosted runners, the persistent directory itself on the mini —
  verifies the staged dylib, and publishes LIBKRUN_PREFIX. minvmd's
  build.rs prefers LIBKRUN_PREFIX over /opt/homebrew and bakes it as
  an rpath, so a leftover brew libkrun on the runner is ignored; brew
  leaves the CI path entirely (and with it the installed-but-unlinked
  keg failure mode).
- release build-libkrun-macos-arm64 ships from the same composite:
  the shipped dylib is by construction the one every macOS CI lane
  linked and booted. Developer ID signing flow unchanged (#680).

With the @rpath install name, minvmd records @rpath/libkrun.1.dylib
at link time — verified locally: the script builds a self-contained
dylib (Hypervisor.framework/libiconv/libSystem only), minvmd links
with the @rpath load command + @loader_path and prefix rpaths, and
the binary loads. rewrite-macos-linkage.sh's -change becomes a
natural no-op; only its @loader_path -> @loader_path/../lib retarget
still mutates release binaries.

Refs: #687

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* ci(macos): address CodeRabbit review on the libkrun build

- Key the on-disk libkrun prefix by BOTH build inputs (pinned commit
  + build-script hash), not commit alone: the persistent mini could
  otherwise keep serving a dylib built by an older
  build-libkrun-macos.sh after a script change. This mirrors the
  invalidation the actions/cache key already gave GitHub-hosted
  runners; stale sibling prefixes on the mini are inert.
- cargo build --locked: build exactly upstream's committed
  Cargo.lock (verified present at the pin) so a silent dependency
  re-resolve cannot undermine the reproducible-build guarantee.

Verified locally: the --locked build completes at the pin and stages
into the new hash-suffixed prefix.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* ci(macos): build minimal in its own cargo invocation in the e2e job

The e2e's combined `cargo build -p minvmd --bin minvmd -p minimal
--bin minimal` unified minvmd's default `libkrun` feature into the
CLI's default-features=false opt-out, so `minimal` linked libkrun —
the exact footgun release.yml already documents and avoids with
separate invocations. The brew era masked it: brew's libkrun carries
an absolute /opt/homebrew install name, so the mislinked CLI still
loaded. The own-build dylib's @rpath install name exposed it — the
autospawn e2e died with dyld "Library not loaded:
@rpath/libkrun.1.dylib / no LC_RPATH's found" from target/debug/
minimal, which bakes no rpaths.

Split the build (mirroring release.yml) and add the release job's
"minimal links only system libraries" assert to the e2e, so a
unification regression fails at build time with a pointed message
instead of a dyld error mid-test. Net effect of this PR's own-build
switch: CI now catches a mislinked CLI that brew silently tolerated.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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.

3 participants