From e86db5146016b913af6421fa82655cf7b1a82cfb Mon Sep 17 00:00:00 2001 From: J Robert Ray Date: Fri, 31 May 2024 12:02:31 -0700 Subject: [PATCH 1/5] WIP: Add "compat" to var options Signed-off-by: J Robert Ray --- crates/spk-schema/src/option.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/crates/spk-schema/src/option.rs b/crates/spk-schema/src/option.rs index 1bded2733c..b0496247ee 100644 --- a/crates/spk-schema/src/option.rs +++ b/crates/spk-schema/src/option.rs @@ -9,6 +9,7 @@ use indexmap::set::IndexSet; use serde::{Deserialize, Serialize}; use spk_schema_foundation::ident_component::ComponentBTreeSetBuf; use spk_schema_foundation::option_map::Stringified; +use spk_schema_foundation::version::Compat; use spk_schema_ident::{NameAndValue, PinnableValue, RangeIdent}; use crate::foundation::name::{OptName, OptNameBuf, PkgName, PkgNameBuf}; @@ -181,6 +182,7 @@ impl TryFrom for Opt { choices: Default::default(), inheritance: Default::default(), description, + compat: None, value: None, })), } @@ -214,6 +216,7 @@ impl<'de> Deserialize<'de> for Opt { var: Option, choices: Option>, inheritance: Option, + compat: Option, // Both default: Option, @@ -280,6 +283,9 @@ impl<'de> Deserialize<'de> for Opt { "description" => { self.description = Some(map.next_value::()?.0); } + "compat" => { + self.compat = Some(map.next_value::()?); + } _ => { // unrecognized fields are explicitly ignored in case // they were added in a newer version of spk. We assume @@ -305,6 +311,7 @@ impl<'de> Deserialize<'de> for Opt { inheritance: self.inheritance.unwrap_or_default(), default: self.default.unwrap_or_default(), description: self.description, + compat: self.compat, value: self.value, })), (Some(_), Some(_)) => Err(serde::de::Error::custom( @@ -328,6 +335,7 @@ pub struct VarOpt { pub choices: IndexSet, pub inheritance: Inheritance, pub description: Option, + pub compat: Option, value: Option, } @@ -385,6 +393,7 @@ impl VarOpt { choices: IndexSet::default(), inheritance: Inheritance::default(), description: None, + compat: None, value: None, }) } @@ -464,6 +473,8 @@ struct VarOptSchema { inheritance: Inheritance, #[serde(skip_serializing_if = "String::is_empty")] description: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + compat: Option, #[serde(rename = "static", skip_serializing_if = "String::is_empty")] value: String, } @@ -478,6 +489,7 @@ impl Serialize for VarOpt { choices: self.choices.iter().map(String::to_owned).collect(), inheritance: self.inheritance, description: self.description.clone().unwrap_or_default(), + compat: self.compat.clone(), value: self.value.clone().unwrap_or_default(), }; if !self.default.is_empty() { From 15c57cd279880feaae4dd820f126ee9900c464c5 Mon Sep 17 00:00:00 2001 From: J Robert Ray Date: Fri, 31 May 2024 15:22:31 -0700 Subject: [PATCH 2/5] Populate compat values for known distros For example, on Rocky it is expected that all versions of 9.x are binary compatible. When creating the "rocky" build option, set its compat value accordingly. Signed-off-by: J Robert Ray --- crates/spk-schema/src/build_spec.rs | 43 +++++++++++++++++++++-------- 1 file changed, 31 insertions(+), 12 deletions(-) diff --git a/crates/spk-schema/src/build_spec.rs b/crates/spk-schema/src/build_spec.rs index 4562c893f5..37b0553284 100644 --- a/crates/spk-schema/src/build_spec.rs +++ b/crates/spk-schema/src/build_spec.rs @@ -4,12 +4,14 @@ use std::collections::{HashMap, HashSet}; use std::hash::Hash; +use std::str::FromStr; use itertools::Itertools; use serde::{Deserialize, Serialize}; use spk_schema_foundation::ident_build::BuildId; use spk_schema_foundation::name::PkgName; use spk_schema_foundation::option_map::{OptionMap, Stringified, HOST_OPTIONS}; +use spk_schema_foundation::version::Compat; use strum::Display; use super::{v0, Opt, ValidationSpec}; @@ -68,7 +70,16 @@ impl AutoHostVars { pub fn host_options(&self) -> Result> { let all_host_options = HOST_OPTIONS.get()?; - let mut names_added = self.names_added(); + let mut settings = Vec::new(); + let names_to_add = self.names_added(); + for (name, _value) in all_host_options.iter() { + if !names_to_add.contains(&OptName::new(name)?) { + continue; + } + let opt = Opt::Var(VarOpt::new(name)?); + settings.push(opt) + } + let distro_name; let fallback_name: OptNameBuf; if AutoHostVars::Distro == *self { @@ -76,14 +87,30 @@ impl AutoHostVars { Some(distro) => { distro_name = distro.clone(); match OptName::new(&distro_name) { - Ok(name) => _ = names_added.insert(name), + Ok(name) => { + let mut var_opt = VarOpt::new(name.to_owned())?; + // Allow packages built on Rocky 7.3 to run on + // Rocky 7.4 by giving the "rocky" option a compat + // value. + // XXX: hard coded distro names as a proof of + // concept + if distro_name == "centos" + || distro_name == "rocky" + || distro_name == "ubuntu" + { + var_opt.compat = Some( + Compat::from_str("x.ab").expect("valid Compat expression"), + ); + } + settings.push(Opt::Var(var_opt)); + } Err(err) => { fallback_name = OptNameBuf::new_lossy(&distro_name); tracing::warn!("Reported distro id ({}) is not a valid var option name: {err}. A {} var will be used instead.", distro_name.to_string(), fallback_name); - _ = names_added.insert(&fallback_name); + settings.push(Opt::Var(VarOpt::new(&fallback_name)?)); } } } @@ -92,19 +119,11 @@ impl AutoHostVars { "No distro name set by host. A {}= will be used instead.", OptName::unknown_distro() ); - _ = names_added.insert(OptName::unknown_distro()); + settings.push(Opt::Var(VarOpt::new(OptName::unknown_distro().to_owned())?)); } } } - let mut settings = Vec::new(); - for (name, _value) in all_host_options.iter() { - if names_added.contains(&OptName::new(name)?) { - let opt = Opt::Var(VarOpt::new(name)?); - settings.push(opt) - } - } - Ok(settings) } } From 819544f02af2f7e4e3761a0cdb57fabd283a1686 Mon Sep 17 00:00:00 2001 From: J Robert Ray Date: Fri, 31 May 2024 16:52:24 -0700 Subject: [PATCH 3/5] Respect var opt "compat" rule when present Signed-off-by: J Robert Ray --- crates/spk-schema/src/v0/spec.rs | 58 +++++++++++++++++++++++++++----- 1 file changed, 49 insertions(+), 9 deletions(-) diff --git a/crates/spk-schema/src/v0/spec.rs b/crates/spk-schema/src/v0/spec.rs index 7e12e55780..280680f3c8 100644 --- a/crates/spk-schema/src/v0/spec.rs +++ b/crates/spk-schema/src/v0/spec.rs @@ -6,6 +6,7 @@ use std::borrow::Cow; use std::collections::{BTreeSet, HashMap}; use std::convert::TryInto; use std::path::Path; +use std::str::FromStr; use itertools::Itertools; use serde::{Deserialize, Serialize}; @@ -13,6 +14,7 @@ use spk_schema_foundation::ident_build::BuildId; use spk_schema_foundation::ident_component::ComponentBTreeSet; use spk_schema_foundation::name::PkgNameBuf; use spk_schema_foundation::option_map::{OptFilter, Stringified}; +use spk_schema_foundation::version::IncompatibleReason; use spk_schema_ident::{AnyIdent, BuildIdent, Ident, RangeIdent, VersionIdent}; use super::variant_spec::VariantSpecEntryKey; @@ -801,16 +803,54 @@ where Some(Opt::Var(opt)) => { let request_value = var_request.value.as_pinned(); let exact = opt.get_value(request_value); - if exact.as_deref() != request_value { - Compatibility::incompatible(format!( - "Incompatible build option '{}': '{}' != '{}'", - var_request.var, - exact.unwrap_or_else(|| "None".to_string()), - request_value.unwrap_or_default() - )) - } else { - Compatibility::Compatible + if exact.as_deref() == request_value { + return Compatibility::Compatible; } + + // For values that aren't exact matches, if the option specifies + // a compat rule, try treating the values as version numbers + // and see if they satisfy the rule. + if let Some(compat) = &opt.compat { + let base_version = exact.clone(); + let Ok(base_version) = Version::from_str(&base_version.unwrap_or_default()) + else { + return Compatibility::incompatible(format!( + "Incompatible build option '{}': '{base}' != '{}' and '{base}' is not a valid version number", + var_request.var, + request_value.unwrap_or_default(), + base = exact.unwrap_or_default() + )); + }; + + let Ok(request_version) = Version::from_str(request_value.unwrap_or_default()) + else { + return Compatibility::incompatible(format!( + "Incompatible build option '{}': '{base}' != '{request}' and '{request}' is not a valid version number", + var_request.var, + request = request_value.unwrap_or_default(), + base = exact.unwrap_or_default() + )); + }; + + let mut result = compat.is_binary_compatible(&base_version, &request_version); + if let Compatibility::Incompatible(IncompatibleReason::Other(msg)) = &mut result + { + *msg = format!( + "Incompatible build option '{}': '{}' != '{}' and {msg}", + var_request.var, + exact.unwrap_or_else(|| "None".to_string()), + request_value.unwrap_or_default() + ); + } + return result; + } + + Compatibility::incompatible(format!( + "Incompatible build option '{}': '{}' != '{}'", + var_request.var, + exact.unwrap_or_else(|| "None".to_string()), + request_value.unwrap_or_default() + )) } } } From 1b3140c5a9dd197316e4afcb6bb160c4e3c65ebd Mon Sep 17 00:00:00 2001 From: J Robert Ray Date: Tue, 6 Aug 2024 17:25:44 -0700 Subject: [PATCH 4/5] Move the default distro compat values into config Add a new spk config section for setting distro rules. Currently the only rule is setting a compat value for a distro, but others may come later (like renaming the distro, e.g., "rocky" -> "rhel9", for more flexibility). An invalid compat value in the config will produce an error at the point where that value is needed, rather than attempting to parse the compat value when parsing the config. This could be changed later, but for now it prevents a bad config file from competely breaking all spk commands. Signed-off-by: J Robert Ray --- Cargo.lock | 1 + crates/spk-config/src/config.rs | 18 ++++++++++++++++++ crates/spk-schema/Cargo.toml | 1 + crates/spk-schema/src/build_spec.rs | 28 ++++++++++++++++------------ docs/admin/config.md | 8 ++++++++ 5 files changed, 44 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 39ac97b98a..0ab5f6338d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4342,6 +4342,7 @@ dependencies = [ name = "spk-schema" version = "0.42.0" dependencies = [ + "config", "data-encoding", "dunce", "enum_dispatch", diff --git a/crates/spk-config/src/config.rs b/crates/spk-config/src/config.rs index 4d52ac7305..8ae4bde1e6 100644 --- a/crates/spk-config/src/config.rs +++ b/crates/spk-config/src/config.rs @@ -118,6 +118,23 @@ pub struct Cli { pub ls: Ls, } +#[derive(Clone, Default, Debug, Deserialize, Serialize)] +#[serde(default)] +pub struct DistroRule { + /// The name of the distro to match, e.g., "rocky" + pub name: String, + + /// The compat rule to set for the distro, e.g., "x.ab" + pub compat_rule: Option, +} + +#[derive(Clone, Default, Debug, Deserialize, Serialize)] +#[serde(default)] +pub struct HostOptions { + /// A list of distro names to recognize and customizations for each. + pub distro_rules: Vec, +} + /// Configuration values for spk. #[derive(Clone, Debug, Default, Deserialize, Serialize)] #[serde(default)] @@ -130,6 +147,7 @@ pub struct Config { pub statsd: Statsd, pub metadata: Metadata, pub cli: Cli, + pub host_options: HostOptions, } impl Config { diff --git a/crates/spk-schema/Cargo.toml b/crates/spk-schema/Cargo.toml index 9f7f77ca44..2144874b64 100644 --- a/crates/spk-schema/Cargo.toml +++ b/crates/spk-schema/Cargo.toml @@ -16,6 +16,7 @@ workspace = true migration-to-components = ["spk-schema-foundation/migration-to-components"] [dependencies] +config = { workspace = true } data-encoding = "2.3" dunce = { workspace = true } enum_dispatch = "0.3.8" diff --git a/crates/spk-schema/src/build_spec.rs b/crates/spk-schema/src/build_spec.rs index 37b0553284..96ff4d1900 100644 --- a/crates/spk-schema/src/build_spec.rs +++ b/crates/spk-schema/src/build_spec.rs @@ -8,6 +8,7 @@ use std::str::FromStr; use itertools::Itertools; use serde::{Deserialize, Serialize}; +use spk_config::get_config; use spk_schema_foundation::ident_build::BuildId; use spk_schema_foundation::name::PkgName; use spk_schema_foundation::option_map::{OptionMap, Stringified, HOST_OPTIONS}; @@ -89,18 +90,21 @@ impl AutoHostVars { match OptName::new(&distro_name) { Ok(name) => { let mut var_opt = VarOpt::new(name.to_owned())?; - // Allow packages built on Rocky 7.3 to run on - // Rocky 7.4 by giving the "rocky" option a compat - // value. - // XXX: hard coded distro names as a proof of - // concept - if distro_name == "centos" - || distro_name == "rocky" - || distro_name == "ubuntu" - { - var_opt.compat = Some( - Compat::from_str("x.ab").expect("valid Compat expression"), - ); + // Look for any configured compat rules for the + // distro + let config = get_config()?; + for rule in config.host_options.distro_rules.iter() { + if rule.name != distro_name { + continue; + } + + if let Some(compat_rule) = &rule.compat_rule { + var_opt.compat = Some( + Compat::from_str(compat_rule).map_err(|err| { + Error::SpkConfigError(spk_config::Error::Config(config::ConfigError::Message(format!("Invalid compat rule found in config for distro {distro_name}: {err}")))) + })?, + ); + } } settings.push(Opt::Var(var_opt)); } diff --git a/docs/admin/config.md b/docs/admin/config.md index 04d542208e..c67f854fc9 100644 --- a/docs/admin/config.md +++ b/docs/admin/config.md @@ -253,4 +253,12 @@ environment = "" [cli.ls] # Use all current host's host options by default for filtering in ls host_filtering = false + +# SPK supports some customization of the distro host options. +[[host_options.distro_rules]] +# The name of the distro that the customizations apply to. +name = "rocky" +# Set a default compat rule for this distro. For example, on Rocky Linux +# packages built on 9.3 would be usable on 9.4. +compat_rule = "x.ab" ``` From 0699fb000c674096df53aa4a3283c4d5fa9cb3f9 Mon Sep 17 00:00:00 2001 From: J Robert Ray Date: Wed, 7 Aug 2024 16:10:14 -0700 Subject: [PATCH 5/5] Restructure distro rules to use a HashMap Signed-off-by: J Robert Ray --- crates/spk-config/src/config.rs | 8 +++----- crates/spk-schema/src/build_spec.rs | 6 +----- docs/admin/config.md | 4 +--- 3 files changed, 5 insertions(+), 13 deletions(-) diff --git a/crates/spk-config/src/config.rs b/crates/spk-config/src/config.rs index 8ae4bde1e6..59e40dce22 100644 --- a/crates/spk-config/src/config.rs +++ b/crates/spk-config/src/config.rs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 // https://github.com/spkenv/spk +use std::collections::HashMap; use std::sync::{Arc, RwLock}; use config::Environment; @@ -121,9 +122,6 @@ pub struct Cli { #[derive(Clone, Default, Debug, Deserialize, Serialize)] #[serde(default)] pub struct DistroRule { - /// The name of the distro to match, e.g., "rocky" - pub name: String, - /// The compat rule to set for the distro, e.g., "x.ab" pub compat_rule: Option, } @@ -131,8 +129,8 @@ pub struct DistroRule { #[derive(Clone, Default, Debug, Deserialize, Serialize)] #[serde(default)] pub struct HostOptions { - /// A list of distro names to recognize and customizations for each. - pub distro_rules: Vec, + /// A mapping of distro names to recognize and customizations for each. + pub distro_rules: HashMap, } /// Configuration values for spk. diff --git a/crates/spk-schema/src/build_spec.rs b/crates/spk-schema/src/build_spec.rs index 96ff4d1900..befd782f42 100644 --- a/crates/spk-schema/src/build_spec.rs +++ b/crates/spk-schema/src/build_spec.rs @@ -93,11 +93,7 @@ impl AutoHostVars { // Look for any configured compat rules for the // distro let config = get_config()?; - for rule in config.host_options.distro_rules.iter() { - if rule.name != distro_name { - continue; - } - + if let Some(rule) = config.host_options.distro_rules.get(&distro_name) { if let Some(compat_rule) = &rule.compat_rule { var_opt.compat = Some( Compat::from_str(compat_rule).map_err(|err| { diff --git a/docs/admin/config.md b/docs/admin/config.md index c67f854fc9..e13c8238c2 100644 --- a/docs/admin/config.md +++ b/docs/admin/config.md @@ -255,9 +255,7 @@ environment = "" host_filtering = false # SPK supports some customization of the distro host options. -[[host_options.distro_rules]] -# The name of the distro that the customizations apply to. -name = "rocky" +[host_options.distro_rules.rocky] # Set a default compat rule for this distro. For example, on Rocky Linux # packages built on 9.3 would be usable on 9.4. compat_rule = "x.ab"