diff --git a/src/variant_config.rs b/src/variant_config.rs index ac0b026c..256e11f6 100644 --- a/src/variant_config.rs +++ b/src/variant_config.rs @@ -18,7 +18,6 @@ use crate::{ recipe::{ custom_yaml::{HasSpan, Node, RenderedMappingNode, RenderedNode, TryConvertNode}, error::{ErrorKind, ParsingError, PartialParsingError}, - parser::BuildString, Jinja, Render, }, selectors::SelectorConfig, @@ -441,16 +440,19 @@ impl VariantConfig { // Now we need to convert the stage 1 renders to DiscoveredOutputs let mut recipes = IndexSet::new(); for sx in stage_1 { - for (idx, ((node, recipe), variant)) in sx.outputs().enumerate() { + for ((node, recipe), variant) in sx.into_sorted_outputs()? { let target_platform = if recipe.build().noarch().is_none() { selector_config.target_platform } else { Platform::NoArch }; - let mut recipe = recipe.clone(); - let build_string = sx.build_string_for_output(idx); - recipe.build.string = BuildString::Resolved(build_string.clone()); + let build_string = recipe + .build() + .string() + .as_resolved() + .expect("Build string has to be resolved") + .to_string(); recipes.insert(DiscoveredOutput { name: recipe.package().name.as_normalized().to_string(), @@ -458,7 +460,7 @@ impl VariantConfig { build_string, noarch_type: *recipe.build().noarch(), target_platform, - node: node.clone(), + node, used_vars: variant.clone(), recipe: recipe.clone(), hash: HashInfo::from_variant(&variant, recipe.build().noarch()), diff --git a/src/variant_render.rs b/src/variant_render.rs index a952a892..148f3b10 100644 --- a/src/variant_render.rs +++ b/src/variant_render.rs @@ -10,7 +10,11 @@ use crate::{ env_vars, hash::HashInfo, normalized_key::NormalizedKey, - recipe::{custom_yaml::Node, parser::Dependency, Jinja, ParsingError, Recipe}, + recipe::{ + custom_yaml::Node, + parser::{BuildString, Dependency}, + Jinja, ParsingError, Recipe, + }, selectors::SelectorConfig, used_variables::used_vars_from_expressions, variant_config::{ParseErrors, VariantConfig, VariantError}, @@ -55,17 +59,15 @@ pub(crate) fn stage_0_render( used_vars_from_expressions(output, recipe) .map(|x| x.into_iter().map(Into::into).collect()) }) - .collect::>, Vec>>(); - - // If there are any parsing errors, return them - if let Err(errors) = used_vars { - let err: ParseErrors = errors.into(); - return Err(VariantError::RecipeParseErrors(err)); - } + .collect::>, Vec>>() + .map_err(|errs| { + let errs: ParseErrors = errs.into(); + VariantError::RecipeParseErrors(errs) + })?; let raw_output_vec = RawOutputVec { vec: outputs.to_vec(), - used_vars_jinja: used_vars.unwrap(), + used_vars_jinja: used_vars, recipe: recipe.to_string(), }; @@ -74,7 +76,10 @@ pub(crate) fn stage_0_render( for output in outputs { used_vars.extend( used_vars_from_expressions(output, recipe) - .unwrap() + .map_err(|errs| { + let errs: ParseErrors = errs.into(); + VariantError::RecipeParseErrors(errs) + })? .into_iter() .map(Into::into), ); @@ -129,8 +134,6 @@ pub struct Stage1Render { pub(crate) inner: Vec, pub(crate) stage_0_render: Stage0Render, - - order: Vec, } impl Stage1Render { @@ -140,8 +143,10 @@ impl Stage1Render { .position(|x| x.recipe.package().name() == package_name) } - pub fn variant_for_output(&self, idx: usize) -> BTreeMap { - let idx = self.order[idx]; + pub fn variant_for_output( + &self, + idx: usize, + ) -> Result, VariantError> { let inner = &self.inner[idx]; // combine jinja variables and the variables from the dependencies let self_name = self.stage_0_render.rendered_outputs[idx].package().name(); @@ -175,9 +180,11 @@ impl Stage1Render { if pin == self_name { continue; } - let other_idx = self.index_from_name(pin).unwrap(); + let Some(other_idx) = self.index_from_name(pin) else { + return Err(VariantError::MissingOutput(pin.as_source().to_string())); + }; // find the referenced output - let build_string = self.build_string_for_output(other_idx); + let build_string = self.build_string_for_output(other_idx)?; let version = self.inner[other_idx].recipe.package().version(); variant.insert( pin.as_normalized().into(), @@ -190,28 +197,28 @@ impl Stage1Render { variant.insert("target_platform".into(), "noarch".into()); } - variant + Ok(variant) } - pub fn build_string_for_output(&self, idx: usize) -> String { - let variant = self.variant_for_output(idx); - let recipe = &self.stage_0_render.rendered_outputs[self.order[idx]]; + pub fn build_string_for_output(&self, idx: usize) -> Result { + let variant = self.variant_for_output(idx)?; + let recipe = &self.stage_0_render.rendered_outputs[idx]; let hash = HashInfo::from_variant(&variant, recipe.build().noarch()); - let inner = &self.inner[self.order[idx]]; + let inner = &self.inner[idx]; let mut selector_config = inner.selector_config.clone(); selector_config.hash = Some(hash.clone()); let jinja = Jinja::new(selector_config.clone()).with_context(&recipe.context); - recipe + Ok(recipe .build() .string() .resolve(&hash, recipe.build().number, &jinja) - .into_owned() + .into_owned()) } /// sort the outputs topologically - pub fn sort_outputs(self) -> Self { + pub fn sorted_indices(&self) -> Result, VariantError> { // Create an empty directed graph let mut graph = DiGraph::<_, ()>::new(); let mut node_indices = Vec::new(); @@ -249,8 +256,16 @@ impl Stage1Render { } // Sort the outputs topologically - let sorted_indices = - petgraph::algo::toposort(&graph, None).expect("Could not sort topologically."); + let sorted_indices = match petgraph::algo::toposort(&graph, None) { + Ok(sorted_indices) => sorted_indices, + Err(cycle) => { + let cycle = cycle.node_id(); + let cycle_name = graph[cycle].package().name(); + return Err(VariantError::CycleInRecipeOutputs( + cycle_name.as_source().to_string(), + )); + } + }; let sorted_indices = sorted_indices .into_iter() @@ -258,26 +273,30 @@ impl Stage1Render { .collect::>(); // Update the order of the outputs - Stage1Render { - order: sorted_indices, - ..self - } + Ok(sorted_indices) } - pub fn outputs( - &self, - ) -> impl Iterator)> { + pub fn into_sorted_outputs( + self, + ) -> Result)>, VariantError> { // zip node from stage0 and final render output - let raw_nodes = self.stage_0_render.raw_outputs.vec.iter(); - let outputs: Vec<&Recipe> = self.inner.iter().map(|i| &i.recipe).collect(); + let sorted_indices = self.sorted_indices()?; + + let raw_nodes = self.stage_0_render.raw_outputs.vec.clone().into_iter(); + let outputs = self.inner.clone().into_iter().map(|i| i.recipe); let zipped = raw_nodes.zip(outputs).collect::>(); + let mut result = Vec::new(); + for idx in sorted_indices { + let mut recipe = zipped[idx].clone(); + let build_string = self.build_string_for_output(idx)?; + // Resolve the build string and store the resolved one in the recipe + recipe.1.build.string = BuildString::Resolved(build_string); + let variant = self.variant_for_output(idx)?; + result.push((recipe, variant)); + } - (0..zipped.len()).map(move |idx| { - let recipe = zipped[self.order[idx]]; - let variant = self.variant_for_output(idx); - (recipe, variant) - }) + Ok(result) } } @@ -337,7 +356,7 @@ pub(crate) fn stage_1_render( } // special handling of CONDA_BUILD_SYSROOT - let jinja_variables = r.raw_outputs.used_vars_jinja.get(idx).unwrap(); + let jinja_variables = &r.raw_outputs.used_vars_jinja[idx]; if jinja_variables.contains(&"c_compiler".into()) || jinja_variables.contains(&"cxx_compiler".into()) { @@ -383,7 +402,15 @@ pub(crate) fn stage_1_render( let config_with_variant = selector_config .with_variant(combination.clone(), selector_config.target_platform); - let parsed_recipe = Recipe::from_node(output, config_with_variant.clone()).unwrap(); + let parsed_recipe = Recipe::from_node(output, config_with_variant.clone()) + .map_err(|err| { + let errs: ParseErrors = err + .into_iter() + .map(|err| ParsingError::from_partial(&r.raw_outputs.recipe, err)) + .collect::>() + .into(); + errs + })?; inner.push(Stage1Inner { used_vars_from_dependencies: extra_vars_per_output[idx].clone(), @@ -397,9 +424,7 @@ pub(crate) fn stage_1_render( inner, variables: combination, stage_0_render: r.clone(), - order: (0..r.rendered_outputs.len()).collect(), - } - .sort_outputs(); + }; stage_1_renders.push(stage_1); } diff --git a/test-data/recipes/race-condition/recipe-cycle.yaml b/test-data/recipes/race-condition/recipe-cycle.yaml new file mode 100644 index 00000000..78f6e78b --- /dev/null +++ b/test-data/recipes/race-condition/recipe-cycle.yaml @@ -0,0 +1,14 @@ +recipe: + version: "1.2.3" + +outputs: + - package: + name: foobar + requirements: + host: + - bazbus + - package: + name: bazbus + requirements: + host: + - foobar diff --git a/test-data/recipes/race-condition/recipe-pin-invalid.yaml b/test-data/recipes/race-condition/recipe-pin-invalid.yaml new file mode 100644 index 00000000..f813deda --- /dev/null +++ b/test-data/recipes/race-condition/recipe-pin-invalid.yaml @@ -0,0 +1,19 @@ +recipe: + name: test-split + version: v1 + +build: + number: 0 + noarch: python + +source: + path: ./ + +outputs: + - package: + name: test.a + - package: + name: test.b + requirements: + run: + - ${{ pin_subpackage('test1', exact=True) }} # this package does not exist! diff --git a/test-data/recipes/race-condition/recipe-pin-subpackage.yaml b/test-data/recipes/race-condition/recipe-pin-subpackage.yaml new file mode 100644 index 00000000..4c2049d3 --- /dev/null +++ b/test-data/recipes/race-condition/recipe-pin-subpackage.yaml @@ -0,0 +1,19 @@ +recipe: + name: test-split + version: v1 + +build: + number: 0 + noarch: python + +source: + path: ./ + +outputs: + - package: + name: test2 + requirements: + run: + - ${{ pin_subpackage('test1', exact=True) }} + - package: + name: test1 diff --git a/test-data/recipes/race-condition/recipe-python-min.yaml b/test-data/recipes/race-condition/recipe-python-min.yaml new file mode 100644 index 00000000..d959a763 --- /dev/null +++ b/test-data/recipes/race-condition/recipe-python-min.yaml @@ -0,0 +1,15 @@ +package: + name: test-python-min + version: '1.0.0' + +requirements: + host: + - python ${{ python_min }} + - python >=${{ python_min }} + - python ${{ python_min }}.* + - python ${{ python_min ~ ".*,<4.0a0" }} + run: + - python ${{ python_min }} + - python >=${{ python_min }} + - python ${{ python_min }}.* + - python ${{ python_min ~ ".*,<4.0a0" }} diff --git a/test-data/recipes/race-condition/variants.yaml b/test-data/recipes/race-condition/variants.yaml index 22601765..cbcd84ce 100644 --- a/test-data/recipes/race-condition/variants.yaml +++ b/test-data/recipes/race-condition/variants.yaml @@ -1,3 +1,6 @@ variant: - a - b + +python_min: + - "3.8.0" diff --git a/test/end-to-end/__snapshots__/test_simple/test_python_min_render.json b/test/end-to-end/__snapshots__/test_simple/test_python_min_render.json new file mode 100644 index 00000000..b334593a --- /dev/null +++ b/test/end-to-end/__snapshots__/test_simple/test_python_min_render.json @@ -0,0 +1,14 @@ +{ + "host": [ + "python ==3.8.0", + "python >=3.8.0", + "python 3.8.0.*", + "python 3.8.0.*,<4.0a0" + ], + "run": [ + "python ==3.8.0", + "python >=3.8.0", + "python 3.8.0.*", + "python 3.8.0.*,<4.0a0" + ] +} diff --git a/test/end-to-end/helpers.py b/test/end-to-end/helpers.py index 166065a1..d01785b3 100644 --- a/test/end-to-end/helpers.py +++ b/test/end-to-end/helpers.py @@ -13,8 +13,9 @@ def __call__(self, *args: Any, **kwds: Any) -> Any: try: return check_output([str(self.path), *args], **kwds).decode("utf-8") except CalledProcessError as e: - print(e.output) - print(e.stderr) + if kwds.get("stderr") is None: + print(e.output) + print(e.stderr) raise e def build_args( @@ -77,6 +78,7 @@ def render( custom_channels: Optional[list[str]] = None, extra_args: list[str] = None, extra_meta: dict[str, Any] = None, + **kwargs: Any, ) -> Any: args = self.build_args( recipe_folder, @@ -88,7 +90,7 @@ def render( ) if with_solve: args += ["--with-solve"] - output = self(*args, "--render-only") + output = self(*args, "--render-only", **kwargs) return json.loads(output) diff --git a/test/end-to-end/test_simple.py b/test/end-to-end/test_simple.py index e07d2772..5dedd6d2 100644 --- a/test/end-to-end/test_simple.py +++ b/test/end-to-end/test_simple.py @@ -1103,9 +1103,7 @@ def test_pin_compatible( assert snapshot_json == rendered[0]["recipe"]["requirements"] -def test_render_variants( - rattler_build: RattlerBuild, recipes: Path, tmp_path: Path, snapshot_json -): +def test_render_variants(rattler_build: RattlerBuild, recipes: Path, tmp_path: Path): rendered = rattler_build.render( recipes / "race-condition/recipe-undefined-variant.yaml", tmp_path ) @@ -1115,8 +1113,50 @@ def test_render_variants( ] -def test_race_condition( - rattler_build: RattlerBuild, recipes: Path, tmp_path: Path, snapshot_json -): +def test_race_condition(rattler_build: RattlerBuild, recipes: Path, tmp_path: Path): # make sure that tests are ran in the right order and that the packages are built correctly rattler_build.build(recipes / "race-condition", tmp_path) + + +def test_variant_sorting(rattler_build: RattlerBuild, recipes: Path, tmp_path: Path): + # make sure that tests are ran in the right order and that the packages are built correctly + rendered = rattler_build.render( + recipes / "race-condition" / "recipe-pin-subpackage.yaml", tmp_path + ) + assert [rx["recipe"]["package"]["name"] for rx in rendered] == ["test1", "test2"] + + +def test_missing_pin_subpackage( + rattler_build: RattlerBuild, recipes: Path, tmp_path: Path +): + # make sure that tests are ran in the right order and that the packages are built correctly + with pytest.raises(CalledProcessError) as e: + rattler_build.render( + recipes / "race-condition" / "recipe-pin-invalid.yaml", + tmp_path, + stderr=STDOUT, + ) + stdout = e.value.output.decode("utf-8") + assert "Missing output: test1 (used in pin_subpackage)" in stdout + + +def test_cycle_detection(rattler_build: RattlerBuild, recipes: Path, tmp_path: Path): + # make sure that tests are ran in the right order and that the packages are built correctly + with pytest.raises(CalledProcessError) as e: + rattler_build.render( + recipes / "race-condition" / "recipe-cycle.yaml", + tmp_path, + stderr=STDOUT, + ) + stdout = e.value.output.decode("utf-8") + assert "Found a cycle in the recipe outputs: bazbus" in stdout + + +def test_python_min_render( + rattler_build: RattlerBuild, recipes: Path, tmp_path: Path, snapshot_json +): + rendered = rattler_build.render( + recipes / "race-condition" / "recipe-python-min.yaml", tmp_path + ) + + assert snapshot_json == rendered[0]["recipe"]["requirements"]