From b59386d75aafd40bdc59911ed459d78c8b5eb741 Mon Sep 17 00:00:00 2001 From: J Robert Ray Date: Sat, 6 Dec 2025 12:41:31 -0800 Subject: [PATCH] Hotfix packages with unpinned var opts on load Given a package with an embedded package that defines a var with a default value, such as in: ```yaml install: embedded: - pkg: python/3.10.8 build: options: - var: abi/3.10.8 ``` Until the fix in the previous commit, these vars were not getting pinned when the package is built and the stub created: ```yaml pkg: python/3.10.8/embedded[some-parent-pkg:run/3.10.8/P7SZECZD] api: v0/package build: options: - var: python.abi/cp310 ``` This created a problem for solving correctly because `Satisfy` would incorrectly claim that this package is compatible with _any_ value for `python.abi`. In order to fix solves for the existing embedded stubs, these missing pinned values are pinned as the package specs are read from storage, so they end up with the intended content: ```yaml pkg: python/3.10.8/embedded[some-parent-pkg:run/3.10.8/P7SZECZD] api: v0/package build: options: - var: python.abi/cp310 static: cp310 ``` Now `Satisfy` will correctly reject this package as not being compatible with, e.g., `python.abi/cp311`. The best we can do is assume `python.abi` was not overridden to some other value at the time the parent package was built and that the default on the option is the appropriate value for the pin. Signed-off-by: J Robert Ray --- crates/spk-schema/src/option.rs | 9 +++++++ crates/spk-storage/src/storage/spfs.rs | 33 ++++++++++++++++++++++++-- 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/crates/spk-schema/src/option.rs b/crates/spk-schema/src/option.rs index 545d976f90..62144c48da 100644 --- a/crates/spk-schema/src/option.rs +++ b/crates/spk-schema/src/option.rs @@ -416,6 +416,15 @@ impl VarOpt { } } + /// If this option is not already pinned, and it has a non-empty default, + /// pin the default value. + pub fn pin_with_default(&mut self) { + if self.value.is_some() || self.default.is_empty() { + return; + } + self.value = Some(self.default.clone()); + } + pub fn set_value(&mut self, value: String) -> Result<()> { if !self.choices.is_empty() && !value.is_empty() && !self.choices.contains(&value) { return Err(Error::String(format!( diff --git a/crates/spk-storage/src/storage/spfs.rs b/crates/spk-storage/src/storage/spfs.rs index f6fa4375db..2490b9149b 100644 --- a/crates/spk-storage/src/storage/spfs.rs +++ b/crates/spk-storage/src/storage/spfs.rs @@ -26,7 +26,7 @@ use spk_schema::ident::{AsVersionIdent, VersionIdent}; use spk_schema::ident_build::parsing::embedded_source_package; use spk_schema::ident_build::{EmbeddedSource, EmbeddedSourcePackage}; use spk_schema::ident_ops::TagPath; -use spk_schema::{AnyIdent, BuildIdent, FromYaml, Package, Recipe, Spec, SpecRecipe}; +use spk_schema::{AnyIdent, BuildIdent, FromYaml, Opt, Package, Recipe, Spec, SpecRecipe}; use tokio::io::AsyncReadExt; use super::CachePolicy; @@ -504,7 +504,6 @@ impl Storage for SpfsRepository { { return v.value().clone().into(); } - let r: Result> = async move { let tag_path = Self::build_spec_tag(pkg); let tag_spec = spfs::tracking::TagSpec::parse(tag_path.as_str())?; @@ -517,6 +516,25 @@ impl Storage for SpfsRepository { .await .map_err(|err| Error::FileReadError(filename, err))?; Spec::from_yaml(&yaml) + .map(|spec| match spec { + Spec::V0Package(mut spec) => { + for opt in spec.build.options.iter_mut() { + let Opt::Var(var_opt) = opt else { + continue; + }; + var_opt.pin_with_default() + } + for embedded in spec.install.embedded.iter_mut() { + for opt in embedded.build.options.iter_mut() { + let Opt::Var(var_opt) = opt else { + continue; + }; + var_opt.pin_with_default() + } + } + Spec::V0Package(spec) + } + }) .map_err(|err| { Error::InvalidPackageSpec(Box::new(InvalidPackageSpec( pkg.to_any_ident(), @@ -768,6 +786,17 @@ impl crate::Repository for SpfsRepository { .await .map_err(|err| Error::FileReadError(tag.target.to_string().into(), err))?; Spec::from_yaml(yaml) + .map(|spec| match spec { + Spec::V0Package(mut spec) => { + for opt in spec.build.options.iter_mut() { + let Opt::Var(var_opt) = opt else { + continue; + }; + var_opt.pin_with_default() + } + Spec::V0Package(spec) + } + }) .map_err(|err| { Error::InvalidPackageSpec(Box::new(InvalidPackageSpec( pkg.to_any_ident(),