Skip to content

Commit

Permalink
Add support for python_version in ... markers
Browse files Browse the repository at this point in the history
  • Loading branch information
zanieb committed Aug 17, 2024
1 parent 0091adf commit 3506104
Show file tree
Hide file tree
Showing 6 changed files with 190 additions and 16 deletions.
65 changes: 65 additions & 0 deletions crates/pep508-rs/src/marker/algebra.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,11 +168,32 @@ impl InternerGuard<'_> {
),
Err(node) => return node,
},
MarkerExpression::VersionIn {
key: MarkerValueVersion::PythonVersion,
versions,
negated,
} => match Edges::from_python_versions(versions, negated) {
Ok(edges) => (
Variable::Version(MarkerValueVersion::PythonFullVersion),
edges,
),
Err(node) => return node,
},
// A variable representing the output of a version key. Edges correspond
// to disjoint version ranges.
MarkerExpression::Version { key, specifier } => {
(Variable::Version(key), Edges::from_specifier(specifier))
}
// A variable representing the output of a version key. Edges correspond
// to disjoint version ranges.
MarkerExpression::VersionIn {
key,
versions,
negated,
} => (
Variable::Version(key),
Edges::from_versions(&versions, negated),
),
// The `in` and `contains` operators are a bit different than other operators.
// In particular, they do not represent a particular value for the corresponding
// variable, and can overlap. For example, `'nux' in os_name` and `os_name == 'Linux'`
Expand Down Expand Up @@ -587,6 +608,50 @@ impl Edges {
}
}

/// Returns an [`Edges`] where values in the given range are `true`.
///
/// Only for use when the `key` is a `PythonVersion`. Casts to `PythonFullVersion`.
fn from_python_versions(versions: Vec<Version>, negated: bool) -> Result<Edges, NodeId> {
let mut range = Range::empty();

// TODO(zanieb): We need to make sure this is performant, repeated unions like this do not
// seem efficient.
for version in versions {
let specifier = VersionSpecifier::equals_version(version.clone());
let specifier = python_version_to_full_version(specifier)?;
let pubgrub_specifier =
PubGrubSpecifier::from_release_specifier(&normalize_specifier(specifier)).unwrap();
range = range.union(&pubgrub_specifier.into());
}

if negated {
range = range.complement();
}

Ok(Edges::Version {
edges: Edges::from_range(&range),
})
}

/// Returns an [`Edges`] where values in the given range are `true`.
fn from_versions(versions: &Vec<Version>, negated: bool) -> Edges {
let mut range = Range::empty();

// TODO(zanieb): We need to make sure this is performant, repeated unions like this do not
// seem efficient.
for version in versions {
range = range.union(&Range::singleton(version.clone()));
}

if negated {
range = range.complement();
}

Edges::Version {
edges: Edges::from_range(&range),
}
}

/// Returns an [`Edges`] where values in the given range are `true`.
fn from_range<T>(range: &Range<T>) -> SmallVec<(Range<T>, NodeId)>
where
Expand Down
56 changes: 56 additions & 0 deletions crates/pep508-rs/src/marker/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,10 @@ pub(crate) fn parse_marker_key_op_value<T: Pep508Url>(
return Ok(None);
};

if let Some(expr) = parse_version_in_expr(key.clone(), operator, &value, reporter) {
return Ok(Some(expr));
}

parse_version_expr(key.clone(), operator, &value, reporter)
}
// The only sound choice for this is `<env key> <op> <string>`
Expand Down Expand Up @@ -238,6 +242,58 @@ pub(crate) fn parse_marker_key_op_value<T: Pep508Url>(
Ok(expr)
}

/// Creates an instance of [`MarkerExpression::VersionIn`] with the given values.
///
/// Reports a warning on failure, and returns `None`.
fn parse_version_in_expr(
key: MarkerValueVersion,
operator: MarkerOperator,
value: &str,
reporter: &mut impl Reporter,
) -> Option<MarkerExpression> {
if !matches!(operator, MarkerOperator::In | MarkerOperator::NotIn) {
return None;
}
let negated = matches!(operator, MarkerOperator::NotIn);

let mut cursor = Cursor::new(value);
let mut versions = Vec::new();

// Parse all of the values in the list as versions
loop {
// Allow arbitrary whitespace between versions
cursor.eat_whitespace();

let (start, len) = cursor.take_while(|c| !c.is_whitespace());
if len == 0 {
break;
}

let version = match Version::from_str(cursor.slice(start, len)) {
Ok(version) => version,
Err(err) => {
reporter.report(
MarkerWarningKind::Pep440Error,
format!(
"Expected PEP 440 versions to compare with {key}, found {value},
will be ignored: {err}"
),
);

return None;
}
};

versions.push(version);
}

Some(MarkerExpression::VersionIn {
key,
versions,
negated,
})
}

