Repository navigation
refactor(vm): source runtime pins only from pins.env - #4373
Merged
elezar merged 1 commit intoOct 13, 2026
Merged
Conversation
benoitf
requested review from
a team,
derekwaynecarr,
mrunalp and
sjenning
as code owners
October 9, 2026 13:03
The VM runtime scripts sourced pins.env optionally and duplicated the LIBKRUN_REF and UMOCI_VERSION defaults, so every bump had to update the same value in several files. Require pins.env, drop the duplicated defaults, and fail on an invalid LIBKRUN_REF instead of silently building upstream HEAD. Environment overrides still take precedence through the defaults in pins.env. Signed-off-by: Florent Benoit <fbenoit@redhat.com>
benoitf
force-pushed
the
refactor/dedupe-vm-runtime-pins/benoitf
branch
from
October 9, 2026 13:10
5583ed4 to
ac0d48e
Compare
Contributor
Author
|
cc @elezar @TaylorMutch following discussion #4282 (comment) |
|
Label |
Member
|
/ok-to-test ac0d48e |
elezar
approved these changes
Oct 12, 2026
elezar
left a comment
elezar
left a comment
Member
There was a problem hiding this comment.
Reviewed at ac0d48e. No actionable issues found. Verified Bash syntax, pin defaults and environment overrides, checkout success/fallback/failure handling, and fail-fast behavior when pins.env is missing. Full runtime builds were not run; merge remains contingent on required CI passing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The VM runtime scripts loaded
pins.envoptionally and kept their own copies of theLIBKRUN_REFandUMOCI_VERSIONdefaults, so bumping a pin meant updating the same value in several files. This PR makespins.envthe only place these values are defined. Bumping a pin now touches one file.Related Issue
Follow-up to this review discussion on #4282, which asked why the pin was not specified only once in
pins.env.Changes
pins.envwithout2>/dev/null || truein the 5 VM scripts, so they fail if the file is missing.LIBKRUN_REFdefaults frombuild-libkrun.shandbuild-libkrun-macos.sh.UMOCI_VERSIONdefaults fromcompress-vm-runtime.sh,download-kernel-runtime.sh, andpackage-vm-runtime.sh.build-libkrun.sh, remove the code paths that ran with no pinned ref, now thatLIBKRUN_REFandLIBKRUNFW_REFare always set. An invalidLIBKRUN_REFnow fails at checkout. Before, a trailing|| truelet the build continue on upstream HEAD.Environment overrides such as
LIBKRUN_REF=v1.19.4 tasks/scripts/vm/build-libkrun.shstill work.pins.envassigns each pin asVAR="${VAR:-<default>}", so a value set in the environment takes precedence.Testing
bash -non all VM scripts;pins.envis now the only file undertasks/andcrates/that defines these pins;source pins.envkeeps an env-providedLIBKRUN_REF)Checklist