Remove python_version from lowered marker representation

charliermarsh · charliermarsh · commit 2a90f0657dd2 · 2024-11-26T09:27:55.000-05:00
diff --git a/crates/uv-pep508/src/lib.rs b/crates/uv-pep508/src/lib.rs
@@ -16,7 +16,6 @@
 
 #![warn(missing_docs)]
 
-use std::collections::HashSet;
 use std::error::Error;
 use std::fmt::{Debug, Display, Formatter};
 use std::path::Path;
@@ -41,7 +40,7 @@ pub use uv_normalize::{ExtraName, InvalidNameError, PackageName};
 /// Version and version specifiers used in requirements (reexport).
 // https://github.com/konstin/pep508_rs/issues/19
 pub use uv_pep440;
-use uv_pep440::{Version, VersionSpecifier, VersionSpecifiers};
+use uv_pep440::{VersionSpecifier, VersionSpecifiers};
 pub use verbatim_url::{
     expand_env_vars, split_scheme, strip_host, Scheme, VerbatimUrl, VerbatimUrlError,
 };
@@ -207,21 +206,6 @@ impl<T: Pep508Url> Requirement<T> {
         self.marker.evaluate(env, extras)
     }
 
-    /// Returns whether the requirement would be satisfied, independent of environment markers, i.e.
-    /// if there is potentially an environment that could activate this requirement.
-    ///
-    /// Note that unlike [`Self::evaluate_markers`] this does not perform any checks for bogus
-    /// expressions but will simply return true. As caller you should separately perform a check
-    /// with an environment and forward all warnings.
-    pub fn evaluate_extras_and_python_version(
-        &self,
-        extras: &HashSet<ExtraName>,
-        python_versions: &[Version],
-    ) -> bool {
-        self.marker
-            .evaluate_extras_and_python_version(extras, python_versions)
-    }
-
     /// Return the requirement with an additional marker added, to require the given extra.
     ///
     /// For example, given `flask >= 2.0.2`, calling `with_extra_marker("dotenv")` would return