/// Creates an instance of [`MarkerExpression::Version`] with the given values.
///
/// Reports a warning on failure, and returns `None`.
Expand Down
16 changes: 16 additions & 0 deletions crates/pep508-rs/src/marker/simplify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,22 @@ fn is_negation(left: &MarkerExpression, right: &MarkerExpression) -> bool {
.negate()
.is_some_and(|negated| negated == *specifier2.operator())
}
MarkerExpression::VersionIn {
key,
versions,
negated,
} => {
let MarkerExpression::VersionIn {
key: key2,
versions: versions2,
negated: negated2,
} = right
else {
return false;
};

key == key2 && versions == versions2 && negated != negated2
}
MarkerExpression::String {
key,
operator,
Expand Down
52 changes: 52 additions & 0 deletions crates/pep508-rs/src/marker/tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use std::fmt::{self, Display, Formatter};
use std::ops::{Bound, Deref};
use std::str::FromStr;

use itertools::Itertools;
use pubgrub::Range;
#[cfg(feature = "pyo3")]
use pyo3::{basic::CompareOp, pyclass, pymethods};
Expand Down Expand Up @@ -436,6 +437,11 @@ pub enum MarkerExpression {
key: MarkerValueVersion,
specifier: VersionSpecifier,
},
VersionIn {
key: MarkerValueVersion,
versions: Vec<Version>,
negated: bool,
},
/// An string marker comparison, e.g. `sys_platform == '...'`.
///
/// Inverted string expressions, e.g `'...' == sys_platform`, are also normalized to this form.
Expand Down Expand Up @@ -534,6 +540,15 @@ impl Display for MarkerExpression {
}
write!(f, "{key} {op} '{version}'")
}
MarkerExpression::VersionIn {
key,
versions,
negated,
} => {
let op = if *negated { "not in" } else { "in" };
let versions = versions.iter().map(ToString::to_string).join(" ");
write!(f, "{key} {op} '{versions}'")
}
MarkerExpression::String {
key,
operator,
Expand Down Expand Up @@ -1542,6 +1557,34 @@ mod test {
assert!(!marker3.evaluate(&env37, &[]));
}

#[test]
fn test_python_version_in_evaluation() {
let env27 = MarkerEnvironment::try_from(MarkerEnvironmentBuilder {
implementation_name: "",
implementation_version: "2.7",
os_name: "linux",
platform_machine: "",
platform_python_implementation: "",
platform_release: "",
platform_system: "",
platform_version: "",
python_full_version: "2.7",
python_version: "2.7",
sys_platform: "linux",
})
.unwrap();
let env37 = env37();
let marker1 = MarkerTree::from_str("python_version in \"2.7 3.2 3.3\"").unwrap();
let marker2 = MarkerTree::from_str("python_version in \"2.7 3.7\"").unwrap();
let marker3 = MarkerTree::from_str("python_version in \"2.4 3.8 4.0\"").unwrap();
assert!(marker1.evaluate(&env27, &[]));
assert!(!marker1.evaluate(&env37, &[]));
assert!(marker2.evaluate(&env27, &[]));
assert!(marker2.evaluate(&env37, &[]));
assert!(!marker3.evaluate(&env27, &[]));
assert!(!marker3.evaluate(&env37, &[]));
}

#[test]
#[cfg(feature = "tracing")]
fn warnings() {
Expand Down Expand Up @@ -1814,6 +1857,15 @@ mod test {
"python_full_version == '3.9.*'",
);

assert_simplifies(
"python_version in '3.9 3.11'",
"python_full_version == '3.9.*' or python_full_version == '3.11.*'",
);
assert_simplifies(
"python_version in '3.9 3.10 3.11'",
"python_full_version >= '3.9' and python_full_version < '3.12'",
);

assert_simplifies("python_version != '3.9'", "python_full_version != '3.9.*'");

assert_simplifies("python_version >= '3.9.0'", "python_full_version >= '3.9'");
Expand Down
15 changes: 0 additions & 15 deletions crates/uv/tests/snapshots/ecosystem__black-lock-file.snap
Original file line number Diff line number Diff line change
Expand Up @@ -510,18 +510,6 @@ wheels = [
{ url = "https://files.pythonhosted.org/packages/c6/ac/dac4a63f978e4dcb3c6d3a78c4d8e0192a113d288502a1216950c41b1027/parso-0.8.4-py2.py3-none-any.whl", hash = "sha256:a418670a20291dacd2dddc80c377c5c3791378ee1e8d12bffc35420643d43f18", size = 103650 },
]

[[package]]
name = "pathlib2"
version = "2.3.7.post1"
source = { registry = "https://pypi.org/simple" }
dependencies = [
{ name = "six" },
]
sdist = { url = "https://files.pythonhosted.org/packages/31/51/99caf463dc7c18eb18dad1fffe465a3cf3ee50ac3d1dccbd1781336fe9c7/pathlib2-2.3.7.post1.tar.gz", hash = "sha256:9fe0edad898b83c0c3e199c842b27ed216645d2e177757b2dd67384d4113c641", size = 211190 }
wheels = [
{ url = "https://files.pythonhosted.org/packages/09/eb/4af4bcd5b8731366b676192675221c5324394a580dfae469d498313b5c4a/pathlib2-2.3.7.post1-py2.py3-none-any.whl", hash = "sha256:5266a0fd000452f1b3467d782f079a4343c63aaa119221fbdc4e39577489ca5b", size = 18027 },
]

[[package]]
name = "pathspec"
version = "0.12.1"
Expand All @@ -547,9 +535,6 @@ wheels = [
name = "pickleshare"
version = "0.7.5"
source = { registry = "https://pypi.org/simple" }
dependencies = [
{ name = "pathlib2" },
]
sdist = { url = "https://files.pythonhosted.org/packages/d8/b6/df3c1c9b616e9c0edbc4fbab6ddd09df9535849c64ba51fcb6531c32d4d8/pickleshare-0.7.5.tar.gz", hash = "sha256:87683d47965c1da65cdacaf31c8441d12b8044cdec9aca500cd78fc2c683afca", size = 6161 }
wheels = [
{ url = "https://files.pythonhosted.org/packages/9a/41/220f49aaea88bc6fa6cba8d05ecf24676326156c23b991e80b3f2fc24c77/pickleshare-0.7.5-py2.py3-none-any.whl", hash = "sha256:9649af414d74d4df115d5d718f82acb59c9d418196b7b4290ed47a12ce62df56", size = 6877 },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,4 @@ exit_code: 0

----- stderr -----
warning: `uv lock` is experimental and may change without warning
Resolved 40 packages in [TIME]
Resolved 39 packages in [TIME]

0 comments on commit 3506104

Please sign in to comment.