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 9b9241f
Show file tree
Hide file tree
Showing 5 changed files with 155 additions and 1 deletion.
29 changes: 29 additions & 0 deletions crates/pep508-rs/src/marker/algebra.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,16 @@ impl InternerGuard<'_> {
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 +597,25 @@ impl Edges {
}
}

/// 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
53 changes: 53 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,16 @@ mod test {
"python_full_version == '3.9.*'",
);

// TODO(zanieb): These clauses should simplify to version ranges
// 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
2 changes: 1 addition & 1 deletion crates/uv/tests/snapshots/ecosystem__black-lock-file.snap
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,7 @@ name = "pickleshare"
version = "0.7.5"
source = { registry = "https://pypi.org/simple" }
dependencies = [
{ name = "pathlib2" },
{ name = "pathlib2", marker = "python_version == '2.6' or python_version == '2.7' or python_version == '3.2' or python_version == '3.3'" },
]
sdist = { url = "https://files.pythonhosted.org/packages/d8/b6/df3c1c9b616e9c0edbc4fbab6ddd09df9535849c64ba51fcb6531c32d4d8/pickleshare-0.7.5.tar.gz", hash = "sha256:87683d47965c1da65cdacaf31c8441d12b8044cdec9aca500cd78fc2c683afca", size = 6161 }
wheels = [
Expand Down

0 comments on commit 9b9241f

Please sign in to comment.