From 866b671a5c642e8c7ee84e0388d4b4fe0c984128 Mon Sep 17 00:00:00 2001 From: Wolf Vollprecht Date: Mon, 12 Feb 2024 13:50:12 +0100 Subject: [PATCH] fix: order of builds and `recipe` key schema (#617) --- src/recipe/parser/output.rs | 38 ++++++++++++----- src/variant_config.rs | 38 ++++++++++++++++- test-data/recipes/output_order/order_1.yaml | 47 +++++++++++++++++++++ 3 files changed, 111 insertions(+), 12 deletions(-) create mode 100644 test-data/recipes/output_order/order_1.yaml diff --git a/src/recipe/parser/output.rs b/src/recipe/parser/output.rs index 3fa65290c..0a927dad6 100644 --- a/src/recipe/parser/output.rs +++ b/src/recipe/parser/output.rs @@ -83,7 +83,32 @@ pub fn find_outputs_from_src(src: &str) -> Result, ParsingError> { return Ok(vec![recipe]); }; - // TODO: Schema + let mut recipe_version: Option = None; + // If `recipe` exists in root we will use the version as default for all outputs + // We otherwise ignore the `recipe.name` value. + if let Some(recipe_mapping) = root_map + .get("recipe") + .and_then(|recipe| recipe.as_mapping()) + { + // make sure that mapping only contains name and version + for (k, v) in recipe_mapping.iter() { + match k.as_str() { + "name" => {} + "version" => recipe_version = Some(v.clone()), + _ => { + return Err(ParsingError::from_partial( + src, + _partialerror!( + *k.span(), + ErrorKind::InvalidField(k.as_str().to_string().into()), + help = "recipe can only contain `name` and `version` fields" + ), + )); + } + } + } + } + let outputs = outputs.as_sequence().ok_or_else(|| { ParsingError::from_partial( src, @@ -107,18 +132,9 @@ pub fn find_outputs_from_src(src: &str) -> Result, ParsingError> { // 4. merge skip values (make sure to preserve the spans) // Note: Make sure to preserve the spans of the original root span so the error // messages remain accurate and point the correct part of the original recipe src - let mut root = root_map.clone(); root.remove("outputs"); - // recipe.version, if exists in root, and package.version doesn't exist in output, we will - // use that instead - // ignore recipe.name - let version = root - .get("recipe") - .and_then(|recipe| recipe.as_mapping()) - .and_then(|recipe| recipe.get("version")); - let mut output_node = output.clone(); let output_map = output_node.as_mapping_mut().ok_or_else(|| { @@ -173,7 +189,7 @@ pub fn find_outputs_from_src(src: &str) -> Result, ParsingError> { } } - if let Some(version) = version { + if let Some(version) = recipe_version.as_ref() { let Some(package_map) = output_map .get_mut("package") .and_then(|node| node.as_mapping_mut()) diff --git a/src/variant_config.rs b/src/variant_config.rs index 23cd94f75..1f8b19a19 100644 --- a/src/variant_config.rs +++ b/src/variant_config.rs @@ -403,12 +403,15 @@ impl VariantConfig { let noarch_type = parsed_recipe.build().noarch(); // add in any host and build dependencies - used_vars.extend(parsed_recipe.requirements().build_time().filter_map(|dep| { + used_vars.extend(parsed_recipe.requirements().all().filter_map(|dep| { match dep { Dependency::Spec(spec) => spec .name .as_ref() .and_then(|name| name.as_normalized().to_string().into()), + Dependency::PinSubpackage(pin) => { + Some(pin.pin_value().name.as_normalized().to_string()) + } _ => None, } })); @@ -525,6 +528,9 @@ impl VariantConfig { None } } + Dependency::PinSubpackage(pin_sub) => { + Some(pin_sub.pin_value().name.as_normalized().to_string()) + } _ => None, }) .collect::>(); @@ -986,4 +992,34 @@ mod tests { let combinations = config.combinations(&used_vars).unwrap(); assert_eq!(combinations.len(), 2 * 2 * 3); } + + #[test] + fn test_order() { + let test_data_dir = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("test-data"); + let selector_config = SelectorConfig { + target_platform: Platform::Linux64, + build_platform: Platform::Linux64, + ..Default::default() + }; + + for _ in 1..3 { + // First find all outputs from the recipe + let recipe_text = + std::fs::read_to_string(test_data_dir.join("recipes/output_order/order_1.yaml")) + .unwrap(); + let outputs = crate::recipe::parser::find_outputs_from_src(&recipe_text).unwrap(); + let variant_config = VariantConfig::from_files(&vec![], &selector_config).unwrap(); + let outputs_and_variants = variant_config + .find_variants(&outputs, &recipe_text, &selector_config) + .unwrap(); + + // assert output order + let order = vec!["some-pkg-a", "some-pkg", "some_pkg"]; + let outputs: Vec<_> = outputs_and_variants + .iter() + .map(|o| o.name.clone()) + .collect(); + assert_eq!(outputs, order); + } + } } diff --git a/test-data/recipes/output_order/order_1.yaml b/test-data/recipes/output_order/order_1.yaml new file mode 100644 index 000000000..e30dfc427 --- /dev/null +++ b/test-data/recipes/output_order/order_1.yaml @@ -0,0 +1,47 @@ +context: + name: some-pkg + name_a: ${{ name }}-a + version: 1.0.0 + build_number: 0 + +recipe: + name: ${{ name }} + version: ${{ version }} + +source: + path: ../ + +outputs: + - package: + name: ${{ name }} + version: ${{ version }} + build: + noarch: python + number: ${{ build_number }} + requirements: + host: + - python >=3.10 + - pip + run: + - ${{ pin_subpackage(name_a, min_pin="x.x.x") }} + - python >=3.10 + - package: + name: ${{ name_a }} + version: ${{ version }} + build: + noarch: python + number: ${{ build_number }} + requirements: + host: + - python >=3.10 + - pip + run: + - python >=3.10 + - package: + name: ${{ name | replace('-', '_') }} + version: ${{ version }} + build: + noarch: generic + requirements: + run: + - ${{ pin_subpackage(name, exact=true) }}