diff --git a/crates/uv-pep508/src/marker/algebra.rs b/crates/uv-pep508/src/marker/algebra.rs
@@ -158,44 +158,54 @@ impl InternerGuard<'_> {
     /// Returns a decision node for a single marker expression.
     pub(crate) fn expression(&mut self, expr: MarkerExpression) -> NodeId {
         let (var, children) = match expr {
-            // Normalize `python_version` markers to `python_full_version` nodes.
-            MarkerExpression::Version {
-                key: MarkerValueVersion::PythonVersion,
-                specifier,
-            } => match python_version_to_full_version(normalize_specifier(specifier)) {
-                Ok(specifier) => (
-                    Variable::Version(LoweredMarkerValueVersion::PythonFullVersion),
+            // A variable representing the output of a version key. Edges correspond
+            // to disjoint version ranges.
+            MarkerExpression::Version { key, specifier } => match key {
+                MarkerValueVersion::ImplementationVersion => (
+                    Variable::Version(LoweredMarkerValueVersion::ImplementationVersion),
                     Edges::from_specifier(specifier),
                 ),
-                Err(node) => return node,
-            },
-            MarkerExpression::VersionIn {
-                key: MarkerValueVersion::PythonVersion,
-                versions,
-                negated,
-            } => match Edges::from_python_versions(versions, negated) {
-                Ok(edges) => (
+                MarkerValueVersion::PythonFullVersion => (
                     Variable::Version(LoweredMarkerValueVersion::PythonFullVersion),
-                    edges,
+                    Edges::from_specifier(specifier),
                 ),
-                Err(node) => return node,
+                // Normalize `python_version` markers to `python_full_version` nodes.
+                MarkerValueVersion::PythonVersion => {
+                    match python_version_to_full_version(normalize_specifier(specifier)) {
+                        Ok(specifier) => (
+                            Variable::Version(LoweredMarkerValueVersion::PythonFullVersion),
+                            Edges::from_specifier(specifier),
+                        ),
+                        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.into()),
-                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.into()),
-                Edges::from_versions(&versions, negated),
-            ),
+            } => match key {
+                MarkerValueVersion::ImplementationVersion => (
+                    Variable::Version(LoweredMarkerValueVersion::ImplementationVersion),
+                    Edges::from_versions(&versions, negated),
+                ),
+                MarkerValueVersion::PythonFullVersion => (
+                    Variable::Version(LoweredMarkerValueVersion::PythonFullVersion),
+                    Edges::from_versions(&versions, negated),
+                ),
+                // Normalize `python_version` markers to `python_full_version` nodes.
+                MarkerValueVersion::PythonVersion => {
+                    match Edges::from_python_versions(versions, negated) {
+                        Ok(edges) => (
+                            Variable::Version(LoweredMarkerValueVersion::PythonFullVersion),
+                            edges,
+                        ),
+                        Err(node) => return node,
+                    }
+                }
+            },
             // 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'`
diff --git a/crates/uv-pep508/src/marker/environment.rs b/crates/uv-pep508/src/marker/environment.rs
@@ -39,7 +39,6 @@ impl MarkerEnvironment {
                 &self.implementation_version().version
             }
             LoweredMarkerValueVersion::PythonFullVersion => &self.python_full_version().version,
-            LoweredMarkerValueVersion::PythonVersion => &self.python_version().version,
         }
     }
 
diff --git a/crates/uv-pep508/src/marker/lowering.rs b/crates/uv-pep508/src/marker/lowering.rs
@@ -12,26 +12,13 @@ pub enum LoweredMarkerValueVersion {
     ImplementationVersion,
     /// `python_full_version`
     PythonFullVersion,
-    /// `python_version`
-    PythonVersion,
 }
 
 impl Display for LoweredMarkerValueVersion {
     fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
         match self {
             Self::ImplementationVersion => f.write_str("implementation_version"),
             Self::PythonFullVersion => f.write_str("python_full_version"),
-            Self::PythonVersion => f.write_str("python_version"),
-        }
-    }
-}
-
-impl From<MarkerValueVersion> for LoweredMarkerValueVersion {
-    fn from(value: MarkerValueVersion) -> Self {
-        match value {
-            MarkerValueVersion::ImplementationVersion => Self::ImplementationVersion,
-            MarkerValueVersion::PythonFullVersion => Self::PythonFullVersion,
-            MarkerValueVersion::PythonVersion => Self::PythonVersion,
         }
     }
 }
@@ -41,7 +28,6 @@ impl From<LoweredMarkerValueVersion> for MarkerValueVersion {
         match value {
             LoweredMarkerValueVersion::ImplementationVersion => Self::ImplementationVersion,
             LoweredMarkerValueVersion::PythonFullVersion => Self::PythonFullVersion,
-            LoweredMarkerValueVersion::PythonVersion => Self::PythonVersion,
         }
     }
 }
diff --git a/crates/uv-pep508/src/marker/tree.rs b/crates/uv-pep508/src/marker/tree.rs
@@ -1,5 +1,4 @@
 use std::cmp::Ordering;
-use std::collections::HashSet;
 use std::fmt::{self, Display, Formatter};
 use std::ops::{Bound, Deref};
 use std::str::FromStr;
@@ -922,49 +921,6 @@ impl MarkerTree {
         false
     }
 
-    /// Checks if the requirement should be activated with the given set of active extras and a set
-    /// of possible python versions (from `requires-python`) without evaluating the remaining
-    /// environment markers, i.e. if there is potentially an environment that could activate this
-    /// requirement.
-    ///
-    /// Note that unlike [`Self::evaluate`] this does not perform any checks for bogus expressions but
-    /// will simply return true. As caller you should separately perform a check with an environment
-    /// and forward all warnings.
-    pub fn evaluate_extras_and_python_version(
-        &self,
-        extras: &HashSet<ExtraName>,
-        python_versions: &[Version],
-    ) -> bool {
-        match self.kind() {
-            MarkerTreeKind::True => true,
-            MarkerTreeKind::False => false,
-            MarkerTreeKind::Version(marker) => marker.edges().any(|(range, tree)| {
-                if marker.key() == LoweredMarkerValueVersion::PythonVersion {
-                    if !python_versions
-                        .iter()
-                        .any(|version| range.contains(version))
-                    {
-                        return false;
-                    }
-                }
-
-                tree.evaluate_extras_and_python_version(extras, python_versions)
-            }),
-            MarkerTreeKind::String(marker) => marker
-                .children()
-                .any(|(_, tree)| tree.evaluate_extras_and_python_version(extras, python_versions)),
-            MarkerTreeKind::In(marker) => marker
-                .children()
-                .any(|(_, tree)| tree.evaluate_extras_and_python_version(extras, python_versions)),
-            MarkerTreeKind::Contains(marker) => marker
-                .children()
-                .any(|(_, tree)| tree.evaluate_extras_and_python_version(extras, python_versions)),
-            MarkerTreeKind::Extra(marker) => marker
-                .edge(extras.contains(marker.name().extra()))
-                .evaluate_extras_and_python_version(extras, python_versions),
-        }
-    }
-
     /// Checks if the requirement should be activated with the given set of active extras without evaluating
     /// the remaining environment markers, i.e. if there is potentially an environment that could activate this
     /// requirement.
