Add some TODOs

astral-sh · Jul 4, 2024 · 309d708 · 309d708
1 parent 11cb005
commit 309d708
Show file tree

Hide file tree

Showing 12 changed files with 389 additions and 172 deletions.
diff --git a/crates/pep440-rs/src/version.rs b/crates/pep440-rs/src/version.rs
@@ -537,6 +537,13 @@ impl Version {
         self
     }
 
+    /// Return the version with any segments apart from the release removed.
+    #[inline]
+    #[must_use]
+    pub fn only_release(&self) -> Self {
+        Self::new(self.release().iter().copied())
+    }
+
     /// Set the min-release component and return the updated version.
     ///
     /// The "min" component is internal-only, and does not exist in PEP 440.

diff --git a/crates/uv-resolver/src/marker.rs b/crates/uv-resolver/src/marker.rs
@@ -86,10 +86,25 @@ fn string_is_disjoint(this: &MarkerExpression, other: &MarkerExpression) -> bool
 pub(crate) fn requires_python_marker(tree: &MarkerTree) -> Option<RequiresPythonBound> {
     match tree {
         MarkerTree::Expression(MarkerExpression::Version {
-            key: MarkerValueVersion::PythonFullVersion | MarkerValueVersion::PythonVersion,
+            key: MarkerValueVersion::PythonFullVersion,
             specifier,
         }) => {
-            let specifier = PubGrubSpecifier::try_from(specifier).ok()?;
+            // Simplify using PEP 440 semantics.
+            let specifier = PubGrubSpecifier::from_pep440_specifier(specifier).ok()?;
+
+            // Convert to PubGrub range and perform a union.
+            let range = PubGrubRange::from(specifier);
+            let (lower, _) = range.iter().next()?;
+
+            // Extract the lower bound.
+            Some(RequiresPythonBound::new(lower.clone()))
+        }
+        MarkerTree::Expression(MarkerExpression::Version {
+            key: MarkerValueVersion::PythonVersion,
+            specifier,
+        }) => {
+            // Simplify using release-only semantics, since `python_version` is always `major.minor`.
+            let specifier = PubGrubSpecifier::from_release_specifier(specifier).ok()?;
 
             // Convert to PubGrub range and perform a union.
             let range = PubGrubRange::from(specifier);
@@ -348,7 +363,26 @@ fn keyed_range(expr: &MarkerExpression) -> Option<(&MarkerValueVersion, PubGrubR
         _ => return None,
     };
 
-    let pubgrub_specifier = PubGrubSpecifier::try_from(&specifier).ok()?;
+    // Simplify using either PEP 440 or release-only semantics.
+    let pubgrub_specifier = match expr {
+        MarkerExpression::Version {
+            key: MarkerValueVersion::PythonVersion,
+            ..
+        } => PubGrubSpecifier::from_release_specifier(&specifier).ok()?,
+        MarkerExpression::Version {
+            key: MarkerValueVersion::PythonFullVersion,
+            ..
+        } => PubGrubSpecifier::from_pep440_specifier(&specifier).ok()?,
+        MarkerExpression::VersionInverted {
+            key: MarkerValueVersion::PythonVersion,
+            ..
+        } => PubGrubSpecifier::from_release_specifier(&specifier).ok()?,
+        MarkerExpression::VersionInverted {
+            key: MarkerValueVersion::PythonFullVersion,
+            ..
+        } => PubGrubSpecifier::from_pep440_specifier(&specifier).ok()?,
+        _ => return None,
+    };
 
     Some((key, pubgrub_specifier.into()))
 }

diff --git a/crates/uv-resolver/src/pubgrub/dependencies.rs b/crates/uv-resolver/src/pubgrub/dependencies.rs
@@ -179,13 +179,15 @@ impl PubGrubRequirement {
                 .map(|specifier| {
                     Locals::map(expected, specifier)
                         .map_err(ResolveError::InvalidVersion)
-                        .and_then(|specifier| Ok(PubGrubSpecifier::try_from(&specifier)?))
+                        .and_then(|specifier| {
+                            Ok(PubGrubSpecifier::from_pep440_specifier(&specifier)?)
+                        })
                 })
                 .fold_ok(Range::full(), |range, specifier| {
                     range.intersection(&specifier.into())
                 })?
         } else {
-            PubGrubSpecifier::try_from(specifier)?.into()
+            PubGrubSpecifier::from_pep440_specifiers(specifier)?.into()
         };
 
         let requirement = Self {

diff --git a/crates/uv-resolver/src/pubgrub/specifier.rs b/crates/uv-resolver/src/pubgrub/specifier.rs
@@ -35,26 +35,26 @@ impl From<PubGrubSpecifier> for Range<Version> {
     }
 }
 
-impl TryFrom<&VersionSpecifiers> for PubGrubSpecifier {
-    type Error = PubGrubSpecifierError;
-
-    /// Convert a PEP 440 specifier to a PubGrub-compatible version range.
-    fn try_from(specifiers: &VersionSpecifiers) -> Result<Self, PubGrubSpecifierError> {
+impl PubGrubSpecifier {
+    /// Convert [`VersionSpecifiers`] to a PubGrub-compatible version range, using PEP 440
+    /// semantics.
+    pub(crate) fn from_pep440_specifiers(
+        specifiers: &VersionSpecifiers,
+    ) -> Result<Self, PubGrubSpecifierError> {
         let range = specifiers
             .iter()
-            .map(crate::pubgrub::PubGrubSpecifier::try_from)
+            .map(Self::from_pep440_specifier)
             .fold_ok(Range::full(), |range, specifier| {
                 range.intersection(&specifier.into())
             })?;
         Ok(Self(range))
     }
-}
-
-impl TryFrom<&VersionSpecifier> for PubGrubSpecifier {
-    type Error = PubGrubSpecifierError;
 
-    /// Convert a PEP 440 specifier to a PubGrub-compatible version range.
-    fn try_from(specifier: &VersionSpecifier) -> Result<Self, PubGrubSpecifierError> {
+    /// Convert the [`VersionSpecifier`] to a PubGrub-compatible version range, using PEP 440
+    /// semantics.
+    pub(crate) fn from_pep440_specifier(
+        specifier: &VersionSpecifier,
+    ) -> Result<Self, PubGrubSpecifierError> {
         let ranges = match specifier.operator() {
             Operator::Equal => {
                 let version = specifier.version().clone();
@@ -147,4 +147,104 @@ impl TryFrom<&VersionSpecifier> for PubGrubSpecifier {
 
         Ok(Self(ranges))
     }
+
+    /// Convert the [`VersionSpecifiers`] to a PubGrub-compatible version range, using release-only
+    /// semantics.
+    ///
+    /// Assumes that the range will only be tested against versions that consist solely of release
+    /// segments (e.g., `3.12.0`, but not `3.12.0b1`).
+    ///
+    /// These semantics are used for testing Python compatibility (e.g., `requires-python` against
+    /// the user's installed Python version). In that context, it's more intuitive that `3.13.0b0`
+    /// is allowed for projects that declare `requires-python = ">3.13"`.
+    ///
+    /// See: <https://github.com/pypa/pip/blob/a432c7f4170b9ef798a15f035f5dfdb4cc939f35/src/pip/_internal/resolution/resolvelib/candidates.py#L540>
+    pub(crate) fn from_release_specifiers(
+        specifiers: &VersionSpecifiers,
+    ) -> Result<Self, PubGrubSpecifierError> {
+        let range = specifiers
+            .iter()
+            .map(Self::from_release_specifier)
+            .fold_ok(Range::full(), |range, specifier| {
+                range.intersection(&specifier.into())
+            })?;
+        Ok(Self(range))
+    }
+
+    /// Convert the [`VersionSpecifier`] to a PubGrub-compatible version range, using release-only
+    /// semantics.
+    ///
+    /// Assumes that the range will only be tested against versions that consist solely of release
+    /// segments (e.g., `3.12.0`, but not `3.12.0b1`).
+    ///
+    /// These semantics are used for testing Python compatibility (e.g., `requires-python` against
+    /// the user's installed Python version). In that context, it's more intuitive that `3.13.0b0`
+    /// is allowed for projects that declare `requires-python = ">3.13"`.
+    ///
+    /// See: <https://github.com/pypa/pip/blob/a432c7f4170b9ef798a15f035f5dfdb4cc939f35/src/pip/_internal/resolution/resolvelib/candidates.py#L540>
+    pub(crate) fn from_release_specifier(
+        specifier: &VersionSpecifier,
+    ) -> Result<Self, PubGrubSpecifierError> {
+        let ranges = match specifier.operator() {
+            Operator::Equal => {
+                let version = specifier.version().only_release();
+                Range::singleton(version)
+            }
+            Operator::ExactEqual => {
+                let version = specifier.version().only_release();
+                Range::singleton(version)
+            }
+            Operator::NotEqual => {
+                let version = specifier.version().only_release();
+                Range::singleton(version).complement()
+            }
+            Operator::TildeEqual => {
+                let [rest @ .., last, _] = specifier.version().release() else {
+                    return Err(PubGrubSpecifierError::InvalidTildeEquals(specifier.clone()));
+                };
+                let upper = Version::new(rest.iter().chain([&(last + 1)]));
+                let version = specifier.version().only_release();
+                Range::from_range_bounds(version..upper)
+            }
+            Operator::LessThan => {
+                let version = specifier.version().only_release();
+                Range::strictly_lower_than(version)
+            }
+            Operator::LessThanEqual => {
+                let version = specifier.version().only_release();
+                Range::lower_than(version)
+            }
+            Operator::GreaterThan => {
+                let version = specifier.version().only_release();
+                Range::strictly_higher_than(version)
+            }
+            Operator::GreaterThanEqual => {
+                let version = specifier.version().only_release();
+                Range::higher_than(version)
+            }
+            Operator::EqualStar => {
+                let low = specifier.version().only_release();
+                let high = {
+                    let mut high = low.clone();
+                    let mut release = high.release().to_vec();
+                    *release.last_mut().unwrap() += 1;
+                    high = high.with_release(release);
+                    high
+                };
+                Range::from_range_bounds(low..high)
+            }
+            Operator::NotEqualStar => {
+                let low = specifier.version().only_release();
+                let high = {
+                    let mut high = low.clone();
+                    let mut release = high.release().to_vec();
+                    *release.last_mut().unwrap() += 1;
+                    high = high.with_release(release);
+                    high
+                };
+                Range::from_range_bounds(low..high).complement()
+            }
+        };
+        Ok(Self(ranges))
+    }
 }
diff --git a/crates/uv-resolver/src/python_requirement.rs b/crates/uv-resolver/src/python_requirement.rs
@@ -1,13 +1,13 @@
-use pep440_rs::VersionSpecifiers;
-use pep508_rs::{MarkerTree, StringVersion};
+use pep440_rs::{Version, VersionSpecifiers};
+use pep508_rs::MarkerTree;
 use uv_python::{Interpreter, PythonVersion};
 
 use crate::{RequiresPython, RequiresPythonBound};
 
 #[derive(Debug, Clone, Eq, PartialEq)]
 pub struct PythonRequirement {
     /// The installed version of Python.
-    installed: StringVersion,
+    installed: Version,
     /// The target version of Python; that is, the version of Python for which we are resolving
     /// dependencies. This is typically the same as the installed version, but may be different
     /// when specifying an alternate Python version for the resolution.
@@ -21,11 +21,10 @@ impl PythonRequirement {
     /// [`PythonVersion`].
     pub fn from_python_version(interpreter: &Interpreter, python_version: &PythonVersion) -> Self {
         Self {
-            installed: interpreter.python_full_version().clone(),
-            target: Some(PythonTarget::Version(StringVersion {
-                string: python_version.to_string(),
-                version: python_version.python_full_version(),
-            })),
+            installed: interpreter.python_full_version().version.only_release(),
+            target: Some(PythonTarget::Version(
+                python_version.python_full_version().only_release(),
+            )),
         }
     }
 
@@ -36,15 +35,15 @@ impl PythonRequirement {
         requires_python: &RequiresPython,
     ) -> Self {
         Self {
-            installed: interpreter.python_full_version().clone(),
+            installed: interpreter.python_full_version().version.only_release(),
             target: Some(PythonTarget::RequiresPython(requires_python.clone())),
         }
     }
 
     /// Create a [`PythonRequirement`] to resolve against an [`Interpreter`].
     pub fn from_interpreter(interpreter: &Interpreter) -> Self {
         Self {
-            installed: interpreter.python_full_version().clone(),
+            installed: interpreter.python_full_version().version.only_release(),
             target: None,
         }
     }
@@ -63,7 +62,7 @@ impl PythonRequirement {
     }
 
     /// Return the installed version of Python.
-    pub fn installed(&self) -> &StringVersion {
+    pub fn installed(&self) -> &Version {
         &self.installed
     }
 
@@ -91,7 +90,7 @@ pub enum PythonTarget {
     ///
     /// The use of a separate enum variant allows us to use a verbatim representation when reporting
     /// back to the user.
-    Version(StringVersion),
+    Version(Version),
     /// The [`PythonTarget`] specifier is a set of version specifiers, as extracted from the
     /// `Requires-Python` field in a `pyproject.toml` or `METADATA` file.
     RequiresPython(RequiresPython),