Skip to content

Commit ce42d94

Browse files
committed
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<VarRequest>` 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<VarRequest>` 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 <jrray@imageworks.com>
1 parent e7dc7f2 commit ce42d94

File tree

2 files changed

+44
-2
lines changed

2 files changed

+44
-2
lines changed

crates/spk-schema/src/option.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -416,6 +416,15 @@ impl VarOpt {
416416
}
417417
}
418418

419+
/// If this option is not already pinned, and it has a non-empty default,
420+
/// pin the default value.
421+
pub fn pin_with_default(&mut self) {
422+
if self.value.is_some() || self.default.is_empty() {
423+
return;
424+
}
425+
self.value = Some(self.default.clone());
426+
}
427+
419428
pub fn set_value(&mut self, value: String) -> Result<()> {
420429
if !self.choices.is_empty() && !value.is_empty() && !self.choices.contains(&value) {
421430
return Err(Error::String(format!(

crates/spk-storage/src/storage/spfs.rs

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ use spk_schema::ident::{AsVersionIdent, VersionIdent};
2626
use spk_schema::ident_build::parsing::embedded_source_package;
2727
use spk_schema::ident_build::{EmbeddedSource, EmbeddedSourcePackage};
2828
use spk_schema::ident_ops::TagPath;
29-
use spk_schema::{AnyIdent, BuildIdent, FromYaml, Package, Recipe, Spec, SpecRecipe};
29+
use spk_schema::{AnyIdent, BuildIdent, FromYaml, Opt, Package, Recipe, Spec, SpecRecipe};
3030
use tokio::io::AsyncReadExt;
3131

3232
use super::CachePolicy;
@@ -504,7 +504,6 @@ impl Storage for SpfsRepository {
504504
{
505505
return v.value().clone().into();
506506
}
507-
508507
let r: Result<Arc<Spec>> = async move {
509508
let tag_path = Self::build_spec_tag(pkg);
510509
let tag_spec = spfs::tracking::TagSpec::parse(tag_path.as_str())?;
@@ -517,6 +516,29 @@ impl Storage for SpfsRepository {
517516
.await
518517
.map_err(|err| Error::FileReadError(filename, err))?;
519518
Spec::from_yaml(&yaml)
519+
.map(|spec| match spec {
520+
Spec::V0Package(mut spec) => {
521+
for opt in spec.build.options.iter_mut() {
522+
let Opt::Var(var_opt) = opt else {
523+
continue;
524+
};
525+
var_opt.pin_with_default()
526+
}
527+
for embedded in spec.install.embedded.iter_mut() {
528+
match embedded {
529+
Spec::V0Package(spec) => {
530+
for opt in spec.build.options.iter_mut() {
531+
let Opt::Var(var_opt) = opt else {
532+
continue;
533+
};
534+
var_opt.pin_with_default()
535+
}
536+
}
537+
}
538+
}
539+
Spec::V0Package(spec)
540+
}
541+
})
520542
.map_err(|err| {
521543
Error::InvalidPackageSpec(Box::new(InvalidPackageSpec(
522544
pkg.to_any_ident(),
@@ -768,6 +790,17 @@ impl crate::Repository for SpfsRepository {
768790
.await
769791
.map_err(|err| Error::FileReadError(tag.target.to_string().into(), err))?;
770792
Spec::from_yaml(yaml)
793+
.map(|spec| match spec {
794+
Spec::V0Package(mut spec) => {
795+
for opt in spec.build.options.iter_mut() {
796+
let Opt::Var(var_opt) = opt else {
797+
continue;
798+
};
799+
var_opt.pin_with_default()
800+
}
801+
Spec::V0Package(spec)
802+
}
803+
})
771804
.map_err(|err| {
772805
Error::InvalidPackageSpec(Box::new(InvalidPackageSpec(
773806
pkg.to_any_ident(),

0 commit comments

Comments
 (0)