From cbeca6ea9ebcf06dd0fd942a3a19f7a6c897d83b Mon Sep 17 00:00:00 2001 From: Wolf Vollprecht Date: Tue, 17 Dec 2024 14:07:11 +0100 Subject: [PATCH 1/8] use parse instead of explicit parsing --- src/package_test/run_test.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/package_test/run_test.rs b/src/package_test/run_test.rs index e37df3e8..f749b689 100644 --- a/src/package_test/run_test.rs +++ b/src/package_test/run_test.rs @@ -511,7 +511,7 @@ impl PythonTest { // Add `pip` if pip_check is set to true if self.pip_check { dependencies_map.iter_mut().for_each(|(_, v)| { - v.push(MatchSpec::from_str("pip", ParseStrictness::Strict).unwrap()) + v.push("pip".parse().unwrap()) }); } @@ -614,7 +614,7 @@ impl PerlTest { )?; let dependencies = vec![ - MatchSpec::from_str("perl", ParseStrictness::Strict).unwrap(), + "perl".parse().unwrap(), match_spec, ]; From 72af83cf85940809e8990e722320013fdab0a443 Mon Sep 17 00:00:00 2001 From: Wolf Vollprecht Date: Tue, 17 Dec 2024 17:18:39 +0100 Subject: [PATCH 2/8] simplify matchspec parsing --- src/build.rs | 8 +++----- src/package_test/run_test.rs | 11 ++++------- src/recipe/parser/requirements.rs | 2 +- src/render/resolved_dependencies.rs | 16 ++-------------- 4 files changed, 10 insertions(+), 27 deletions(-) diff --git a/src/build.rs b/src/build.rs index 66293a26..d28a3c8b 100644 --- a/src/build.rs +++ b/src/build.rs @@ -3,7 +3,7 @@ use std::{path::PathBuf, vec}; use miette::{Context, IntoDiagnostic}; -use rattler_conda_types::{Channel, MatchSpec, ParseStrictness}; +use rattler_conda_types::{Channel, MatchSpec}; use crate::{ metadata::{build_reindexed_channels, Output}, @@ -39,10 +39,8 @@ pub async fn skip_existing( let match_specs = outputs .iter() - .map(|o| { - MatchSpec::from_str(o.name().as_normalized(), ParseStrictness::Strict).into_diagnostic() - }) - .collect::, _>>()?; + .map(|o| o.name().clone().into()) + .collect::>(); let channels = if only_local { vec![ diff --git a/src/package_test/run_test.rs b/src/package_test/run_test.rs index f749b689..52a9ff49 100644 --- a/src/package_test/run_test.rs +++ b/src/package_test/run_test.rs @@ -510,9 +510,9 @@ impl PythonTest { // Add `pip` if pip_check is set to true if self.pip_check { - dependencies_map.iter_mut().for_each(|(_, v)| { - v.push("pip".parse().unwrap()) - }); + dependencies_map + .iter_mut() + .for_each(|(_, v)| v.push("pip".parse().unwrap())); } // Run tests for each python version @@ -613,10 +613,7 @@ impl PerlTest { ParseStrictness::Lenient, )?; - let dependencies = vec![ - "perl".parse().unwrap(), - match_spec, - ]; + let dependencies = vec!["perl".parse().unwrap(), match_spec]; create_environment( "test", diff --git a/src/recipe/parser/requirements.rs b/src/recipe/parser/requirements.rs index 16e9cd8b..ef37cb97 100644 --- a/src/recipe/parser/requirements.rs +++ b/src/recipe/parser/requirements.rs @@ -366,7 +366,7 @@ impl TryConvertNode for RenderedNode { impl TryConvertNode for RenderedScalarNode { fn try_convert(&self, _name: &str) -> Result> { - MatchSpec::from_str(self.as_str(), ParseStrictness::Strict).map_err(|err| { + MatchSpec::from_str(self.as_str(), ParseStrictness::Lenient).map_err(|err| { let str = self.as_str(); vec![_partialerror!( *self.span(), diff --git a/src/render/resolved_dependencies.rs b/src/render/resolved_dependencies.rs index b801e824..0756a92c 100644 --- a/src/render/resolved_dependencies.rs +++ b/src/render/resolved_dependencies.rs @@ -467,23 +467,11 @@ pub fn apply_variant( if m.version.is_none() && m.build.is_none() { if let Some(name) = &m.name { if let Some(version) = variant.get(&name.into()) { - // if the variant starts with an alphanumeric character, - // we have to add a '=' to the version spec - let mut spec = version.clone(); - - // check if all characters are alphanumeric or ., in that case add - // a '=' to get "startswith" behavior - if spec.chars().all(|c| c.is_alphanumeric() || c == '.') { - spec = format!("={}", version); - } else { - spec = version.clone(); - } - // we split at whitespace to separate into version and build - let mut splitter = spec.split_whitespace(); + let mut splitter = version.split_whitespace(); let version_spec = splitter .next() - .map(|v| VersionSpec::from_str(v, ParseStrictness::Strict)) + .map(|v| VersionSpec::from_str(v, ParseStrictness::Lenient)) .transpose()?; let build_spec = splitter.next().map(StringMatcher::from_str).transpose()?; From 30093a41ab1c4c6237eedebbfe8c860239f82964 Mon Sep 17 00:00:00 2001 From: Wolf Vollprecht Date: Tue, 17 Dec 2024 17:26:44 +0100 Subject: [PATCH 3/8] implement an error for ambigous matchspecs --- src/recipe/parser/requirements.rs | 29 ++++++++++++++++++++++++++++- src/render/resolved_dependencies.rs | 16 ++++++++++++++-- 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/src/recipe/parser/requirements.rs b/src/recipe/parser/requirements.rs index ef37cb97..6c898969 100644 --- a/src/recipe/parser/requirements.rs +++ b/src/recipe/parser/requirements.rs @@ -366,7 +366,34 @@ impl TryConvertNode for RenderedNode { impl TryConvertNode for RenderedScalarNode { fn try_convert(&self, _name: &str) -> Result> { - MatchSpec::from_str(self.as_str(), ParseStrictness::Lenient).map_err(|err| { + let string = self.as_str(); + + // if we have a matchspec that is only numbers, and ., we complain and ask the user to add a + // `.*` or `==` in front of it. + let splitted = string.split_whitespace().collect::>(); + if splitted.len() >= 2 { + if splitted[1].chars().all(|c| c.is_numeric() || c == '.') { + let name = splitted[0]; + let version = splitted[1]; + let rest = splitted[2..].join(" "); + let rest = if rest.is_empty() { + "".to_string() + } else { + format!(" {}", rest) + }; + + return Err(vec![_partialerror!( + *self.span(), + ErrorKind::Other, + label = format!( + "This match spec is ambiguous. Do you mean `{name} =={version}{rest}` or `{name} {version}.*{rest}`?" + ) + )]); + } + } + + + MatchSpec::from_str(self.as_str(), ParseStrictness::Strict).map_err(|err| { let str = self.as_str(); vec![_partialerror!( *self.span(), diff --git a/src/render/resolved_dependencies.rs b/src/render/resolved_dependencies.rs index 0756a92c..b801e824 100644 --- a/src/render/resolved_dependencies.rs +++ b/src/render/resolved_dependencies.rs @@ -467,11 +467,23 @@ pub fn apply_variant( if m.version.is_none() && m.build.is_none() { if let Some(name) = &m.name { if let Some(version) = variant.get(&name.into()) { + // if the variant starts with an alphanumeric character, + // we have to add a '=' to the version spec + let mut spec = version.clone(); + + // check if all characters are alphanumeric or ., in that case add + // a '=' to get "startswith" behavior + if spec.chars().all(|c| c.is_alphanumeric() || c == '.') { + spec = format!("={}", version); + } else { + spec = version.clone(); + } + // we split at whitespace to separate into version and build - let mut splitter = version.split_whitespace(); + let mut splitter = spec.split_whitespace(); let version_spec = splitter .next() - .map(|v| VersionSpec::from_str(v, ParseStrictness::Lenient)) + .map(|v| VersionSpec::from_str(v, ParseStrictness::Strict)) .transpose()?; let build_spec = splitter.next().map(StringMatcher::from_str).transpose()?; From 591417f67eed426a244e8cc2e4c66b8cc61e7270 Mon Sep 17 00:00:00 2001 From: Wolf Vollprecht Date: Tue, 17 Dec 2024 17:30:51 +0100 Subject: [PATCH 4/8] .. --- src/recipe/parser/requirements.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/recipe/parser/requirements.rs b/src/recipe/parser/requirements.rs index 6c898969..c2784286 100644 --- a/src/recipe/parser/requirements.rs +++ b/src/recipe/parser/requirements.rs @@ -392,7 +392,6 @@ impl TryConvertNode for RenderedScalarNode { } } - MatchSpec::from_str(self.as_str(), ParseStrictness::Strict).map_err(|err| { let str = self.as_str(); vec![_partialerror!( From 66274e9a71a7e9e9948309114681a86b9129941b Mon Sep 17 00:00:00 2001 From: Wolf Vollprecht Date: Tue, 17 Dec 2024 18:23:52 +0100 Subject: [PATCH 5/8] parse run exports from other packages as lenient --- src/render/run_exports.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/render/run_exports.rs b/src/render/run_exports.rs index 3ad05d9e..5910dec3 100644 --- a/src/render/run_exports.rs +++ b/src/render/run_exports.rs @@ -45,7 +45,8 @@ impl IgnoreRunExports { let to_specs = |strings: &Vec| -> Result, ParseMatchSpecError> { strings .iter() - .map(|s| MatchSpec::from_str(s, ParseStrictness::Strict)) + // We have to parse these as lenient as they come from packages + .map(|s| MatchSpec::from_str(s, ParseStrictness::Lenient)) .filter_map(|result| match result { Ok(spec) => { if spec From 454dbb5ccdc141b59f69af532fb01a9d4c4198f7 Mon Sep 17 00:00:00 2001 From: Wolf Vollprecht Date: Tue, 17 Dec 2024 18:39:35 +0100 Subject: [PATCH 6/8] fix more recipes --- README.md | 4 ++-- docs/index.md | 4 ++-- examples/rich/recipe.yaml | 4 ++-- test-data/recipes/package-content-tests/rich-recipe.yaml | 4 ++-- test-data/recipes/rich/recipe.yaml | 4 ++-- test-data/recipes/toml/recipe.yaml | 6 +++--- 6 files changed, 13 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 55774a08..70dfc0c6 100644 --- a/README.md +++ b/README.md @@ -216,12 +216,12 @@ requirements: host: - pip - poetry-core >=1.0.0 - - python 3.10 + - python 3.10.* run: # sync with normalized deps from poetry-generated setup.py - markdown-it-py >=2.2.0 - pygments >=2.13.0,<3.0.0 - - python 3.10 + - python 3.10.* - typing_extensions >=4.0.0,<5.0.0 tests: diff --git a/docs/index.md b/docs/index.md index daaf76eb..aa0f8764 100644 --- a/docs/index.md +++ b/docs/index.md @@ -238,12 +238,12 @@ requirements: host: - pip - poetry-core >=1.0.0 - - python 3.10 + - python 3.10.* run: # sync with normalized deps from poetry-generated setup.py - markdown-it-py >=2.2.0 - pygments >=2.13.0,<3.0.0 - - python 3.10 + - python 3.10.* - typing_extensions >=4.0.0,<5.0.0 tests: diff --git a/examples/rich/recipe.yaml b/examples/rich/recipe.yaml index 1832f046..533508d5 100644 --- a/examples/rich/recipe.yaml +++ b/examples/rich/recipe.yaml @@ -23,12 +23,12 @@ requirements: host: - pip - poetry-core >=1.0.0 - - python 3.10 + - python 3.10.* run: # sync with normalized deps from poetry-generated setup.py - markdown-it-py >=2.2.0 - pygments >=2.13.0,<3.0.0 - - python 3.10 + - python 3.10.* - typing_extensions >=4.0.0,<5.0.0 tests: diff --git a/test-data/recipes/package-content-tests/rich-recipe.yaml b/test-data/recipes/package-content-tests/rich-recipe.yaml index 97f26af8..1052396b 100644 --- a/test-data/recipes/package-content-tests/rich-recipe.yaml +++ b/test-data/recipes/package-content-tests/rich-recipe.yaml @@ -21,12 +21,12 @@ requirements: host: - pip - poetry-core >=1.0.0 - - python 3.10 + - python 3.10.* run: # sync with normalized deps from poetry-generated setup.py - markdown-it-py >=2.2.0 - pygments >=2.13.0,<3.0.0 - - python 3.10 + - python 3.10.* - typing_extensions >=4.0.0,<5.0.0 tests: diff --git a/test-data/recipes/rich/recipe.yaml b/test-data/recipes/rich/recipe.yaml index 406e4958..4c494aa2 100644 --- a/test-data/recipes/rich/recipe.yaml +++ b/test-data/recipes/rich/recipe.yaml @@ -21,12 +21,12 @@ requirements: host: - pip - poetry-core >=1.0.0 - - python 3.10 + - python 3.10.* run: # sync with normalized deps from poetry-generated setup.py - markdown-it-py >=2.2.0 - pygments >=2.13.0,<3.0.0 - - python 3.10 + - python 3.10.* - typing_extensions >=4.0.0,<5.0.0 tests: diff --git a/test-data/recipes/toml/recipe.yaml b/test-data/recipes/toml/recipe.yaml index a54cd460..786b44e5 100644 --- a/test-data/recipes/toml/recipe.yaml +++ b/test-data/recipes/toml/recipe.yaml @@ -18,9 +18,9 @@ build: requirements: host: - - python 3.12.1 - - pip 23.3.2 - - setuptools 68 + - python 3.12.1.* + - pip 23.3.2.* + - setuptools 68.* run: - python From 9a5e4141492a0798b4fa1edd834dea5f4017430e Mon Sep 17 00:00:00 2001 From: Wolf Vollprecht Date: Tue, 17 Dec 2024 19:29:59 +0100 Subject: [PATCH 7/8] fix test --- src/recipe/parser/requirements.rs | 12 ++++++------ .../recipes/race-condition/recipe-python-min.yaml | 2 -- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/recipe/parser/requirements.rs b/src/recipe/parser/requirements.rs index c2784286..7e76131a 100644 --- a/src/recipe/parser/requirements.rs +++ b/src/recipe/parser/requirements.rs @@ -370,12 +370,12 @@ impl TryConvertNode for RenderedScalarNode { // if we have a matchspec that is only numbers, and ., we complain and ask the user to add a // `.*` or `==` in front of it. - let splitted = string.split_whitespace().collect::>(); - if splitted.len() >= 2 { - if splitted[1].chars().all(|c| c.is_numeric() || c == '.') { - let name = splitted[0]; - let version = splitted[1]; - let rest = splitted[2..].join(" "); + let split_string = string.split_whitespace().collect::>(); + if split_string.len() >= 2 { + if split_string[1].chars().all(|c| c.is_numeric() || c == '.') { + let name = split_string[0]; + let version = split_string[1]; + let rest = split_string[2..].join(" "); let rest = if rest.is_empty() { "".to_string() } else { diff --git a/test-data/recipes/race-condition/recipe-python-min.yaml b/test-data/recipes/race-condition/recipe-python-min.yaml index d959a763..e647b8f7 100644 --- a/test-data/recipes/race-condition/recipe-python-min.yaml +++ b/test-data/recipes/race-condition/recipe-python-min.yaml @@ -4,12 +4,10 @@ package: 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" }} From 81603363e33b18636e99328c0eac07932e87c751 Mon Sep 17 00:00:00 2001 From: Wolf Vollprecht Date: Tue, 17 Dec 2024 19:43:19 +0100 Subject: [PATCH 8/8] .. --- .../__snapshots__/test_simple/test_python_min_render.json | 2 -- 1 file changed, 2 deletions(-) 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 index b334593a..207528c6 100644 --- 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 @@ -1,12 +1,10 @@ { "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"