Skip to content

Commit

Permalink
feat!: optional strict parsing of matchspec and versionspec (#552)
Browse files Browse the repository at this point in the history 
Adds a mode to all `from_str` functions for `MatchSpec`,
`NamelessMatchSpec`, `VersionSpec`, and `Constraint`.

Strict mode disallows some ambigous syntax like:

* `>=1.*` should just be `>=1`
* `*.*` should just be `*`

All parsing functions accept a `ParseStrictness` that can either be
`Strict` or `Lenient`. `Lenient` mode should be used when parsing
repodata etc, but `Strict` mode can be used to validate new user input.
  • Loading branch information
@baszalmstra
baszalmstra authored Mar 6, 2024
1 parent e0b9840 commit f97c97b
Show file tree
Hide file tree
Showing 17 changed files with 584 additions and 342 deletions.
6 changes: 3 additions & 3 deletions crates/rattler-bin/src/commands/create.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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::<Result<Vec<_>, _>>()?;

// Find the default cache directory. Create it if it doesnt exist yet.
Expand Down
2 changes: 2 additions & 0 deletions crates/rattler_conda_types/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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};
Expand Down
68 changes: 38 additions & 30 deletions crates/rattler_conda_types/src/match_spec/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
/// ```
Expand Down Expand Up @@ -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();
Expand All @@ -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();
Expand Down Expand Up @@ -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));
}

Expand All @@ -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::<Vec<String>>()
.join("\n"));
Expand Down
Loading

0 comments on commit f97c97b

Please sign in to comment.