fix: matchspec build / version from brackets and string serialization (…

…#917)

crates/rattler_conda_types/src/match_spec/mod.rs

@@ -181,11 +181,23 @@ impl Display for MatchSpec {
         let mut keys = Vec::new();
 
         if let Some(md5) = &self.md5 {
-            keys.push(format!("md5={md5:x}"));
+            keys.push(format!("md5=\"{md5:x}\""));
         }
 
         if let Some(sha256) = &self.sha256 {
-            keys.push(format!("sha256={sha256:x}"));
+            keys.push(format!("sha256=\"{sha256:x}\""));
+        }
+
+        if let Some(build_number) = &self.build_number {
+            keys.push(format!("build_number=\"{build_number}\""));
+        }
+
+        if let Some(file_name) = &self.file_name {
+            keys.push(format!("fn=\"{file_name}\""));
+        }
+
+        if let Some(url) = &self.url {
+            keys.push(format!("url=\"{url}\""));
         }
 
         if !keys.is_empty() {
@@ -477,7 +489,7 @@ mod tests {
 
     use crate::{
         match_spec::Matches, MatchSpec, NamelessMatchSpec, PackageName, PackageRecord,
-        ParseStrictness::*, RepoDataRecord, Version,
+        ParseStrictness::*, RepoDataRecord, StringMatcher, Version,
     };
     use insta::assert_snapshot;
     use std::hash::{Hash, Hasher};
@@ -493,7 +505,7 @@ mod tests {
 
     #[test]
     fn test_nameless_matchspec_format_eq() {
-        let spec = NamelessMatchSpec::from_str("*[version==1.0, sha256=aaac4bc9c6916ecc0e33137431645b029ade22190c7144eead61446dcbcc6f97, md5=dede6252c964db3f3e41c7d30d07f6bf]", Strict).unwrap();
+        let spec = NamelessMatchSpec::from_str("*[version==1.0, sha256=aaac4bc9c6916ecc0e33137431645b029ade22190c7144eead61446dcbcc6f97, md5=dede6252c964db3f3e41c7d30d07f6bf]", Lenient).unwrap();
         let spec_as_string = spec.to_string();
         let rebuild_spec = NamelessMatchSpec::from_str(&spec_as_string, Strict).unwrap();
 
@@ -544,7 +556,7 @@ mod tests {
             ..PackageRecord::new(
                 PackageName::new_unchecked("mamba"),
                 Version::from_str("1.0").unwrap(),
-                String::from(""),
+                String::from("foo_bar_py310_1"),
             )
         };
 
@@ -573,6 +585,85 @@ mod tests {
 
         let spec = MatchSpec::from_str("mamba[version==1.0, md5=dede6252c964db3f3e41c7d30d07f6bf, sha256=aaac4bc9c6916ecc0e33137431645b029ade22190c7144eead61446dcbcc6f97]", Strict).unwrap();
         assert!(!spec.matches(&record));
+
+        let spec = MatchSpec::from_str("mamba[build=*py310_1]", Strict).unwrap();
+        assert!(spec.matches(&record));
+
+        let spec = MatchSpec::from_str("mamba[build=*py310*]", Strict).unwrap();
+        assert!(spec.matches(&record));
+
+        let spec = MatchSpec::from_str("mamba[build=*py39*]", Strict).unwrap();
+        assert!(!spec.matches(&record));
+
+        let spec = MatchSpec::from_str("mamba * [build=*py310*]", Strict).unwrap();
+        assert!(spec.matches(&record));
+
+        let spec = MatchSpec::from_str("mamba *[build=*py39*]", Strict).unwrap();
+        assert!(!spec.matches(&record));
+        assert!(spec.build == Some(StringMatcher::from_str("*py39*").unwrap()));
+
+        let spec = MatchSpec::from_str("mamba * [build=*py39*]", Strict).unwrap();
+        println!("Build: {:?}", spec.build);
+        assert!(!spec.matches(&record));
+    }
+
+    #[test]
+    fn precedence_version_build() {
+        let spec =
+            MatchSpec::from_str("foo 3.0.* [version=1.2.3, build='foobar']", Lenient).unwrap();
+        assert_eq!(spec.version.unwrap(), "1.2.3".parse().unwrap());
+        assert_eq!(spec.build.unwrap(), "foobar".parse().unwrap());
+
+        let spec = MatchSpec::from_str("foo 3.0.* abcdef[build='foobar', version=1.2.3]", Lenient)
+            .unwrap();
+        assert_eq!(spec.build.unwrap(), "foobar".parse().unwrap());
+        assert_eq!(spec.version.unwrap(), "1.2.3".parse().unwrap());
+
+        let spec =
+            NamelessMatchSpec::from_str("3.0.* [version=1.2.3, build='foobar']", Lenient).unwrap();
+        assert_eq!(spec.version.unwrap(), "1.2.3".parse().unwrap());
+        assert_eq!(spec.build.unwrap(), "foobar".parse().unwrap());
+
+        let spec =
+            NamelessMatchSpec::from_str("3.0.* abcdef[build='foobar', version=1.2.3]", Lenient)
+                .unwrap();
+        assert_eq!(spec.build.unwrap(), "foobar".parse().unwrap());
+        assert_eq!(spec.version.unwrap(), "1.2.3".parse().unwrap());
+    }
+
+    #[test]
+    fn strict_parsing_multiple_values() {
+        let spec = NamelessMatchSpec::from_str("3.0.* [version=1.2.3]", Strict);
+        assert!(spec.is_err());
+
+        let spec = NamelessMatchSpec::from_str("3.0.* foo[build='foobar']", Strict);
+        assert!(spec.is_err());
+
+        let spec = NamelessMatchSpec::from_str(
+            "3.0.* [build=baz, fn='/home/bla.tar.bz2' build='foobar']",
+            Strict,
+        );
+        assert!(spec.is_err());
+
+        let spec = MatchSpec::from_str("foo 3.0.* [version=1.2.3]", Strict);
+        assert!(spec.is_err());
+
+        let spec = MatchSpec::from_str("foo 3.0.* foo[build='foobar']", Strict);
+        assert!(spec.is_err());
+        assert!(spec
+            .unwrap_err()
+            .to_string()
+            .contains("multiple values for: build"));
+
+        let spec = MatchSpec::from_str(
+            "foo 3.0.* [build=baz, fn='/home/foo.tar.bz2', build='foobar']",
+            Strict,
+        );
+        assert!(spec.is_err());
+        assert!(spec
+            .unwrap_err()
+            .to_string()
+            .contains("multiple values for: build"));
     }
 
     #[test]

crates/rattler_conda_types/src/match_spec/parse.rs

@@ -1,4 +1,4 @@
-use std::{borrow::Cow, ops::Not, str::FromStr, sync::Arc};
+use std::{borrow::Cow, collections::HashSet, ops::Not, str::FromStr, sync::Arc};
 
 use nom::{
     branch::alt,
@@ -71,11 +71,11 @@ pub enum ParseMatchSpecError {
     MultipleBracketSectionsNotAllowed,
 
     /// Invalid version and build
-    #[error("Unable to parse version spec: {0}")]
+    #[error("unable to parse version spec: {0}")]
     InvalidVersionAndBuild(String),
 
     /// Invalid build string
-    #[error("The build string '{0}' is not valid, it can only contain alphanumeric characters and underscores"
+    #[error("the build string '{0}' is not valid, it can only contain alphanumeric characters and underscores"
     )]
     InvalidBuildString(String),
 
@@ -92,12 +92,16 @@ pub enum ParseMatchSpecError {
     InvalidBuildNumber(#[from] ParseBuildNumberSpecError),
 
     /// Unable to parse hash digest from hex
-    #[error("Unable to parse hash digest from hex")]
+    #[error("unable to parse hash digest from hex")]
     InvalidHashDigest,
 
     /// The package name was invalid
     #[error(transparent)]
     InvalidPackageName(#[from] InvalidPackageNameError),
+
+    /// Multiple values for a key in the matchspec
+    #[error("found multiple values for: {0}")]
+    MultipleValueForKey(String),
 }
 
 impl FromStr for MatchSpec {
@@ -227,6 +231,17 @@ fn parse_bracket_vec_into_components(
 ) -> Result<NamelessMatchSpec, ParseMatchSpecError> {
     let mut match_spec = match_spec;
 
+    if strictness == Strict {
+        // check for duplicate keys
+        let mut seen = HashSet::new();
+        for (key, _) in &bracket {
+            if seen.contains(key) {
+                return Err(ParseMatchSpecError::MultipleValueForKey(key.to_string()));
+            }
+            seen.insert(key);
+        }
+    }
+
     for elem in bracket {
         let (key, value) = elem;
         match key {
@@ -482,8 +497,19 @@ impl NamelessMatchSpec {
         // Get the version and optional build string
         if !input.is_empty() {
             let (version, build) = parse_version_and_build(input, strictness)?;
-            match_spec.version = version;
-            match_spec.build = build;
+            if strictness == Strict {
+                if match_spec.version.is_some() && version.is_some() {
+                    return Err(ParseMatchSpecError::MultipleValueForKey(
+                        "version".to_owned(),
+                    ));
+                }
+
+                if match_spec.build.is_some() && build.is_some() {
+                    return Err(ParseMatchSpecError::MultipleValueForKey("build".to_owned()));
+                }
+            }
+            match_spec.version = match_spec.version.or(version);
+            match_spec.build = match_spec.build.or(build);
         }
 
         Ok(match_spec)
@@ -578,8 +604,19 @@ fn matchspec_parser(
     let input = input.trim();
     if !input.is_empty() {
         let (version, build) = parse_version_and_build(input, strictness)?;
-        match_spec.version = version;
-        match_spec.build = build;
+        if strictness == Strict {
+            if match_spec.version.is_some() && version.is_some() {
+                return Err(ParseMatchSpecError::MultipleValueForKey(
+                    "version".to_owned(),
+                ));
+            }
+
+            if match_spec.build.is_some() && build.is_some() {
+                return Err(ParseMatchSpecError::MultipleValueForKey("build".to_owned()));
+            }
+        }
+        match_spec.version = match_spec.version.or(version);
+        match_spec.build = match_spec.build.or(build);
     }
 
     Ok(match_spec)
@@ -772,9 +809,11 @@ mod tests {
             NamelessMatchSpec::from_str("*cpu*", Strict).unwrap(),
             NamelessMatchSpec::from_str("conda-forge::foobar", Strict).unwrap(),
             NamelessMatchSpec::from_str("foobar[channel=conda-forge]", Strict).unwrap(),
+            NamelessMatchSpec::from_str("* [build=foo]", Strict).unwrap(),
+            NamelessMatchSpec::from_str(">=1.2[build=foo]", Strict).unwrap(),
+            NamelessMatchSpec::from_str("[version='>=1.2', build=foo]", Strict).unwrap(),
         ],
         @r###"
-        ---
         - version: 3.8.*
           build: "*_cpython"
         - version: "==1.0"
@@ -792,6 +831,12 @@ mod tests {
           channel:
             base_url: "https://conda.anaconda.org/conda-forge/"
             name: conda-forge
+        - version: "*"
+          build: foo
+        - version: ">=1.2"
+          build: foo
+        - version: ">=1.2"
+          build: foo
         "###);
     }
 
@@ -1279,4 +1324,50 @@ mod tests {
             assert_eq!(subdir, expected_subdir.map(ToString::to_string));
         }
     }
+
+    #[test]
+    fn test_matchspec_to_string() {
+        let mut specs: Vec<MatchSpec> =
+            vec![MatchSpec::from_str("foo[version=1.0.*, build_number=\">6\"]", Strict).unwrap()];
+
+        // complete matchspec to verify that we print all fields
+        specs.push(MatchSpec {
+            name: Some("foo".parse().unwrap()),
+            version: Some(VersionSpec::from_str("1.0.*", Strict).unwrap()),
+            build: "py27_0*".parse().ok(),
+            build_number: Some(BuildNumberSpec::from_str(">=6").unwrap()),
+            file_name: Some("foo-1.0-py27_0.tar.bz2".to_string()),
+            channel: Some(
+                Channel::from_str("conda-forge", &channel_config())
+                    .map(Arc::new)
+                    .unwrap(),
+            ),
+            subdir: Some("linux-64".to_string()),
+            namespace: Some("foospace".to_string()),
+            md5: Some(parse_digest_from_hex::<Md5>("8b1a9953c4611296a827abf8c47804d7").unwrap()),
+            sha256: Some(
+                parse_digest_from_hex::<Sha256>(
+                    "315f5bdb76d078c43b8ac0064e4a0164612b1fce77c869345bfc94c75894edd3",
+                )
+                .unwrap(),
+            ),
+            url: Some(
+                Url::parse(
+                    "https://conda.anaconda.org/conda-forge/linux-64/foo-1.0-py27_0.tar.bz2",
+                )
+                .unwrap(),
+            ),
+        });
+
+        // insta check all the strings
+        let vec_strings = specs.iter().map(|s| s.to_string()).collect::<Vec<_>>();
+        insta::assert_debug_snapshot!(vec_strings);
+
+        // parse back the strings and check if they are the same
+        let parsed_specs = vec_strings
+            .iter()
+            .map(|s| MatchSpec::from_str(s, Strict).unwrap())
+            .collect::<Vec<_>>();
+        assert_eq!(specs, parsed_specs);
+    }
 }

crates/rattler_conda_types/src/match_spec/snapshots/rattler_conda_types__match_spec__parse__tests__matchspec_to_string.snap

@@ -0,0 +1,8 @@
+---
+source: crates/rattler_conda_types/src/match_spec/parse.rs
+expression: vec_strings
+---
+[
+    "foo 1.0.*[build_number=\">6\"]",
+    "conda-forge/linux-64:foospace:foo 1.0.* py27_0*[md5=\"8b1a9953c4611296a827abf8c47804d7\", sha256=\"315f5bdb76d078c43b8ac0064e4a0164612b1fce77c869345bfc94c75894edd3\", build_number=\">=6\", fn=\"foo-1.0-py27_0.tar.bz2\", url=\"https://conda.anaconda.org/conda-forge/linux-64/foo-1.0-py27_0.tar.bz2\"]",
+]

crates/rattler_conda_types/src/match_spec/snapshots/rattler_conda_types__match_spec__parse__tests__test_from_string_Strict.snap

@@ -10,9 +10,9 @@ blas *.* mkl:
   name: foo
   url: "file:///C:/Users/user/conda-bld/linux-64/foo-1.0-py27_0.tar.bz2"
 foo=1.0=py27_0:
-  error: "The build string '=py27_0' is not valid, it can only contain alphanumeric characters and underscores"
+  error: "the build string '=py27_0' is not valid, it can only contain alphanumeric characters and underscores"
 foo==1.0=py27_0:
-  error: "The build string '=py27_0' is not valid, it can only contain alphanumeric characters and underscores"
+  error: "the build string '=py27_0' is not valid, it can only contain alphanumeric characters and underscores"
 "https://conda.anaconda.org/conda-forge/linux-64/py-rattler-0.6.1-py39h8169da8_0.conda":
   name: py-rattler
   url: "https://conda.anaconda.org/conda-forge/linux-64/py-rattler-0.6.1-py39h8169da8_0.conda"
@@ -24,7 +24,7 @@ python 3.8.* *_cpython:
   version: 3.8.*
   build: "*_cpython"
 pytorch=*=cuda*:
-  error: "The build string '=cuda*' is not valid, it can only contain alphanumeric characters and underscores"
+  error: "the build string '=cuda*' is not valid, it can only contain alphanumeric characters and underscores"
 "x264 >=1!164.3095,<1!165":
   name: x264
   version: ">=1!164.3095,<1!165"

crates/rattler_conda_types/src/match_spec/snapshots/rattler_conda_types__match_spec__parse__tests__test_nameless_from_string_Strict.snap

@@ -14,9 +14,9 @@ expression: evaluated
 "C:\\Users\\user\\conda-bld\\linux-64\\foo-1.0-py27_0.tar.bz2":
   url: "file:///C:/Users/user/conda-bld/linux-64/foo-1.0-py27_0.tar.bz2"
 "=1.0=py27_0":
-  error: "The build string '=py27_0' is not valid, it can only contain alphanumeric characters and underscores"
+  error: "the build string '=py27_0' is not valid, it can only contain alphanumeric characters and underscores"
 "==1.0=py27_0":
-  error: "The build string '=py27_0' is not valid, it can only contain alphanumeric characters and underscores"
+  error: "the build string '=py27_0' is not valid, it can only contain alphanumeric characters and underscores"
 "https://conda.anaconda.org/conda-forge/linux-64/py-rattler-0.6.1-py39h8169da8_0.conda":
   url: "https://conda.anaconda.org/conda-forge/linux-64/py-rattler-0.6.1-py39h8169da8_0.conda"
 "https://repo.prefix.dev/ruben-arts/linux-64/boost-cpp-1.78.0-h75c5d50_1.tar.bz2":
@@ -25,7 +25,7 @@ expression: evaluated
   version: 3.8.*
   build: "*_cpython"
 "=*=cuda*":
-  error: "The build string '=cuda*' is not valid, it can only contain alphanumeric characters and underscores"
+  error: "the build string '=cuda*' is not valid, it can only contain alphanumeric characters and underscores"
 ">=1!164.3095,<1!165":
   version: ">=1!164.3095,<1!165"
 /home/user/conda-bld/linux-64/foo-1.0-py27_0.tar.bz2:

crates/rattler_conda_types/src/match_spec/snapshots/rattler_conda_types__match_spec__tests__serialize_matchspec.snap

@@ -1,9 +1,9 @@
 ---
 source: crates/rattler_conda_types/src/match_spec/mod.rs
-expression: "specs.into_iter().map(|s|\n                    MatchSpec::from_str(s).unwrap()).map(|s|\n                s.to_string()).collect::<Vec<String>>().join(\"\\n\")"
+expression: "specs.into_iter().map(|s|\nMatchSpec::from_str(s,\nStrict).unwrap()).map(|s| s.to_string()).collect::<Vec<String>>().join(\"\\n\")"
 ---
 mamba ==1.0 py37_0
-conda-forge::pytest ==1.0[md5=dede6252c964db3f3e41c7d30d07f6bf, sha256=aaac4bc9c6916ecc0e33137431645b029ade22190c7144eead61446dcbcc6f97]
+conda-forge::pytest ==1.0[md5="dede6252c964db3f3e41c7d30d07f6bf", sha256="aaac4bc9c6916ecc0e33137431645b029ade22190c7144eead61446dcbcc6f97"]
 conda-forge/linux-64::pytest
 conda-forge/linux-64::pytest ==1.0
 conda-forge/linux-64::pytest ==1.0 py37_0