Skip to content

feat(release): ship the VM stack to Linux installs [blocked on pkgs#533] - #988

Closed
norrietaylor wants to merge 8 commits into
mainfrom
fix/980-linux-vm-parity
Closed

feat(release): ship the VM stack to Linux installs [blocked on pkgs#533]#988
norrietaylor wants to merge 8 commits into
mainfrom
fix/980-linux-vm-parity

Conversation

@norrietaylor

@norrietaylor norrietaylor commented Jul 27, 2026

Copy link
Copy Markdown
Member

Blocked, and being rebuilt. Do not review the current diff — it ships a
minvmd that gominimal/pkgs#533
has since shown we should not ship. See "Why this is parked" below.

Part of #980.

What already split off

PR Scope
#994 The switch binary: shipped to Linux (fixes own-ip on installs) and installed as gvproxy-min
#999 Hide --network / --ingress while own-ip is not usable from an install

That leaves this PR as the VM stack: minvmd for both Linux arches, the libkrun it links, and the guest payload.

Why this is parked

This PR ships minvmd as a native-glibc binary with a dynamically-linked libkrun, which is what forced everything else in it:

  • lib/libkrun.so.1 as a shipped component (~29 MB of libraries)
  • scripts/rewrite-linux-linkage.sh + its test harness, to rewrite RUNPATHs for the shipped layout
  • a bin/lib sibling constraint, and an installer warning when a host's prefixes diverge
  • a new glibc version floor on Linux — the first time an install has had one, since min/mip/minimald are static musl

gominimal/pkgs#533 has now proven minvmd can ship as a single static-musl binary: it builds, links, boots a VM, and passes the session e2e — verified against a deliberate stub control, because build.rs silently falls back to a stub that links and runs fine and cannot boot anything.

Every item above then disappears. ~29 MB of shipped libraries becomes one ~8 MB binary, and the glibc floor goes with it. Landing the dynamic version first would mean shipping all of that to users and deleting it days later.

The unblocking discovery was @twitchyliquid64's, in review on this PR: libkrunfw is dead weight, because it exists to carry a bundled GPL-2 kernel and minvmd supplies its own. That mattered beyond one artifact — musl's static libc cannot dlopen, so "libkrun must dlopen libkrunfw" was the sole reason static musl looked unreachable. #533 confirmed it the strong way: with libkrunfw removed system-wide, the dynamic libkrun searched all five loader paths, got ENOENT on every one, and booted anyway.

What comes back

Rebuilt on top of #994, with:

  • minvmd built for *-unknown-linux-musl, statically linked
  • a scripts/build-libkrun-linux.sh mirroring the macOS from-source build (same pin, same patch series, same trim), plus the archive merge/localize pass [aw] sdd-execute (haiku tier) failed #533 worked out
  • no lib/ component, no RUNPATH rewriting, no sibling constraint
  • the guest payload, which is only useful once minvmd ships

Outstanding before that lands: #533 verified on aarch64 only, so x86_64 needs proving — which will happen in CI, since the release lane builds both arches.

Worth keeping from the review here

Both reviews on this PR produced findings that survive the rewrite, and the fixes are already in this branch's history:

  • the nightly ldd assertion could never match glibc's un-normalized $ORIGIN/../lib output (permanently red nightly)
  • pipefail + grep -q silently masking the unresolved-libraries guard
  • the pre-swap daemon stop never targeting a running minvmd on Linux
  • the spec gaps in 07 (lib prefix, R5.5's second stop) and 02 ("libkrun is resolved from the system")

The spec text will need another pass once the static build lands, since it currently documents the shipped-lib/ layout that is going away.

norrietaylor and others added 2 commits July 27, 2026 14:56
Linux installs shipped only min/mip/minimald + AppArmor, so own-ip was
dead on every installed Linux host (minimald resolved a switch binary no
install provides) and VM mode was macOS-only. Most artifacts already
existed in release.yml; they were never mapped to Linux components.

Linux now installs the same session stack macOS does:

- minvmd for BOTH arches. The arm64 build is new: setup-libkrun-linux
  is already arch-parameterized and ubuntu-26.04-arm already runs the
  arm64 lane, so it mirrors the amd64 steps.
- libkrun + libkrunfw, uploaded from the materialized prefix and staged
  under lib/ like the macOS dylib. Nothing produced a redistributable
  Linux libkrun before this.
- the guest payload (vmlinuz, rootfs.img, initramfs.cpio) for each arch,
  which release.yml already built for both and never staged.
- the switch binary, fixing own-ip on native (DM2) Linux.

scripts/rewrite-linux-linkage.sh is the Linux twin of the macOS @rpath
rewrite. Two RUNPATHs matter: minvmd -> $ORIGIN/../lib, and libkrun ->
$ORIGIN, the latter because glibc resolves a dlopen from a library
against the CALLING object's RUNPATH, so a shipped libkrun would
otherwise never find its libkrunfw sibling. It also hard-fails on a
soname bump rather than letting the lib/ dests go quietly stale.

The switch installs as `mingvproxy`, not `gvproxy`: the bin prefix is
~/.local/bin, which is on PATH, and podman/crc ship their own gvproxy
there — under the upstream name whichever landed last would win a PATH
lookup, in either direction. The bytes are stock gvproxy. install.sh
removes a bin/gvproxy left by a pre-rename install when its bytes are
still the ones we recorded writing, so an upgrade doesn't strand the
very collision the rename removes.

switch::installed_gvproxy_bin() is now the one definition of where an
install puts that binary; minimald defaults through it (it previously
only ever looked at the fixed system path, so a user-local install was
invisible to it) and minvmd delegates its lower tiers to it.

nightly's smoke-linux-kvm now consumes the SHIPPED libkrun from that
lib/ layout instead of materializing its own, with no LD_LIBRARY_PATH
and an explicit ldd assertion. Materializing would have masked exactly
the failure this lane exists to catch — the same shape of blind spot
that let #980 sit undetected (CI passed because it fetched gvproxy
itself and set GVPROXY_BIN).

Also drops the --network/--ingress rows from the min activate flag
table.

Refs: #980

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
rewrite-linux-linkage.sh only ever runs on a release runner, so its
RUNPATH logic and hard-fail paths were unverifiable outside a release.
Stub `patchelf` the way verify-nightly-provenance_test.sh stubs `gh`:
canned per-file --print-{needed,rpath,soname} state that --set-rpath
writes back, so no ELF binaries and no patchelf install are needed and
the harness runs on macOS too.

Covers the shapes build.rs emits (bare $ORIGIN, $ORIGIN + system dirs,
empty), idempotence on re-run, and each failure that would otherwise
ship a broken install: a stub-linked minvmd with no libkrun DT_NEEDED,
a libkrun/libkrunfw soname bump that would strand the lib/ dests, and
an ephemeral build prefix leaking into the shipped RUNPATH.

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

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR standardizes gvproxy-min discovery and installation, stages Linux minvmd runtime artifacts with verified RUNPATHs and SONAMEs, adds installer migration handling, and expands release, nightly smoke-test, and linkage-harness coverage.

Changes

Installed gvproxy resolution

Layer / File(s) Summary
Shared gvproxy path resolution
crates/switch/*, crates/minvmd/src/image.rs, crates/minimald/src/*, crates/minimal/src/diag/mod.rs
Installed proxy lookup now prefers user-local gvproxy-min, supports system fallbacks, and is shared by minvmd and minimald. Diagnostics recognize the renamed process.

Release runtime packaging

Layer / File(s) Summary
Linux linkage rewrite and validation
scripts/rewrite-linux-linkage*.sh, crates/common/tests/script_harnesses.rs, justfile
RUNPATH and SONAME checks validate shipped minvmd and libkrun layouts, including failure and idempotency scenarios.
Platform artifact staging and smoke layouts
.github/workflows/{release,nightly}.yml, scripts/stage-release.sh
Linux builds upload patched runtime libraries, manifests include session artifacts and guest payloads, and smoke lanes assemble sibling bin/ and lib/ layouts using gvproxy-min.

Installer migration

Layer / File(s) Summary
Installer replacement and rename migration
scripts/install.sh, scripts/install_test.sh
Installer updates warn about incompatible bin/lib locations, stop both daemon backends, and safely remove or preserve legacy gvproxy files during upgrades.

Documentation alignment

Layer / File(s) Summary
Release, installer, and CLI documentation
docs/internal/*, docs/reference/*, docs/specs/*
Documentation records the shipped runtime layout, linkage constraints, installer migration behavior, renamed proxy defaults, and activation synchronization option.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ReleaseWorkflow
  participant LinkageScript
  participant ArtifactStore
  participant Installer
  participant minvmd
  ReleaseWorkflow->>LinkageScript: rewrite and verify shipped linkage
  LinkageScript-->>ReleaseWorkflow: return validated RUNPATH and SONAMEs
  ReleaseWorkflow->>ArtifactStore: upload minvmd and libkrun
  Installer->>ArtifactStore: install bin and lib components
  minvmd->>Installer: resolve installed gvproxy-min and sibling libraries
Loading

Possibly related issues

  • Issue 687 — The shipped-artifact smoke-test changes directly implement its validation objective.

Possibly related PRs

Suggested reviewers: twitchyliquid64

Poem

A rabbit packed libraries tight,
With RUNPATH trails aligned just right.
“gvproxy-min” now leads the way,
While old names safely hop away.
Tests thump their paws: “All clear!”

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is off-template and omits the required Summary, Testing, and Checklist sections. Rewrite it to match the template, adding Summary, Testing evidence, and Checklist items for docs and BREAKING CHANGE.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR covers the Linux-install gaps: it stages gvproxy-min, minvmd for amd64/arm64, libkrun, and guest payloads, addressing #980.
Out of Scope Changes check ✅ Passed The added docs, tests, and installer cleanup all support the Linux release and rename work, with no clear unrelated changes.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Title check ✅ Passed The title is specific and matches the main change: shipping the VM stack to Linux installs.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

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

norrietaylor and others added 2 commits July 27, 2026 15:43
shellcheck 0.9.0/0.10.0 flag `A && B || C` inside the record loop; the
locally-installed 0.11.0 relaxed that check, so the pattern passed here
and failed both the dedicated --shell=sh job and the shell_lint gate on
CI's older binary. Two plain guards say the same thing unambiguously.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Renames the installed name chosen in the parent commit from `mingvproxy`
to `gvproxy-min`, so it sorts and reads next to the upstream tool it is.
Same rationale, same collision it avoids: the bin prefix is ~/.local/bin
and podman/crc ship their own `gvproxy` there.

Installed name only — the release artifacts stay `gvproxy-*`, since the
bytes are stock gvproxy.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@norrietaylor norrietaylor changed the title feat(release): ship the VM stack to Linux installs; rename gvproxy -> mingvproxy feat(release): ship the VM stack to Linux installs; rename gvproxy -> gvproxy-min Jul 27, 2026
@norrietaylor
norrietaylor marked this pull request as ready for review July 27, 2026 22:57
Round one — defects in this PR's own new code:

- nightly's shipped-layout assertion could never pass. ld.so expands
  $ORIGIN but does not normalize the rest of the entry, so it reports
  <bin>/../lib/libkrun.so.1 verbatim; grepping for the canonical
  "$LIB/libkrun.so.1" never matched, which would have made the Linux VM
  smoke permanently red and blocked the nightly channel cut. Now
  canonicalizes both sides. Its sibling guard also piped into `grep -q`
  under `set -o pipefail`, where an early grep exit reports SIGPIPE and
  silently skips the check; ldd output is captured once instead.
- the gvproxy migration deleted a binary the same run had just
  installed. It keyed only off the PREVIOUS record, so a post-rename
  installer pointed at a still-shipping manifest — which channels
  advancing independently guarantees — removed the file every run,
  leaving no switch at all. Skips when this run installed a gvproxy
  component.
- that migration also got exactly one attempt (the record it reads is
  replaced immediately after) and swallowed a failed `rm` via an
  AND-list. A failure is now reported and the row carried forward so
  the next run retries.
- the soname check pattern-matched the whole resolved path, so a lib
  dir whose path contained "krunfw" compared libkrun against the wrong
  soname and aborted the release citing a bump that never happened.
  Explicit (file, soname) pairs now.
- the ephemeral-build-prefix guard applied only to minvmd. We prepend
  to libkrun's own RUNPATH rather than replacing it, so an upstream
  build path would ship to users unexamined. Same check both sides.
- the pre-swap daemon stop ran bare `min stop`, which resolves the
  DEFAULT provider (local-minimald on Linux) and so never stopped a
  running minvmd whose binary and libkrun were about to be replaced.

Round three — gaps the review is right about:

- make the linkage harness a real gate. `just test-linkage` was
  reachable from no CI lane, so a regression in the RUNPATH logic was
  green on all five aggregators. crates/common/tests/script_harnesses.rs
  drives it from the workspace suite, the same convention-discovered
  extension point shell_lint.rs uses (the workflow YAML is frozen).
- honour the pre-rename system-wide path. That directory is not on
  PATH, so the collision rationale never applied to it, and an operator
  who provisioned it has no install record to migrate — renaming
  without a fallback would silently break their own-IP.
- warn when the bin and lib prefixes are not siblings. They resolve
  from unrelated knobs (MINIMAL_BIN vs XDG_LIB_HOME) while minvmd finds
  libkrun through a binary-relative rpath, so a mismatched pair fails
  at autospawn with an opaque loader error.
- minimald's --help still promised the old fixed-path default.

Not addressed here, by design: the glibc floor and the unconditional
guest-payload download are product decisions, tracked separately
(gominimal/pkgs#533 covers musl libkrun for the former).

Refs: #980
Refs: https://github.com/gominimal/pkgs/issues/533

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (2)
scripts/install_test.sh (1)

947-1015: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Cover failed removal and retry behavior.

The suite does not exercise the record re-queue path in scripts/install.sh lines 837-844. Stub a first rm failure, then verify the next install retries and removes the legacy binary; otherwise a regression could leave gvproxy on PATH permanently.

🤖 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 `@scripts/install_test.sh` around lines 947 - 1015, Extend the rename-migration
tests around the gvproxy scenarios to simulate a failed first removal, using the
existing rm-failure stubbing mechanisms. Verify the initial install exits as
expected and preserves or re-queues the legacy gvproxy record, then run a
subsequent install without the failure and assert that gvproxy is removed and
the rename message is emitted.

Source: Coding guidelines

.github/workflows/release.yml (1)

206-241: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift

Duplicated rewrite/stage/upload logic between amd64 and arm64 jobs.

The "Rewrite + verify linkage", "Stage libkrun pair for upload", and the three upload steps are near-identical copies differing only by arch suffix. A future fix to this logic (e.g., soname handling, path resolution) risks being applied to only one architecture. Consider extracting this into a composite action (parameterized by arch), similar to the existing ./.github/actions/setup-libkrun-linux.

Also applies to: 319-363

🤖 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 around lines 206 - 241, Extract the duplicated
Linux libkrun rewrite, staging, and three artifact-upload steps from the amd64
and arm64 jobs into a parameterized composite action, following the existing
setup-libkrun-linux action pattern. Pass the architecture-specific binary path,
libkrun prefix, artifact names, and platform suffixes through action inputs,
then replace both job-local copies with calls to the shared action while
preserving their current outputs and behavior.
🤖 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 `@crates/minimald/src/server.rs`:
- Around line 64-67: The gvproxy binary path fallback in
ServerState::gvproxy_bin_path performs synchronous filesystem probing on an
async worker. Resolve installed_gvproxy_bin through tokio::task::spawn_blocking
in ServerState::new before constructing the switch client, and pass the resolved
path into the initialization flow while preserving the explicitly configured
gvproxy_bin path.

In `@crates/minvmd/src/image.rs`:
- Around line 116-124: The gvproxy resolver documentation and tests assume only
the renamed system path, but switch::installed_gvproxy_bin() can fall back to
the legacy /usr/lib/minimal/bin/gvproxy path. Update the shared resolver’s
filesystem probing to be injectable, then test both the renamed and legacy
fallback branches deterministically without relying on host filesystem state.
Revise the resolver documentation to describe both possible system-path outcomes
and adjust the unconditional tier-3 assertion accordingly.

In `@crates/switch/src/lib.rs`:
- Around line 379-385: Update installed_gvproxy_bin and the
installed_gvproxy_falls_back_to_system_path test so system-path existence is
accessed through a testable helper rather than directly probing the host
filesystem. Allow the test to control that helper and deterministically cover
both the legacy /usr/lib/minimal/bin/gvproxy path and DEFAULT_GVPROXY_BIN
without depending on installed host binaries.

---

Nitpick comments:
In @.github/workflows/release.yml:
- Around line 206-241: Extract the duplicated Linux libkrun rewrite, staging,
and three artifact-upload steps from the amd64 and arm64 jobs into a
parameterized composite action, following the existing setup-libkrun-linux
action pattern. Pass the architecture-specific binary path, libkrun prefix,
artifact names, and platform suffixes through action inputs, then replace both
job-local copies with calls to the shared action while preserving their current
outputs and behavior.

In `@scripts/install_test.sh`:
- Around line 947-1015: Extend the rename-migration tests around the gvproxy
scenarios to simulate a failed first removal, using the existing rm-failure
stubbing mechanisms. Verify the initial install exits as expected and preserves
or re-queues the legacy gvproxy record, then run a subsequent install without
the failure and assert that gvproxy is removed and the rename message is
emitted.
🪄 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: 44007716-83e6-4376-9e97-fc5d8a1c7c89

📥 Commits

Reviewing files that changed from the base of the PR and between f633a2a and 83be120.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (18)
  • .github/workflows/nightly.yml
  • .github/workflows/release.yml
  • crates/common/tests/script_harnesses.rs
  • crates/minimal/src/diag/mod.rs
  • crates/minimald/src/main.rs
  • crates/minimald/src/server.rs
  • crates/minvmd/src/image.rs
  • crates/switch/Cargo.toml
  • crates/switch/src/lib.rs
  • docs/internal/release-pipeline.md
  • docs/reference/cli-min.md
  • docs/reference/cli-minimald.md
  • justfile
  • scripts/install.sh
  • scripts/install_test.sh
  • scripts/rewrite-linux-linkage.sh
  • scripts/rewrite-linux-linkage_test.sh
  • scripts/stage-release.sh
💤 Files with no reviewable changes (1)
  • docs/reference/cli-min.md

Comment thread crates/minimald/src/server.rs
Comment on lines +116 to 124
/// Resolve the host gvproxy binary path: the `MINVMD_GVPROXY_BIN` override,
/// then the installed location — the user-local `bin/gvproxy-min` the curl|sh
/// installer stamps, else the system-wide `switch::DEFAULT_GVPROXY_BIN`
/// ([`switch::installed_gvproxy_bin`], the definition shared with minimald).
///
/// Unlike the kernel/rootfs/initramfs resolvers this never errors: gvproxy is
/// only spawned for an own-IP VM, so an absent override is the common case, and
/// the system path is returned as a last resort even if nothing exists there
/// (the caller surfaces a spawn failure with that concrete path).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Document and test the legacy-system fallback.

switch::installed_gvproxy_bin() may return the pre-rename /usr/lib/minimal/bin/gvproxy when gvproxy-min is absent. The current docs and unconditional tier-3 assertion incorrectly describe the renamed system path as the only fallback, causing failures on hosts the compatibility branch is meant to support. Make the shared resolver’s filesystem probe injectable and cover both branches deterministically.

Also applies to: 261-264

🤖 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 `@crates/minvmd/src/image.rs` around lines 116 - 124, The gvproxy resolver
documentation and tests assume only the renamed system path, but
switch::installed_gvproxy_bin() can fall back to the legacy
/usr/lib/minimal/bin/gvproxy path. Update the shared resolver’s filesystem
probing to be injectable, then test both the renamed and legacy fallback
branches deterministically without relying on host filesystem state. Revise the
resolver documentation to describe both possible system-path outcomes and adjust
the unconditional tier-3 assertion accordingly.

Comment thread crates/switch/src/lib.rs Outdated
The legacy-path fallback added in the previous commit made three
assertions depend on what the host has under /usr/lib/minimal/bin: they
pinned DEFAULT_GVPROXY_BIN while the resolver returns the legacy path
when only that exists — so they failed on exactly the pre-rename hosts
the fallback was written to serve.

Split the resolution into `resolve_installed(bin_dir, system, legacy)`
with every probed location passed in, and drive the system-path branches
from temp dirs. Public API is unchanged. Both branches are now covered
deterministically (current wins when both exist; legacy when only it
does; current named when neither does; user-local beats both), which the
host-probing version could not express at all.

minvmd's tier-3 test asserts the result is under the system bin dir
rather than one exact path; `switch`'s own tests pin which branch wins,
and that test only needs to prove the tier-2 miss falls through.

Also documents the legacy branch on minvmd's resolver, which described
the renamed path as the only system fallback.

Reported by CodeRabbit on #988.

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

Copy link
Copy Markdown
Member

libkrun.so.1 → $ORIGIN (finds its libkrunfw.so.5 sibling)

There is no libkrunfw as we bring our own kernel

Comment thread .github/workflows/nightly.yml Outdated
chmod +x "$BIN/min" "$BIN/minvmd" "$BIN/mip" "$BIN/gvproxy"
mv "$BIN/gvproxy-linux-amd64" "$BIN/gvproxy-min"
mv "$BIN/libkrun-linux-amd64.so" "$LIB/libkrun.so.1"
mv "$BIN/libkrunfw-linux-amd64.so" "$LIB/libkrunfw.so.5"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We bring our own kernel so shouldnt need libkrunfw - its purpose is to contain the GPL-2 kernel

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That matches what I can see from here, and it has bigger consequences than this line — flagging them rather than quietly dropping the artifact.

Corroborating evidence:

  • minvmd hands libkrun its own kernel explicitly (ctx.set_kernel(&self.kernel_path, …), crates/minvmd/src/vm.rs:237), which is the GPL-2 payload libkrunfw exists to provide.
  • Nothing in crates/minvmd/src/ references libkrunfw at all — the only mentions are in build.rs comments, the crate README and the packaging scripts.
  • crates/minvmd/README.md:100 already records that it is loaded "from the prefix (it is not a link-time DT_NEEDED)".
  • macOS already ships libkrun with no libkrunfw: stage-release.sh has exactly one darwin libkrun row (lib/libkrun.1.dylib), and the macOS e2e + nightly smoke boot a real VM from that layout. So Linux shipping libkrunfw is the inconsistency, not the other way round.

If you're right, three things fall out beyond this assertion:

  1. Drop the libkrunfw rows/uploads entirely — one fewer artifact per arch, and one fewer soname to keep in lockstep.
  2. scripts/rewrite-linux-linkage.sh no longer needs to set $ORIGIN on libkrun's own RUNPATH; that entry exists only so its dlopen of libkrunfw finds the shipped sibling.
  3. It removes the blocker I filed in gominimal/pkgs#533 for building minvmd as static musl — musl's static libc can't dlopen, so "libkrun must dlopen libkrunfw" was the thing standing in the way of dropping the new glibc floor entirely. If libkrunfw is unnecessary, static musl is back on the table.

The one thing I can't confirm from macOS is whether Linux libkrun probes for libkrunfw at init and hard-fails when it is absent, versus only touching it when no external kernel was set. Homebrew has libkrunfw installed here, so the local macOS boot doesn't isolate it.

Happy to drop it in this PR — the nightly Linux smoke now runs with no LD_LIBRARY_PATH and no materialize step, so a wrong guess fails loudly there rather than silently. Want me to, or would you rather confirm against a Linux host first?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m fairly sure it doesn’t prove for libkrunfw

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*probe

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry my agent got carried away here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dropped in 403c784 — thanks, this was worth more than the one line it was on.

Gone: the artifact, both uploads, the lib/libkrunfw.so.5 staging rows, and the nightly smoke's placement of it. Also gone is the machinery that existed only for it:

  • libkrun's $ORIGIN RUNPATH. Its sole justification was that libkrun dlopens libkrunfw by soname and glibc resolves that against the calling object's RUNPATH. With nothing to find, the entry was pointless — the shipped .so is now left verbatim and only leak-checked.
  • The (file, soname) dispatch I'd added an hour earlier to fix a path-vs-basename bug in the soname check. With one library there is no dispatch, so that bug is now unreachable by construction and its regression test went with it.

The larger consequence is in gominimal/pkgs#533, which I've updated: this unblocks static-musl minvmd. The chain was musl's static libc can't dlopen → libkrun dlopens libkrunfw → so static musl could never get its kernel payload. With no dlopen on the path, minvmd can be fully static musl rather than the musl-dynamic fallback that issue was hedging on — which would retire the glibc floor entirely instead of tracking it, and take the lib/ component and the bin/lib sibling constraint with it.

Also folded in @norrietaylor's point that we already build our own libkrun from source on macOS (build-libkrun-macos.sh, vendored + patched, pinned in vendor/libkrun/libkrun.lock), so the musl build reuses an existing pipeline rather than starting one.

Comment thread scripts/install.sh
# replaces it.
bindir="$(resolve_prefix bin)"

# minvmd finds its libkrun through a BINARY-RELATIVE rpath (`$ORIGIN/../lib` on

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might want to update the corresponding spec for the installer (07 iirc) with this logic as well

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh good call!

Spec 07 (installer):

- R4.1's prefix table omitted `lib` entirely, so read literally a `lib`
  dest was an unknown token that must exit with an error. Both platforms
  now ship `lib/` components.
- New R4.1a for the bin/lib sibling constraint. minvmd finds libkrun
  through a binary-relative rpath, but the two prefixes resolve from
  unrelated variables, so a host can set one and not the other. Records
  why the mismatch warns rather than fails: it breaks only VM-backed
  sessions, and diagnosing it at install time beats an opaque loader
  error at autospawn.
- R5.5 and its proof artifact said the daemon stop runs "exactly `stop
  --force`, once". It now runs once per backend, because bare `min stop`
  resolves the default provider and would leave a live minvmd holding
  its socket while its own binary and libkrun are swapped.
- New R6.1a for the renamed-component migration, which had four tests
  and no requirement to trace to. Covers the three rules that make it
  safe: skip when the manifest still ships the old name (a pre-rename
  manifest on an independently-advancing channel would otherwise have us
  delete the binary we just installed), report a refusal, retry a
  failure.
- R8.1's prune list omitted `lib`, which is shared like `bin` and so is
  rmdir'd-if-empty rather than purged.

Spec 02 (minvmd on Linux/KVM): "libkrun installation on Linux" said
libkrun is resolved from the host's system paths. That is now only the
dev/CI story — an installed host resolves nothing from the system and
loads the shipped libkrun through a binary-relative rpath. Splits the
two cases and states the consequences that are easy to get wrong: no
absolute build prefix may survive into a shipped binary, the lib prefix
must stay a bin sibling, and shipping libkrun sets a glibc floor
(gominimal/pkgs#533).

Spec 01: the libkrun-linking note described the absolute Homebrew rpath
as the whole story; marked as development-only with a pointer to 02.

Spec 08: two citations pointed at scripts/stage-release.sh:118, which
this PR's new COMPONENTS rows moved. Repointed by name instead of line.

Pre-existing drift found while surveying and deliberately NOT touched
here (it predates this PR and would bury the diff): spec 07's macOS
ad-hoc-codesigning narrative, spec 01/02's "Linux is a stub / autospawn
is a no-op" statements, and spec 03's source-build-gvproxy lines.

Raised by @twitchyliquid64 in review.

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 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 `@docs/specs/02-spec-minvmd-linux-kvm/02-spec-minvmd-linux-kvm.md`:
- Around line 316-320: Revise the opening claim in the release-resolution
paragraph to apply only to libkrun: state that the installed binary resolves
libkrun from the shipped sibling lib directory via the binary-relative rpath. Do
not imply that minvmd has no host dependencies, since its glibc and KVM
requirements remain unchanged.

In `@docs/specs/07-spec-installer/07-spec-installer.md`:
- Around line 283-286: Update the R5.5 code fence containing the `/min stop
--force` commands to specify the shell language by changing its opening fence to
```sh, while leaving the commands unchanged.
- Line 193: Update scripts/install.sh’s resolve_prefix handling for the lib
prefix to reject any non-absolute XDG_LIB_HOME override, such as “lib”. Perform
this validation before the component loop and fail immediately, while preserving
valid absolute overrides and the existing default path behavior.
🪄 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: dd29b71c-4216-4495-829d-904115e00776

📥 Commits

Reviewing files that changed from the base of the PR and between 83be120 and 146ec31.

📒 Files selected for processing (7)
  • crates/minvmd/src/image.rs
  • crates/switch/src/lib.rs
  • docs/specs/01-spec-minvmd-host-daemon/01-spec-minvmd-host-daemon.md
  • docs/specs/02-spec-minvmd-linux-kvm/02-spec-minvmd-linux-kvm.md
  • docs/specs/07-spec-installer/07-spec-installer.md
  • docs/specs/08-spec-vm-ext4-volume/08-spec-vm-ext4-volume.md
  • docs/specs/08-spec-vm-ext4-volume/architecture.md
🚧 Files skipped from review as they are similar to previous changes (2)
  • crates/minvmd/src/image.rs
  • crates/switch/src/lib.rs

Comment thread docs/specs/02-spec-minvmd-linux-kvm/02-spec-minvmd-linux-kvm.md
| token | resolves to |
|---------|---------------------------------------------------------|
| `bin` | `${MINIMAL_BIN:-$HOME/.local/bin}` |
| `lib` | `${XDG_LIB_HOME:-$HOME/.local/lib}` |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

set -euo pipefail

printf '\n== Spec excerpt ==\n'
sed -n '180,210p' docs/specs/07-spec-installer/07-spec-installer.md

printf '\n== Search for resolve_prefix and XDG_LIB_HOME ==\n'
rg -n -A4 -B4 'resolve_prefix|XDG_LIB_HOME|XDG_LIB_HOME:-|absolute prefix|absolute.*prefix|prefix override|install destinations' .

Repository: gominimal/minimal

Length of output: 22801


🏁 Script executed:

set -euo pipefail

printf '\n== Files mentioning installer spec ==\n'
git ls-files | rg '07-spec-installer|installer|resolve_prefix|prefix' 

printf '\n== Outline candidate files if found ==\n'
for f in $(git ls-files | rg 'resolve_prefix|installer'); do
  [ -f "$f" ] && echo "--- $f ---" && wc -l "$f" && sed -n '1,220p' "$f" | head -n 220
done

Repository: gominimal/minimal

Length of output: 16723


🏁 Script executed:

set -euo pipefail

printf '\n== Search for XDG_LIB_HOME in repository ==\n'
rg -n -A6 -B6 'XDG_LIB_HOME' .

printf '\n== Search for resolve_prefix symbol ==\n'
rg -n -A8 -B8 'resolve_prefix' .

Repository: gominimal/minimal

Length of output: 29212


Reject relative XDG_LIB_HOME overrides. In scripts/install.sh, resolve_prefix lib still accepts values like XDG_LIB_HOME=lib, which makes the lib prefix relative to the current directory instead of absolute. Validate the override up front and fail before the component loop.

🤖 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 `@docs/specs/07-spec-installer/07-spec-installer.md` at line 193, Update
scripts/install.sh’s resolve_prefix handling for the lib prefix to reject any
non-absolute XDG_LIB_HOME override, such as “lib”. Perform this validation
before the component loop and fail immediately, while preserving valid absolute
overrides and the existing default path behavior.

Comment on lines +283 to +286
```
<bin>/min stop --force
<bin>/min --provider local-minvmd stop --force
```

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Specify the shell language for the R5.5 fence.

Change the opening fence to ```sh; markdownlint reports the current untyped fence.

🧰 Tools
🪛 markdownlint-cli2 (0.23.0)

[warning] 283-283: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 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 `@docs/specs/07-spec-installer/07-spec-installer.md` around lines 283 - 286,
Update the R5.5 code fence containing the `/min stop --force` commands to
specify the shell language by changing its opening fence to ```sh, while leaving
the commands unchanged.

Source: Linters/SAST tools

libkrunfw exists to carry a bundled GPL-2 guest kernel. minvmd supplies
its own via `ctx.set_kernel` from the shipped data/vmlinuz, so libkrunfw
is neither linked nor loaded — macOS has shipped libkrun alone all
along, which made Linux the odd one out rather than the other way round.

Drops the artifact, its uploads, its staging rows, and the machinery
that existed only for it:

- the libkrun `$ORIGIN` RUNPATH. Its sole justification was that
  libkrun dlopen()s libkrunfw by soname and glibc resolves that against
  the calling object's RUNPATH; with nothing to find, the entry is
  pointless. The shipped .so is now left verbatim and only leak-checked.
- the libkrunfw soname check, and with it the (file, soname) dispatch
  the last commit introduced — one library needs no dispatch, so the
  path-vs-basename bug it fixed is now unreachable by construction and
  its regression test went with it.
- the nightly smoke's libkrunfw placement.

This also removes the blocker recorded in gominimal/pkgs#533: musl's
static libc cannot dlopen, so "libkrun must dlopen libkrunfw" was the
thing standing between us and a static-musl minvmd — and therefore
between us and dropping the new Linux glibc floor entirely.

Caught by @twitchyliquid64 in review.

Refs: https://github.com/gominimal/pkgs/issues/533

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
.github/workflows/release.yml (1)

123-125: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift

Move this workflow change out of the frozen CI layer.

These additions modify .github/workflows/release.yml, which repository policy reserves for CODEOWNER-gated scheduler maintenance. Put the verification/staging logic in existing scripts/ or justfile entry points, or obtain the required CI-owner review before merging.

As per coding guidelines, “Do not edit files under .github/workflows/; the workflows are frozen and CODEOWNER-gated.”

Also applies to: 198-233, 261-262, 311-348

🤖 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 around lines 123 - 125, Move the added release
verification, staging, and dependency-installation logic out of
.github/workflows/release.yml into existing scripts/ or justfile entry points,
updating the workflow only to invoke those shared entry points if necessary.
Preserve the behavior associated with the patchelf, staging, and verification
steps, and do not modify the frozen workflow unless required CODEOWNER approval
is obtained.

Sources: Coding guidelines, Learnings

🤖 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.

Outside diff comments:
In @.github/workflows/release.yml:
- Around line 123-125: Move the added release verification, staging, and
dependency-installation logic out of .github/workflows/release.yml into existing
scripts/ or justfile entry points, updating the workflow only to invoke those
shared entry points if necessary. Preserve the behavior associated with the
patchelf, staging, and verification steps, and do not modify the frozen workflow
unless required CODEOWNER approval is obtained.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e534c772-d322-413b-9ecc-9110ee02f4bf

📥 Commits

Reviewing files that changed from the base of the PR and between 146ec31 and 403c784.

📒 Files selected for processing (7)
  • .github/workflows/nightly.yml
  • .github/workflows/release.yml
  • docs/internal/release-pipeline.md
  • docs/specs/02-spec-minvmd-linux-kvm/02-spec-minvmd-linux-kvm.md
  • scripts/rewrite-linux-linkage.sh
  • scripts/rewrite-linux-linkage_test.sh
  • scripts/stage-release.sh
🚧 Files skipped from review as they are similar to previous changes (4)
  • scripts/stage-release.sh
  • .github/workflows/nightly.yml
  • docs/specs/02-spec-minvmd-linux-kvm/02-spec-minvmd-linux-kvm.md
  • docs/internal/release-pipeline.md

@norrietaylor
norrietaylor marked this pull request as draft July 28, 2026 03:35
norrietaylor added a commit that referenced this pull request Jul 28, 2026
The installer's `bin` prefix is `~/.local/bin`, which is on PATH, and
podman/crc/Docker Desktop all ship their own `gvproxy` there. Under the
upstream name whichever was installed last wins a PATH lookup — in
either direction — so we take a minimal-specific name. The bytes are
stock gvproxy, still pinned and SHA-256-verified by fetch-gvproxy.sh;
only the installed name changes, and the release artifacts keep theirs.

`switch::installed_gvproxy_bin()` becomes the one definition of where an
install puts that binary, in the crate whose charter is already "one
definition rather than drifting copies". This fixes a real bug on the
way past: minimald only ever looked at the fixed system path, so a
user-local install was invisible to it and own-IP could not find a
switch it had. minvmd already had that tier and now delegates to the
shared one.

The system-wide fallback moves too, but that directory is not on PATH,
so the collision rationale never applied there — and an operator who
provisioned the old path has no install record for us to migrate. The
resolver therefore still honours `/usr/lib/minimal/bin/gvproxy` when
only that exists.

install.sh reverses the rename from the prior record, on the same
bytes-still-ours terms uninstall uses: the old file is removed only when
its hash still matches what we recorded writing, so a user-replaced
binary is kept and reported. Three rules keep that safe, all covered by
the harness and specified as R6.1a:

- skip entirely when the manifest THIS run installed still ships a
  `gvproxy` component. Channels advance independently, so a post-rename
  installer will be pointed at a pre-rename manifest, where the file on
  disk is the one we just placed — deleting it would leave the host with
  no switch binary at all, on every run.
- report a refusal rather than deleting a modified file.
- carry the row forward when removal fails, so the next run retries
  instead of stranding the binary the rename exists to remove.

Split out of #988 to keep that PR to release packaging; it depends on
none of it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
norrietaylor added a commit that referenced this pull request Jul 28, 2026
The installer's `bin` prefix is `~/.local/bin`, which is on PATH, and
podman/crc/Docker Desktop all ship their own `gvproxy` there. Under the
upstream name whichever was installed last wins a PATH lookup — in
either direction — so we take a minimal-specific name. The bytes are
stock gvproxy, still pinned and SHA-256-verified by fetch-gvproxy.sh;
only the installed name changes, and the release artifacts keep theirs.

`switch::installed_gvproxy_bin()` becomes the one definition of where an
install puts that binary, in the crate whose charter is already "one
definition rather than drifting copies". This fixes a real bug on the
way past: minimald only ever looked at the fixed system path, so a
user-local install was invisible to it and own-IP could not find a
switch it had. minvmd already had that tier and now delegates to the
shared one.

The system-wide fallback moves too, but that directory is not on PATH,
so the collision rationale never applied there — and an operator who
provisioned the old path has no install record for us to migrate. The
resolver therefore still honours `/usr/lib/minimal/bin/gvproxy` when
only that exists.

install.sh reverses the rename from the prior record, on the same
bytes-still-ours terms uninstall uses: the old file is removed only when
its hash still matches what we recorded writing, so a user-replaced
binary is kept and reported. Three rules keep that safe, all covered by
the harness and specified as R6.1a:

- skip entirely when the manifest THIS run installed still ships a
  `gvproxy` component. Channels advance independently, so a post-rename
  installer will be pointed at a pre-rename manifest, where the file on
  disk is the one we just placed — deleting it would leave the host with
  no switch binary at all, on every run.
- report a refusal rather than deleting a modified file.
- carry the row forward when removal fails, so the next run retries
  instead of stranding the binary the rename exists to remove.

Split out of #988 to keep that PR to release packaging; it depends on
none of it.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
@norrietaylor norrietaylor changed the title feat(release): ship the VM stack to Linux installs; rename gvproxy -> gvproxy-min feat(release): ship the VM stack to Linux installs [blocked on pkgs#533] Jul 29, 2026
@norrietaylor
norrietaylor deleted the fix/980-linux-vm-parity branch July 30, 2026 22:34
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