Skip to content

Commit

Permalink
feat: require a range specifier for version spec in strict mode (#989)
Browse files Browse the repository at this point in the history
Co-authored-by: Bas Zalmstra <bas@prefix.dev>
  • Loading branch information
wolfv and baszalmstra authored Jan 1, 2025
1 parent f922ccd commit 94ecc07
Show file tree
Hide file tree
Showing 13 changed files with 295 additions and 230 deletions.
16 changes: 8 additions & 8 deletions crates/rattler_conda_types/src/match_spec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,12 @@ use matcher::StringMatcher;
/// use std::sync::Arc;
///
/// let channel_config = ChannelConfig::default_with_root_dir(std::env::current_dir().unwrap());
/// let spec = MatchSpec::from_str("foo 1.0 py27_0", Strict).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", Strict).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", Strict).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", Strict).unwrap()));
/// assert_eq!(spec.build, Some(StringMatcher::from_str("py27_0").unwrap()));
Expand Down Expand Up @@ -702,12 +702,12 @@ mod tests {

#[test]
fn test_serialize_matchspec() {
let specs = ["mamba 1.0 py37_0",
"conda-forge::pytest[version=1.0, sha256=aaac4bc9c6916ecc0e33137431645b029ade22190c7144eead61446dcbcc6f97, md5=dede6252c964db3f3e41c7d30d07f6bf]",
let specs = ["mamba 1.0.* py37_0",
"conda-forge::pytest[version='==1.0', sha256=aaac4bc9c6916ecc0e33137431645b029ade22190c7144eead61446dcbcc6f97, md5=dede6252c964db3f3e41c7d30d07f6bf]",
"conda-forge/linux-64::pytest",
"conda-forge/linux-64::pytest[version=1.0]",
"conda-forge/linux-64::pytest[version=1.0, build=py37_0]",
"conda-forge/linux-64::pytest 1.2.3"];
"conda-forge/linux-64::pytest[version=1.0.*]",
"conda-forge/linux-64::pytest[version=1.0.*, build=py37_0]",
"conda-forge/linux-64::pytest ==1.2.3"];

assert_snapshot!(specs
.into_iter()
Expand Down
7 changes: 4 additions & 3 deletions crates/rattler_conda_types/src/match_spec/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -804,11 +804,12 @@ mod tests {
fn test_nameless_match_spec() {
insta::assert_yaml_snapshot!([
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[fn=\"bla\"]", Strict).unwrap(),
NamelessMatchSpec::from_str("=1.0 py27_0", Strict).unwrap(),
NamelessMatchSpec::from_str("*cpu*", Strict).unwrap(),
NamelessMatchSpec::from_str("conda-forge::foobar", Strict).unwrap(),
NamelessMatchSpec::from_str("foobar[channel=conda-forge]", Strict).unwrap(),
// the next two tests are a bit weird, the version is `foobar` and the channel is `conda-forge`
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(),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
---
source: crates/rattler_conda_types/src/match_spec/parse.rs
expression: evaluated
snapshot_kind: text
---
blas *.* mkl:
name: blas
Expand Down Expand Up @@ -54,32 +55,13 @@ python=3.9:
python=*:
error: "invalid version constraint: '*' is incompatible with '=' operator'"
"https://software.repos.intel.com/python/conda::python[version=3.9]":
name: python
version: "==3.9"
channel:
base_url: "https://software.repos.intel.com/python/conda"
name: python/conda
error: "invalid version constraint: missing range specifier for '3.9'. Did you mean '==3.9' or '3.9.*'?"
"https://c.com/p/conda/linux-64::python[version=3.9]":
name: python
version: "==3.9"
channel:
base_url: "https://c.com/p/conda"
name: p/conda
subdir: linux-64
error: "invalid version constraint: missing range specifier for '3.9'. Did you mean '==3.9' or '3.9.*'?"
"https://c.com/p/conda::python[version=3.9, subdir=linux-64]":
name: python
version: "==3.9"
channel:
base_url: "https://c.com/p/conda"
name: p/conda
subdir: linux-64
error: "invalid version constraint: missing range specifier for '3.9'. Did you mean '==3.9' or '3.9.*'?"
"conda-forge/linux-32::python[version=3.9, subdir=linux-64]":
name: python
version: "==3.9"
channel:
base_url: "https://conda.anaconda.org/conda-forge"
name: conda-forge
subdir: linux-64
error: "invalid version constraint: missing range specifier for '3.9'. Did you mean '==3.9' or '3.9.*'?"
"conda-forge/linux-32::python ==3.9[subdir=linux-64, build_number=\"0\"]":
name: python
version: "==3.9"
Expand Down Expand Up @@ -119,11 +101,7 @@ rust ~=1.2.3:
base_url: "file://<CRATE>/relative/channel"
name: "./relative/channel"
"python[channel=https://conda.anaconda.org/python/conda,version=3.9]":
name: python
version: "==3.9"
channel:
base_url: "https://conda.anaconda.org/python/conda"
name: python/conda
error: "invalid version constraint: missing range specifier for '3.9'. Did you mean '==3.9' or '3.9.*'?"
"channel/win-64::foobar[channel=conda-forge, subdir=linux-64]":
name: foobar
channel:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
---
source: crates/rattler_conda_types/src/match_spec/parse.rs
expression: evaluated
snapshot_kind: text
---
2.7|>=3.6:
version: "==2.7|>=3.6"
error: "invalid version constraint: missing range specifier for '2.7'. Did you mean '==2.7' or '2.7.*'?"
"https://conda.anaconda.org/conda-forge/linux-64/_libgcc_mutex-0.1-conda_forge.tar.bz2":
url: "https://conda.anaconda.org/conda-forge/linux-64/_libgcc_mutex-0.1-conda_forge.tar.bz2"
~=1.2.3:
Expand Down Expand Up @@ -40,14 +41,13 @@ expression: evaluated
"==2.7.*.*|>=3.6":
error: "invalid version constraint: regex constraints are not supported"
"3.9":
version: "==3.9"
error: "invalid version constraint: missing range specifier for '3.9'. Did you mean '==3.9' or '3.9.*'?"
"*":
version: "*"
"[version=3.9]":
version: "==3.9"
error: "invalid version constraint: missing range specifier for '3.9'. Did you mean '==3.9' or '3.9.*'?"
"[version=3.9, subdir=linux-64]":
version: "==3.9"
subdir: linux-64
error: "invalid version constraint: missing range specifier for '3.9'. Did you mean '==3.9' or '3.9.*'?"
"==3.9[subdir=linux-64, build_number=\"0\"]":
version: "==3.9"
build_number:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
source: crates/rattler_conda_types/src/match_spec/mod.rs
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
mamba 1.0.* py37_0
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
conda-forge/linux-64::pytest 1.0.*
conda-forge/linux-64::pytest 1.0.* py37_0
conda-forge/linux-64::pytest ==1.2.3
2 changes: 1 addition & 1 deletion crates/rattler_conda_types/src/version_spec/constraint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ mod test {
#[rstest]
fn test_exact(#[values(Lenient, Strict)] strictness: ParseStrictness) {
assert_eq!(
Constraint::from_str("1.2.3", strictness),
Constraint::from_str("==1.2.3", strictness),
Ok(Constraint::Exact(
EqualityOperator::Equals,
Version::from_str("1.2.3").unwrap(),
Expand Down
19 changes: 14 additions & 5 deletions crates/rattler_conda_types/src/version_spec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -350,15 +350,15 @@ mod tests {
use crate::{
version_spec::{
parse::ParseConstraintError, EqualityOperator, LogicalOperator, ParseVersionSpecError,
RangeOperator,
RangeOperator, StrictRangeOperator,
},
ParseStrictness, Version, VersionSpec,
ParseStrictness, StrictVersion, Version, VersionSpec,
};

#[test]
fn test_simple() {
assert_eq!(
VersionSpec::from_str("1.2.3", ParseStrictness::Strict),
VersionSpec::from_str("==1.2.3", ParseStrictness::Strict),
Ok(VersionSpec::Exact(
EqualityOperator::Equals,
Version::from_str("1.2.3").unwrap(),
Expand All @@ -371,6 +371,13 @@ mod tests {
Version::from_str("1.2.3").unwrap(),
))
);
assert_eq!(
VersionSpec::from_str("=1.2.3", ParseStrictness::Strict),
Ok(VersionSpec::StrictRange(
StrictRangeOperator::StartsWith,
StrictVersion::from_str("1.2.3").unwrap(),
))
);
}

#[test]
Expand Down Expand Up @@ -422,21 +429,23 @@ mod tests {
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", ParseStrictness::Strict).unwrap();
let vs2 = VersionSpec::from_str("==1.2.0", ParseStrictness::Strict).unwrap();
assert!(vs2.matches(&v1));

let v2 = Version::from_str("1.2.3").unwrap();
assert!(vs1.matches(&v2));
assert!(!vs2.matches(&v2));

let v3 = Version::from_str("1!1.2.3").unwrap();
println!("{v3:?}");

assert!(!vs1.matches(&v3));
assert!(!vs2.matches(&v3));

let vs3 = VersionSpec::from_str(">=1!1.2,<1!2", ParseStrictness::Strict).unwrap();
assert!(vs3.matches(&v3));

let vs4 = VersionSpec::from_str("1!1.2.*", ParseStrictness::Strict).unwrap();
assert!(vs4.matches(&v3));
}

#[test]
Expand Down
25 changes: 21 additions & 4 deletions crates/rattler_conda_types/src/version_spec/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ pub enum ParseConstraintError {
InvalidOperator(String),
#[error(transparent)]
InvalidVersion(#[from] ParseVersionError),
#[error("missing range specifier for '{0}'. Did you mean '=={0}' or '{0}.*'?")]
AmbiguousVersion(String),
/// Expected a version
#[error("expected a version")]
ExpectedVersion,
Expand Down Expand Up @@ -208,8 +210,15 @@ fn logical_constraint_parser(
let op = match (version_rest, op, strictness) {
// The version was successfully parsed
("", Some(op), _) => op,
("", None, _) => VersionOperators::Exact(EqualityOperator::Equals),

("", None, _) => {
if strictness == Strict {
return Err(nom::Err::Failure(ParseConstraintError::AmbiguousVersion(
version_str.to_owned(),
)));
} else {
VersionOperators::Exact(EqualityOperator::Equals)
}
}
// The version ends in a wildcard pattern
(
"*" | ".*",
Expand Down Expand Up @@ -316,6 +325,7 @@ mod test {
use std::str::FromStr;

use assert_matches::assert_matches;
use insta::assert_snapshot;
use rstest::rstest;

use super::*;
Expand Down Expand Up @@ -401,7 +411,7 @@ mod test {
#[rstest]
fn parse_logical_constraint(#[values(Lenient, Strict)] strictness: ParseStrictness) {
assert_eq!(
logical_constraint_parser(strictness)("3.1"),
logical_constraint_parser(strictness)("==3.1"),
Ok((
"",
Constraint::Exact(EqualityOperator::Equals, Version::from_str("3.1").unwrap())
Expand Down Expand Up @@ -510,7 +520,14 @@ mod test {

#[test]
fn pixi_issue_278() {
assert!(VersionSpec::from_str("1.8.1+g6b29558", Strict).is_ok());
assert!(VersionSpec::from_str("=1.8.1+g6b29558", Strict).is_ok());
}

#[test]
fn test_exact_strict() {
assert!(VersionSpec::from_str("==3.1", Strict).is_ok());
assert!(VersionSpec::from_str("3.1", Strict).is_err());
assert_snapshot!(VersionSpec::from_str("1.2.3", Strict).unwrap_err());
}

#[test]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
source: crates/rattler_conda_types/src/version_spec/parse.rs
expression: "VersionSpec::from_str(\"1.2.3\", Strict).unwrap_err()"
snapshot_kind: text
---
invalid version constraint: missing range specifier for '1.2.3'. Did you mean '==1.2.3' or '1.2.3.*'?
7 changes: 4 additions & 3 deletions crates/rattler_repodata_gateway/src/gateway/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,8 @@ mod test {
.query(
vec![index],
vec![Platform::Linux64],
vec![MatchSpec::from_str("openssl 3.3.1 h2466b09_1", Strict).unwrap()].into_iter(),
vec![MatchSpec::from_str("openssl ==3.3.1 h2466b09_1", Strict).unwrap()]
.into_iter(),
)
.recursive(true)
.await
Expand Down Expand Up @@ -454,7 +455,7 @@ mod test {
vec![index.clone()],
vec![Platform::Linux64],
vec![
MatchSpec::from_str("mamba 0.9.2 py39h951de11_0", Strict).unwrap(),
MatchSpec::from_str("mamba ==0.9.2 py39h951de11_0", Strict).unwrap(),
MatchSpec::from_str(openssl_url, Strict).unwrap(),
]
.into_iter(),
Expand Down Expand Up @@ -491,7 +492,7 @@ mod test {
.query(
vec![index.clone()],
vec![Platform::Linux64],
vec![MatchSpec::from_str("mamba 0.9.2 py39h951de11_0", Strict).unwrap()]
vec![MatchSpec::from_str("mamba ==0.9.2 py39h951de11_0", Strict).unwrap()]
.into_iter(),
)
.recursive(true)
Expand Down
Loading

0 comments on commit 94ecc07

Please sign in to comment.