From f3dc457d2a96fb27b88c7cfafe97b7f04ffe78aa Mon Sep 17 00:00:00 2001 From: konsti Date: Mon, 21 Jul 2025 18:21:46 +0200 Subject: [PATCH] Introduce a generic type for list operations (#14792) We currently have two marker keys that a list, `extras` and `dependency_groups`, both from PEP 751. With the variants PEP, we will add three more. This change is broken out of the wheel variants PR to introduce generic marker list support, plus a change to use `ContainerOperator` in more places. --- crates/uv-pep508/src/lib.rs | 4 +- crates/uv-pep508/src/marker/algebra.rs | 83 ++--- crates/uv-pep508/src/marker/lowering.rs | 41 +-- crates/uv-pep508/src/marker/mod.rs | 4 +- crates/uv-pep508/src/marker/parse.rs | 155 ++++---- crates/uv-pep508/src/marker/simplify.rs | 77 ++-- crates/uv-pep508/src/marker/tree.rs | 375 +++++++------------- crates/uv-pep508/src/verbatim_url.rs | 1 + crates/uv-resolver/src/marker.rs | 7 +- crates/uv-resolver/src/resolution/output.rs | 7 +- 10 files changed, 276 insertions(+), 478 deletions(-) diff --git a/crates/uv-pep508/src/lib.rs b/crates/uv-pep508/src/lib.rs index f63d46206..10e4142e7 100644 --- a/crates/uv-pep508/src/lib.rs +++ b/crates/uv-pep508/src/lib.rs @@ -32,8 +32,8 @@ pub use marker::{ CanonicalMarkerValueExtra, CanonicalMarkerValueString, CanonicalMarkerValueVersion, ContainsMarkerTree, ExtraMarkerTree, ExtraOperator, InMarkerTree, MarkerEnvironment, MarkerEnvironmentBuilder, MarkerExpression, MarkerOperator, MarkerTree, MarkerTreeContents, - MarkerTreeKind, MarkerValue, MarkerValueExtra, MarkerValueString, MarkerValueVersion, - MarkerWarningKind, StringMarkerTree, StringVersion, VersionMarkerTree, + MarkerTreeKind, MarkerValue, MarkerValueExtra, MarkerValueList, MarkerValueString, + MarkerValueVersion, MarkerWarningKind, StringMarkerTree, StringVersion, VersionMarkerTree, }; pub use origin::RequirementOrigin; #[cfg(feature = "non-pep508-extensions")] diff --git a/crates/uv-pep508/src/marker/algebra.rs b/crates/uv-pep508/src/marker/algebra.rs index d1a369491..6b166dbc6 100644 --- a/crates/uv-pep508/src/marker/algebra.rs +++ b/crates/uv-pep508/src/marker/algebra.rs @@ -59,10 +59,10 @@ use uv_pep440::{Operator, Version, VersionSpecifier, release_specifier_to_range} use crate::marker::MarkerValueExtra; use crate::marker::lowering::{ - CanonicalMarkerValueDependencyGroup, CanonicalMarkerValueExtra, CanonicalMarkerValueString, + CanonicalMarkerListPair, CanonicalMarkerValueExtra, CanonicalMarkerValueString, CanonicalMarkerValueVersion, }; -use crate::marker::tree::{ContainerOperator, MarkerValueDependencyGroup}; +use crate::marker::tree::ContainerOperator; use crate::{ ExtraOperator, MarkerExpression, MarkerOperator, MarkerValueString, MarkerValueVersion, }; @@ -188,19 +188,19 @@ impl InternerGuard<'_> { MarkerExpression::VersionIn { key, versions, - negated, + operator, } => match key { MarkerValueVersion::ImplementationVersion => ( Variable::Version(CanonicalMarkerValueVersion::ImplementationVersion), - Edges::from_versions(&versions, negated), + Edges::from_versions(&versions, operator), ), MarkerValueVersion::PythonFullVersion => ( Variable::Version(CanonicalMarkerValueVersion::PythonFullVersion), - Edges::from_versions(&versions, negated), + Edges::from_versions(&versions, operator), ), // Normalize `python_version` markers to `python_full_version` nodes. MarkerValueVersion::PythonVersion => { - match Edges::from_python_versions(versions, negated) { + match Edges::from_python_versions(versions, operator) { Ok(edges) => ( Variable::Version(CanonicalMarkerValueVersion::PythonFullVersion), edges, @@ -315,6 +315,10 @@ impl InternerGuard<'_> { }; (Variable::String(key), Edges::from_string(operator, value)) } + MarkerExpression::List { pair, operator } => ( + Variable::List(pair), + Edges::from_bool(operator == ContainerOperator::In), + ), // A variable representing the existence or absence of a particular extra. MarkerExpression::Extra { name: MarkerValueExtra::Extra(extra), @@ -335,48 +339,6 @@ impl InternerGuard<'_> { name: MarkerValueExtra::Arbitrary(_), .. } => return NodeId::FALSE, - // A variable representing the existence or absence of a particular extra, in the - // context of a PEP 751 lockfile. - MarkerExpression::Extras { - name: MarkerValueExtra::Extra(extra), - operator: ContainerOperator::In, - } => ( - Variable::Extras(CanonicalMarkerValueExtra::Extra(extra)), - Edges::from_bool(true), - ), - MarkerExpression::Extras { - name: MarkerValueExtra::Extra(extra), - operator: ContainerOperator::NotIn, - } => ( - Variable::Extras(CanonicalMarkerValueExtra::Extra(extra)), - Edges::from_bool(false), - ), - // Invalid `extras` names are always `false`. - MarkerExpression::Extras { - name: MarkerValueExtra::Arbitrary(_), - .. - } => return NodeId::FALSE, - // A variable representing the existence or absence of a particular extra, in the - // context of a PEP 751 lockfile. - MarkerExpression::DependencyGroups { - name: MarkerValueDependencyGroup::Group(group), - operator: ContainerOperator::In, - } => ( - Variable::DependencyGroups(CanonicalMarkerValueDependencyGroup::Group(group)), - Edges::from_bool(true), - ), - MarkerExpression::DependencyGroups { - name: MarkerValueDependencyGroup::Group(group), - operator: ContainerOperator::NotIn, - } => ( - Variable::DependencyGroups(CanonicalMarkerValueDependencyGroup::Group(group)), - Edges::from_bool(false), - ), - // Invalid `dependency_group` names are always `false`. - MarkerExpression::DependencyGroups { - name: MarkerValueDependencyGroup::Arbitrary(_), - .. - } => return NodeId::FALSE, }; self.create_node(var, children) @@ -1090,18 +1052,12 @@ pub(crate) enum Variable { /// We keep extras at the leaves of the tree, so when simplifying extras we can /// trivially remove the leaves without having to reconstruct the entire tree. Extra(CanonicalMarkerValueExtra), - /// A variable representing the existence or absence of a given extra, in the context of a - /// PEP 751 lockfile marker. + /// A variable representing whether a ` in ` or ` not in ` + /// expression, where the key is a list. /// - /// We keep extras at the leaves of the tree, so when simplifying extras we can + /// We keep extras and groups at the leaves of the tree, so when simplifying extras we can /// trivially remove the leaves without having to reconstruct the entire tree. - Extras(CanonicalMarkerValueExtra), - /// A variable representing the existence or absence of a given dependency group, in the context of a - /// PEP 751 lockfile marker. - /// - /// We keep groups at the leaves of the tree, so when simplifying groups we can - /// trivially remove the leaves without having to reconstruct the entire tree. - DependencyGroups(CanonicalMarkerValueDependencyGroup), + List(CanonicalMarkerListPair), } impl Variable { @@ -1279,7 +1235,10 @@ impl Edges { /// Returns an [`Edges`] where values in the given range are `true`. /// /// Only for use when the `key` is a `PythonVersion`. Normalizes to `PythonFullVersion`. - fn from_python_versions(versions: Vec, negated: bool) -> Result { + fn from_python_versions( + versions: Vec, + operator: ContainerOperator, + ) -> Result { let mut range: Ranges = versions .into_iter() .map(|version| { @@ -1290,7 +1249,7 @@ impl Edges { .flatten_ok() .collect::, NodeId>>()?; - if negated { + if operator == ContainerOperator::NotIn { range = range.complement(); } @@ -1300,7 +1259,7 @@ impl Edges { } /// Returns an [`Edges`] where values in the given range are `true`. - fn from_versions(versions: &[Version], negated: bool) -> Edges { + fn from_versions(versions: &[Version], operator: ContainerOperator) -> Edges { let mut range: Ranges = versions .iter() .map(|version| { @@ -1311,7 +1270,7 @@ impl Edges { }) .collect(); - if negated { + if operator == ContainerOperator::NotIn { range = range.complement(); } diff --git a/crates/uv-pep508/src/marker/lowering.rs b/crates/uv-pep508/src/marker/lowering.rs index dadfeac53..e52669840 100644 --- a/crates/uv-pep508/src/marker/lowering.rs +++ b/crates/uv-pep508/src/marker/lowering.rs @@ -2,7 +2,7 @@ use std::fmt::{Display, Formatter}; use uv_normalize::{ExtraName, GroupName}; -use crate::marker::tree::MarkerValueDependencyGroup; +use crate::marker::tree::MarkerValueList; use crate::{MarkerValueExtra, MarkerValueString, MarkerValueVersion}; /// Those environment markers with a PEP 440 version as value such as `python_version` @@ -161,34 +161,35 @@ impl Display for CanonicalMarkerValueExtra { } } -/// The [`GroupName`] value used in `dependency_group` markers. +/// A key-value pair for ` in ` or ` not in `, where the key is a list. +/// +/// Used for PEP 751 markers. #[derive(Clone, Debug, Eq, Hash, PartialEq, PartialOrd, Ord)] -pub enum CanonicalMarkerValueDependencyGroup { +pub enum CanonicalMarkerListPair { + /// A valid [`ExtraName`]. + Extras(ExtraName), /// A valid [`GroupName`]. - Group(GroupName), + DependencyGroup(GroupName), + /// For leniency, preserve invalid values. + Arbitrary { key: MarkerValueList, value: String }, } -impl CanonicalMarkerValueDependencyGroup { - /// Returns the [`GroupName`] value. - pub fn group(&self) -> &GroupName { +impl CanonicalMarkerListPair { + /// The key (RHS) of the marker expression. + pub(crate) fn key(&self) -> MarkerValueList { match self { - Self::Group(group) => group, + Self::Extras(_) => MarkerValueList::Extras, + Self::DependencyGroup(_) => MarkerValueList::DependencyGroups, + Self::Arbitrary { key, .. } => *key, } } -} -impl From for MarkerValueDependencyGroup { - fn from(value: CanonicalMarkerValueDependencyGroup) -> Self { - match value { - CanonicalMarkerValueDependencyGroup::Group(group) => Self::Group(group), - } - } -} - -impl Display for CanonicalMarkerValueDependencyGroup { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + /// The value (LHS) of the marker expression. + pub(crate) fn value(&self) -> String { match self { - Self::Group(group) => group.fmt(f), + Self::Extras(extra) => extra.to_string(), + Self::DependencyGroup(group) => group.to_string(), + Self::Arbitrary { value, .. } => value.clone(), } } } diff --git a/crates/uv-pep508/src/marker/mod.rs b/crates/uv-pep508/src/marker/mod.rs index f5ac7f1da..55d21c69f 100644 --- a/crates/uv-pep508/src/marker/mod.rs +++ b/crates/uv-pep508/src/marker/mod.rs @@ -23,8 +23,8 @@ pub use lowering::{ pub use tree::{ ContainsMarkerTree, ExtraMarkerTree, ExtraOperator, InMarkerTree, MarkerExpression, MarkerOperator, MarkerTree, MarkerTreeContents, MarkerTreeDebugGraph, MarkerTreeKind, - MarkerValue, MarkerValueExtra, MarkerValueString, MarkerValueVersion, MarkerWarningKind, - StringMarkerTree, StringVersion, VersionMarkerTree, + MarkerValue, MarkerValueExtra, MarkerValueList, MarkerValueString, MarkerValueVersion, + MarkerWarningKind, StringMarkerTree, StringVersion, VersionMarkerTree, }; /// `serde` helpers for [`MarkerTree`]. diff --git a/crates/uv-pep508/src/marker/parse.rs b/crates/uv-pep508/src/marker/parse.rs index de8bfac72..8e4a39078 100644 --- a/crates/uv-pep508/src/marker/parse.rs +++ b/crates/uv-pep508/src/marker/parse.rs @@ -5,7 +5,8 @@ use uv_pep440::{Version, VersionPattern, VersionSpecifier}; use crate::cursor::Cursor; use crate::marker::MarkerValueExtra; -use crate::marker::tree::{ContainerOperator, MarkerValueContains, MarkerValueDependencyGroup}; +use crate::marker::lowering::CanonicalMarkerListPair; +use crate::marker::tree::{ContainerOperator, MarkerValueList}; use crate::{ ExtraOperator, MarkerExpression, MarkerOperator, MarkerTree, MarkerValue, MarkerValueString, MarkerValueVersion, MarkerWarningKind, Pep508Error, Pep508ErrorSource, Pep508Url, Reporter, @@ -169,6 +170,7 @@ pub(crate) fn parse_marker_key_op_value( reporter: &mut impl Reporter, ) -> Result, Pep508Error> { cursor.eat_whitespace(); + let start = cursor.pos(); let l_value = parse_marker_value(cursor, reporter)?; cursor.eat_whitespace(); // "not in" and "in" must be preceded by whitespace. We must already have matched a whitespace @@ -177,6 +179,7 @@ pub(crate) fn parse_marker_key_op_value( let operator = parse_marker_operator(cursor)?; cursor.eat_whitespace(); let r_value = parse_marker_value(cursor, reporter)?; + let len = cursor.pos() - start; // Convert a ` ` expression into its // typed equivalent. @@ -211,7 +214,7 @@ pub(crate) fn parse_marker_key_op_value( MarkerValue::Extra | MarkerValue::MarkerEnvVersion(_) | MarkerValue::MarkerEnvString(_) - | MarkerValue::MarkerEnvContains(_) => { + | MarkerValue::MarkerEnvList(_) => { reporter.report( MarkerWarningKind::MarkerMarkerComparison, "Comparing two markers with each other doesn't make any sense, @@ -239,12 +242,23 @@ pub(crate) fn parse_marker_key_op_value( value, }) } + // `extras in "test"` or `dependency_groups not in "dev"` are invalid. + MarkerValue::MarkerEnvList(key) => { + return Err(Pep508Error { + message: Pep508ErrorSource::String(format!( + "The marker {key} must be on the right hand side of the expression" + )), + start, + len, + input: cursor.to_string(), + }); + } // `extra == '...'` MarkerValue::Extra => { let value = match r_value { MarkerValue::MarkerEnvVersion(_) | MarkerValue::MarkerEnvString(_) - | MarkerValue::MarkerEnvContains(_) + | MarkerValue::MarkerEnvList(_) | MarkerValue::Extra => { reporter.report( MarkerWarningKind::ExtraInvalidComparison, @@ -274,16 +288,56 @@ pub(crate) fn parse_marker_key_op_value( operator: operator.invert(), value: l_string, }), + // `"test" in extras` or `"dev" in dependency_groups` + MarkerValue::MarkerEnvList(key) => { + let operator = + ContainerOperator::from_marker_operator(operator).ok_or_else(|| { + Pep508Error { + message: Pep508ErrorSource::String(format!( + "The operator {operator} is not supported with the marker {key}, only the `in` and `not in` operators are supported" + )), + start, + len, + input: cursor.to_string(), + } + })?; + let pair = match key { + // `'...' in extras` + MarkerValueList::Extras => match ExtraName::from_str(&l_string) { + Ok(name) => CanonicalMarkerListPair::Extras(name), + Err(err) => { + reporter.report( + MarkerWarningKind::ExtrasInvalidComparison, + format!("Expected extra name (found `{l_string}`): {err}"), + ); + CanonicalMarkerListPair::Arbitrary { + key, + value: l_string.to_string(), + } + } + }, + // `'...' in dependency_groups` + MarkerValueList::DependencyGroups => { + match GroupName::from_str(&l_string) { + Ok(name) => CanonicalMarkerListPair::DependencyGroup(name), + Err(err) => { + reporter.report( + MarkerWarningKind::ExtrasInvalidComparison, + format!("Expected dependency group name (found `{l_string}`): {err}"), + ); + CanonicalMarkerListPair::Arbitrary { + key, + value: l_string.to_string(), + } + } + } + } + }; + + Some(MarkerExpression::List { pair, operator }) + } // `'...' == extra` MarkerValue::Extra => parse_extra_expr(operator, &l_string, reporter), - // `'...' in extras` - MarkerValue::MarkerEnvContains(MarkerValueContains::Extras) => { - parse_extras_expr(operator, &l_string, reporter) - } - // `'...' in dependency_groups` - MarkerValue::MarkerEnvContains(MarkerValueContains::DependencyGroups) => { - parse_dependency_groups_expr(operator, &l_string, reporter) - } // `'...' == '...'`, doesn't make much sense MarkerValue::QuotedString(_) => { // Not even pypa/packaging 22.0 supports this @@ -300,16 +354,6 @@ pub(crate) fn parse_marker_key_op_value( } } } - MarkerValue::MarkerEnvContains(key) => { - reporter.report( - MarkerWarningKind::Pep440Error, - format!( - "The `{key}` marker must be used as '...' in {key}' or '... not in {key}', - found `{key} {operator} {r_value}`, will be ignored" - ), - ); - return Ok(None); - } }; Ok(expr) @@ -340,10 +384,7 @@ fn parse_version_in_expr( value: &str, reporter: &mut impl Reporter, ) -> Option { - if !matches!(operator, MarkerOperator::In | MarkerOperator::NotIn) { - return None; - } - let negated = matches!(operator, MarkerOperator::NotIn); + let operator = ContainerOperator::from_marker_operator(operator)?; let mut cursor = Cursor::new(value); let mut versions = Vec::new(); @@ -379,7 +420,7 @@ fn parse_version_in_expr( Some(MarkerExpression::VersionIn { key, versions, - negated, + operator, }) } @@ -519,68 +560,6 @@ fn parse_extra_expr( None } -/// Creates an instance of [`MarkerExpression::Extras`] with the given values, falling back to -/// [`MarkerExpression::Arbitrary`] on failure. -fn parse_extras_expr( - operator: MarkerOperator, - value: &str, - reporter: &mut impl Reporter, -) -> Option { - let name = match ExtraName::from_str(value) { - Ok(name) => MarkerValueExtra::Extra(name), - Err(err) => { - reporter.report( - MarkerWarningKind::ExtrasInvalidComparison, - format!("Expected extra name (found `{value}`): {err}"), - ); - MarkerValueExtra::Arbitrary(value.to_string()) - } - }; - - if let Some(operator) = ContainerOperator::from_marker_operator(operator) { - return Some(MarkerExpression::Extras { operator, name }); - } - - reporter.report( - MarkerWarningKind::ExtrasInvalidComparison, - "Comparing `extras` with any operator other than `in` or `not in` is wrong and will be ignored" - .to_string(), - ); - - None -} - -/// Creates an instance of [`MarkerExpression::DependencyGroups`] with the given values, falling -/// back to [`MarkerExpression::Arbitrary`] on failure. -fn parse_dependency_groups_expr( - operator: MarkerOperator, - value: &str, - reporter: &mut impl Reporter, -) -> Option { - let name = match GroupName::from_str(value) { - Ok(name) => MarkerValueDependencyGroup::Group(name), - Err(err) => { - reporter.report( - MarkerWarningKind::ExtrasInvalidComparison, - format!("Expected extra name (found `{value}`): {err}"), - ); - MarkerValueDependencyGroup::Arbitrary(value.to_string()) - } - }; - - if let Some(operator) = ContainerOperator::from_marker_operator(operator) { - return Some(MarkerExpression::DependencyGroups { operator, name }); - } - - reporter.report( - MarkerWarningKind::ExtrasInvalidComparison, - "Comparing `extras` with any operator other than `in` or `not in` is wrong and will be ignored" - .to_string(), - ); - - None -} - /// ```text /// marker_expr = marker_var:l marker_op:o marker_var:r -> (o, l, r) /// | wsp* '(' marker:m wsp* ')' -> m diff --git a/crates/uv-pep508/src/marker/simplify.rs b/crates/uv-pep508/src/marker/simplify.rs index 6897615c4..b1565835b 100644 --- a/crates/uv-pep508/src/marker/simplify.rs +++ b/crates/uv-pep508/src/marker/simplify.rs @@ -162,6 +162,22 @@ fn collect_dnf( path.pop(); } } + MarkerTreeKind::List(marker) => { + for (is_high, tree) in marker.children() { + let expr = MarkerExpression::List { + pair: marker.pair().clone(), + operator: if is_high { + ContainerOperator::In + } else { + ContainerOperator::NotIn + }, + }; + + path.push(expr); + collect_dnf(tree, dnf, path); + path.pop(); + } + } MarkerTreeKind::Extra(marker) => { for (value, tree) in marker.children() { let operator = if value { @@ -175,42 +191,6 @@ fn collect_dnf( operator, }; - path.push(expr); - collect_dnf(tree, dnf, path); - path.pop(); - } - } - MarkerTreeKind::Extras(marker) => { - for (value, tree) in marker.children() { - let operator = if value { - ContainerOperator::In - } else { - ContainerOperator::NotIn - }; - - let expr = MarkerExpression::Extras { - name: marker.name().clone().into(), - operator, - }; - - path.push(expr); - collect_dnf(tree, dnf, path); - path.pop(); - } - } - MarkerTreeKind::DependencyGroups(marker) => { - for (value, tree) in marker.children() { - let operator = if value { - ContainerOperator::In - } else { - ContainerOperator::NotIn - }; - - let expr = MarkerExpression::DependencyGroups { - name: marker.name().clone().into(), - operator, - }; - path.push(expr); collect_dnf(tree, dnf, path); path.pop(); @@ -433,18 +413,18 @@ fn is_negation(left: &MarkerExpression, right: &MarkerExpression) -> bool { MarkerExpression::VersionIn { key, versions, - negated, + operator, } => { let MarkerExpression::VersionIn { key: key2, versions: versions2, - negated: negated2, + operator: operator2, } = right else { return false; }; - key == key2 && versions == versions2 && negated != negated2 + key == key2 && versions == versions2 && operator != operator2 } MarkerExpression::String { key, @@ -477,27 +457,16 @@ fn is_negation(left: &MarkerExpression, right: &MarkerExpression) -> bool { name == name2 && operator.negate() == *operator2 } - MarkerExpression::Extras { name, operator } => { - let MarkerExpression::Extras { - name: name2, + MarkerExpression::List { pair, operator } => { + let MarkerExpression::List { + pair: pair2, operator: operator2, } = right else { return false; }; - name == name2 && *operator == operator2.negate() - } - MarkerExpression::DependencyGroups { name, operator } => { - let MarkerExpression::DependencyGroups { - name: name2, - operator: operator2, - } = right - else { - return false; - }; - - name == name2 && *operator == operator2.negate() + pair == pair2 && operator != operator2 } } } diff --git a/crates/uv-pep508/src/marker/tree.rs b/crates/uv-pep508/src/marker/tree.rs index 756c90ade..f874fd447 100644 --- a/crates/uv-pep508/src/marker/tree.rs +++ b/crates/uv-pep508/src/marker/tree.rs @@ -16,12 +16,12 @@ use super::algebra::{Edges, INTERNER, NodeId, Variable}; use super::simplify; use crate::cursor::Cursor; use crate::marker::lowering::{ - CanonicalMarkerValueDependencyGroup, CanonicalMarkerValueExtra, CanonicalMarkerValueString, - CanonicalMarkerValueVersion, + CanonicalMarkerListPair, CanonicalMarkerValueString, CanonicalMarkerValueVersion, }; use crate::marker::parse; use crate::{ - MarkerEnvironment, Pep508Error, Pep508ErrorSource, Pep508Url, Reporter, TracingReporter, + CanonicalMarkerValueExtra, MarkerEnvironment, Pep508Error, Pep508ErrorSource, Pep508Url, + Reporter, TracingReporter, }; /// Ways in which marker evaluation can fail @@ -126,16 +126,18 @@ impl Display for MarkerValueString { } } -/// Those markers with exclusively `in` and `not in` operators (PEP 751) -#[derive(Clone, Debug, Eq, Hash, PartialEq, PartialOrd, Ord)] -pub enum MarkerValueContains { +/// Those markers with exclusively `in` and `not in` operators. +/// +/// Contains PEP 751 lockfile markers. +#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq, PartialOrd, Ord)] +pub enum MarkerValueList { /// `extras`. This one is special because it's a list, and user-provided Extras, /// `dependency_groups`. This one is special because it's a list, and user-provided DependencyGroups, } -impl Display for MarkerValueContains { +impl Display for MarkerValueList { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { match self { Self::Extras => f.write_str("extras"), @@ -153,10 +155,10 @@ pub enum MarkerValue { MarkerEnvVersion(MarkerValueVersion), /// Those environment markers with an arbitrary string as value such as `sys_platform` MarkerEnvString(MarkerValueString), + /// Those markers with exclusively `in` and `not in` operators + MarkerEnvList(MarkerValueList), /// `extra`. This one is special because it's a list, and user-provided Extra, - /// Those markers with exclusively `in` and `not in` operators (PEP 751) - MarkerEnvContains(MarkerValueContains), /// Not a constant, but a user given quoted string with a value inside such as '3.8' or "windows" QuotedString(ArcStr), } @@ -196,9 +198,9 @@ impl FromStr for MarkerValue { "python_version" => Self::MarkerEnvVersion(MarkerValueVersion::PythonVersion), "sys_platform" => Self::MarkerEnvString(MarkerValueString::SysPlatform), "sys.platform" => Self::MarkerEnvString(MarkerValueString::SysPlatformDeprecated), + "extras" => Self::MarkerEnvList(MarkerValueList::Extras), + "dependency_groups" => Self::MarkerEnvList(MarkerValueList::DependencyGroups), "extra" => Self::Extra, - "extras" => Self::MarkerEnvContains(MarkerValueContains::Extras), - "dependency_groups" => Self::MarkerEnvContains(MarkerValueContains::DependencyGroups), _ => return Err(format!("Invalid key: {s}")), }; Ok(value) @@ -210,8 +212,8 @@ impl Display for MarkerValue { match self { Self::MarkerEnvVersion(marker_value_version) => marker_value_version.fmt(f), Self::MarkerEnvString(marker_value_string) => marker_value_string.fmt(f), + Self::MarkerEnvList(marker_value_contains) => marker_value_contains.fmt(f), Self::Extra => f.write_str("extra"), - Self::MarkerEnvContains(marker_value_contains) => marker_value_contains.fmt(f), Self::QuotedString(value) => write!(f, "'{value}'"), } } @@ -499,24 +501,6 @@ impl Display for MarkerValueExtra { } } -/// The [`GroupName`] value used in `dependency_group` markers. -#[derive(Clone, Debug, Eq, Hash, PartialEq, PartialOrd, Ord)] -pub enum MarkerValueDependencyGroup { - /// A valid [`GroupName`]. - Group(GroupName), - /// An invalid name, preserved as an arbitrary string. - Arbitrary(String), -} - -impl Display for MarkerValueDependencyGroup { - fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - match self { - Self::Group(group) => group.fmt(f), - Self::Arbitrary(string) => string.fmt(f), - } - } -} - /// Represents one clause such as `python_version > "3.8"`. #[derive(Clone, Debug, Eq, Hash, PartialEq, PartialOrd, Ord)] #[allow(missing_docs)] @@ -540,7 +524,7 @@ pub enum MarkerExpression { VersionIn { key: MarkerValueVersion, versions: Vec, - negated: bool, + operator: ContainerOperator, }, /// An string marker comparison, e.g. `sys_platform == '...'`. /// @@ -550,21 +534,16 @@ pub enum MarkerExpression { operator: MarkerOperator, value: ArcStr, }, + /// `'...' in `, a PEP 751 expression. + List { + pair: CanonicalMarkerListPair, + operator: ContainerOperator, + }, /// `extra '...'` or `'...' extra`. Extra { name: MarkerValueExtra, operator: ExtraOperator, }, - /// `'...' in extras` - Extras { - name: MarkerValueExtra, - operator: ContainerOperator, - }, - /// `'...' in dependency_groups` - DependencyGroups { - name: MarkerValueDependencyGroup, - operator: ContainerOperator, - }, } /// The kind of a [`MarkerExpression`]. @@ -572,16 +551,14 @@ pub enum MarkerExpression { pub(crate) enum MarkerExpressionKind { /// A version expression, e.g. ` `. Version(MarkerValueVersion), - /// A version "in" expression, e.g. ` in `. + /// A version `in` expression, e.g. ` in `. VersionIn(MarkerValueVersion), /// A string marker comparison, e.g. `sys_platform == '...'`. String(MarkerValueString), + /// A list `in` or `not in` expression, e.g. `'...' in dependency_groups`. + List(MarkerValueList), /// An extra expression, e.g. `extra == '...'`. Extra, - /// An extras expression, e.g. `'...' in extras`. - Extras, - /// A dependency groups expression, e.g. `'...' in dependency_groups`. - DependencyGroups, } /// The operator for an extra expression, either '==' or '!='. @@ -624,7 +601,7 @@ impl Display for ExtraOperator { } /// The operator for a container expression, either 'in' or 'not in'. -#[derive(Clone, Debug, Eq, Hash, PartialEq, PartialOrd, Ord)] +#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq, PartialOrd, Ord)] pub enum ContainerOperator { /// `in` In, @@ -643,14 +620,6 @@ impl ContainerOperator { _ => None, } } - - /// Negates this operator. - pub(crate) fn negate(&self) -> ContainerOperator { - match *self { - ContainerOperator::In => ContainerOperator::NotIn, - ContainerOperator::NotIn => ContainerOperator::In, - } - } } impl Display for ContainerOperator { @@ -700,9 +669,8 @@ impl MarkerExpression { MarkerExpression::Version { key, .. } => MarkerExpressionKind::Version(*key), MarkerExpression::VersionIn { key, .. } => MarkerExpressionKind::VersionIn(*key), MarkerExpression::String { key, .. } => MarkerExpressionKind::String(*key), + MarkerExpression::List { pair, .. } => MarkerExpressionKind::List(pair.key()), MarkerExpression::Extra { .. } => MarkerExpressionKind::Extra, - MarkerExpression::Extras { .. } => MarkerExpressionKind::Extras, - MarkerExpression::DependencyGroups { .. } => MarkerExpressionKind::DependencyGroups, } } } @@ -721,11 +689,10 @@ impl Display for MarkerExpression { MarkerExpression::VersionIn { key, versions, - negated, + operator, } => { - let op = if *negated { "not in" } else { "in" }; let versions = versions.iter().map(ToString::to_string).join(" "); - write!(f, "{key} {op} '{versions}'") + write!(f, "{key} {operator} '{versions}'") } MarkerExpression::String { key, @@ -741,15 +708,12 @@ impl Display for MarkerExpression { write!(f, "{key} {operator} '{value}'") } + MarkerExpression::List { pair, operator } => { + write!(f, "'{}' {} {}", pair.value(), operator, pair.key()) + } MarkerExpression::Extra { operator, name } => { write!(f, "extra {operator} '{name}'") } - MarkerExpression::Extras { operator, name } => { - write!(f, "'{name}' {operator} extras") - } - MarkerExpression::DependencyGroups { operator, name } => { - write!(f, "'{name}' {operator} dependency_groups") - } } } } @@ -777,24 +741,24 @@ impl<'a> ExtrasEnvironment<'a> { /// Returns the `extra` names in this environment. fn extra(&self) -> &[ExtraName] { match self { - ExtrasEnvironment::Extras(extra) => extra, - ExtrasEnvironment::Pep751(..) => &[], + Self::Extras(extra) => extra, + Self::Pep751(..) => &[], } } /// Returns the `extras` names in this environment, as in a PEP 751 lockfile. fn extras(&self) -> &[ExtraName] { match self { - ExtrasEnvironment::Extras(..) => &[], - ExtrasEnvironment::Pep751(extras, ..) => extras, + Self::Extras(..) => &[], + Self::Pep751(extras, ..) => extras, } } /// Returns the `dependency_group` group names in this environment, as in a PEP 751 lockfile. fn dependency_groups(&self) -> &[GroupName] { match self { - ExtrasEnvironment::Extras(..) => &[], - ExtrasEnvironment::Pep751(.., groups) => groups, + Self::Extras(..) => &[], + Self::Pep751(.., groups) => groups, } } } @@ -1006,6 +970,16 @@ impl MarkerTree { low: low.negate(self.0), }) } + Variable::List(key) => { + let Edges::Boolean { low, high } = node.children else { + unreachable!() + }; + MarkerTreeKind::List(ListMarkerTree { + pair: key, + high: high.negate(self.0), + low: low.negate(self.0), + }) + } Variable::Extra(name) => { let Edges::Boolean { low, high } = node.children else { unreachable!() @@ -1016,26 +990,6 @@ impl MarkerTree { low: low.negate(self.0), }) } - Variable::Extras(name) => { - let Edges::Boolean { low, high } = node.children else { - unreachable!() - }; - MarkerTreeKind::Extras(ExtrasMarkerTree { - name, - high: high.negate(self.0), - low: low.negate(self.0), - }) - } - Variable::DependencyGroups(name) => { - let Edges::Boolean { low, high } = node.children else { - unreachable!() - }; - MarkerTreeKind::DependencyGroups(DependencyGroupsMarkerTree { - name, - high: high.negate(self.0), - low: low.negate(self.0), - }) - } } } @@ -1160,14 +1114,18 @@ impl MarkerTree { .edge(extras.extra().contains(marker.name().extra())) .evaluate_reporter_impl(env, extras, reporter); } - MarkerTreeKind::Extras(marker) => { + MarkerTreeKind::List(marker) => { + let edge = match marker.pair() { + CanonicalMarkerListPair::Extras(extra) => extras.extras().contains(extra), + CanonicalMarkerListPair::DependencyGroup(dependency_group) => { + extras.dependency_groups().contains(dependency_group) + } + // Invalid marker expression + CanonicalMarkerListPair::Arbitrary { .. } => return false, + }; + return marker - .edge(extras.extras().contains(marker.name().extra())) - .evaluate_reporter_impl(env, extras, reporter); - } - MarkerTreeKind::DependencyGroups(marker) => { - return marker - .edge(extras.dependency_groups().contains(marker.name().group())) + .edge(edge) .evaluate_reporter_impl(env, extras, reporter); } } @@ -1194,15 +1152,12 @@ impl MarkerTree { MarkerTreeKind::Contains(marker) => marker .children() .any(|(_, tree)| tree.evaluate_extras(extras)), + MarkerTreeKind::List(marker) => marker + .children() + .any(|(_, tree)| tree.evaluate_extras(extras)), MarkerTreeKind::Extra(marker) => marker .edge(extras.contains(marker.name().extra())) .evaluate_extras(extras), - MarkerTreeKind::Extras(marker) => marker - .children() - .any(|(_, tree)| tree.evaluate_extras(extras)), - MarkerTreeKind::DependencyGroups(marker) => marker - .children() - .any(|(_, tree)| tree.evaluate_extras(extras)), } } @@ -1430,6 +1385,11 @@ impl MarkerTree { imp(tree, f); } } + MarkerTreeKind::List(kind) => { + for (_, tree) in kind.children() { + imp(tree, f); + } + } MarkerTreeKind::Extra(kind) => { if kind.low.is_false() { f(MarkerOperator::Equal, kind.name().extra()); @@ -1440,16 +1400,6 @@ impl MarkerTree { imp(tree, f); } } - MarkerTreeKind::Extras(kind) => { - for (_, tree) in kind.children() { - imp(tree, f); - } - } - MarkerTreeKind::DependencyGroups(kind) => { - for (_, tree) in kind.children() { - imp(tree, f); - } - } } } imp(self, &mut f); @@ -1557,6 +1507,21 @@ impl MarkerTree { write!(f, "{} not in {} -> ", kind.value(), kind.key())?; kind.edge(false).fmt_graph(f, level + 1)?; } + MarkerTreeKind::List(kind) => { + writeln!(f)?; + for _ in 0..level { + write!(f, " ")?; + } + write!(f, "{} in {} -> ", kind.value(), kind.key())?; + kind.edge(true).fmt_graph(f, level + 1)?; + + writeln!(f)?; + for _ in 0..level { + write!(f, " ")?; + } + write!(f, "{} not in {} -> ", kind.value(), kind.key())?; + kind.edge(false).fmt_graph(f, level + 1)?; + } MarkerTreeKind::Extra(kind) => { writeln!(f)?; for _ in 0..level { @@ -1572,36 +1537,6 @@ impl MarkerTree { write!(f, "extra != {} -> ", kind.name())?; kind.edge(false).fmt_graph(f, level + 1)?; } - MarkerTreeKind::Extras(kind) => { - writeln!(f)?; - for _ in 0..level { - write!(f, " ")?; - } - write!(f, "{} in extras -> ", kind.name())?; - kind.edge(true).fmt_graph(f, level + 1)?; - - writeln!(f)?; - for _ in 0..level { - write!(f, " ")?; - } - write!(f, "{} not in extras -> ", kind.name())?; - kind.edge(false).fmt_graph(f, level + 1)?; - } - MarkerTreeKind::DependencyGroups(kind) => { - writeln!(f)?; - for _ in 0..level { - write!(f, " ")?; - } - write!(f, "{} in dependency_groups -> ", kind.name())?; - kind.edge(true).fmt_graph(f, level + 1)?; - - writeln!(f)?; - for _ in 0..level { - write!(f, " ")?; - } - write!(f, "{} not in dependency_groups -> ", kind.name())?; - kind.edge(false).fmt_graph(f, level + 1)?; - } } Ok(()) @@ -1671,12 +1606,10 @@ pub enum MarkerTreeKind<'a> { In(InMarkerTree<'a>), /// A string expression with the `contains` operator. Contains(ContainsMarkerTree<'a>), - /// A string expression (e.g., `extra == 'dev'`). + /// A `in` or `not in` expression. + List(ListMarkerTree<'a>), + /// An extra expression (e.g., `extra == 'dev'`). Extra(ExtraMarkerTree<'a>), - /// A string expression (e.g., `'dev' in extras`). - Extras(ExtrasMarkerTree<'a>), - /// A string expression (e.g., `'dev' in dependency_groups`). - DependencyGroups(DependencyGroupsMarkerTree<'a>), } /// A version marker node, such as `python_version < '3.7'`. @@ -1851,6 +1784,59 @@ impl Ord for ContainsMarkerTree<'_> { } } +#[derive(PartialEq, Eq, Clone, Debug)] +pub struct ListMarkerTree<'a> { + // No separate canonical type, the type is already canonical. + pair: &'a CanonicalMarkerListPair, + high: NodeId, + low: NodeId, +} + +impl ListMarkerTree<'_> { + /// The key-value pair for this expression + pub fn pair(&self) -> &CanonicalMarkerListPair { + self.pair + } + + /// The key (RHS) for this expression. + pub fn key(&self) -> MarkerValueList { + self.pair.key() + } + + /// The value (LHS) for this expression. + pub fn value(&self) -> String { + self.pair.value() + } + + /// The edges of this node, corresponding to the boolean evaluation of the expression. + pub fn children(&self) -> impl Iterator { + [(true, MarkerTree(self.high)), (false, MarkerTree(self.low))].into_iter() + } + + /// Returns the subtree associated with the given edge value. + pub fn edge(&self, value: bool) -> MarkerTree { + if value { + MarkerTree(self.high) + } else { + MarkerTree(self.low) + } + } +} + +impl PartialOrd for ListMarkerTree<'_> { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl Ord for ListMarkerTree<'_> { + fn cmp(&self, other: &Self) -> Ordering { + self.pair() + .cmp(other.pair()) + .then_with(|| self.children().cmp(other.children())) + } +} + /// A node representing the existence or absence of a given extra, such as `extra == 'bar'`. #[derive(PartialEq, Eq, Clone, Debug)] pub struct ExtraMarkerTree<'a> { @@ -1894,93 +1880,6 @@ impl Ord for ExtraMarkerTree<'_> { } } -/// A node representing the existence or absence of a given extra, such as `'bar' in extras`. -#[derive(PartialEq, Eq, Clone, Debug)] -pub struct ExtrasMarkerTree<'a> { - name: &'a CanonicalMarkerValueExtra, - high: NodeId, - low: NodeId, -} - -impl ExtrasMarkerTree<'_> { - /// Returns the name of the extra in this expression. - pub fn name(&self) -> &CanonicalMarkerValueExtra { - self.name - } - - /// The edges of this node, corresponding to the boolean evaluation of the expression. - pub fn children(&self) -> impl Iterator { - [(true, MarkerTree(self.high)), (false, MarkerTree(self.low))].into_iter() - } - - /// Returns the subtree associated with the given edge value. - pub fn edge(&self, value: bool) -> MarkerTree { - if value { - MarkerTree(self.high) - } else { - MarkerTree(self.low) - } - } -} - -impl PartialOrd for ExtrasMarkerTree<'_> { - fn partial_cmp(&self, other: &Self) -> Option { - Some(self.cmp(other)) - } -} - -impl Ord for ExtrasMarkerTree<'_> { - fn cmp(&self, other: &Self) -> Ordering { - self.name() - .cmp(other.name()) - .then_with(|| self.children().cmp(other.children())) - } -} - -/// A node representing the existence or absence of a given dependency group, such as -/// `'bar' in dependency_groups`. -#[derive(PartialEq, Eq, Clone, Debug)] -pub struct DependencyGroupsMarkerTree<'a> { - name: &'a CanonicalMarkerValueDependencyGroup, - high: NodeId, - low: NodeId, -} - -impl DependencyGroupsMarkerTree<'_> { - /// Returns the name of the group in this expression. - pub fn name(&self) -> &CanonicalMarkerValueDependencyGroup { - self.name - } - - /// The edges of this node, corresponding to the boolean evaluation of the expression. - pub fn children(&self) -> impl Iterator { - [(true, MarkerTree(self.high)), (false, MarkerTree(self.low))].into_iter() - } - - /// Returns the subtree associated with the given edge value. - pub fn edge(&self, value: bool) -> MarkerTree { - if value { - MarkerTree(self.high) - } else { - MarkerTree(self.low) - } - } -} - -impl PartialOrd for DependencyGroupsMarkerTree<'_> { - fn partial_cmp(&self, other: &Self) -> Option { - Some(self.cmp(other)) - } -} - -impl Ord for DependencyGroupsMarkerTree<'_> { - fn cmp(&self, other: &Self) -> Ordering { - self.name() - .cmp(other.name()) - .then_with(|| self.children().cmp(other.children())) - } -} - /// A marker tree that contains at least one expression. /// /// See [`MarkerTree::contents`] for details. @@ -2090,7 +1989,7 @@ mod test { implementation_name: "", implementation_version: "3.7", os_name: "linux", - platform_machine: "", + platform_machine: "x86_64", platform_python_implementation: "", platform_release: "", platform_system: "", diff --git a/crates/uv-pep508/src/verbatim_url.rs b/crates/uv-pep508/src/verbatim_url.rs index 2911de938..9f0e9a5ee 100644 --- a/crates/uv-pep508/src/verbatim_url.rs +++ b/crates/uv-pep508/src/verbatim_url.rs @@ -62,6 +62,7 @@ impl VerbatimUrl { /// /// If no root directory is provided, relative paths are resolved against the current working /// directory. + #[cfg(feature = "non-pep508-extensions")] // PEP 508 arguably only allows absolute file URLs. pub fn from_url_or_path( input: &str, root_dir: Option<&Path>, diff --git a/crates/uv-resolver/src/marker.rs b/crates/uv-resolver/src/marker.rs index 5a2203f9b..02ea1d6df 100644 --- a/crates/uv-resolver/src/marker.rs +++ b/crates/uv-resolver/src/marker.rs @@ -54,12 +54,7 @@ pub(crate) fn requires_python(tree: MarkerTree) -> Option { collect_python_markers(tree, markers, range); } } - MarkerTreeKind::Extras(marker) => { - for (_, tree) in marker.children() { - collect_python_markers(tree, markers, range); - } - } - MarkerTreeKind::DependencyGroups(marker) => { + MarkerTreeKind::List(marker) => { for (_, tree) in marker.children() { collect_python_markers(tree, markers, range); } diff --git a/crates/uv-resolver/src/resolution/output.rs b/crates/uv-resolver/src/resolution/output.rs index 2afbf2c6b..8df52f4f0 100644 --- a/crates/uv-resolver/src/resolution/output.rs +++ b/crates/uv-resolver/src/resolution/output.rs @@ -698,12 +698,7 @@ impl ResolverOutput { add_marker_params_from_tree(tree, set); } } - MarkerTreeKind::Extras(marker) => { - for (_, tree) in marker.children() { - add_marker_params_from_tree(tree, set); - } - } - MarkerTreeKind::DependencyGroups(marker) => { + MarkerTreeKind::List(marker) => { for (_, tree) in marker.children() { add_marker_params_from_tree(tree, set); }