Skip to content

Commit

Permalink
Narrow requires-python requirement in resolver forks (#4707)
Browse files Browse the repository at this point in the history
## Summary

Given:

```text
numpy >=1.26 ; python_version >= '3.9'
numpy <1.26 ; python_version < '3.9'
```

When resolving for Python 3.8, we need to narrow the `requires-python`
requirement in the top branch of the fork, because `numpy >=1.26` all
require Python 3.9 or later -- but we know (in that branch) that we only
need to _solve_ for Python 3.9 or later.

Closes #4669.
  • Loading branch information
charliermarsh authored Jul 2, 2024
1 parent 89b3324 commit d9f389a
Show file tree
Hide file tree
Showing 9 changed files with 251 additions and 58 deletions.
4 changes: 2 additions & 2 deletions crates/pep440-rs/src/version_specifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@ impl VersionSpecifier {
}

/// Returns a version specifier representing the given lower bound.
fn from_lower_bound(bound: &Bound<Version>) -> Option<VersionSpecifier> {
pub fn from_lower_bound(bound: &Bound<Version>) -> Option<VersionSpecifier> {
match bound {
Bound::Included(version) => Some(
VersionSpecifier::from_version(Operator::GreaterThanEqual, version.clone())
Expand All @@ -450,7 +450,7 @@ impl VersionSpecifier {
}

/// Returns a version specifier representing the given upper bound.
fn from_upper_bound(bound: &Bound<Version>) -> Option<VersionSpecifier> {
pub fn from_upper_bound(bound: &Bound<Version>) -> Option<VersionSpecifier> {
match bound {
Bound::Included(version) => Some(
VersionSpecifier::from_version(Operator::LessThanEqual, version.clone()).unwrap(),
Expand Down
2 changes: 1 addition & 1 deletion crates/uv-resolver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ pub use preferences::{Preference, PreferenceError, Preferences};
pub use prerelease_mode::PreReleaseMode;
pub use pubgrub::{PubGrubSpecifier, PubGrubSpecifierError};
pub use python_requirement::PythonRequirement;
pub use requires_python::{RequiresPython, RequiresPythonError};
pub use requires_python::{RequiresPython, RequiresPythonBound, RequiresPythonError};
pub use resolution::{AnnotationStyle, DisplayResolutionGraph, ResolutionGraph};
pub use resolution_mode::ResolutionMode;
pub use resolver::{
Expand Down
40 changes: 39 additions & 1 deletion crates/uv-resolver/src/marker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,16 @@ use std::collections::HashMap;
use std::ops::Bound::{self, *};
use std::ops::RangeBounds;

use pubgrub::range::Range as PubGrubRange;

use pep440_rs::{Operator, Version, VersionSpecifier};
use pep508_rs::{
ExtraName, ExtraOperator, MarkerExpression, MarkerOperator, MarkerTree, MarkerValueString,
MarkerValueVersion,
};

use crate::pubgrub::PubGrubSpecifier;
use pubgrub::range::Range as PubGrubRange;
use crate::RequiresPythonBound;

/// Returns `true` if there is no environment in which both marker trees can both apply, i.e.
/// the expression `first and second` is always false.
Expand Down Expand Up @@ -80,6 +82,42 @@ fn string_is_disjoint(this: &MarkerExpression, other: &MarkerExpression) -> bool
true
}

/// Returns the minimum Python version that can satisfy the [`MarkerTree`], if it's constrained.
pub(crate) fn requires_python_marker(tree: &MarkerTree) -> Option<RequiresPythonBound> {
match tree {
MarkerTree::Expression(MarkerExpression::Version {
key: MarkerValueVersion::PythonFullVersion | MarkerValueVersion::PythonVersion,
specifier,
}) => {
let specifier = PubGrubSpecifier::try_from(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::And(trees) => {
// Take the maximum of any nested expressions.
trees.iter().filter_map(requires_python_marker).max()
}
MarkerTree::Or(trees) => {
// If all subtrees have a bound, take the minimum.
let mut min_version = None;
for tree in trees {
let version = requires_python_marker(tree)?;
min_version = match min_version {
Some(min_version) => Some(std::cmp::min(min_version, version)),
None => Some(version),
};
}
min_version
}
MarkerTree::Expression(_) => None,
}
}

/// Normalizes this marker tree.
///
/// This function does a number of operations to normalize a marker tree recursively:
Expand Down
7 changes: 3 additions & 4 deletions crates/uv-resolver/src/pubgrub/package.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use std::fmt::{Display, Formatter};
use std::ops::Deref;
use std::sync::Arc;

Expand All @@ -17,9 +16,9 @@ impl Deref for PubGrubPackage {
}
}

impl Display for PubGrubPackage {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
Display::fmt(&self.0, f)
impl std::fmt::Display for PubGrubPackage {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
std::fmt::Display::fmt(&self.0, f)
}
}

Expand Down
15 changes: 14 additions & 1 deletion crates/uv-resolver/src/python_requirement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use pep440_rs::VersionSpecifiers;
use pep508_rs::{MarkerTree, StringVersion};
use uv_toolchain::{Interpreter, PythonVersion};

use crate::RequiresPython;
use crate::{RequiresPython, RequiresPythonBound};

#[derive(Debug, Clone, Eq, PartialEq)]
pub struct PythonRequirement {
Expand Down Expand Up @@ -49,6 +49,19 @@ impl PythonRequirement {
}
}

/// Narrow the [`PythonRequirement`] to the given version, if it's stricter (i.e., greater)
/// than the current `Requires-Python` minimum.
pub fn narrow(&self, target: &RequiresPythonBound) -> Option<Self> {
let Some(PythonTarget::RequiresPython(requires_python)) = self.target.as_ref() else {
return None;
};
let requires_python = requires_python.narrow(target)?;
Some(Self {
installed: self.installed.clone(),
target: Some(PythonTarget::RequiresPython(requires_python)),
})
}

/// Return the installed version of Python.
pub fn installed(&self) -> &StringVersion {
&self.installed
Expand Down
94 changes: 72 additions & 22 deletions crates/uv-resolver/src/requires_python.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use std::cmp::Ordering;
use std::collections::Bound;
use std::ops::Deref;

use itertools::Itertools;
use pubgrub::range::Range;
Expand All @@ -24,7 +26,7 @@ pub enum RequiresPythonError {
#[derive(Debug, Clone, Eq, PartialEq, Hash)]
pub struct RequiresPython {
specifiers: VersionSpecifiers,
bound: Bound<Version>,
bound: RequiresPythonBound,
}

impl RequiresPython {
Expand All @@ -34,7 +36,7 @@ impl RequiresPython {
specifiers: VersionSpecifiers::from(VersionSpecifier::greater_than_equal_version(
version.clone(),
)),
bound: Bound::Included(version),
bound: RequiresPythonBound(Bound::Included(version)),
}
}

Expand All @@ -61,11 +63,13 @@ impl RequiresPython {
};

// Extract the lower bound.
let bound = range
.iter()
.next()
.map(|(lower, _)| lower.clone())
.unwrap_or(Bound::Unbounded);
let bound = RequiresPythonBound(
range
.iter()
.next()
.map(|(lower, _)| lower.clone())
.unwrap_or(Bound::Unbounded),
);

// Convert back to PEP 440 specifiers.
let specifiers = range
Expand All @@ -76,6 +80,16 @@ impl RequiresPython {
Ok(Some(Self { specifiers, bound }))
}

/// Narrow the [`RequiresPython`] to the given version, if it's stricter (i.e., greater) than
/// the current target.
pub fn narrow(&self, target: &RequiresPythonBound) -> Option<Self> {
let target = VersionSpecifiers::from(VersionSpecifier::from_lower_bound(target)?);
Self::union(std::iter::once(&target))
.ok()
.flatten()
.filter(|next| next.bound > self.bound)
}

/// Returns `true` if the `Requires-Python` is compatible with the given version.
pub fn contains(&self, version: &Version) -> bool {
self.specifiers.contains(version)
Expand Down Expand Up @@ -140,7 +154,7 @@ impl RequiresPython {
// Alternatively, we could vary the semantics depending on whether or not the user included
// a pre-release in their specifier, enforcing pre-release compatibility only if the user
// explicitly requested it.
match (target, &self.bound) {
match (target, self.bound.as_ref()) {
(Bound::Included(target_lower), Bound::Included(requires_python_lower)) => {
target_lower.release() <= requires_python_lower.release()
}
Expand All @@ -166,9 +180,9 @@ impl RequiresPython {
&self.specifiers
}

/// Returns the lower [`Bound`] for the `Requires-Python` specifier.
pub fn bound(&self) -> &Bound<Version> {
&self.bound
/// Returns `true` if the `Requires-Python` specifier is unbounded.
pub fn is_unbounded(&self) -> bool {
self.bound.as_ref() == Bound::Unbounded
}

/// Returns this `Requires-Python` specifier as an equivalent marker
Expand All @@ -184,16 +198,14 @@ impl RequiresPython {
/// returns a marker tree that evaluates to `true` for all possible marker
/// environments.
pub fn to_marker_tree(&self) -> MarkerTree {
let (op, version) = match self.bound {
let (op, version) = match self.bound.as_ref() {
// If we see this anywhere, then it implies the marker
// tree we would generate would always evaluate to
// `true` because every possible Python version would
// satisfy it.
Bound::Unbounded => return MarkerTree::And(vec![]),
Bound::Excluded(ref version) => {
(Operator::GreaterThan, version.clone().without_local())
}
Bound::Included(ref version) => {
Bound::Excluded(version) => (Operator::GreaterThan, version.clone().without_local()),
Bound::Included(version) => {
(Operator::GreaterThanEqual, version.clone().without_local())
}
};
Expand Down Expand Up @@ -247,12 +259,50 @@ impl serde::Serialize for RequiresPython {
impl<'de> serde::Deserialize<'de> for RequiresPython {
fn deserialize<D: serde::Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
let specifiers = VersionSpecifiers::deserialize(deserializer)?;
let bound = crate::pubgrub::PubGrubSpecifier::try_from(&specifiers)
.map_err(serde::de::Error::custom)?
.iter()
.next()
.map(|(lower, _)| lower.clone())
.unwrap_or(Bound::Unbounded);
let bound = RequiresPythonBound(
crate::pubgrub::PubGrubSpecifier::try_from(&specifiers)
.map_err(serde::de::Error::custom)?
.iter()
.next()
.map(|(lower, _)| lower.clone())
.unwrap_or(Bound::Unbounded),
);
Ok(Self { specifiers, bound })
}
}

#[derive(Debug, Clone, Eq, PartialEq, Hash)]
pub struct RequiresPythonBound(Bound<Version>);

impl RequiresPythonBound {
pub fn new(bound: Bound<Version>) -> Self {
Self(bound)
}
}

impl Deref for RequiresPythonBound {
type Target = Bound<Version>;

fn deref(&self) -> &Self::Target {
&self.0
}
}

impl PartialOrd for RequiresPythonBound {
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
Some(self.cmp(other))
}
}

impl Ord for RequiresPythonBound {
fn cmp(&self, other: &Self) -> Ordering {
match (self.as_ref(), other.as_ref()) {
(Bound::Included(a), Bound::Included(b)) => a.cmp(b),
(Bound::Included(_), Bound::Excluded(_)) => Ordering::Less,
(Bound::Excluded(_), Bound::Included(_)) => Ordering::Greater,
(Bound::Excluded(a), Bound::Excluded(b)) => a.cmp(b),
(Bound::Unbounded, _) => Ordering::Less,
(_, Bound::Unbounded) => Ordering::Greater,
}
}
}
Loading

0 comments on commit d9f389a

Please sign in to comment.