diff --git a/crates/uv-resolver/src/marker.rs b/crates/uv-resolver/src/marker.rs
@@ -10,8 +10,7 @@ pub(crate) fn requires_python(tree: &MarkerTree) -> Option<RequiresPythonRange>
         match tree.kind() {
             MarkerTreeKind::True | MarkerTreeKind::False => {}
             MarkerTreeKind::Version(marker) => match marker.key() {
-                LoweredMarkerValueVersion::PythonVersion
-                | LoweredMarkerValueVersion::PythonFullVersion => {
+                LoweredMarkerValueVersion::PythonFullVersion => {
                     for (range, tree) in marker.edges() {
                         if !tree.is_false() {
                             markers.push(range.clone());

Original file line number	Diff line number	Diff line change
`@@ -39,7 +39,6 @@ impl MarkerEnvironment {`
`39`	`39`	`&self.implementation_version().version`
`40`	`40`	`}`
`41`	`41`	`LoweredMarkerValueVersion::PythonFullVersion => &self.python_full_version().version,`
`42`		`- LoweredMarkerValueVersion::PythonVersion => &self.python_version().version,`
`43`	`42`	`}`
`44`	`43`	`}`
`45`	`44`
Original file line number	Diff line number	Diff line change
`@@ -12,26 +12,13 @@ pub enum LoweredMarkerValueVersion {`
`12`	`12`	`ImplementationVersion,`
`13`	`13`	/// `python_full_version`
`14`	`14`	`PythonFullVersion,`
`15`		- /// `python_version`
`16`		`- PythonVersion,`
`17`	`15`	`}`
`18`	`16`
`19`	`17`	`impl Display for LoweredMarkerValueVersion {`
`20`	`18`	`fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {`
`21`	`19`	`match self {`
`22`	`20`	`Self::ImplementationVersion => f.write_str("implementation_version"),`
`23`	`21`	`Self::PythonFullVersion => f.write_str("python_full_version"),`
`24`		`- Self::PythonVersion => f.write_str("python_version"),`
`25`		`- }`
`26`		`- }`
`27`		`-}`
`28`		`-`
`29`		`-impl From<MarkerValueVersion> for LoweredMarkerValueVersion {`
`30`		`- fn from(value: MarkerValueVersion) -> Self {`
`31`		`- match value {`
`32`		`- MarkerValueVersion::ImplementationVersion => Self::ImplementationVersion,`
`33`		`- MarkerValueVersion::PythonFullVersion => Self::PythonFullVersion,`
`34`		`- MarkerValueVersion::PythonVersion => Self::PythonVersion,`
`35`	`22`	`}`
`36`	`23`	`}`
`37`	`24`	`}`
`@@ -41,7 +28,6 @@ impl From<LoweredMarkerValueVersion> for MarkerValueVersion {`
`41`	`28`	`match value {`
`42`	`29`	`LoweredMarkerValueVersion::ImplementationVersion => Self::ImplementationVersion,`
`43`	`30`	`LoweredMarkerValueVersion::PythonFullVersion => Self::PythonFullVersion,`
`44`		`- LoweredMarkerValueVersion::PythonVersion => Self::PythonVersion,`
`45`	`31`	`}`
`46`	`32`	`}`
`47`	`33`	`}`