diff --git a/crates/rattler-bin/src/commands/create.rs b/crates/rattler-bin/src/commands/create.rs index 02d7a49b6..1f057e747 100644 --- a/crates/rattler-bin/src/commands/create.rs +++ b/crates/rattler-bin/src/commands/create.rs @@ -11,8 +11,8 @@ use rattler::{ package_cache::PackageCache, }; use rattler_conda_types::{ - Channel, ChannelConfig, GenericVirtualPackage, MatchSpec, PackageRecord, Platform, - PrefixRecord, RepoDataRecord, Version, + Channel, ChannelConfig, GenericVirtualPackage, MatchSpec, PackageRecord, ParseStrictness, + Platform, PrefixRecord, RepoDataRecord, Version, }; use rattler_networking::{ retry_policies::default_retry_policy, AuthenticationMiddleware, AuthenticationStorage, @@ -80,7 +80,7 @@ pub async fn create(opt: Opt) -> anyhow::Result<()> { let specs = opt .specs .iter() - .map(|spec| MatchSpec::from_str(spec)) + .map(|spec| MatchSpec::from_str(spec, ParseStrictness::Strict)) .collect::, _>>()?; // Find the default cache directory. Create it if it doesnt exist yet. diff --git a/crates/rattler_conda_types/src/lib.rs b/crates/rattler_conda_types/src/lib.rs index eedff4a41..e3719cf2b 100644 --- a/crates/rattler_conda_types/src/lib.rs +++ b/crates/rattler_conda_types/src/lib.rs @@ -8,6 +8,7 @@ mod channel_data; mod explicit_environment_spec; mod match_spec; mod no_arch_type; +mod parse_mode; mod platform; mod repo_data; mod repo_data_record; @@ -34,6 +35,7 @@ pub use match_spec::parse::ParseMatchSpecError; pub use match_spec::{MatchSpec, NamelessMatchSpec}; pub use no_arch_type::{NoArchKind, NoArchType}; pub use package_name::{InvalidPackageNameError, PackageName}; +pub use parse_mode::ParseStrictness; pub use platform::{Arch, ParseArchError, ParsePlatformError, Platform}; pub use prefix_record::PrefixRecord; pub use repo_data::patches::{PackageRecordPatch, PatchInstructions, RepoDataPatch}; diff --git a/crates/rattler_conda_types/src/match_spec/mod.rs b/crates/rattler_conda_types/src/match_spec/mod.rs index 7af4fed07..1f65c5af4 100644 --- a/crates/rattler_conda_types/src/match_spec/mod.rs +++ b/crates/rattler_conda_types/src/match_spec/mod.rs @@ -68,38 +68,38 @@ use matcher::StringMatcher; /// # Examples: /// /// ```rust -/// use rattler_conda_types::{MatchSpec, VersionSpec, StringMatcher, PackageName, Channel}; +/// use rattler_conda_types::{MatchSpec, VersionSpec, StringMatcher, PackageName, Channel, ParseStrictness::*}; /// use std::str::FromStr; /// use std::sync::Arc; /// -/// let spec = MatchSpec::from_str("foo 1.0 py27_0").unwrap(); +/// let spec = MatchSpec::from_str("foo 1.0 py27_0", Strict).unwrap(); /// assert_eq!(spec.name, Some(PackageName::new_unchecked("foo"))); -/// assert_eq!(spec.version, Some(VersionSpec::from_str("1.0").unwrap())); +/// assert_eq!(spec.version, Some(VersionSpec::from_str("1.0", Strict).unwrap())); /// assert_eq!(spec.build, Some(StringMatcher::from_str("py27_0").unwrap())); /// -/// let spec = MatchSpec::from_str("foo=1.0=py27_0").unwrap(); +/// let spec = MatchSpec::from_str("foo=1.0=py27_0", Strict).unwrap(); /// assert_eq!(spec.name, Some(PackageName::new_unchecked("foo"))); -/// assert_eq!(spec.version, Some(VersionSpec::from_str("==1.0").unwrap())); +/// assert_eq!(spec.version, Some(VersionSpec::from_str("==1.0", Strict).unwrap())); /// assert_eq!(spec.build, Some(StringMatcher::from_str("py27_0").unwrap())); /// -/// let spec = MatchSpec::from_str(r#"conda-forge::foo[version="1.0.*"]"#).unwrap(); +/// let spec = MatchSpec::from_str(r#"conda-forge::foo[version="1.0.*"]"#, Strict).unwrap(); /// assert_eq!(spec.name, Some(PackageName::new_unchecked("foo"))); -/// assert_eq!(spec.version, Some(VersionSpec::from_str("1.0.*").unwrap())); +/// assert_eq!(spec.version, Some(VersionSpec::from_str("1.0.*", Strict).unwrap())); /// assert_eq!(spec.channel, Some(Channel::from_str("conda-forge", &Default::default()).map(|channel| Arc::new(channel)).unwrap())); /// -/// let spec = MatchSpec::from_str("conda-forge/linux-64::foo>=1.0").unwrap(); +/// let spec = MatchSpec::from_str("conda-forge/linux-64::foo>=1.0", Strict).unwrap(); /// assert_eq!(spec.name, Some(PackageName::new_unchecked("foo"))); -/// assert_eq!(spec.version, Some(VersionSpec::from_str(">=1.0").unwrap())); +/// assert_eq!(spec.version, Some(VersionSpec::from_str(">=1.0", Strict).unwrap())); /// assert_eq!(spec.channel, Some(Channel::from_str("conda-forge", &Default::default()).map(|channel| Arc::new(channel)).unwrap())); /// assert_eq!(spec.subdir, Some("linux-64".to_string())); /// -/// let spec = MatchSpec::from_str("*/linux-64::foo>=1.0").unwrap(); +/// let spec = MatchSpec::from_str("*/linux-64::foo>=1.0", Strict).unwrap(); /// assert_eq!(spec.name, Some(PackageName::new_unchecked("foo"))); -/// assert_eq!(spec.version, Some(VersionSpec::from_str(">=1.0").unwrap())); +/// assert_eq!(spec.version, Some(VersionSpec::from_str(">=1.0", Strict).unwrap())); /// assert_eq!(spec.channel, Some(Channel::from_str("*", &Default::default()).map(|channel| Arc::new(channel)).unwrap())); /// assert_eq!(spec.subdir, Some("linux-64".to_string())); /// -/// let spec = MatchSpec::from_str(r#"foo[build="py2*"]"#).unwrap(); +/// let spec = MatchSpec::from_str(r#"foo[build="py2*"]"#, Strict).unwrap(); /// assert_eq!(spec.name, Some(PackageName::new_unchecked("foo"))); /// assert_eq!(spec.build, Some(StringMatcher::from_str("py2*").unwrap())); /// ``` @@ -404,32 +404,34 @@ mod tests { use rattler_digest::{parse_digest_from_hex, Md5, Sha256}; - use crate::{MatchSpec, NamelessMatchSpec, PackageName, PackageRecord, Version}; + use crate::{ + MatchSpec, NamelessMatchSpec, PackageName, PackageRecord, ParseStrictness::*, Version, + }; use insta::assert_snapshot; use std::hash::{Hash, Hasher}; #[test] fn test_matchspec_format_eq() { - let spec = MatchSpec::from_str("conda-forge::mamba[version==1.0, sha256=aaac4bc9c6916ecc0e33137431645b029ade22190c7144eead61446dcbcc6f97, md5=dede6252c964db3f3e41c7d30d07f6bf]").unwrap(); + let spec = MatchSpec::from_str("conda-forge::mamba[version==1.0, sha256=aaac4bc9c6916ecc0e33137431645b029ade22190c7144eead61446dcbcc6f97, md5=dede6252c964db3f3e41c7d30d07f6bf]", Strict).unwrap(); let spec_as_string = spec.to_string(); - let rebuild_spec = MatchSpec::from_str(&spec_as_string).unwrap(); + let rebuild_spec = MatchSpec::from_str(&spec_as_string, Strict).unwrap(); assert_eq!(spec, rebuild_spec); } #[test] fn test_nameless_matchspec_format_eq() { - let spec = NamelessMatchSpec::from_str("*[version==1.0, sha256=aaac4bc9c6916ecc0e33137431645b029ade22190c7144eead61446dcbcc6f97, md5=dede6252c964db3f3e41c7d30d07f6bf]").unwrap(); + let spec = NamelessMatchSpec::from_str("*[version==1.0, sha256=aaac4bc9c6916ecc0e33137431645b029ade22190c7144eead61446dcbcc6f97, md5=dede6252c964db3f3e41c7d30d07f6bf]", Strict).unwrap(); let spec_as_string = spec.to_string(); - let rebuild_spec = NamelessMatchSpec::from_str(&spec_as_string).unwrap(); + let rebuild_spec = NamelessMatchSpec::from_str(&spec_as_string, Strict).unwrap(); assert_eq!(spec, rebuild_spec); } #[test] fn test_hash_match() { - let spec1 = MatchSpec::from_str("tensorflow 2.6.*").unwrap(); - let spec2 = MatchSpec::from_str("tensorflow 2.6.*").unwrap(); + let spec1 = MatchSpec::from_str("tensorflow 2.6.*", Strict).unwrap(); + let spec2 = MatchSpec::from_str("tensorflow 2.6.*", Strict).unwrap(); assert_eq!(spec1, spec2); let mut hasher = std::collections::hash_map::DefaultHasher::new(); @@ -445,8 +447,8 @@ mod tests { #[test] fn test_hash_no_match() { - let spec1 = MatchSpec::from_str("tensorflow 2.6.0.*").unwrap(); - let spec2 = MatchSpec::from_str("tensorflow 2.6.*").unwrap(); + let spec1 = MatchSpec::from_str("tensorflow 2.6.0.*", Strict).unwrap(); + let spec2 = MatchSpec::from_str("tensorflow 2.6.*", Strict).unwrap(); assert_ne!(spec1, spec2); let mut hasher = std::collections::hash_map::DefaultHasher::new(); @@ -474,24 +476,30 @@ mod tests { ) }; - let spec = MatchSpec::from_str("mamba[version==1.0, sha256=aaac4bc9c6916ecc0e33137431645b029ade22190c7144eead61446dcbcc6f97]").unwrap(); + let spec = MatchSpec::from_str("mamba[version==1.0, sha256=aaac4bc9c6916ecc0e33137431645b029ade22190c7144eead61446dcbcc6f97]", Strict).unwrap(); assert!(!spec.matches(&record)); - let spec = MatchSpec::from_str("mamba[version==1.0, sha256=f44c4bc9c6916ecc0e33137431645b029ade22190c7144eead61446dcbcc6f97]").unwrap(); + let spec = MatchSpec::from_str("mamba[version==1.0, sha256=f44c4bc9c6916ecc0e33137431645b029ade22190c7144eead61446dcbcc6f97]", Strict).unwrap(); assert!(spec.matches(&record)); - let spec = MatchSpec::from_str("mamba[version==1.0, md5=aaaa6252c964db3f3e41c7d30d07f6bf]") - .unwrap(); + let spec = MatchSpec::from_str( + "mamba[version==1.0, md5=aaaa6252c964db3f3e41c7d30d07f6bf]", + Strict, + ) + .unwrap(); assert!(!spec.matches(&record)); - let spec = MatchSpec::from_str("mamba[version==1.0, md5=dede6252c964db3f3e41c7d30d07f6bf]") - .unwrap(); + let spec = MatchSpec::from_str( + "mamba[version==1.0, md5=dede6252c964db3f3e41c7d30d07f6bf]", + Strict, + ) + .unwrap(); assert!(spec.matches(&record)); - let spec = MatchSpec::from_str("mamba[version==1.0, md5=dede6252c964db3f3e41c7d30d07f6bf, sha256=f44c4bc9c6916ecc0e33137431645b029ade22190c7144eead61446dcbcc6f97]").unwrap(); + let spec = MatchSpec::from_str("mamba[version==1.0, md5=dede6252c964db3f3e41c7d30d07f6bf, sha256=f44c4bc9c6916ecc0e33137431645b029ade22190c7144eead61446dcbcc6f97]", Strict).unwrap(); assert!(spec.matches(&record)); - let spec = MatchSpec::from_str("mamba[version==1.0, md5=dede6252c964db3f3e41c7d30d07f6bf, sha256=aaac4bc9c6916ecc0e33137431645b029ade22190c7144eead61446dcbcc6f97]").unwrap(); + let spec = MatchSpec::from_str("mamba[version==1.0, md5=dede6252c964db3f3e41c7d30d07f6bf, sha256=aaac4bc9c6916ecc0e33137431645b029ade22190c7144eead61446dcbcc6f97]", Strict).unwrap(); assert!(!spec.matches(&record)); } @@ -506,7 +514,7 @@ mod tests { assert_snapshot!(specs .into_iter() - .map(|s| MatchSpec::from_str(s).unwrap()) + .map(|s| MatchSpec::from_str(s, Strict).unwrap()) .map(|s| s.to_string()) .collect::>() .join("\n")); diff --git a/crates/rattler_conda_types/src/match_spec/parse.rs b/crates/rattler_conda_types/src/match_spec/parse.rs index 7ef085206..834361e98 100644 --- a/crates/rattler_conda_types/src/match_spec/parse.rs +++ b/crates/rattler_conda_types/src/match_spec/parse.rs @@ -6,7 +6,7 @@ use crate::version_spec::version_tree::{recognize_constraint, recognize_version} use crate::version_spec::{is_start_of_version_constraint, ParseVersionSpecError}; use crate::{ Channel, ChannelConfig, InvalidPackageNameError, NamelessMatchSpec, PackageName, - ParseChannelError, VersionSpec, + ParseChannelError, ParseStrictness, VersionSpec, }; use nom::branch::alt; use nom::bytes::complete::{tag, take_till1, take_until, take_while, take_while1}; @@ -84,7 +84,17 @@ impl FromStr for MatchSpec { type Err = ParseMatchSpecError; fn from_str(s: &str) -> Result { - parse(s) + Self::from_str(s, ParseStrictness::Lenient) + } +} + +impl MatchSpec { + /// Parses a [`MatchSpec`] from a string with a given strictness. + pub fn from_str( + source: &str, + strictness: ParseStrictness, + ) -> Result { + matchspec_parser(source, strictness) } } @@ -197,13 +207,14 @@ fn strip_brackets(input: &str) -> Result<(Cow<'_, str>, BracketVec<'_>), ParseMa fn parse_bracket_vec_into_components( bracket: BracketVec<'_>, match_spec: NamelessMatchSpec, + strictness: ParseStrictness, ) -> Result { let mut match_spec = match_spec; for elem in bracket { let (key, value) = elem; match key { - "version" => match_spec.version = Some(VersionSpec::from_str(value)?), + "version" => match_spec.version = Some(VersionSpec::from_str(value, strictness)?), "build" => match_spec.build = Some(StringMatcher::from_str(value)?), "build_number" => match_spec.build_number = Some(BuildNumberSpec::from_str(value)?), "sha256" => { @@ -309,10 +320,17 @@ impl FromStr for NamelessMatchSpec { type Err = ParseMatchSpecError; fn from_str(input: &str) -> Result { + Self::from_str(input, ParseStrictness::Lenient) + } +} + +impl NamelessMatchSpec { + /// Parses a [`NamelessMatchSpec`] from a string with a given strictness. + pub fn from_str(input: &str, strictness: ParseStrictness) -> Result { // Strip off brackets portion let (input, brackets) = strip_brackets(input.trim())?; let mut match_spec = - parse_bracket_vec_into_components(brackets, NamelessMatchSpec::default())?; + parse_bracket_vec_into_components(brackets, NamelessMatchSpec::default(), strictness)?; // Get the version and optional build string let input = input.trim(); @@ -331,7 +349,7 @@ impl FromStr for NamelessMatchSpec { // Parse the version spec match_spec.version = Some( - VersionSpec::from_str(version_str.as_ref()) + VersionSpec::from_str(version_str.as_ref(), strictness) .map_err(ParseMatchSpecError::InvalidVersionSpec)?, ); @@ -346,7 +364,10 @@ impl FromStr for NamelessMatchSpec { /// Parses a conda match spec. /// This is based on: -fn parse(input: &str) -> Result { +fn matchspec_parser( + input: &str, + strictness: ParseStrictness, +) -> Result { // Step 1. Strip '#' and `if` statement let (input, _comment) = strip_comment(input); let (input, _if_clause) = strip_if(input); @@ -372,7 +393,7 @@ fn parse(input: &str) -> Result { // 3. Strip off brackets portion let (input, brackets) = strip_brackets(input.trim())?; let mut nameless_match_spec = - parse_bracket_vec_into_components(brackets, NamelessMatchSpec::default())?; + parse_bracket_vec_into_components(brackets, NamelessMatchSpec::default(), strictness)?; // 4. Strip off parens portion // TODO: What is this? I've never seen in @@ -453,7 +474,7 @@ fn parse(input: &str) -> Result { // Parse the version spec match_spec.version = Some( - VersionSpec::from_str(version_str.as_ref()) + VersionSpec::from_str(version_str.as_ref(), strictness) .map_err(ParseMatchSpecError::InvalidVersionSpec)?, ); @@ -467,6 +488,7 @@ fn parse(input: &str) -> Result { #[cfg(test)] mod tests { + use crate::ParseStrictness::*; use assert_matches::assert_matches; use rattler_digest::{parse_digest_from_hex, Md5, Sha256}; use serde::Serialize; @@ -549,9 +571,9 @@ mod tests { #[test] fn test_nameless_match_spec() { insta::assert_yaml_snapshot!([ - NamelessMatchSpec::from_str("3.8.* *_cpython").unwrap(), - NamelessMatchSpec::from_str("1.0 py27_0[fn=\"bla\"]").unwrap(), - NamelessMatchSpec::from_str("=1.0 py27_0").unwrap(), + NamelessMatchSpec::from_str("3.8.* *_cpython", Strict).unwrap(), + NamelessMatchSpec::from_str("1.0 py27_0[fn=\"bla\"]", Strict).unwrap(), + NamelessMatchSpec::from_str("=1.0 py27_0", Strict).unwrap(), ], @r###" --- @@ -567,9 +589,12 @@ mod tests { #[test] fn test_match_spec_more() { - let spec = MatchSpec::from_str("conda-forge::foo[version=\"1.0.*\"]").unwrap(); + let spec = MatchSpec::from_str("conda-forge::foo[version=\"1.0.*\"]", Strict).unwrap(); assert_eq!(spec.name, Some("foo".parse().unwrap())); - assert_eq!(spec.version, Some(VersionSpec::from_str("1.0.*").unwrap())); + assert_eq!( + spec.version, + Some(VersionSpec::from_str("1.0.*", Strict).unwrap()) + ); assert_eq!( spec.channel, Some( @@ -579,9 +604,12 @@ mod tests { ) ); - let spec = MatchSpec::from_str("conda-forge::foo[version=1.0.*]").unwrap(); + let spec = MatchSpec::from_str("conda-forge::foo[version=1.0.*]", Strict).unwrap(); assert_eq!(spec.name, Some("foo".parse().unwrap())); - assert_eq!(spec.version, Some(VersionSpec::from_str("1.0.*").unwrap())); + assert_eq!( + spec.version, + Some(VersionSpec::from_str("1.0.*", Strict).unwrap()) + ); assert_eq!( spec.channel, Some( @@ -591,10 +619,16 @@ mod tests { ) ); - let spec = - MatchSpec::from_str(r#"conda-forge::foo[version=1.0.*, build_number=">6"]"#).unwrap(); + let spec = MatchSpec::from_str( + r#"conda-forge::foo[version=1.0.*, build_number=">6"]"#, + Strict, + ) + .unwrap(); assert_eq!(spec.name, Some("foo".parse().unwrap())); - assert_eq!(spec.version, Some(VersionSpec::from_str("1.0.*").unwrap())); + assert_eq!( + spec.version, + Some(VersionSpec::from_str("1.0.*", Strict).unwrap()) + ); assert_eq!( spec.channel, Some( @@ -611,13 +645,13 @@ mod tests { #[test] fn test_hash_spec() { - let spec = MatchSpec::from_str("conda-forge::foo[md5=1234567890]"); + let spec = MatchSpec::from_str("conda-forge::foo[md5=1234567890]", Strict); assert_matches!(spec, Err(ParseMatchSpecError::InvalidHashDigest)); - let spec = MatchSpec::from_str("conda-forge::foo[sha256=1234567890]"); + let spec = MatchSpec::from_str("conda-forge::foo[sha256=1234567890]", Strict); assert_matches!(spec, Err(ParseMatchSpecError::InvalidHashDigest)); - let spec = MatchSpec::from_str("conda-forge::foo[sha256=315f5bdb76d078c43b8ac0064e4a0164612b1fce77c869345bfc94c75894edd3]").unwrap(); + let spec = MatchSpec::from_str("conda-forge::foo[sha256=315f5bdb76d078c43b8ac0064e4a0164612b1fce77c869345bfc94c75894edd3]", Strict).unwrap(); assert_eq!( spec.sha256, Some( @@ -628,8 +662,11 @@ mod tests { ) ); - let spec = - MatchSpec::from_str("conda-forge::foo[md5=8b1a9953c4611296a827abf8c47804d7]").unwrap(); + let spec = MatchSpec::from_str( + "conda-forge::foo[md5=8b1a9953c4611296a827abf8c47804d7]", + Strict, + ) + .unwrap(); assert_eq!( spec.md5, Some(parse_digest_from_hex::("8b1a9953c4611296a827abf8c47804d7").unwrap()) @@ -713,7 +750,7 @@ mod tests { .map(|spec| { ( spec, - MatchSpec::from_str(spec).map_or_else( + MatchSpec::from_str(spec, Strict).map_or_else( |err| MatchSpecOrError::Error { error: err.to_string(), }, @@ -740,7 +777,7 @@ mod tests { #[test] fn test_invalid_bracket_key() { let _unknown_key = String::from("unknown"); - let spec = MatchSpec::from_str("conda-forge::foo[unknown=1.0.*]"); + let spec = MatchSpec::from_str("conda-forge::foo[unknown=1.0.*]", Strict); assert_matches!( spec, Err(ParseMatchSpecError::InvalidBracketKey(_unknown_key)) @@ -749,7 +786,7 @@ mod tests { #[test] fn test_invalid_number_of_colons() { - let spec = MatchSpec::from_str("conda-forge::::foo[version=\"1.0.*\"]"); + let spec = MatchSpec::from_str("conda-forge::::foo[version=\"1.0.*\"]", Strict); assert_matches!(spec, Err(ParseMatchSpecError::InvalidNumberOfColons)); } diff --git a/crates/rattler_conda_types/src/parse_mode.rs b/crates/rattler_conda_types/src/parse_mode.rs new file mode 100644 index 000000000..f93316986 --- /dev/null +++ b/crates/rattler_conda_types/src/parse_mode.rs @@ -0,0 +1,9 @@ +/// Defines how strict a parser should behave. +#[derive(Debug, Copy, Clone, Eq, PartialEq)] +pub enum ParseStrictness { + /// Allows guessing the users intent. + Lenient, + + /// Very strictly follow parsing rules. + Strict, +} diff --git a/crates/rattler_conda_types/src/version_spec/constraint.rs b/crates/rattler_conda_types/src/version_spec/constraint.rs index dd4280a14..c041e5282 100644 --- a/crates/rattler_conda_types/src/version_spec/constraint.rs +++ b/crates/rattler_conda_types/src/version_spec/constraint.rs @@ -2,7 +2,7 @@ use super::ParseConstraintError; use super::RangeOperator; use crate::version_spec::parse::constraint_parser; use crate::version_spec::{EqualityOperator, StrictRangeOperator}; -use crate::Version; +use crate::{ParseStrictness, Version}; use std::str::FromStr; /// A single version constraint (e.g. `>3.4.5` or `1.2.*`) @@ -30,8 +30,17 @@ pub(crate) fn is_start_of_version_constraint(c: char) -> bool { impl FromStr for Constraint { type Err = ParseConstraintError; - fn from_str(input: &str) -> Result { - match constraint_parser(input) { + fn from_str(s: &str) -> Result { + Constraint::from_str(s, ParseStrictness::Lenient) + } +} + +impl Constraint { + pub fn from_str( + input: &str, + strictness: ParseStrictness, + ) -> Result { + match constraint_parser(strictness)(input) { Ok(("", version)) => Ok(version), Ok((_, _)) => Err(ParseConstraintError::ExpectedEof), Err(nom::Err::Failure(e) | nom::Err::Error(e)) => Err(e), @@ -45,212 +54,252 @@ mod test { use super::Constraint; use crate::version_spec::constraint::ParseConstraintError; use crate::version_spec::{EqualityOperator, RangeOperator, StrictRangeOperator}; - use crate::Version; + use crate::{ParseStrictness, ParseStrictness::*, Version}; + use assert_matches::assert_matches; + use rstest::rstest; use std::str::FromStr; - #[test] - fn test_empty() { + #[rstest] + fn test_empty(#[values(Lenient, Strict)] strictness: ParseStrictness) { assert!(matches!( - Constraint::from_str(""), + Constraint::from_str("", strictness), Err(ParseConstraintError::InvalidVersion(_)) )); } #[test] fn test_any() { - assert_eq!(Constraint::from_str("*"), Ok(Constraint::Any)); + assert_eq!(Constraint::from_str("*", Lenient), Ok(Constraint::Any)); + assert_eq!(Constraint::from_str("*", Strict), Ok(Constraint::Any)); + assert_eq!(Constraint::from_str("*.*", Lenient), Ok(Constraint::Any)); + assert_eq!( + Constraint::from_str("*.*", Strict), + Err(ParseConstraintError::InvalidGlob) + ); } - #[test] - fn test_invalid_op() { + #[rstest] + fn test_invalid_op(#[values(Lenient, Strict)] strictness: ParseStrictness) { assert_eq!( - Constraint::from_str("<>1.2.3"), + Constraint::from_str("<>1.2.3", strictness), Err(ParseConstraintError::InvalidOperator(String::from("<>"))) ); assert_eq!( - Constraint::from_str("=!1.2.3"), + Constraint::from_str("=!1.2.3", strictness), Err(ParseConstraintError::InvalidOperator(String::from("=!"))) ); assert_eq!( - Constraint::from_str("1.2.3"), + Constraint::from_str("1.2.3", strictness), Err(ParseConstraintError::InvalidOperator(String::from(""))) ); assert_eq!( - Constraint::from_str("!=!1.2.3"), + Constraint::from_str("!=!1.2.3", strictness), Err(ParseConstraintError::InvalidOperator(String::from("!=!"))) ); assert_eq!( - Constraint::from_str("<=>1.2.3"), + Constraint::from_str("<=>1.2.3", strictness), Err(ParseConstraintError::InvalidOperator(String::from("<=>"))) ); assert_eq!( - Constraint::from_str("=>1.2.3"), + Constraint::from_str("=>1.2.3", strictness), Err(ParseConstraintError::InvalidOperator(String::from("=>"))) ); } - #[test] - fn test_op() { + #[rstest] + fn test_op(#[values(Lenient, Strict)] strictness: ParseStrictness) { assert_eq!( - Constraint::from_str(">1.2.3"), + Constraint::from_str(">1.2.3", strictness), Ok(Constraint::Comparison( RangeOperator::Greater, - Version::from_str("1.2.3").unwrap() + Version::from_str("1.2.3").unwrap(), )) ); assert_eq!( - Constraint::from_str("<1.2.3"), + Constraint::from_str("<1.2.3", strictness), Ok(Constraint::Comparison( RangeOperator::Less, - Version::from_str("1.2.3").unwrap() + Version::from_str("1.2.3").unwrap(), )) ); assert_eq!( - Constraint::from_str("=1.2.3"), + Constraint::from_str("=1.2.3", strictness), Ok(Constraint::StrictComparison( StrictRangeOperator::StartsWith, - Version::from_str("1.2.3").unwrap() + Version::from_str("1.2.3").unwrap(), )) ); assert_eq!( - Constraint::from_str("==1.2.3"), + Constraint::from_str("==1.2.3", strictness), Ok(Constraint::Exact( EqualityOperator::Equals, - Version::from_str("1.2.3").unwrap() + Version::from_str("1.2.3").unwrap(), )) ); assert_eq!( - Constraint::from_str("!=1.2.3"), + Constraint::from_str("!=1.2.3", strictness), Ok(Constraint::Exact( EqualityOperator::NotEquals, - Version::from_str("1.2.3").unwrap() + Version::from_str("1.2.3").unwrap(), )) ); assert_eq!( - Constraint::from_str("~=1.2.3"), + Constraint::from_str("~=1.2.3", strictness), Ok(Constraint::StrictComparison( StrictRangeOperator::Compatible, - Version::from_str("1.2.3").unwrap() + Version::from_str("1.2.3").unwrap(), )) ); assert_eq!( - Constraint::from_str(">=1.2.3"), + Constraint::from_str(">=1.2.3", strictness), Ok(Constraint::Comparison( RangeOperator::GreaterEquals, - Version::from_str("1.2.3").unwrap() + Version::from_str("1.2.3").unwrap(), )) ); assert_eq!( - Constraint::from_str("<=1.2.3"), + Constraint::from_str("<=1.2.3", strictness), Ok(Constraint::Comparison( RangeOperator::LessEquals, - Version::from_str("1.2.3").unwrap() + Version::from_str("1.2.3").unwrap(), )) ); assert_eq!( - Constraint::from_str(">=1!1.2"), + Constraint::from_str(">=1!1.2", strictness), Ok(Constraint::Comparison( RangeOperator::GreaterEquals, - Version::from_str("1!1.2").unwrap() + Version::from_str("1!1.2").unwrap(), )) ); } #[test] - fn test_glob_op() { + fn test_glob_op_lenient() { assert_eq!( - Constraint::from_str("=1.2.*"), + Constraint::from_str("=1.2.*", Lenient), Ok(Constraint::StrictComparison( StrictRangeOperator::StartsWith, - Version::from_str("1.2").unwrap() + Version::from_str("1.2").unwrap(), )) ); assert_eq!( - Constraint::from_str("!=1.2.*"), + Constraint::from_str("!=1.2.*", Lenient), Ok(Constraint::StrictComparison( StrictRangeOperator::NotStartsWith, - Version::from_str("1.2").unwrap() + Version::from_str("1.2").unwrap(), )) ); assert_eq!( - Constraint::from_str(">=1.2.*"), + Constraint::from_str(">=1.2.*", Lenient), Ok(Constraint::Comparison( RangeOperator::GreaterEquals, - Version::from_str("1.2").unwrap() + Version::from_str("1.2").unwrap(), )) ); assert_eq!( - Constraint::from_str("==1.2.*"), + Constraint::from_str("==1.2.*", Lenient), Ok(Constraint::Exact( EqualityOperator::Equals, - Version::from_str("1.2").unwrap() + Version::from_str("1.2").unwrap(), )) ); assert_eq!( - Constraint::from_str(">1.2.*"), + Constraint::from_str(">1.2.*", Lenient), Ok(Constraint::Comparison( RangeOperator::GreaterEquals, - Version::from_str("1.2").unwrap() + Version::from_str("1.2").unwrap(), )) ); assert_eq!( - Constraint::from_str("<=1.2.*"), + Constraint::from_str("<=1.2.*", Lenient), Ok(Constraint::Comparison( RangeOperator::LessEquals, - Version::from_str("1.2").unwrap() + Version::from_str("1.2").unwrap(), )) ); assert_eq!( - Constraint::from_str("<1.2.*"), + Constraint::from_str("<1.2.*", Lenient), Ok(Constraint::Comparison( RangeOperator::Less, - Version::from_str("1.2").unwrap() + Version::from_str("1.2").unwrap(), )) ); } #[test] - fn test_starts_with() { + fn test_glob_op_strict() { + assert_matches!( + Constraint::from_str("=1.2.*", Strict), + Err(ParseConstraintError::GlobVersionIncompatibleWithOperator(_)) + ); + assert_matches!( + Constraint::from_str("!=1.2.*", Strict), + Err(ParseConstraintError::GlobVersionIncompatibleWithOperator(_)) + ); + assert_matches!( + Constraint::from_str(">=1.2.*", Strict), + Err(ParseConstraintError::GlobVersionIncompatibleWithOperator(_)) + ); + assert_matches!( + Constraint::from_str("==1.2.*", Strict), + Err(ParseConstraintError::GlobVersionIncompatibleWithOperator(_)) + ); + assert_matches!( + Constraint::from_str(">1.2.*", Strict), + Err(ParseConstraintError::GlobVersionIncompatibleWithOperator(_)) + ); + assert_matches!( + Constraint::from_str("<=1.2.*", Strict), + Err(ParseConstraintError::GlobVersionIncompatibleWithOperator(_)) + ); + assert_matches!( + Constraint::from_str("<1.2.*", Strict), + Err(ParseConstraintError::GlobVersionIncompatibleWithOperator(_)) + ); + } + + #[rstest] + fn test_starts_with(#[values(Lenient, Strict)] strictness: ParseStrictness) { assert_eq!( - Constraint::from_str("1.2.*"), + Constraint::from_str("1.2.*", strictness), Ok(Constraint::StrictComparison( StrictRangeOperator::StartsWith, - Version::from_str("1.2").unwrap() + Version::from_str("1.2").unwrap(), )) ); assert_eq!( - Constraint::from_str("1.2.*.*"), + Constraint::from_str("1.2.*.*", strictness), Err(ParseConstraintError::RegexConstraintsNotSupported) ); } - #[test] - fn test_exact() { + #[rstest] + fn test_exact(#[values(Lenient, Strict)] strictness: ParseStrictness) { assert_eq!( - Constraint::from_str("1.2.3"), + Constraint::from_str("1.2.3", strictness), Ok(Constraint::Exact( EqualityOperator::Equals, - Version::from_str("1.2.3").unwrap() + Version::from_str("1.2.3").unwrap(), )) ); } - #[test] - fn test_regex() { + #[rstest] + fn test_regex(#[values(Lenient, Strict)] strictness: ParseStrictness) { assert_eq!( - Constraint::from_str("^1.2.3"), + Constraint::from_str("^1.2.3", strictness), Err(ParseConstraintError::UnterminatedRegex) ); assert_eq!( - Constraint::from_str("1.2.3$"), + Constraint::from_str("1.2.3$", strictness), Err(ParseConstraintError::UnterminatedRegex) ); assert_eq!( - Constraint::from_str("1.*.3"), + Constraint::from_str("1.*.3", strictness), Err(ParseConstraintError::RegexConstraintsNotSupported) ); } diff --git a/crates/rattler_conda_types/src/version_spec/mod.rs b/crates/rattler_conda_types/src/version_spec/mod.rs index 6571e8ce6..25fa4e555 100644 --- a/crates/rattler_conda_types/src/version_spec/mod.rs +++ b/crates/rattler_conda_types/src/version_spec/mod.rs @@ -6,7 +6,7 @@ pub(crate) mod parse; pub(crate) mod version_tree; use crate::version_spec::version_tree::ParseVersionTreeError; -use crate::{ParseVersionError, Version}; +use crate::{ParseStrictness, ParseVersionError, Version}; use constraint::Constraint; use serde::{Deserialize, Serialize, Serializer}; use std::convert::TryFrom; @@ -160,25 +160,38 @@ impl FromStr for VersionSpec { type Err = ParseVersionSpecError; fn from_str(s: &str) -> Result { - fn parse_tree(tree: VersionTree<'_>) -> Result { + VersionSpec::from_str(s, ParseStrictness::Lenient) + } +} + +impl VersionSpec { + /// Parse a [`VersionSpec`] from a string. + pub fn from_str( + source: &str, + strictness: ParseStrictness, + ) -> Result { + fn parse_tree( + tree: VersionTree<'_>, + strictness: ParseStrictness, + ) -> Result { match tree { - VersionTree::Term(str) => Ok(Constraint::from_str(str) + VersionTree::Term(str) => Ok(Constraint::from_str(str, strictness) .map_err(ParseVersionSpecError::InvalidConstraint)? .into()), VersionTree::Group(op, groups) => Ok(VersionSpec::Group( op, groups .into_iter() - .map(parse_tree) + .map(|group| parse_tree(group, strictness)) .collect::>()?, )), } } let version_tree = - VersionTree::try_from(s).map_err(ParseVersionSpecError::InvalidVersionTree)?; + VersionTree::try_from(source).map_err(ParseVersionSpecError::InvalidVersionTree)?; - parse_tree(version_tree) + parse_tree(version_tree, strictness) } } @@ -323,23 +336,23 @@ mod tests { use crate::version_spec::{ EqualityOperator, LogicalOperator, ParseVersionSpecError, RangeOperator, }; - use crate::{Version, VersionSpec}; + use crate::{ParseStrictness, Version, VersionSpec}; use std::str::FromStr; #[test] fn test_simple() { assert_eq!( - VersionSpec::from_str("1.2.3"), + VersionSpec::from_str("1.2.3", ParseStrictness::Strict), Ok(VersionSpec::Exact( EqualityOperator::Equals, - Version::from_str("1.2.3").unwrap() + Version::from_str("1.2.3").unwrap(), )) ); assert_eq!( - VersionSpec::from_str(">=1.2.3"), + VersionSpec::from_str(">=1.2.3", ParseStrictness::Strict), Ok(VersionSpec::Range( RangeOperator::GreaterEquals, - Version::from_str("1.2.3").unwrap() + Version::from_str("1.2.3").unwrap(), )) ); } @@ -347,42 +360,42 @@ mod tests { #[test] fn test_group() { assert_eq!( - VersionSpec::from_str(">=1.2.3,<2.0.0"), + VersionSpec::from_str(">=1.2.3,<2.0.0", ParseStrictness::Strict), Ok(VersionSpec::Group( LogicalOperator::And, vec![ VersionSpec::Range( RangeOperator::GreaterEquals, - Version::from_str("1.2.3").unwrap() + Version::from_str("1.2.3").unwrap(), ), VersionSpec::Range(RangeOperator::Less, Version::from_str("2.0.0").unwrap()), - ] + ], )) ); assert_eq!( - VersionSpec::from_str(">=1.2.3|<1.0.0"), + VersionSpec::from_str(">=1.2.3|<1.0.0", ParseStrictness::Strict), Ok(VersionSpec::Group( LogicalOperator::Or, vec![ VersionSpec::Range( RangeOperator::GreaterEquals, - Version::from_str("1.2.3").unwrap() + Version::from_str("1.2.3").unwrap(), ), VersionSpec::Range(RangeOperator::Less, Version::from_str("1.0.0").unwrap()), - ] + ], )) ); assert_eq!( - VersionSpec::from_str("((>=1.2.3)|<1.0.0)"), + VersionSpec::from_str("((>=1.2.3)|<1.0.0)", ParseStrictness::Strict), Ok(VersionSpec::Group( LogicalOperator::Or, vec![ VersionSpec::Range( RangeOperator::GreaterEquals, - Version::from_str("1.2.3").unwrap() + Version::from_str("1.2.3").unwrap(), ), VersionSpec::Range(RangeOperator::Less, Version::from_str("1.0.0").unwrap()), - ] + ], )) ); } @@ -390,10 +403,10 @@ mod tests { #[test] fn test_matches() { let v1 = Version::from_str("1.2.0").unwrap(); - let vs1 = VersionSpec::from_str(">=1.2.3,<2.0.0").unwrap(); + let vs1 = VersionSpec::from_str(">=1.2.3,<2.0.0", ParseStrictness::Strict).unwrap(); assert!(!vs1.matches(&v1)); - let vs2 = VersionSpec::from_str("1.2").unwrap(); + let vs2 = VersionSpec::from_str("1.2", ParseStrictness::Strict).unwrap(); assert!(vs2.matches(&v1)); let v2 = Version::from_str("1.2.3").unwrap(); @@ -406,18 +419,18 @@ mod tests { assert!(!vs1.matches(&v3)); assert!(!vs2.matches(&v3)); - let vs3 = VersionSpec::from_str(">=1!1.2,<1!2").unwrap(); + let vs3 = VersionSpec::from_str(">=1!1.2,<1!2", ParseStrictness::Strict).unwrap(); assert!(vs3.matches(&v3)); } #[test] fn issue_204() { - assert!(VersionSpec::from_str(">=3.8<3.9").is_err()); + assert!(VersionSpec::from_str(">=3.8<3.9", ParseStrictness::Strict).is_err()); } #[test] fn issue_225() { - let spec = VersionSpec::from_str("~=2.4").unwrap(); + let spec = VersionSpec::from_str("~=2.4", ParseStrictness::Strict).unwrap(); assert!(!spec.matches(&Version::from_str("3.1").unwrap())); assert!(spec.matches(&Version::from_str("2.4").unwrap())); assert!(spec.matches(&Version::from_str("2.5").unwrap())); @@ -427,48 +440,79 @@ mod tests { #[test] fn issue_235() { assert_eq!( - VersionSpec::from_str(">2.10*").unwrap(), - VersionSpec::from_str(">=2.10").unwrap() + VersionSpec::from_str(">2.10*", ParseStrictness::Lenient).unwrap(), + VersionSpec::from_str(">=2.10", ParseStrictness::Strict).unwrap() ); } #[test] fn issue_star_operator() { assert_eq!( - VersionSpec::from_str(">=*").unwrap(), - VersionSpec::from_str("*").unwrap() + VersionSpec::from_str(">=*", ParseStrictness::Lenient).unwrap(), + VersionSpec::from_str("*", ParseStrictness::Lenient).unwrap() ); assert_eq!( - VersionSpec::from_str("==*").unwrap(), - VersionSpec::from_str("*").unwrap() + VersionSpec::from_str("==*", ParseStrictness::Lenient).unwrap(), + VersionSpec::from_str("*", ParseStrictness::Lenient).unwrap() ); assert_eq!( - VersionSpec::from_str("=*").unwrap(), - VersionSpec::from_str("*").unwrap() + VersionSpec::from_str("=*", ParseStrictness::Lenient).unwrap(), + VersionSpec::from_str("*", ParseStrictness::Lenient).unwrap() ); assert_eq!( - VersionSpec::from_str("~=*").unwrap(), - VersionSpec::from_str("*").unwrap() + VersionSpec::from_str("~=*", ParseStrictness::Lenient).unwrap(), + VersionSpec::from_str("*", ParseStrictness::Lenient).unwrap() ); assert_eq!( - VersionSpec::from_str("<=*").unwrap(), - VersionSpec::from_str("*").unwrap() + VersionSpec::from_str("<=*", ParseStrictness::Lenient).unwrap(), + VersionSpec::from_str("*", ParseStrictness::Lenient).unwrap() + ); + + assert_matches!( + VersionSpec::from_str(">*", ParseStrictness::Lenient).unwrap_err(), + ParseVersionSpecError::InvalidConstraint( + ParseConstraintError::GlobVersionIncompatibleWithOperator(_) + ) + ); + assert_matches!( + VersionSpec::from_str("!=*", ParseStrictness::Lenient).unwrap_err(), + ParseVersionSpecError::InvalidConstraint( + ParseConstraintError::GlobVersionIncompatibleWithOperator(_) + ) + ); + assert_matches!( + VersionSpec::from_str("<*", ParseStrictness::Lenient).unwrap_err(), + ParseVersionSpecError::InvalidConstraint( + ParseConstraintError::GlobVersionIncompatibleWithOperator(_) + ) ); assert_matches!( - VersionSpec::from_str(">*").unwrap_err(), + VersionSpec::from_str(">=*", ParseStrictness::Strict).unwrap_err(), + ParseVersionSpecError::InvalidConstraint( + ParseConstraintError::GlobVersionIncompatibleWithOperator(_) + ) + ); + assert_matches!( + VersionSpec::from_str("==*", ParseStrictness::Strict).unwrap_err(), + ParseVersionSpecError::InvalidConstraint( + ParseConstraintError::GlobVersionIncompatibleWithOperator(_) + ) + ); + assert_matches!( + VersionSpec::from_str("=*", ParseStrictness::Strict).unwrap_err(), ParseVersionSpecError::InvalidConstraint( ParseConstraintError::GlobVersionIncompatibleWithOperator(_) ) ); assert_matches!( - VersionSpec::from_str("!=*").unwrap_err(), + VersionSpec::from_str("~=*", ParseStrictness::Strict).unwrap_err(), ParseVersionSpecError::InvalidConstraint( ParseConstraintError::GlobVersionIncompatibleWithOperator(_) ) ); assert_matches!( - VersionSpec::from_str("<*").unwrap_err(), + VersionSpec::from_str("<=*", ParseStrictness::Strict).unwrap_err(), ParseVersionSpecError::InvalidConstraint( ParseConstraintError::GlobVersionIncompatibleWithOperator(_) ) diff --git a/crates/rattler_conda_types/src/version_spec/parse.rs b/crates/rattler_conda_types/src/version_spec/parse.rs index 1854b4b1e..93dee07a9 100644 --- a/crates/rattler_conda_types/src/version_spec/parse.rs +++ b/crates/rattler_conda_types/src/version_spec/parse.rs @@ -1,14 +1,14 @@ use crate::version::parse::version_parser; use crate::version_spec::constraint::Constraint; use crate::version_spec::{EqualityOperator, RangeOperator, StrictRangeOperator, VersionOperators}; -use crate::{ParseVersionError, ParseVersionErrorKind}; +use crate::{ParseStrictness, ParseVersionError, ParseVersionErrorKind}; use nom::{ branch::alt, bytes::complete::{tag, take_while, take_while1}, character::complete::char, - combinator::{opt, value}, + combinator::opt, error::{ErrorKind, ParseError}, - sequence::{terminated, tuple}, + sequence::tuple, IResult, }; use thiserror::Error; @@ -42,7 +42,7 @@ fn operator_parser(input: &str) -> IResult<&str, VersionOperators, ParseVersionO _ => { return Err(nom::Err::Failure( ParseVersionOperatorError::InvalidOperator(operator_str), - )) + )); } }; @@ -70,6 +70,9 @@ pub enum ParseConstraintError { /// Nom error #[error("{0:?}")] Nom(ErrorKind), + + #[error("invalid glob pattern")] + InvalidGlob, } impl<'i> ParseError<&'i str> for ParseConstraintError { @@ -83,145 +86,193 @@ impl<'i> ParseError<&'i str> for ParseConstraintError { } /// Parses a regex constraint. Returns an error if no terminating `$` is found. -fn regex_constraint_parser(input: &str) -> IResult<&str, Constraint, ParseConstraintError> { - let (_rest, (preceder, _, terminator)) = - tuple((opt(char('^')), take_while(|c| c != '$'), opt(char('$'))))(input)?; - match (preceder, terminator) { - (None, None) => Err(nom::Err::Error(ParseConstraintError::UnterminatedRegex)), - (_, None) | (None, _) => Err(nom::Err::Failure(ParseConstraintError::UnterminatedRegex)), - _ => Err(nom::Err::Failure( - ParseConstraintError::RegexConstraintsNotSupported, - )), +fn regex_constraint_parser( + _strictness: ParseStrictness, +) -> impl FnMut(&str) -> IResult<&str, Constraint, ParseConstraintError> { + move |input: &str| { + let (_rest, (preceder, _, terminator)) = + tuple((opt(char('^')), take_while(|c| c != '$'), opt(char('$'))))(input)?; + match (preceder, terminator) { + (None, None) => Err(nom::Err::Error(ParseConstraintError::UnterminatedRegex)), + (_, None) | (None, _) => { + Err(nom::Err::Failure(ParseConstraintError::UnterminatedRegex)) + } + _ => Err(nom::Err::Failure( + ParseConstraintError::RegexConstraintsNotSupported, + )), + } } } -/// Parses the any constraint. This matches "*" and ".*" -fn any_constraint_parser(input: &str) -> IResult<&str, Constraint, ParseConstraintError> { - value(Constraint::Any, terminated(tag("*"), opt(tag(".*"))))(input) +/// Parses an any constraint. This matches "*" and ".*". +fn any_constraint_parser( + strictness: ParseStrictness, +) -> impl FnMut(&str) -> IResult<&str, Constraint, ParseConstraintError> { + move |input: &str| { + let (remaining, (_, trailing)) = tuple((tag("*"), opt(tag(".*"))))(input)?; + + // `*.*` is not allowed in strict mode + if trailing.is_some() && strictness == ParseStrictness::Strict { + return Err(nom::Err::Failure(ParseConstraintError::InvalidGlob)); + } + + Ok((remaining, Constraint::Any)) + } } /// Parses a constraint with an operator in front of it. -fn logical_constraint_parser(input: &str) -> IResult<&str, Constraint, ParseConstraintError> { - // Parse the optional preceding operator - let (input, op) = match operator_parser(input) { - Err( - nom::Err::Failure(ParseVersionOperatorError::InvalidOperator(op)) - | nom::Err::Error(ParseVersionOperatorError::InvalidOperator(op)), - ) => { - return Err(nom::Err::Failure(ParseConstraintError::InvalidOperator( - op.to_owned(), - ))) - } - Err(nom::Err::Error(_)) => (input, None), - Ok((rest, op)) => (rest, Some(op)), - _ => unreachable!(), - }; +fn logical_constraint_parser( + strictness: ParseStrictness, +) -> impl FnMut(&str) -> IResult<&str, Constraint, ParseConstraintError> { + use ParseStrictness::{Lenient, Strict}; + + move |input: &str| { + // Parse the optional preceding operator + let (input, op) = match operator_parser(input) { + Err( + nom::Err::Failure(ParseVersionOperatorError::InvalidOperator(op)) + | nom::Err::Error(ParseVersionOperatorError::InvalidOperator(op)), + ) => { + return Err(nom::Err::Failure(ParseConstraintError::InvalidOperator( + op.to_owned(), + ))); + } + Err(nom::Err::Error(_)) => (input, None), + Ok((rest, op)) => (rest, Some(op)), + _ => unreachable!(), + }; - // Take everything that looks like a version and use that to parse the version. Any error means - // no characters were detected that belong to the version. - let (rest, version_str) = take_while1::<_, _, (&str, ErrorKind)>(|c: char| { - c.is_alphanumeric() || "!-_.*+".contains(c) - })(input) - .map_err(|_err| { - nom::Err::Error(ParseConstraintError::InvalidVersion(ParseVersionError { - kind: ParseVersionErrorKind::Empty, - version: String::from(""), - })) - })?; - - // Handle the case where no version was specified. These cases don't make any sense (e.g. - // ``>=*``) but they do exist in the wild. This code here tries to map it to something that at - // least makes some sort of sense. But this is not the case for everything, for instance what - // what is ment with `!=*` or `<*`? - // See: https://github.com/AnacondaRecipes/repodata-hotfixes/issues/220 - if version_str == "*" { - return match op.expect( - "if no operator was specified for the star then this is not a logical constraint", - ) { - VersionOperators::Range(RangeOperator::GreaterEquals | RangeOperator::LessEquals) - | VersionOperators::StrictRange( - StrictRangeOperator::Compatible | StrictRangeOperator::StartsWith, - ) - | VersionOperators::Exact(EqualityOperator::Equals) => Ok((rest, Constraint::Any)), - op => { - return Err(nom::Err::Error( + // Take everything that looks like a version and use that to parse the version. Any error means + // no characters were detected that belong to the version. + let (rest, version_str) = take_while1::<_, _, (&str, ErrorKind)>(|c: char| { + c.is_alphanumeric() || "!-_.*+".contains(c) + })(input) + .map_err(|_err| { + nom::Err::Error(ParseConstraintError::InvalidVersion(ParseVersionError { + kind: ParseVersionErrorKind::Empty, + version: String::from(""), + })) + })?; + + // Handle the case where no version was specified. These cases don't make any sense (e.g. + // ``>=*``) but they do exist in the wild. This code here tries to map it to something that at + // least makes some sort of sense. But this is not the case for everything, for instance what + // what is ment with `!=*` or `<*`? + // See: https://github.com/AnacondaRecipes/repodata-hotfixes/issues/220 + if version_str == "*" { + let op = op.expect( + "if no operator was specified for the star then this is not a logical constraint", + ); + + if strictness == Strict { + return Err(nom::Err::Failure( ParseConstraintError::GlobVersionIncompatibleWithOperator(op), )); } - }; - } - // Parse the string as a version - let (version_rest, version) = version_parser(version_str).map_err(|e| { - e.map(|e| { - ParseConstraintError::InvalidVersion(ParseVersionError { - kind: e, - version: String::from(""), - }) - }) - })?; - - // Convert the operator and the wildcard to something understandable - let op = match (version_rest, op) { - // The version was successfully parsed - ("", Some(op)) => op, - ("", None) => VersionOperators::Exact(EqualityOperator::Equals), - - // The version ends in a wildcard pattern - ("*" | ".*", Some(VersionOperators::StrictRange(StrictRangeOperator::StartsWith))) => { - VersionOperators::StrictRange(StrictRangeOperator::StartsWith) - } - ( - "*" | ".*", - Some(VersionOperators::Range(RangeOperator::GreaterEquals | RangeOperator::Greater)), - ) => VersionOperators::Range(RangeOperator::GreaterEquals), - ("*" | ".*", Some(VersionOperators::Exact(EqualityOperator::NotEquals))) => { - VersionOperators::StrictRange(StrictRangeOperator::NotStartsWith) - } - (glob @ ("*" | ".*"), Some(op)) => { - tracing::warn!("Using {glob} with relational operator is superfluous and deprecated and will be removed in a future version of conda."); - op + return match op { + VersionOperators::Range( + RangeOperator::GreaterEquals | RangeOperator::LessEquals, + ) + | VersionOperators::StrictRange( + StrictRangeOperator::Compatible | StrictRangeOperator::StartsWith, + ) + | VersionOperators::Exact(EqualityOperator::Equals) => Ok((rest, Constraint::Any)), + op => { + return Err(nom::Err::Failure( + ParseConstraintError::GlobVersionIncompatibleWithOperator(op), + )); + } + }; } - ("*" | ".*", None) => VersionOperators::StrictRange(StrictRangeOperator::StartsWith), - // The version string kinda looks like a regular expression. - (version_remainder, _) if version_str.contains('*') || version_remainder.ends_with('$') => { - return Err(nom::Err::Error( - ParseConstraintError::RegexConstraintsNotSupported, - )); - } + // Parse the string as a version + let (version_rest, version) = version_parser(version_str).map_err(|e| { + e.map(|e| { + ParseConstraintError::InvalidVersion(ParseVersionError { + kind: e, + version: String::from(""), + }) + }) + })?; + + // Convert the operator and the wildcard to something understandable + let op = match (version_rest, op, strictness) { + // The version was successfully parsed + ("", Some(op), _) => op, + ("", None, _) => VersionOperators::Exact(EqualityOperator::Equals), + + // The version ends in a wildcard pattern + ( + "*" | ".*", + Some(VersionOperators::Range( + RangeOperator::GreaterEquals | RangeOperator::Greater, + )), + Lenient, + ) => VersionOperators::Range(RangeOperator::GreaterEquals), + ("*" | ".*", Some(VersionOperators::Exact(EqualityOperator::NotEquals)), Lenient) => { + VersionOperators::StrictRange(StrictRangeOperator::NotStartsWith) + } + ("*" | ".*", Some(op), Lenient) => { + // In lenient mode we simply ignore the glob. + op + } + ("*" | ".*", Some(op), Strict) => { + return Err(nom::Err::Failure( + ParseConstraintError::GlobVersionIncompatibleWithOperator(op), + )); + } + ("*" | ".*", None, _) => VersionOperators::StrictRange(StrictRangeOperator::StartsWith), + + // The version string kinda looks like a regular expression. + (version_remainder, _, _) + if version_str.contains('*') || version_remainder.ends_with('$') => + { + return Err(nom::Err::Failure( + ParseConstraintError::RegexConstraintsNotSupported, + )); + } - // Otherwise its just a generic error. - _ => { - return Err(nom::Err::Error(ParseConstraintError::InvalidVersion( - ParseVersionError { - version: version_str.to_owned(), - kind: ParseVersionErrorKind::ExpectedEof, - }, - ))) - } - }; + // Otherwise its just a generic error. + _ => { + return Err(nom::Err::Failure(ParseConstraintError::InvalidVersion( + ParseVersionError { + version: version_str.to_owned(), + kind: ParseVersionErrorKind::ExpectedEof, + }, + ))); + } + }; - match op { - VersionOperators::Range(r) => Ok((rest, Constraint::Comparison(r, version))), - VersionOperators::Exact(e) => Ok((rest, Constraint::Exact(e, version))), - VersionOperators::StrictRange(s) => Ok((rest, Constraint::StrictComparison(s, version))), + match op { + VersionOperators::Range(r) => Ok((rest, Constraint::Comparison(r, version))), + VersionOperators::Exact(e) => Ok((rest, Constraint::Exact(e, version))), + VersionOperators::StrictRange(s) => { + Ok((rest, Constraint::StrictComparison(s, version))) + } + } } } /// Parses a version constraint. -pub fn constraint_parser(input: &str) -> IResult<&str, Constraint, ParseConstraintError> { - alt(( - regex_constraint_parser, - any_constraint_parser, - logical_constraint_parser, - ))(input) +pub fn constraint_parser( + strictness: ParseStrictness, +) -> impl FnMut(&str) -> IResult<&str, Constraint, ParseConstraintError> { + move |input| { + alt(( + regex_constraint_parser(strictness), + any_constraint_parser(strictness), + logical_constraint_parser(strictness), + ))(input) + } } #[cfg(test)] mod test { use super::*; - use crate::{Version, VersionSpec}; + use crate::{ParseStrictness::*, Version, VersionSpec}; + use assert_matches::assert_matches; + use rstest::rstest; use std::str::FromStr; #[test] @@ -277,34 +328,34 @@ mod test { ); } - #[test] - fn parse_regex_constraint() { + #[rstest] + fn parse_regex(#[values(Lenient, Strict)] strictness: ParseStrictness) { assert_eq!( - regex_constraint_parser("^.*"), + regex_constraint_parser(strictness)("^.*"), Err(nom::Err::Failure(ParseConstraintError::UnterminatedRegex)) ); assert_eq!( - regex_constraint_parser("^"), + regex_constraint_parser(strictness)("^"), Err(nom::Err::Failure(ParseConstraintError::UnterminatedRegex)) ); assert_eq!( - regex_constraint_parser("^$"), + regex_constraint_parser(strictness)("^$"), Err(nom::Err::Failure( ParseConstraintError::RegexConstraintsNotSupported )) ); assert_eq!( - regex_constraint_parser("^1.2.3$"), + regex_constraint_parser(strictness)("^1.2.3$"), Err(nom::Err::Failure( ParseConstraintError::RegexConstraintsNotSupported )) ); } - #[test] - fn parse_logical_constraint() { + #[rstest] + fn parse_logical_constraint(#[values(Lenient, Strict)] strictness: ParseStrictness) { assert_eq!( - logical_constraint_parser("3.1"), + logical_constraint_parser(strictness)("3.1"), Ok(( "", Constraint::Exact(EqualityOperator::Equals, Version::from_str("3.1").unwrap()) @@ -312,7 +363,7 @@ mod test { ); assert_eq!( - logical_constraint_parser(">3.1"), + logical_constraint_parser(strictness)(">3.1"), Ok(( "", Constraint::Comparison(RangeOperator::Greater, Version::from_str("3.1").unwrap()) @@ -320,71 +371,99 @@ mod test { ); assert_eq!( - logical_constraint_parser("3.1*"), + logical_constraint_parser(strictness)("3.1*"), Ok(( "", Constraint::StrictComparison( StrictRangeOperator::StartsWith, - Version::from_str("3.1").unwrap() + Version::from_str("3.1").unwrap(), ) )) ); assert_eq!( - logical_constraint_parser("3.1.*"), + logical_constraint_parser(strictness)("3.1.*"), Ok(( "", Constraint::StrictComparison( StrictRangeOperator::StartsWith, - Version::from_str("3.1").unwrap() + Version::from_str("3.1").unwrap(), ) )) ); assert_eq!( - logical_constraint_parser("~=3.1"), + logical_constraint_parser(strictness)("~=3.1"), Ok(( "", Constraint::StrictComparison( StrictRangeOperator::Compatible, - Version::from_str("3.1").unwrap() + Version::from_str("3.1").unwrap(), ) )) ); + } + #[test] + fn parse_logical_constraint_lenient() { assert_eq!( - logical_constraint_parser(">=3.1*"), + logical_constraint_parser(Lenient)(">=3.1*"), Ok(( "", Constraint::Comparison( RangeOperator::GreaterEquals, - Version::from_str("3.1").unwrap() + Version::from_str("3.1").unwrap(), ) )) ); + assert_matches!( + logical_constraint_parser(Strict)(">=3.1*"), + Err(nom::Err::Failure( + ParseConstraintError::GlobVersionIncompatibleWithOperator(_) + )) + ); } - #[test] - fn parse_constraint() { + #[rstest] + fn parse_regex_constraint(#[values(Lenient, Strict)] strictness: ParseStrictness) { // Regular expressions assert_eq!( - constraint_parser("^1.2.3$"), + constraint_parser(strictness)("^1.2.3$"), Err(nom::Err::Failure( ParseConstraintError::RegexConstraintsNotSupported )) ); assert_eq!( - constraint_parser("^1.2.3"), + constraint_parser(strictness)("^1.2.3"), Err(nom::Err::Failure(ParseConstraintError::UnterminatedRegex)) ); // Any constraints - assert_eq!(constraint_parser("*"), Ok(("", Constraint::Any))); - assert_eq!(constraint_parser("*.*"), Ok(("", Constraint::Any))); + assert_eq!( + constraint_parser(strictness)("*"), + Ok(("", Constraint::Any)) + ); + } + + #[rstest] + fn parse_any_constraint(#[values(Lenient, Strict)] strictness: ParseStrictness) { + assert_eq!( + constraint_parser(strictness)("*"), + Ok(("", Constraint::Any)) + ); + } + + #[test] + fn parse_any_constraint_lenient() { + assert_eq!(constraint_parser(Lenient)("*.*"), Ok(("", Constraint::Any))); + assert_matches!( + constraint_parser(Strict)("*.*"), + Err(nom::Err::Failure(ParseConstraintError::InvalidGlob)) + ); } #[test] fn pixi_issue_278() { - assert!(VersionSpec::from_str("1.8.1+g6b29558").is_ok()); + assert!(VersionSpec::from_str("1.8.1+g6b29558", Strict).is_ok()); } } diff --git a/crates/rattler_solve/src/libsolv_c/wrapper/pool.rs b/crates/rattler_solve/src/libsolv_c/wrapper/pool.rs index 5f99d815d..b41ca5a7d 100644 --- a/crates/rattler_solve/src/libsolv_c/wrapper/pool.rs +++ b/crates/rattler_solve/src/libsolv_c/wrapper/pool.rs @@ -275,10 +275,10 @@ impl From for Id { #[cfg(test)] mod test { - use std::{ffi::CString, str::FromStr}; + use std::ffi::CString; use super::super::pool::Pool; - use rattler_conda_types::MatchSpec; + use rattler_conda_types::{MatchSpec, ParseStrictness}; #[test] fn test_pool_string_interning() { @@ -319,7 +319,7 @@ mod test { #[test] fn test_matchspec_interning() { // Create a matchspec - let spec = MatchSpec::from_str("foo=1.0=py27_0").unwrap(); + let spec = MatchSpec::from_str("foo=1.0=py27_0", ParseStrictness::Lenient).unwrap(); // Intern it into the pool let pool = Pool::default(); pool.intern_matchspec(&spec); diff --git a/crates/rattler_solve/src/resolvo/mod.rs b/crates/rattler_solve/src/resolvo/mod.rs index bbcb33002..4b214b6d4 100644 --- a/crates/rattler_solve/src/resolvo/mod.rs +++ b/crates/rattler_solve/src/resolvo/mod.rs @@ -4,7 +4,7 @@ use crate::{IntoRepoData, SolveError, SolverRepoData, SolverTask}; use rattler_conda_types::package::ArchiveType; use rattler_conda_types::{ GenericVirtualPackage, MatchSpec, NamelessMatchSpec, PackageRecord, ParseMatchSpecError, - RepoDataRecord, + ParseStrictness, RepoDataRecord, }; use resolvo::{ Candidates, Dependencies, DependencyProvider, KnownDependencies, NameId, Pool, SolvableDisplay, @@ -19,7 +19,6 @@ use std::{ fmt::{Display, Formatter}, marker::PhantomData, ops::Deref, - str::FromStr, }; use itertools::Itertools; @@ -499,7 +498,7 @@ fn parse_match_spec<'a>( if let Some(spec_id) = parse_match_spec_cache.get(spec_str) { Ok(*spec_id) } else { - let match_spec = MatchSpec::from_str(spec_str)?; + let match_spec = MatchSpec::from_str(spec_str, ParseStrictness::Lenient)?; let (name, spec) = match_spec.into_nameless(); let dependency_name = pool.intern_package_name( name.as_ref() diff --git a/crates/rattler_solve/tests/backends.rs b/crates/rattler_solve/tests/backends.rs index f1b9a0b7b..4cfed7151 100644 --- a/crates/rattler_solve/tests/backends.rs +++ b/crates/rattler_solve/tests/backends.rs @@ -1,7 +1,7 @@ use once_cell::sync::Lazy; use rattler_conda_types::{ - Channel, ChannelConfig, GenericVirtualPackage, MatchSpec, NoArchType, PackageRecord, RepoData, - RepoDataRecord, Version, + Channel, ChannelConfig, GenericVirtualPackage, MatchSpec, NoArchType, PackageRecord, + ParseStrictness, RepoData, RepoDataRecord, Version, }; use rattler_repodata_gateway::sparse::SparseRepoData; use rattler_solve::{SolveError, SolverImpl, SolverTask}; @@ -112,7 +112,7 @@ fn installed_package( fn solve_real_world(specs: Vec<&str>) -> Vec { let specs = specs .iter() - .map(|s| MatchSpec::from_str(s).unwrap()) + .map(|s| MatchSpec::from_str(s, ParseStrictness::Lenient).unwrap()) .collect::>(); let sparse_repo_datas = read_real_world_repo_data(); @@ -620,7 +620,7 @@ fn solve( let specs: Vec<_> = match_specs .iter() - .map(|m| MatchSpec::from_str(m).unwrap()) + .map(|m| MatchSpec::from_str(m, ParseStrictness::Lenient).unwrap()) .collect(); let task = SolverTask { @@ -644,7 +644,7 @@ fn solve( fn compare_solve(specs: Vec<&str>) { let specs = specs .iter() - .map(|s| MatchSpec::from_str(s).unwrap()) + .map(|s| MatchSpec::from_str(s, ParseStrictness::Lenient).unwrap()) .collect::>(); let sparse_repo_datas = read_real_world_repo_data(); @@ -766,7 +766,7 @@ fn solve_to_get_channel_of_spec( expected_channel: &str, repo_data: Vec<&SparseRepoData>, ) { - let spec = MatchSpec::from_str(spec_str).unwrap(); + let spec = MatchSpec::from_str(spec_str, ParseStrictness::Lenient).unwrap(); let specs = vec![spec.clone()]; let names = specs.iter().filter_map(|s| s.name.as_ref().cloned());