From d8631714a0bfd910f6cd925c195ba0ab0946dde6 Mon Sep 17 00:00:00 2001 From: Bas Zalmstra Date: Thu, 7 Dec 2023 14:58:09 +0100 Subject: [PATCH] refactor: serde Dependency instead of string magic (#405) --- src/recipe/parser/requirements.rs | 128 +++++++----------- ..._requirements__test__compiler_serde-2.snap | 2 +- ...r__requirements__test__compiler_serde.snap | 2 +- ...d__metadata__test__read_full_recipe-2.snap | 2 +- test-data/rendered_recipes/curl_recipe.yaml | 2 +- 5 files changed, 52 insertions(+), 84 deletions(-) diff --git a/src/recipe/parser/requirements.rs b/src/recipe/parser/requirements.rs index 64418916..a09e12a9 100644 --- a/src/recipe/parser/requirements.rs +++ b/src/recipe/parser/requirements.rs @@ -1,9 +1,10 @@ //! Parsing for the requirements section of the recipe. use indexmap::IndexSet; -use std::{fmt, str::FromStr}; +use std::str::FromStr; use rattler_conda_types::{MatchSpec, PackageName}; +use serde::de::Error; use serde::{Deserialize, Serialize}; use crate::recipe::custom_yaml::RenderedSequenceNode; @@ -205,7 +206,8 @@ impl PinCompatible { /// For example, a c-compiler will resolve to the variant key `c_compiler`. /// If that value is `gcc`, the rendered compiler will read `gcc_linux-64` because /// it is always resolved with the target_platform. -#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] +#[serde(transparent)] pub struct Compiler { /// The language such as c, cxx, rust, etc. language: String, @@ -218,29 +220,6 @@ impl Compiler { } } -impl Serialize for Compiler { - fn serialize(&self, serializer: S) -> Result - where - S: serde::ser::Serializer, - { - format!("__COMPILER {}", self.language).serialize(serializer) - } -} - -impl FromStr for Compiler { - type Err = String; - - fn from_str(s: &str) -> Result { - if let Some(lang) = s.strip_prefix("__COMPILER ") { - Ok(Self { - language: lang.into(), - }) - } else { - Err(format!("compiler without prefix: {}", s)) - } - } -} - /// A combination of all possible dependencies. #[derive(Debug, Clone)] pub enum Dependency { @@ -324,43 +303,28 @@ impl<'de> Deserialize<'de> for Dependency { where D: serde::Deserializer<'de>, { - struct DependencyVisitor; - - impl<'de> serde::de::Visitor<'de> for DependencyVisitor { - type Value = Dependency; - - fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { - formatter.write_str( - "a string starting with '__COMPILER', '__PIN_SUBPACKAGE', or a MatchSpec", - ) - } + #[derive(Deserialize)] + #[serde(rename_all = "snake_case")] + enum RawDependency { + PinSubpackage(PinSubpackage), + PinCompatible(PinCompatible), + Compiler(Compiler), + } - fn visit_str(self, value: &str) -> Result - where - E: serde::de::Error, - { - if let Some(compiler_language) = value.strip_prefix("__COMPILER ") { - Ok(Dependency::Compiler(Compiler { - language: compiler_language.to_lowercase(), - })) - } else if let Some(pin) = value.strip_prefix("__PIN_SUBPACKAGE ") { - Ok(Dependency::PinSubpackage(PinSubpackage { - pin_subpackage: Pin::from_internal_repr(pin), - })) - } else if let Some(pin) = value.strip_prefix("__PIN_COMPATIBLE ") { - Ok(Dependency::PinCompatible(PinCompatible { - pin_compatible: Pin::from_internal_repr(pin), - })) - } else { - // Assuming MatchSpec can be constructed from a string. - MatchSpec::from_str(value) - .map(Dependency::Spec) - .map_err(serde::de::Error::custom) - } - } + #[derive(Deserialize)] + #[serde(untagged)] + enum RawSpec { + String(String), + Explicit(#[serde(with = "serde_yaml::with::singleton_map")] RawDependency), } - deserializer.deserialize_str(DependencyVisitor) + let raw_spec = RawSpec::deserialize(deserializer)?; + Ok(match raw_spec { + RawSpec::String(spec) => Dependency::Spec(spec.parse().map_err(D::Error::custom)?), + RawSpec::Explicit(RawDependency::PinSubpackage(dep)) => Dependency::PinSubpackage(dep), + RawSpec::Explicit(RawDependency::PinCompatible(dep)) => Dependency::PinCompatible(dep), + RawSpec::Explicit(RawDependency::Compiler(dep)) => Dependency::Compiler(dep), + }) } } @@ -369,20 +333,29 @@ impl Serialize for Dependency { where S: serde::ser::Serializer, { - match self { - Dependency::Spec(spec) => serializer.serialize_str(&spec.to_string()), - Dependency::PinSubpackage(pin) => serializer.serialize_str(&format!( - "__PIN_SUBPACKAGE {}", - pin.pin_subpackage.internal_repr() - )), - Dependency::PinCompatible(pin) => serializer.serialize_str(&format!( - "__PIN_COMPATIBLE {}", - pin.pin_compatible.internal_repr() - )), - Dependency::Compiler(compiler) => { - serializer.serialize_str(&format!("__COMPILER {}", compiler.language())) - } + #[derive(Serialize)] + #[serde(rename_all = "snake_case")] + enum RawDependency<'a> { + PinSubpackage(&'a PinSubpackage), + PinCompatible(&'a PinCompatible), + Compiler(&'a Compiler), + } + + #[derive(Serialize)] + #[serde(untagged)] + enum RawSpec<'a> { + String(String), + Explicit(#[serde(with = "serde_yaml::with::singleton_map")] RawDependency<'a>), } + + let raw = match self { + Dependency::Spec(dep) => RawSpec::String(dep.to_string()), + Dependency::PinSubpackage(dep) => RawSpec::Explicit(RawDependency::PinSubpackage(dep)), + Dependency::PinCompatible(dep) => RawSpec::Explicit(RawDependency::PinCompatible(dep)), + Dependency::Compiler(dep) => RawSpec::Explicit(RawDependency::Compiler(dep)), + }; + + raw.serialize(serializer) } } @@ -619,12 +592,7 @@ impl TryConvertNode for RenderedMappingNode { #[cfg(test)] mod test { - use requirements::{Dependency, Requirements}; - - use crate::recipe::parser::requirements; - - /// test serialization and deserialization of Compiler - use super::Compiler; + use super::*; #[test] fn test_compiler_serde() { @@ -633,7 +601,7 @@ mod test { }; let serialized = serde_yaml::to_string(&compiler).unwrap(); - assert_eq!(serialized, "__COMPILER gcc\n"); + assert_eq!(serialized, "gcc\n"); let requirements = Requirements { build: vec![Dependency::Compiler(compiler)], @@ -643,7 +611,7 @@ mod test { insta::assert_yaml_snapshot!(requirements); let yaml = serde_yaml::to_string(&requirements).unwrap(); - assert_eq!(yaml, "build:\n- __COMPILER gcc\n"); + assert_eq!(yaml, "build:\n- compiler: gcc\n"); let deserialized: Requirements = serde_yaml::from_str(&yaml).unwrap(); insta::assert_yaml_snapshot!(deserialized); diff --git a/src/recipe/parser/snapshots/rattler_build__recipe__parser__requirements__test__compiler_serde-2.snap b/src/recipe/parser/snapshots/rattler_build__recipe__parser__requirements__test__compiler_serde-2.snap index 780e4b56..8864f489 100644 --- a/src/recipe/parser/snapshots/rattler_build__recipe__parser__requirements__test__compiler_serde-2.snap +++ b/src/recipe/parser/snapshots/rattler_build__recipe__parser__requirements__test__compiler_serde-2.snap @@ -3,5 +3,5 @@ source: src/recipe/parser/requirements.rs expression: deserialized --- build: - - __COMPILER gcc + - compiler: gcc diff --git a/src/recipe/parser/snapshots/rattler_build__recipe__parser__requirements__test__compiler_serde.snap b/src/recipe/parser/snapshots/rattler_build__recipe__parser__requirements__test__compiler_serde.snap index 2a7ca2b0..5bddf5d4 100644 --- a/src/recipe/parser/snapshots/rattler_build__recipe__parser__requirements__test__compiler_serde.snap +++ b/src/recipe/parser/snapshots/rattler_build__recipe__parser__requirements__test__compiler_serde.snap @@ -3,5 +3,5 @@ source: src/recipe/parser/requirements.rs expression: requirements --- build: - - __COMPILER gcc + - compiler: gcc diff --git a/src/snapshots/rattler_build__metadata__test__read_full_recipe-2.snap b/src/snapshots/rattler_build__metadata__test__read_full_recipe-2.snap index 4e8a44bf..88b15bc5 100644 --- a/src/snapshots/rattler_build__metadata__test__read_full_recipe-2.snap +++ b/src/snapshots/rattler_build__metadata__test__read_full_recipe-2.snap @@ -14,7 +14,7 @@ recipe: string: h60d57d3_0 requirements: build: - - __COMPILER c + - compiler: c - make - perl - pkg-config diff --git a/test-data/rendered_recipes/curl_recipe.yaml b/test-data/rendered_recipes/curl_recipe.yaml index 0592b0de..7ecc1a3a 100644 --- a/test-data/rendered_recipes/curl_recipe.yaml +++ b/test-data/rendered_recipes/curl_recipe.yaml @@ -10,7 +10,7 @@ recipe: string: h60d57d3_0 requirements: build: - - __COMPILER c + - compiler: c - make - perl - pkg-config