From 523943e60f764d76bff1e5e0697f94e90f64f4e6 Mon Sep 17 00:00:00 2001 From: J Robert Ray Date: Tue, 17 Sep 2024 15:44:33 -0700 Subject: [PATCH 1/3] Refactor Package::runtime_environment Both recipes and packages have the `install.environment` property, so refactor `runtime_environment()` out of the `Package` trait into its own trait, then require both `Recipe` and `Package` implement it. Signed-off-by: J Robert Ray --- crates/spk-schema/src/environ.rs | 37 ++++++++++++++++++++++ crates/spk-schema/src/lib.rs | 11 ++++++- crates/spk-schema/src/package.rs | 27 ++++++---------- crates/spk-schema/src/recipe.rs | 10 +++++- crates/spk-schema/src/spec.rs | 38 ++++++++++++++-------- crates/spk-schema/src/v0/platform.rs | 14 +++++++-- crates/spk-schema/src/v0/spec.rs | 47 +++++++++++++++------------- 7 files changed, 127 insertions(+), 57 deletions(-) diff --git a/crates/spk-schema/src/environ.rs b/crates/spk-schema/src/environ.rs index 74515e25cd..55890996a5 100644 --- a/crates/spk-schema/src/environ.rs +++ b/crates/spk-schema/src/environ.rs @@ -3,6 +3,7 @@ // https://github.com/spkenv/spk use std::collections::HashMap; +use std::sync::Arc; use serde::{Deserialize, Serialize}; use spk_schema_foundation::option_map::Stringified; @@ -23,6 +24,42 @@ const OP_PRIORITY: &str = "priority"; const OP_SET: &str = "set"; const OP_NAMES: &[&str] = &[OP_APPEND, OP_COMMENT, OP_PREPEND, OP_SET]; +/// Some item that contains a list of [`EnvOp`] operations +pub trait RuntimeEnvironment { + /// The set of operations to perform on the environment when running this package + fn runtime_environment(&self) -> &[EnvOp]; +} + +impl RuntimeEnvironment for Box +where + T: RuntimeEnvironment, +{ + #[inline] + fn runtime_environment(&self) -> &[EnvOp] { + (**self).runtime_environment() + } +} + +impl RuntimeEnvironment for &T +where + T: RuntimeEnvironment, +{ + #[inline] + fn runtime_environment(&self) -> &[EnvOp] { + (**self).runtime_environment() + } +} + +impl RuntimeEnvironment for Arc +where + T: RuntimeEnvironment, +{ + #[inline] + fn runtime_environment(&self) -> &[EnvOp] { + (**self).runtime_environment() + } +} + /// The set of operation types for use in deserialization #[derive(Copy, Clone, Debug, PartialEq, strum::Display)] #[strum(serialize_all = "lowercase")] diff --git a/crates/spk-schema/src/lib.rs b/crates/spk-schema/src/lib.rs index 6f00bf873e..1f5a5a93db 100644 --- a/crates/spk-schema/src/lib.rs +++ b/crates/spk-schema/src/lib.rs @@ -30,7 +30,16 @@ pub use component_spec::{ComponentFileMatchMode, ComponentSpec}; pub use component_spec_list::ComponentSpecList; pub use deprecate::{Deprecate, DeprecateMut}; pub use embedded_packages_list::EmbeddedPackagesList; -pub use environ::{AppendEnv, EnvComment, EnvOp, EnvPriority, OpKind, PrependEnv, SetEnv}; +pub use environ::{ + AppendEnv, + EnvComment, + EnvOp, + EnvPriority, + OpKind, + PrependEnv, + RuntimeEnvironment, + SetEnv, +}; pub use error::{Error, Result}; pub use input_variant::InputVariant; pub use install_spec::InstallSpec; diff --git a/crates/spk-schema/src/package.rs b/crates/spk-schema/src/package.rs index f3a2050b30..2e3fdf9803 100644 --- a/crates/spk-schema/src/package.rs +++ b/crates/spk-schema/src/package.rs @@ -15,7 +15,7 @@ use super::RequirementsList; use crate::foundation::ident_component::Component; use crate::foundation::option_map::OptionMap; use crate::foundation::version::Compatibility; -use crate::{DeprecateMut, Opt}; +use crate::{DeprecateMut, Opt, RuntimeEnvironment}; #[cfg(test)] #[path = "./package_test.rs"] @@ -24,7 +24,15 @@ mod package_test; /// Can be resolved into an environment. #[enum_dispatch::enum_dispatch] pub trait Package: - Named + Versioned + super::Deprecate + Clone + Eq + std::hash::Hash + Sync + Send + Named + + Versioned + + super::Deprecate + + RuntimeEnvironment + + Clone + + Eq + + std::hash::Hash + + Sync + + Send { type Package; @@ -62,9 +70,6 @@ pub trait Package: /// The components defined by this package fn components(&self) -> &super::ComponentSpecList; - /// The set of operations to perform on the environment when running this package - fn runtime_environment(&self) -> &Vec; - /// The list of build options for this package fn get_build_options(&self) -> &Vec; @@ -180,10 +185,6 @@ impl Package for std::sync::Arc { (**self).components() } - fn runtime_environment(&self) -> &Vec { - (**self).runtime_environment() - } - fn get_build_options(&self) -> &Vec { (**self).get_build_options() } @@ -260,10 +261,6 @@ impl Package for Box { (**self).components() } - fn runtime_environment(&self) -> &Vec { - (**self).runtime_environment() - } - fn get_build_options(&self) -> &Vec { (**self).get_build_options() } @@ -340,10 +337,6 @@ impl Package for &T { (**self).components() } - fn runtime_environment(&self) -> &Vec { - (**self).runtime_environment() - } - fn get_build_options(&self) -> &Vec { (**self).get_build_options() } diff --git a/crates/spk-schema/src/recipe.rs b/crates/spk-schema/src/recipe.rs index 7f65b407fc..abc682792c 100644 --- a/crates/spk-schema/src/recipe.rs +++ b/crates/spk-schema/src/recipe.rs @@ -26,7 +26,15 @@ pub trait BuildEnv { /// Can be used to build a package. #[enum_dispatch::enum_dispatch] pub trait Recipe: - Named + Versioned + super::Deprecate + Clone + Eq + std::hash::Hash + Sync + Send + Named + + Versioned + + super::Deprecate + + crate::RuntimeEnvironment + + Clone + + Eq + + std::hash::Hash + + Sync + + Send { type Output: super::Package; type Variant: super::Variant + Clone; diff --git a/crates/spk-schema/src/spec.rs b/crates/spk-schema/src/spec.rs index 20c0bcc930..112c022d0a 100644 --- a/crates/spk-schema/src/spec.rs +++ b/crates/spk-schema/src/spec.rs @@ -36,6 +36,7 @@ use crate::{ Recipe, RequirementsList, Result, + RuntimeEnvironment, Template, TemplateExt, Test, @@ -378,6 +379,15 @@ impl Recipe for SpecRecipe { } } +impl HasVersion for SpecRecipe { + fn version(&self) -> &Version { + match self { + SpecRecipe::V0Package(r) => r.version(), + SpecRecipe::V0Platform(r) => r.version(), + } + } +} + impl Named for SpecRecipe { fn name(&self) -> &PkgName { match self { @@ -387,11 +397,11 @@ impl Named for SpecRecipe { } } -impl HasVersion for SpecRecipe { - fn version(&self) -> &Version { +impl RuntimeEnvironment for SpecRecipe { + fn runtime_environment(&self) -> &[crate::EnvOp] { match self { - SpecRecipe::V0Package(r) => r.version(), - SpecRecipe::V0Platform(r) => r.version(), + SpecRecipe::V0Package(r) => r.runtime_environment(), + SpecRecipe::V0Platform(r) => r.runtime_environment(), } } } @@ -529,6 +539,14 @@ impl Satisfy for Spec { } } +impl HasVersion for Spec { + fn version(&self) -> &Version { + match self { + Spec::V0Package(r) => r.version(), + } + } +} + impl Named for Spec { fn name(&self) -> &PkgName { match self { @@ -537,10 +555,10 @@ impl Named for Spec { } } -impl HasVersion for Spec { - fn version(&self) -> &Version { +impl RuntimeEnvironment for Spec { + fn runtime_environment(&self) -> &[crate::EnvOp] { match self { - Spec::V0Package(r) => r.version(), + Spec::V0Package(r) => r.runtime_environment(), } } } @@ -609,12 +627,6 @@ impl Package for Spec { } } - fn runtime_environment(&self) -> &Vec { - match self { - Spec::V0Package(spec) => spec.runtime_environment(), - } - } - fn get_build_options(&self) -> &Vec { match self { Spec::V0Package(spec) => spec.get_build_options(), diff --git a/crates/spk-schema/src/v0/platform.rs b/crates/spk-schema/src/v0/platform.rs index 4be57463b4..e4741546ae 100644 --- a/crates/spk-schema/src/v0/platform.rs +++ b/crates/spk-schema/src/v0/platform.rs @@ -36,6 +36,7 @@ use crate::{ Recipe, RequirementsList, Result, + RuntimeEnvironment, Script, TestStage, Variant, @@ -148,15 +149,22 @@ impl DeprecateMut for Platform { } } +impl HasVersion for Platform { + fn version(&self) -> &Version { + self.platform.version() + } +} + impl Named for Platform { fn name(&self) -> &PkgName { self.platform.name() } } -impl HasVersion for Platform { - fn version(&self) -> &Version { - self.platform.version() +impl RuntimeEnvironment for Platform { + fn runtime_environment(&self) -> &[crate::EnvOp] { + // Platforms don't have any EnvOps + &[] } } diff --git a/crates/spk-schema/src/v0/spec.rs b/crates/spk-schema/src/v0/spec.rs index 280680f3c8..5f63bb5697 100644 --- a/crates/spk-schema/src/v0/spec.rs +++ b/crates/spk-schema/src/v0/spec.rs @@ -58,6 +58,7 @@ use crate::{ Recipe, RequirementsList, Result, + RuntimeEnvironment, SourceSpec, TestStage, ValidationSpec, @@ -181,24 +182,6 @@ impl Spec { } } -impl Named for Spec { - fn name(&self) -> &PkgName { - self.pkg.name() - } -} - -impl HasVersion for Spec { - fn version(&self) -> &Version { - self.pkg.version() - } -} - -impl Versioned for Spec { - fn compat(&self) -> &Compat { - &self.compat - } -} - impl Deprecate for Spec { fn is_deprecated(&self) -> bool { self.deprecated @@ -217,6 +200,30 @@ impl DeprecateMut for Spec { } } +impl HasVersion for Spec { + fn version(&self) -> &Version { + self.pkg.version() + } +} + +impl Named for Spec { + fn name(&self) -> &PkgName { + self.pkg.name() + } +} + +impl RuntimeEnvironment for Spec { + fn runtime_environment(&self) -> &[EnvOp] { + &self.install.environment + } +} + +impl Versioned for Spec { + fn compat(&self) -> &Compat { + &self.compat + } +} + impl Package for Spec { type Package = Self; @@ -279,10 +286,6 @@ impl Package for Spec { &self.install.components } - fn runtime_environment(&self) -> &Vec { - &self.install.environment - } - fn get_build_options(&self) -> &Vec { &self.build.options } From 0efd33925fc35ab9b4d687ab5788f30cff6a687a Mon Sep 17 00:00:00 2001 From: J Robert Ray Date: Tue, 17 Sep 2024 17:37:19 -0700 Subject: [PATCH 2/3] Create newtype EnvOpList for Vec In order to implement foreign traits for this type. Signed-off-by: J Robert Ray --- crates/spk-schema/src/environ.rs | 17 +++++++++++++++++ crates/spk-schema/src/install_spec.rs | 8 ++++---- crates/spk-schema/src/lib.rs | 1 + crates/spk-schema/src/v0/spec.rs | 3 ++- 4 files changed, 24 insertions(+), 5 deletions(-) diff --git a/crates/spk-schema/src/environ.rs b/crates/spk-schema/src/environ.rs index 55890996a5..b2d7130e1c 100644 --- a/crates/spk-schema/src/environ.rs +++ b/crates/spk-schema/src/environ.rs @@ -187,6 +187,23 @@ impl EnvOp { } } +#[derive(Clone, Debug, Default, Deserialize, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)] +pub struct EnvOpList(Vec); + +impl std::ops::Deref for EnvOpList { + type Target = Vec; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl std::ops::DerefMut for EnvOpList { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } +} + impl<'de> Deserialize<'de> for EnvOp { fn deserialize(deserializer: D) -> std::result::Result where diff --git a/crates/spk-schema/src/install_spec.rs b/crates/spk-schema/src/install_spec.rs index 7c3e5807e1..9a4228845c 100644 --- a/crates/spk-schema/src/install_spec.rs +++ b/crates/spk-schema/src/install_spec.rs @@ -27,7 +27,7 @@ pub struct InstallSpec { deserialize_with = "deserialize_env_conf", skip_serializing_if = "Vec::is_empty" )] - pub environment: Vec, + pub environment: EnvOpList, } impl InstallSpec { @@ -53,14 +53,14 @@ impl InstallSpec { } } -fn deserialize_env_conf<'de, D>(deserializer: D) -> std::result::Result, D::Error> +fn deserialize_env_conf<'de, D>(deserializer: D) -> std::result::Result where D: serde::Deserializer<'de>, { struct EnvConfVisitor; impl<'de> serde::de::Visitor<'de> for EnvConfVisitor { - type Value = Vec; + type Value = EnvOpList; fn expecting(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { f.write_str("an environment configuration") @@ -70,7 +70,7 @@ where where A: serde::de::SeqAccess<'de>, { - let mut vec = Vec::new(); + let mut vec = EnvOpList::default(); while let Some(elem) = seq.next_element::()? { if vec.iter().any(|x: &EnvOp| x.kind() == OpKind::Priority) diff --git a/crates/spk-schema/src/lib.rs b/crates/spk-schema/src/lib.rs index 1f5a5a93db..b8475647e4 100644 --- a/crates/spk-schema/src/lib.rs +++ b/crates/spk-schema/src/lib.rs @@ -34,6 +34,7 @@ pub use environ::{ AppendEnv, EnvComment, EnvOp, + EnvOpList, EnvPriority, OpKind, PrependEnv, diff --git a/crates/spk-schema/src/v0/spec.rs b/crates/spk-schema/src/v0/spec.rs index 5f63bb5697..708c4d65cf 100644 --- a/crates/spk-schema/src/v0/spec.rs +++ b/crates/spk-schema/src/v0/spec.rs @@ -47,6 +47,7 @@ use crate::{ DeprecateMut, EmbeddedPackagesList, EnvOp, + EnvOpList, Error, Inheritance, InputVariant, @@ -676,7 +677,7 @@ impl Recipe for Spec { let mut build = updated.map_ident(|i| i.into_build(Build::BuildId(digest))); // Expand env variables from EnvOp. - let mut updated_ops = Vec::new(); + let mut updated_ops = EnvOpList::default(); let mut build_env_vars = build_env.env_vars(); build_env_vars.extend(build.get_build_env()); for op in build.install.environment.iter() { From ae9258aca4a130c3d20ea1c803804390a97dec27 Mon Sep 17 00:00:00 2001 From: J Robert Ray Date: Tue, 17 Sep 2024 17:49:55 -0700 Subject: [PATCH 3/3] Refactor various is_default() methods into IsDefault trait Fixes #1081. The bug causing #1081 was in `InstallSpec::is_default` as the logic was not updated when the `environment` field was added to the struct way back at https://github.com/spkenv/spk/blob/f4899ce1e4ac0634ee95608e322a86a358096d4d/src/api/install_spec.rs#L31 As a guard against this kind of bug in the future, an `IsDefault` trait was created and a derive macro created to derive the `is_default` method semi-automatically. All existing `is_default` methods have been changed to use the trait instead. Signed-off-by: J Robert Ray --- Cargo.lock | 11 +++ Cargo.toml | 2 + crates/is_default_derive_macro/Cargo.toml | 23 ++++++ crates/is_default_derive_macro/src/lib.rs | 72 +++++++++++++++++++ .../cmd-build/src/cmd_build_test/mod.rs | 34 +++++++++ crates/spk-schema/Cargo.toml | 1 + .../crates/foundation/src/is_default.rs | 8 +++ .../spk-schema/crates/foundation/src/lib.rs | 2 + .../crates/foundation/src/version/compat.rs | 14 ++-- crates/spk-schema/crates/ident/src/request.rs | 13 ++-- crates/spk-schema/src/build_spec.rs | 21 +++--- crates/spk-schema/src/build_spec_test.rs | 2 +- crates/spk-schema/src/component_spec_list.rs | 5 +- .../spk-schema/src/embedded_packages_list.rs | 7 ++ crates/spk-schema/src/environ.rs | 7 ++ crates/spk-schema/src/install_spec.rs | 23 ++++-- crates/spk-schema/src/metadata/meta.rs | 11 +-- crates/spk-schema/src/option.rs | 5 +- crates/spk-schema/src/requirements_list.rs | 7 ++ crates/spk-schema/src/v0/platform.rs | 5 +- crates/spk-schema/src/v0/spec.rs | 3 +- crates/spk-schema/src/validation.rs | 11 +-- 22 files changed, 244 insertions(+), 43 deletions(-) create mode 100644 crates/is_default_derive_macro/Cargo.toml create mode 100644 crates/is_default_derive_macro/src/lib.rs create mode 100644 crates/spk-schema/crates/foundation/src/is_default.rs diff --git a/Cargo.lock b/Cargo.lock index 10e357aad6..1f442f3590 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1824,6 +1824,16 @@ version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7655c9839580ee829dfacba1d1278c2b7883e50a277ff7541299489d6bdfdc45" +[[package]] +name = "is_default_derive_macro" +version = "0.42.0" +dependencies = [ + "indicatif", + "quote", + "syn 2.0.48", + "tracing", +] + [[package]] name = "itertools" version = "0.10.5" @@ -4351,6 +4361,7 @@ dependencies = [ "format_serde_error", "ignore", "indexmap 2.2.3", + "is_default_derive_macro", "itertools 0.12.0", "miette", "nom", diff --git a/Cargo.toml b/Cargo.toml index 9106164c72..9b515933f1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,6 +4,7 @@ codegen-units = 1 [workspace] members = [ + "crates/is_default_derive_macro", "crates/progress_bar_derive_macro", "crates/spfs", "crates/spfs-encoding", @@ -64,6 +65,7 @@ glob = "0.3" indicatif = "0.17.8" indexmap = "2.2" itertools = "0.12" +is_default_derive_macro = { path = "crates/is_default_derive_macro" } libc = "0.2.80" miette = "7.0" nix = { version = "0.27.1", features = ["mount", "sched", "user"] } diff --git a/crates/is_default_derive_macro/Cargo.toml b/crates/is_default_derive_macro/Cargo.toml new file mode 100644 index 0000000000..2b5bc66295 --- /dev/null +++ b/crates/is_default_derive_macro/Cargo.toml @@ -0,0 +1,23 @@ +[package] +name = "is_default_derive_macro" +version = { workspace = true } +license-file = { workspace = true } +homepage = { workspace = true } +repository = { workspace = true } +readme = { workspace = true } +description = { workspace = true } +edition = { workspace = true } + +[lints] +workspace = true + +[lib] +proc-macro = true + +[dependencies] +syn = "2.0" +quote = "1.0" +indicatif = { workspace = true } + +[dev-dependencies] +tracing = { workspace = true } diff --git a/crates/is_default_derive_macro/src/lib.rs b/crates/is_default_derive_macro/src/lib.rs new file mode 100644 index 0000000000..45fb526c34 --- /dev/null +++ b/crates/is_default_derive_macro/src/lib.rs @@ -0,0 +1,72 @@ +// Copyright (c) Contributors to the SPK project. +// SPDX-License-Identifier: Apache-2.0 +// https://github.com/spkenv/spk + +extern crate proc_macro; + +use proc_macro::TokenStream; +use quote::quote; +use syn::{parse_macro_input, Data, DeriveInput, Fields}; + +#[proc_macro_derive(IsDefault)] +pub fn derive_is_default(input: TokenStream) -> TokenStream { + // Parse the input tokens into a syntax tree + let input = parse_macro_input!(input as DeriveInput); + + // Get the name of the struct + let name = input.ident; + + // Generate the trait implementation based on the struct's data (fields) + let expanded = match input.data { + Data::Struct(data_struct) => { + match data_struct.fields { + Fields::Named(fields_named) => { + // Generate code that calls `IsDefault::is_default` on each field + let field_checks = fields_named.named.iter().map(|field| { + let field_name = &field.ident; + quote! { + spk_schema_foundation::IsDefault::is_default(&self.#field_name) + } + }); + + quote! { + impl spk_schema_foundation::IsDefault for #name { + fn is_default(&self) -> bool { + true #(&& #field_checks)* + } + } + } + } + Fields::Unnamed(fields_unnamed) => { + let field_checks = fields_unnamed.unnamed.iter().enumerate().map(|(i, _)| { + let index = syn::Index::from(i); + quote! { + spk_schema_foundation::IsDefault::is_default(&self.#index) + } + }); + + quote! { + impl spk_schema_foundation::IsDefault for #name { + fn is_default(&self) -> bool { + true #(&& #field_checks)* + } + } + } + } + Fields::Unit => { + quote! { + impl spk_schema_foundation::IsDefault for #name { + fn is_default(&self) -> bool { + true + } + } + } + } + } + } + _ => panic!("IsDefault can only be derived for structs"), + }; + + // Return the generated code + TokenStream::from(expanded) +} 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 129f7342e9..bd4f46346a 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 @@ -10,6 +10,7 @@ use spk_schema::foundation::option_map; use spk_schema::ident::version_ident; use spk_schema::ident_component::Component; use spk_schema::option_map::HOST_OPTIONS; +use spk_schema::RuntimeEnvironment; use spk_storage::fixtures::*; use super::Build; @@ -728,3 +729,36 @@ build: ); } } + +#[rstest] +#[tokio::test] +async fn test_package_with_environment_ops_preserves_ops_in_recipe(tmpdir: tempfile::TempDir) { + let rt = spfs_runtime().await; + + build_package!( + tmpdir, + "env-ops.spk.yaml", + br#" +api: v0/package +pkg: env-ops/1.0.0 +build: + script: + - true +install: + environment: + - set: FOO + value: bar +"# + ); + + let recipe = rt + .tmprepo + .read_recipe(&version_ident!("env-ops/1.0.0")) + .await + .unwrap(); + + assert!( + !recipe.runtime_environment().is_empty(), + "should have environment ops" + ); +} diff --git a/crates/spk-schema/Cargo.toml b/crates/spk-schema/Cargo.toml index 2144874b64..f5193f5d51 100644 --- a/crates/spk-schema/Cargo.toml +++ b/crates/spk-schema/Cargo.toml @@ -26,6 +26,7 @@ format_serde_error = { version = "0.3", default-features = false, features = [ ] } ignore = "0.4.18" indexmap = { workspace = true } +is_default_derive_macro = { workspace = true } itertools = { workspace = true } nom = { workspace = true } regex = { workspace = true } diff --git a/crates/spk-schema/crates/foundation/src/is_default.rs b/crates/spk-schema/crates/foundation/src/is_default.rs new file mode 100644 index 0000000000..1446a6f25d --- /dev/null +++ b/crates/spk-schema/crates/foundation/src/is_default.rs @@ -0,0 +1,8 @@ +// Copyright (c) Contributors to the SPK project. +// SPDX-License-Identifier: Apache-2.0 +// https://github.com/spkenv/spk + +pub trait IsDefault { + /// Returns true if the value equivalent to the default value. + fn is_default(&self) -> bool; +} diff --git a/crates/spk-schema/crates/foundation/src/lib.rs b/crates/spk-schema/crates/foundation/src/lib.rs index 22e3996f0c..4527d47087 100644 --- a/crates/spk-schema/crates/foundation/src/lib.rs +++ b/crates/spk-schema/crates/foundation/src/lib.rs @@ -9,6 +9,7 @@ mod from_yaml; pub mod ident_build; pub mod ident_component; pub mod ident_ops; +mod is_default; pub mod name; pub mod option_map; pub mod spec_ops; @@ -17,3 +18,4 @@ pub mod version_range; pub use fixtures::*; pub use from_yaml::{FromYaml, SerdeYamlError}; +pub use is_default::IsDefault; diff --git a/crates/spk-schema/crates/foundation/src/version/compat.rs b/crates/spk-schema/crates/foundation/src/version/compat.rs index ccf711751c..3e65ea3be5 100644 --- a/crates/spk-schema/crates/foundation/src/version/compat.rs +++ b/crates/spk-schema/crates/foundation/src/version/compat.rs @@ -12,7 +12,7 @@ use serde::{Deserialize, Serialize}; use super::{Error, Result, Version, VERSION_SEP}; use crate::name::PkgNameBuf; -use crate::version; +use crate::{version, IsDefault}; #[cfg(test)] #[path = "./compat_test.rs"] @@ -324,11 +324,6 @@ impl FromStr for Compat { } impl Compat { - // True if this is the default compatibility specification - pub fn is_default(&self) -> bool { - self == &Self::default() - } - /// Create a compat rule set with two parts pub fn double(first: CompatRuleSet, second: CompatRuleSet) -> Self { Compat { @@ -467,6 +462,13 @@ impl Compat { } } +impl IsDefault for Compat { + // True if this is the default compatibility specification + fn is_default(&self) -> bool { + self == &Self::default() + } +} + /// Parse a string as a compatibility specifier. pub fn parse_compat>(compat: S) -> super::Result { Compat::from_str(compat.as_ref()) diff --git a/crates/spk-schema/crates/ident/src/request.rs b/crates/spk-schema/crates/ident/src/request.rs index e4ab184898..076a20c8d1 100644 --- a/crates/spk-schema/crates/ident/src/request.rs +++ b/crates/spk-schema/crates/ident/src/request.rs @@ -28,6 +28,7 @@ use spk_schema_foundation::version_range::{ RestrictMode, VersionFilter, }; +use spk_schema_foundation::IsDefault; use super::AnyIdent; use crate::{BuildIdent, Error, RangeIdent, Result, Satisfy, VersionIdent}; @@ -45,8 +46,8 @@ pub enum PreReleasePolicy { IncludeAll, } -impl PreReleasePolicy { - pub fn is_default(&self) -> bool { +impl IsDefault for PreReleasePolicy { + fn is_default(&self) -> bool { matches!(self, &PreReleasePolicy::ExcludeAll) } } @@ -73,8 +74,8 @@ pub enum InclusionPolicy { IfAlreadyPresent, } -impl InclusionPolicy { - pub fn is_default(&self) -> bool { +impl IsDefault for InclusionPolicy { + fn is_default(&self) -> bool { matches!(self, &InclusionPolicy::Always) } } @@ -101,9 +102,9 @@ pub enum PinPolicy { IfPresentInBuildEnv, } -impl PinPolicy { +impl IsDefault for PinPolicy { #[inline] - pub fn is_default(&self) -> bool { + fn is_default(&self) -> bool { self == &PinPolicy::default() } } diff --git a/crates/spk-schema/src/build_spec.rs b/crates/spk-schema/src/build_spec.rs index befd782f42..e0474432e9 100644 --- a/crates/spk-schema/src/build_spec.rs +++ b/crates/spk-schema/src/build_spec.rs @@ -13,6 +13,7 @@ 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 spk_schema_foundation::IsDefault; use strum::Display; use super::{v0, Opt, ValidationSpec}; @@ -51,10 +52,6 @@ pub enum AutoHostVars { } impl AutoHostVars { - pub fn is_default(&self) -> bool { - self == &Self::default() - } - fn names_added(&self) -> HashSet<&OptName> { let names = match self { AutoHostVars::Distro => DISTRO_ADDS, @@ -128,6 +125,12 @@ impl AutoHostVars { } } +impl IsDefault for AutoHostVars { + fn is_default(&self) -> bool { + self == &Self::default() + } +} + /// A set of structured inputs used to build a package. #[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)] pub struct BuildSpec { @@ -161,10 +164,6 @@ impl Default for BuildSpec { } impl BuildSpec { - pub fn is_default(&self) -> bool { - self == &Self::default() - } - /// Returns this build's options, plus any additional ones needed /// for building the given variant pub fn opts_for_variant(&self, variant: &V) -> Result> @@ -335,6 +334,12 @@ impl BuildSpec { } } +impl IsDefault for BuildSpec { + fn is_default(&self) -> bool { + self == &Self::default() + } +} + impl TryFrom for BuildSpec { type Error = crate::Error; diff --git a/crates/spk-schema/src/build_spec_test.rs b/crates/spk-schema/src/build_spec_test.rs index e654946434..8783e3e442 100644 --- a/crates/spk-schema/src/build_spec_test.rs +++ b/crates/spk-schema/src/build_spec_test.rs @@ -3,7 +3,7 @@ // https://github.com/spkenv/spk use rstest::rstest; -use spk_schema_foundation::{option_map, pkg_name, FromYaml}; +use spk_schema_foundation::{option_map, pkg_name, FromYaml, IsDefault}; use super::{AutoHostVars, BuildSpec}; use crate::build_spec::UncheckedBuildSpec; diff --git a/crates/spk-schema/src/component_spec_list.rs b/crates/spk-schema/src/component_spec_list.rs index e40716f2a6..b0d69b5fc7 100644 --- a/crates/spk-schema/src/component_spec_list.rs +++ b/crates/spk-schema/src/component_spec_list.rs @@ -5,6 +5,7 @@ use std::collections::{BTreeSet, HashMap, HashSet}; use serde::{Deserialize, Serialize}; +use spk_schema_foundation::IsDefault; use super::ComponentSpec; use crate::foundation::ident_component::Component; @@ -19,8 +20,8 @@ mod component_spec_list_test; #[serde(transparent)] pub struct ComponentSpecList(Vec); -impl ComponentSpecList { - pub fn is_default(&self) -> bool { +impl IsDefault for ComponentSpecList { + fn is_default(&self) -> bool { self == &Self::default() } } diff --git a/crates/spk-schema/src/embedded_packages_list.rs b/crates/spk-schema/src/embedded_packages_list.rs index 2bc0c8f1ac..0ec90be6f4 100644 --- a/crates/spk-schema/src/embedded_packages_list.rs +++ b/crates/spk-schema/src/embedded_packages_list.rs @@ -4,6 +4,7 @@ use serde::{Deserialize, Serialize}; use spk_schema_foundation::ident_build::EmbeddedSource; +use spk_schema_foundation::IsDefault; use spk_schema_ident::AnyIdent; use super::{BuildSpec, InstallSpec, Spec}; @@ -18,6 +19,12 @@ mod embedded_packages_list_test; #[serde(transparent)] pub struct EmbeddedPackagesList(Vec); +impl IsDefault for EmbeddedPackagesList { + fn is_default(&self) -> bool { + self.is_empty() + } +} + impl std::ops::Deref for EmbeddedPackagesList { type Target = Vec; fn deref(&self) -> &Self::Target { diff --git a/crates/spk-schema/src/environ.rs b/crates/spk-schema/src/environ.rs index b2d7130e1c..cd8b680ed2 100644 --- a/crates/spk-schema/src/environ.rs +++ b/crates/spk-schema/src/environ.rs @@ -7,6 +7,7 @@ use std::sync::Arc; use serde::{Deserialize, Serialize}; use spk_schema_foundation::option_map::Stringified; +use spk_schema_foundation::IsDefault; #[cfg(test)] #[path = "./environ_test.rs"] @@ -190,6 +191,12 @@ impl EnvOp { #[derive(Clone, Debug, Default, Deserialize, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)] pub struct EnvOpList(Vec); +impl IsDefault for EnvOpList { + fn is_default(&self) -> bool { + self.0.is_empty() + } +} + impl std::ops::Deref for EnvOpList { type Target = Vec; diff --git a/crates/spk-schema/src/install_spec.rs b/crates/spk-schema/src/install_spec.rs index 9a4228845c..f958dd2459 100644 --- a/crates/spk-schema/src/install_spec.rs +++ b/crates/spk-schema/src/install_spec.rs @@ -3,18 +3,31 @@ // https://github.com/spkenv/spk use serde::{Deserialize, Serialize}; +use spk_schema_foundation::IsDefault; use spk_schema_ident::BuildIdent; use super::{ComponentSpecList, EmbeddedPackagesList, EnvOp, OpKind, RequirementsList}; use crate::foundation::option_map::OptionMap; -use crate::Result; +use crate::{EnvOpList, Result}; #[cfg(test)] #[path = "./install_spec_test.rs"] mod install_spec_test; /// A set of structured installation parameters for a package. -#[derive(Clone, Debug, Default, Deserialize, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)] +#[derive( + Clone, + Debug, + Default, + Deserialize, + Eq, + Hash, + is_default_derive_macro::IsDefault, + Ord, + PartialEq, + PartialOrd, + Serialize, +)] pub struct InstallSpec { #[serde(default, skip_serializing_if = "Vec::is_empty")] pub requirements: RequirementsList, @@ -25,16 +38,12 @@ pub struct InstallSpec { #[serde( default, deserialize_with = "deserialize_env_conf", - skip_serializing_if = "Vec::is_empty" + skip_serializing_if = "IsDefault::is_default" )] pub environment: EnvOpList, } impl InstallSpec { - pub fn is_default(&self) -> bool { - self.requirements.is_empty() && self.embedded.is_empty() && self.components.is_default() - } - /// Render all requests with a package pin using the given resolved packages. pub fn render_all_pins<'a>( &mut self, diff --git a/crates/spk-schema/src/metadata/meta.rs b/crates/spk-schema/src/metadata/meta.rs index 1055f625c6..52c4032338 100644 --- a/crates/spk-schema/src/metadata/meta.rs +++ b/crates/spk-schema/src/metadata/meta.rs @@ -7,6 +7,7 @@ use std::process::{Command, Stdio}; use serde::{Deserialize, Serialize}; use spk_config::Metadata; +use spk_schema_foundation::IsDefault; use crate::{Error, Result}; @@ -27,10 +28,6 @@ pub struct Meta { } impl Meta { - pub fn is_default(&self) -> bool { - self == &Self::default() - } - pub fn has_label_with_value(&self, label: &str, value: &str) -> bool { if let Some(label_value) = self.labels.get(label) { return *label_value == value; @@ -83,3 +80,9 @@ impl Meta { Ok(0) } } + +impl IsDefault for Meta { + fn is_default(&self) -> bool { + self == &Self::default() + } +} diff --git a/crates/spk-schema/src/option.rs b/crates/spk-schema/src/option.rs index b0496247ee..f31ec02d6f 100644 --- a/crates/spk-schema/src/option.rs +++ b/crates/spk-schema/src/option.rs @@ -10,6 +10,7 @@ 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_foundation::IsDefault; use spk_schema_ident::{NameAndValue, PinnableValue, RangeIdent}; use crate::foundation::name::{OptName, OptNameBuf, PkgName, PkgNameBuf}; @@ -60,8 +61,8 @@ impl Default for Inheritance { } } -impl Inheritance { - pub fn is_default(&self) -> bool { +impl IsDefault for Inheritance { + fn is_default(&self) -> bool { self == &Self::default() } } diff --git a/crates/spk-schema/src/requirements_list.rs b/crates/spk-schema/src/requirements_list.rs index 8dad7595c7..84e573a357 100644 --- a/crates/spk-schema/src/requirements_list.rs +++ b/crates/spk-schema/src/requirements_list.rs @@ -8,6 +8,7 @@ use std::fmt::Write; use serde::{Deserialize, Serialize}; use spk_schema_foundation::name::{OptName, PkgName}; use spk_schema_foundation::version::Compatibility; +use spk_schema_foundation::IsDefault; use spk_schema_ident::{BuildIdent, PinPolicy}; use crate::foundation::option_map::OptionMap; @@ -27,6 +28,12 @@ mod requirements_list_test; #[serde(transparent)] pub struct RequirementsList(Vec); +impl IsDefault for RequirementsList { + fn is_default(&self) -> bool { + self.is_empty() + } +} + impl std::ops::Deref for RequirementsList { type Target = Vec; fn deref(&self) -> &Self::Target { diff --git a/crates/spk-schema/src/v0/platform.rs b/crates/spk-schema/src/v0/platform.rs index e4741546ae..a518e9181c 100644 --- a/crates/spk-schema/src/v0/platform.rs +++ b/crates/spk-schema/src/v0/platform.rs @@ -11,6 +11,7 @@ use spk_schema_foundation::name::PkgName; use spk_schema_foundation::option_map::{OptionMap, Stringified, HOST_OPTIONS}; use spk_schema_foundation::spec_ops::{HasVersion, Named, Versioned}; use spk_schema_foundation::version::Version; +use spk_schema_foundation::IsDefault; use spk_schema_ident::{ BuildIdent, InclusionPolicy, @@ -112,9 +113,9 @@ impl PlatformRequirements { #[derive(Debug, Clone, Hash, PartialEq, Eq, Ord, PartialOrd, Serialize)] pub struct Platform { pub platform: VersionIdent, - #[serde(default, skip_serializing_if = "Meta::is_default")] + #[serde(default, skip_serializing_if = "IsDefault::is_default")] pub meta: Meta, - #[serde(default, skip_serializing_if = "Compat::is_default")] + #[serde(default, skip_serializing_if = "IsDefault::is_default")] pub compat: Compat, #[serde(default, skip_serializing_if = "is_false")] pub deprecated: bool, diff --git a/crates/spk-schema/src/v0/spec.rs b/crates/spk-schema/src/v0/spec.rs index 708c4d65cf..2357d4b0e4 100644 --- a/crates/spk-schema/src/v0/spec.rs +++ b/crates/spk-schema/src/v0/spec.rs @@ -15,6 +15,7 @@ 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_foundation::IsDefault; use spk_schema_ident::{AnyIdent, BuildIdent, Ident, RangeIdent, VersionIdent}; use super::variant_spec::VariantSpecEntryKey; @@ -85,7 +86,7 @@ pub struct Spec { pub build: BuildSpec, #[serde(default, skip_serializing_if = "Vec::is_empty")] pub tests: Vec, - #[serde(default, skip_serializing_if = "InstallSpec::is_default")] + #[serde(default, skip_serializing_if = "IsDefault::is_default")] pub install: InstallSpec, } diff --git a/crates/spk-schema/src/validation.rs b/crates/spk-schema/src/validation.rs index 9db140090a..072eeb74aa 100644 --- a/crates/spk-schema/src/validation.rs +++ b/crates/spk-schema/src/validation.rs @@ -5,6 +5,7 @@ use serde::ser::SerializeMap; use serde::{Deserialize, Serialize}; use spk_schema_foundation::name::{PkgName, PkgNameBuf}; +use spk_schema_foundation::IsDefault; #[cfg(test)] #[path = "./validation_test.rs"] @@ -35,10 +36,6 @@ pub struct ValidationSpec { } impl ValidationSpec { - pub fn is_default(&self) -> bool { - self.rules.is_empty() && self.disabled.is_empty() - } - /// The rules as specified in the spec file. Usually this is not /// what you want, see [`Self::to_expanded_rules`]. /// @@ -106,6 +103,12 @@ impl ValidationSpec { } } +impl IsDefault for ValidationSpec { + fn is_default(&self) -> bool { + self.rules.is_empty() && self.disabled.is_empty() + } +} + /// Specifies an additional set of validation criteria for a package /// /// These rules are meant to be evaluated in order with later rules