From d12364f949b32c4a135525799065cce667a8d6e3 Mon Sep 17 00:00:00 2001 From: J Robert Ray Date: Mon, 30 Mar 2026 10:33:50 -0700 Subject: [PATCH] Add suppress for strong var inheritance Allow downstream packages to opt out of inheriting strongly-inherited variables by adding a suppress directive to install requirements: install: requirements: - var: base.inherit-me suppress: "reason for suppressing" The suppress directive prevents the variable from being added to the built package's runtime requirements and tells the InheritRequirements validator to skip checking for it. Closes #1324. Co-Authored-By: Claude Opus 4.6 Signed-off-by: J Robert Ray --- crates/spk-build/src/build/binary.rs | 1 + crates/spk-build/src/report.rs | 6 +- .../validation/alter_existing_files_test.rs | 1 + .../src/validation/collect_all_files_test.rs | 1 + .../validation/collect_existing_files_test.rs | 1 + .../src/validation/empty_package_test.rs | 1 + .../src/validation/inherit_requirements.rs | 6 ++ .../validation/inherit_requirements_test.rs | 2 + .../validation/long_var_description_test.rs | 1 + .../src/validation/recursive_build_test.rs | 1 + .../src/validation/spdx_license_test.rs | 1 + .../strong_inheritance_var_desc_test.rs | 1 + .../cmd-build/src/cmd_build_test/mod.rs | 97 ++++++++++++++++++- .../foundation/src/ident/pinnable_request.rs | 39 +++++++- .../foundation/src/ident/pinned_request.rs | 3 + crates/spk-schema/src/recipe.rs | 13 ++- crates/spk-schema/src/requirements_list.rs | 5 + crates/spk-schema/src/spec.rs | 6 ++ crates/spk-schema/src/v0/recipe_spec.rs | 25 +++++ docs/ref/api/v0/package.md | 1 + 20 files changed, 208 insertions(+), 4 deletions(-) diff --git a/crates/spk-build/src/build/binary.rs b/crates/spk-build/src/build/binary.rs index 229e8d871a..c70f5f0356 100644 --- a/crates/spk-build/src/build/binary.rs +++ b/crates/spk-build/src/build/binary.rs @@ -392,6 +392,7 @@ where package, variant: full_variant, environment_filesystem, + suppressed_requirements: self.recipe.suppressed_requirements(), }; let mut report = BuildReport { setup, diff --git a/crates/spk-build/src/report.rs b/crates/spk-build/src/report.rs index db217a2a39..55247c0e63 100644 --- a/crates/spk-build/src/report.rs +++ b/crates/spk-build/src/report.rs @@ -2,9 +2,10 @@ // SPDX-License-Identifier: Apache-2.0 // https://github.com/spkenv/spk -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use spk_schema::foundation::ident_component::Component; +use spk_schema::foundation::name::OptNameBuf; use spk_schema::{BuildIdent, Package, Variant}; use spk_solve::Solution; @@ -39,6 +40,9 @@ where /// where each entry is tagged with the package that owns /// it within the build environment. pub environment_filesystem: spfs::tracking::Manifest, + /// Var names that the recipe explicitly suppresses from strong + /// inheritance, used by the InheritRequirements validator. + pub suppressed_requirements: HashSet, } /// Details about the generated files and resulting output of diff --git a/crates/spk-build/src/validation/alter_existing_files_test.rs b/crates/spk-build/src/validation/alter_existing_files_test.rs index 433ddabd10..e09ae1007d 100644 --- a/crates/spk-build/src/validation/alter_existing_files_test.rs +++ b/crates/spk-build/src/validation/alter_existing_files_test.rs @@ -34,6 +34,7 @@ async fn test_validate_build_changeset_modified() { environment_filesystem: Manifest::new( spfs::tracking::Entry::empty_dir_with_open_perms_with_data(package.ident().clone()), ), + suppressed_requirements: Default::default(), package, }, }; diff --git a/crates/spk-build/src/validation/collect_all_files_test.rs b/crates/spk-build/src/validation/collect_all_files_test.rs index 760270f647..bec1afb5cf 100644 --- a/crates/spk-build/src/validation/collect_all_files_test.rs +++ b/crates/spk-build/src/validation/collect_all_files_test.rs @@ -52,6 +52,7 @@ async fn test_validate_build_changeset_collected() { environment_filesystem: Manifest::new( spfs::tracking::Entry::empty_dir_with_open_perms_with_data(package.ident().clone()), ), + suppressed_requirements: Default::default(), package, }, }; diff --git a/crates/spk-build/src/validation/collect_existing_files_test.rs b/crates/spk-build/src/validation/collect_existing_files_test.rs index d0f35eda3e..b9cf435a22 100644 --- a/crates/spk-build/src/validation/collect_existing_files_test.rs +++ b/crates/spk-build/src/validation/collect_existing_files_test.rs @@ -43,6 +43,7 @@ async fn test_validate_build_changeset_collect_existing() { environment: Solution::default(), variant: package.option_values(), environment_filesystem, + suppressed_requirements: Default::default(), package, }, }; diff --git a/crates/spk-build/src/validation/empty_package_test.rs b/crates/spk-build/src/validation/empty_package_test.rs index d8486e71cc..b386023099 100644 --- a/crates/spk-build/src/validation/empty_package_test.rs +++ b/crates/spk-build/src/validation/empty_package_test.rs @@ -20,6 +20,7 @@ async fn test_validate_build_changeset_nothing() { environment_filesystem: Manifest::new( spfs::tracking::Entry::empty_dir_with_open_perms_with_data(package.ident().clone()), ), + suppressed_requirements: Default::default(), package, }, output: Default::default(), diff --git a/crates/spk-build/src/validation/inherit_requirements.rs b/crates/spk-build/src/validation/inherit_requirements.rs index dc14532c7e..3666221bb2 100644 --- a/crates/spk-build/src/validation/inherit_requirements.rs +++ b/crates/spk-build/src/validation/inherit_requirements.rs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 // https://github.com/spkenv/spk +use spk_schema::ident::RequestWithOptions; use spk_schema::name::PkgNameBuf; use spk_schema::validation::{ ValidationMatcherDiscriminants, @@ -88,6 +89,11 @@ impl super::Validator for InheritRequirementsValidator<'_> { .spec .downstream_runtime_requirements([component]); for request in downstream_runtime.iter() { + if let RequestWithOptions::Var(var) = request + && setup.suppressed_requirements.contains(&var.var) + { + continue; + } let status = match (self.kind, runtime_requirements.contains_request(request)) { (RuleKind::Allow, Compatibility::Compatible) | (RuleKind::Allow, Compatibility::Incompatible(_)) diff --git a/crates/spk-build/src/validation/inherit_requirements_test.rs b/crates/spk-build/src/validation/inherit_requirements_test.rs index 38758236b9..b2895a3ffa 100644 --- a/crates/spk-build/src/validation/inherit_requirements_test.rs +++ b/crates/spk-build/src/validation/inherit_requirements_test.rs @@ -53,6 +53,7 @@ async fn test_build_package_downstream_build_requests() { environment_filesystem: Manifest::new( spfs::tracking::Entry::empty_dir_with_open_perms_with_data(package.ident().clone()), ), + suppressed_requirements: Default::default(), package, }; let err = ValidationRule::Require { @@ -124,6 +125,7 @@ async fn test_build_package_downstream_runtime_request() { environment_filesystem: Manifest::new( spfs::tracking::Entry::empty_dir_with_open_perms_with_data(package.ident().clone()), ), + suppressed_requirements: Default::default(), package, }; let err = ValidationRule::Require { diff --git a/crates/spk-build/src/validation/long_var_description_test.rs b/crates/spk-build/src/validation/long_var_description_test.rs index 2f25713e47..20ec70f49d 100644 --- a/crates/spk-build/src/validation/long_var_description_test.rs +++ b/crates/spk-build/src/validation/long_var_description_test.rs @@ -44,6 +44,7 @@ async fn test_for_description_over_limit() { environment_filesystem: Manifest::new( spfs::tracking::Entry::empty_dir_with_open_perms_with_data(package.ident().clone()), ), + suppressed_requirements: Default::default(), package, }; diff --git a/crates/spk-build/src/validation/recursive_build_test.rs b/crates/spk-build/src/validation/recursive_build_test.rs index 1f2946baaa..b65f9d4598 100644 --- a/crates/spk-build/src/validation/recursive_build_test.rs +++ b/crates/spk-build/src/validation/recursive_build_test.rs @@ -50,6 +50,7 @@ async fn test_build_with_circular_dependency() { environment_filesystem: Manifest::new( spfs::tracking::Entry::empty_dir_with_open_perms_with_data(new_build.ident().clone()), ), + suppressed_requirements: Default::default(), package: new_build, }; ValidationRule::Deny { diff --git a/crates/spk-build/src/validation/spdx_license_test.rs b/crates/spk-build/src/validation/spdx_license_test.rs index 02d6f94cb3..c128834965 100644 --- a/crates/spk-build/src/validation/spdx_license_test.rs +++ b/crates/spk-build/src/validation/spdx_license_test.rs @@ -24,6 +24,7 @@ macro_rules! basic_setup { environment_filesystem: Manifest::new( spfs::tracking::Entry::empty_dir_with_open_perms_with_data(package.ident().clone()), ), + suppressed_requirements: Default::default(), package, } }}; diff --git a/crates/spk-build/src/validation/strong_inheritance_var_desc_test.rs b/crates/spk-build/src/validation/strong_inheritance_var_desc_test.rs index 082337eac9..6aeee5f42f 100644 --- a/crates/spk-build/src/validation/strong_inheritance_var_desc_test.rs +++ b/crates/spk-build/src/validation/strong_inheritance_var_desc_test.rs @@ -43,6 +43,7 @@ async fn test_strongly_inherited_vars_require_desc() { environment_filesystem: Manifest::new( spfs::tracking::Entry::empty_dir_with_open_perms_with_data(package.ident().clone()), ), + suppressed_requirements: Default::default(), package, }; diff --git a/crates/spk-cli/cmd-build/src/cmd_build_test/mod.rs b/crates/spk-cli/cmd-build/src/cmd_build_test/mod.rs index 98dd20fc63..c5c2a77519 100644 --- a/crates/spk-cli/cmd-build/src/cmd_build_test/mod.rs +++ b/crates/spk-cli/cmd-build/src/cmd_build_test/mod.rs @@ -5,11 +5,11 @@ use clap::Parser; use rstest::rstest; use spfs::storage::prelude::*; -use spk_schema::RuntimeEnvironment; use spk_schema::foundation::fixtures::*; use spk_schema::foundation::{option_map, version_ident}; use spk_schema::ident_component::Component; use spk_schema::option_map::HOST_OPTIONS; +use spk_schema::{Package, RuntimeEnvironment}; use spk_storage::fixtures::*; use super::Build; @@ -837,3 +837,98 @@ install: "should have environment ops" ); } + +#[rstest] +#[case::cli("cli")] +#[case::checks("checks")] +#[case::resolvo("resolvo")] +#[tokio::test] +async fn test_package_can_opt_out_of_strongly_inherited_var( + tmpdir: tempfile::TempDir, + #[case] solver_to_run: &str, +) { + let rt = spfs_runtime().await; + + build_package!( + tmpdir, + "base.spk.yaml", + br#" +api: v0/package +pkg: base/1.0.0 +build: + options: + - var: inherit-me/1.2.3 + inheritance: Strong + description: test content + script: + - true +"#, + solver_to_run + ); + + build_package!( + tmpdir, + "baseline.spk.yaml", + br#" +api: v0/package +pkg: baseline/1.0.0 +build: + options: + - pkg: base + script: + - true +"#, + solver_to_run + ); + + let build = rt + .tmprepo + .list_package_builds(&version_ident!("baseline/1.0.0")) + .await + .unwrap() + .into_iter() + .find(|b| !b.is_source()) + .unwrap(); + + let pkg = rt.tmprepo.read_package(&build).await.unwrap(); + + let requirements = pkg.runtime_requirements(); + // As written, if the strongly-inherited var is not suppressed, then the + // package should have one entry in the runtime_requirements list + assert_eq!(1, requirements.len(), "expected one requirement"); + + build_package!( + tmpdir, + "downstream.spk.yaml", + br#" +api: v0/package +pkg: downstream/1.0.0 +build: + options: + - pkg: base + script: + - true +install: + requirements: + - var: base.inherit-me + suppress: "don't inherit this!" +"#, + solver_to_run + ); + + let build = rt + .tmprepo + .list_package_builds(&version_ident!("downstream/1.0.0")) + .await + .unwrap() + .into_iter() + .find(|b| !b.is_source()) + .unwrap(); + + let pkg = rt.tmprepo.read_package(&build).await.unwrap(); + + let requirements = pkg.runtime_requirements(); + // As written, if the strongly-inherited var is suppressed, then the package + // should have an empty runtime_requirements list + assert_eq!(0, requirements.len(), "expected an empty list"); +} diff --git a/crates/spk-schema/crates/foundation/src/ident/pinnable_request.rs b/crates/spk-schema/crates/foundation/src/ident/pinnable_request.rs index 8daba0a56d..e5409f2129 100644 --- a/crates/spk-schema/crates/foundation/src/ident/pinnable_request.rs +++ b/crates/spk-schema/crates/foundation/src/ident/pinnable_request.rs @@ -195,10 +195,33 @@ impl<'de> Deserialize<'de> for PinPolicy { /// Represents a constraint added to a resolved environment. #[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd, Variantly)] -#[cfg_attr(feature = "parsedbuf-serde", derive(Serialize), serde(untagged))] pub enum PinnableRequest { Pkg(PkgRequest), Var(VarRequest), + /// Prevent the package from inheriting the var named. + /// Contains the var name and an optional comment explaining + /// why the suppression is needed. + Suppress(OptNameBuf, String), +} + +#[cfg(feature = "parsedbuf-serde")] +impl Serialize for PinnableRequest { + fn serialize(&self, serializer: S) -> std::result::Result + where + S: serde::Serializer, + { + match self { + PinnableRequest::Pkg(pkg) => pkg.serialize(serializer), + PinnableRequest::Var(var) => var.serialize(serializer), + PinnableRequest::Suppress(var, comment) => { + use serde::ser::SerializeMap; + let mut map = serializer.serialize_map(Some(2))?; + map.serialize_entry("var", var.as_str())?; + map.serialize_entry("suppress", comment)?; + map.end() + } + } + } } impl crate::spec_ops::Named for PinnableRequest { @@ -206,6 +229,7 @@ impl crate::spec_ops::Named for PinnableRequest { match self { PinnableRequest::Var(r) => &r.var, PinnableRequest::Pkg(r) => r.pkg.name.as_opt_name(), + PinnableRequest::Suppress(var, _) => var, } } } @@ -215,6 +239,7 @@ impl std::fmt::Display for PinnableRequest { match self { Self::Pkg(p) => p.fmt(f), Self::Var(v) => v.fmt(f), + Self::Suppress(v, _) => v.fmt(f), } } } @@ -281,6 +306,7 @@ impl<'de> Deserialize<'de> for PinnableRequest { var: Option, value: Option, description: Option, + suppress: Option, // Both pin: Option, @@ -319,6 +345,7 @@ impl<'de> Deserialize<'de> for PinnableRequest { } "value" => self.value = Some(map.next_value::()?), "description" => self.description = Some(map.next_value::()?), + "suppress" => self.suppress = Some(map.next_value::()?), _ => { // unrecognized fields are explicitly ignored in case // they were added in a newer version of spk. We assume @@ -348,6 +375,16 @@ impl<'de> Deserialize<'de> for PinnableRequest { required_compat: None, requested_by: Default::default(), })), + (None, Some(var)) if self.suppress.is_some() => { + let comment = match self.suppress.unwrap() { + serde_yaml::Value::String(s) => s, + other => serde_yaml::to_string(&other) + .unwrap_or_default() + .trim() + .to_string(), + }; + Ok(PinnableRequest::Suppress(var, comment)) + } (None, Some(var)) => { let mut value = self .pin diff --git a/crates/spk-schema/crates/foundation/src/ident/pinned_request.rs b/crates/spk-schema/crates/foundation/src/ident/pinned_request.rs index 1a36a779f3..9492cdd48d 100644 --- a/crates/spk-schema/crates/foundation/src/ident/pinned_request.rs +++ b/crates/spk-schema/crates/foundation/src/ident/pinned_request.rs @@ -87,6 +87,9 @@ impl TryFrom for PinnedRequest { match req { PinnableRequest::Pkg(r) => Ok(PinnedRequest::Pkg(r)), PinnableRequest::Var(r) => Ok(PinnedRequest::Var(r.try_into()?)), + PinnableRequest::Suppress(..) => { + Err("a var suppression rule cannot be converted into a pinned request".into()) + } } } } diff --git a/crates/spk-schema/src/recipe.rs b/crates/spk-schema/src/recipe.rs index 13dec36ce2..b347f20e00 100644 --- a/crates/spk-schema/src/recipe.rs +++ b/crates/spk-schema/src/recipe.rs @@ -3,10 +3,11 @@ // https://github.com/spkenv/spk use std::borrow::Cow; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::path::Path; use spk_schema_foundation::ident::{RequestWithOptions, VersionIdent}; +use spk_schema_foundation::name::OptNameBuf; use crate::foundation::ident_build::BuildId; use crate::foundation::option_map::OptionMap; @@ -113,6 +114,12 @@ pub trait Recipe: /// Return the set of configured validators when building this package fn validation(&self) -> &super::ValidationSpec; + + /// Return the set of var names that this recipe explicitly suppresses + /// from being inherited via strong inheritance. + fn suppressed_requirements(&self) -> HashSet { + HashSet::new() + } } forward_to_impl!(box = false, Recipe, { @@ -183,4 +190,8 @@ forward_to_impl!(box = false, Recipe, { fn validation(&self) -> &super::ValidationSpec { (**self).validation() } + + fn suppressed_requirements(&self) -> HashSet { + (**self).suppressed_requirements() + } }); diff --git a/crates/spk-schema/src/requirements_list.rs b/crates/spk-schema/src/requirements_list.rs index e0ee9eafe3..d69d1235e7 100644 --- a/crates/spk-schema/src/requirements_list.rs +++ b/crates/spk-schema/src/requirements_list.rs @@ -220,6 +220,11 @@ impl RequirementsList { } } } + PinnableRequest::Suppress(..) => { + // Suppression rules are not carried over into built + // packages + None + } } }).collect::>>()?)) } diff --git a/crates/spk-schema/src/spec.rs b/crates/spk-schema/src/spec.rs index 330b0e07ed..8714f72067 100644 --- a/crates/spk-schema/src/spec.rs +++ b/crates/spk-schema/src/spec.rs @@ -412,6 +412,12 @@ impl Recipe for SpecRecipe { fn validation(&self) -> &ValidationSpec { each_variant!(self, r, r.validation()) } + + fn suppressed_requirements( + &self, + ) -> std::collections::HashSet { + each_variant!(self, r, r.suppressed_requirements()) + } } impl HasVersion for SpecRecipe { diff --git a/crates/spk-schema/src/v0/recipe_spec.rs b/crates/spk-schema/src/v0/recipe_spec.rs index d41fa895a0..482d50b51a 100644 --- a/crates/spk-schema/src/v0/recipe_spec.rs +++ b/crates/spk-schema/src/v0/recipe_spec.rs @@ -13,6 +13,7 @@ use serde::{Deserialize, Serialize}; use spk_schema_foundation::IsDefault; use spk_schema_foundation::ident::{ AsVersionIdent, + PinnableRequest, PinnedRequest, PkgRequestOptions, RangeIdent, @@ -376,6 +377,14 @@ impl Recipe for RecipeSpec { ) }) .collect::>>()?; + let suppressed_vars: std::collections::HashSet = recipe_install + .requirements + .iter() + .filter_map(|r| match r { + PinnableRequest::Suppress(name, _) => Some(name.clone()), + _ => None, + }) + .collect(); let mut package_install = recipe_install.render_all_pins(&build_options, &specs)?; // Update metadata fields from the output of the executable. @@ -433,6 +442,11 @@ impl Recipe for RecipeSpec { Either::Right(embedded) => embedded.downstream_runtime_requirements([]), }; for request in downstream_runtime.iter() { + if let RequestWithOptions::Var(var) = request + && suppressed_vars.contains(&var.var) + { + continue; + } match package_install.requirements.contains_request(request) { Compatibility::Compatible => continue, Compatibility::Incompatible(_) => match request { @@ -509,6 +523,17 @@ impl Recipe for RecipeSpec { fn validation(&self) -> &ValidationSpec { &self.build.validation } + + fn suppressed_requirements(&self) -> std::collections::HashSet { + self.install + .requirements + .iter() + .filter_map(|r| match r { + PinnableRequest::Suppress(name, _) => Some(name.clone()), + _ => None, + }) + .collect() + } } impl Satisfy for RecipeSpec diff --git a/docs/ref/api/v0/package.md b/docs/ref/api/v0/package.md index 688ac5bb2c..b428506c7c 100644 --- a/docs/ref/api/v0/package.md +++ b/docs/ref/api/v0/package.md @@ -386,6 +386,7 @@ A build option can be one of [VariableRequest](#variablerequest), or [PackageReq | var | _str_ | The requested value of a package build variable in the form`name=value`, this can reference a specific package or the global variable (eg `debug=on`, or `python.abi=cp37`) | | fromBuildEnv | _bool_ | If true, replace the requested value of this variable with the value used in the build environment | | ifPresentInBuildEnv | _bool_ | Either true or false; if true, then `fromBuildEnv` only applies if the variable was present in the build environment. This allows different variants to have different runtime requirements. | +| suppress | _str_ | If set, prevents this variable from being inherited from upstream packages with Strong inheritance. The value serves as documentation for why the suppression is needed. | #### PackageRequest