Respect the user's upper-bound in requires-python (#6824)

## Summary

We now respect the user-provided upper-bound in for `requires-python`.
So, if the user has `requires-python = "==3.11.*"`, we won't explore
forks that have `python_version >= '3.12'`, for example.

However, we continue to _only_ compare the lower bounds when assessing
whether a dependency is compatible with a given Python range.

Closes https://github.com/astral-sh/uv/issues/6150.
This commit is contained in:
Charlie Marsh 2024-08-29 14:37:05 -04:00 committed by GitHub
parent a17c1e8e40
commit 34d74501ac
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 308 additions and 77 deletions

View file

@ -12,7 +12,9 @@ pub use preferences::{Preference, PreferenceError, Preferences};
pub use prerelease::PrereleaseMode;
pub use pubgrub::{PubGrubSpecifier, PubGrubSpecifierError};
pub use python_requirement::PythonRequirement;
pub use requires_python::{RequiresPython, RequiresPythonBound, RequiresPythonError};
pub use requires_python::{
RequiresPython, RequiresPythonBound, RequiresPythonError, RequiresPythonRange,
};
pub use resolution::{AnnotationStyle, DisplayResolutionGraph, ResolutionGraph};
pub use resolution_mode::ResolutionMode;
pub use resolver::{

View file

@ -1,19 +1,19 @@
use pep508_rs::{MarkerTree, MarkerTreeKind, MarkerValueVersion};
use crate::requires_python::RequiresPythonRange;
use crate::RequiresPythonBound;
use pep440_rs::Version;
use pep508_rs::{MarkerTree, MarkerTreeKind, MarkerValueVersion};
use pubgrub::Range;
/// Returns the minimum Python version that can satisfy the [`MarkerTree`], if it's constrained.
pub(crate) fn requires_python(tree: &MarkerTree) -> Option<RequiresPythonBound> {
fn collect_python_markers(tree: &MarkerTree, markers: &mut Vec<RequiresPythonBound>) {
/// Returns the bounding Python versions that can satisfy the [`MarkerTree`], if it's constrained.
pub(crate) fn requires_python(tree: &MarkerTree) -> Option<RequiresPythonRange> {
fn collect_python_markers(tree: &MarkerTree, markers: &mut Vec<Range<Version>>) {
match tree.kind() {
MarkerTreeKind::True | MarkerTreeKind::False => {}
MarkerTreeKind::Version(marker) => match marker.key() {
MarkerValueVersion::PythonVersion | MarkerValueVersion::PythonFullVersion => {
for (range, tree) in marker.edges() {
if !tree.is_false() {
// Extract the lower bound.
let (lower, _) = range.iter().next().unwrap();
markers.push(RequiresPythonBound::new(lower.clone()));
markers.push(range.clone());
}
}
}
@ -48,5 +48,21 @@ pub(crate) fn requires_python(tree: &MarkerTree) -> Option<RequiresPythonBound>
let mut markers = Vec::new();
collect_python_markers(tree, &mut markers);
markers.into_iter().min()
// Take the union of all Python version markers.
let range = markers
.into_iter()
.fold(None, |acc: Option<Range<Version>>, range| {
Some(match acc {
Some(acc) => acc.union(&range),
None => range.clone(),
})
})?;
let (lower, upper) = range.bounding_range()?;
Some(RequiresPythonRange::new(
RequiresPythonBound::new(lower.cloned()),
RequiresPythonBound::new(upper.cloned()),
))
}

View file

@ -1,7 +1,7 @@
use pep440_rs::{Version, VersionSpecifiers};
use uv_python::{Interpreter, PythonVersion};
use crate::{RequiresPython, RequiresPythonBound};
use crate::{RequiresPython, RequiresPythonRange};
#[derive(Debug, Clone, Eq, PartialEq)]
pub struct PythonRequirement {
@ -49,7 +49,7 @@ 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> {
pub fn narrow(&self, target: &RequiresPythonRange) -> Option<Self> {
let Some(PythonTarget::RequiresPython(requires_python)) = self.target.as_ref() else {
return None;
};

View file

@ -31,9 +31,8 @@ pub struct RequiresPython {
/// For a workspace, it's the union of all `requires-python` values in the workspace. If no
/// bound was provided by the user, it's greater equal the current Python version.
specifiers: VersionSpecifiers,
/// The lower bound from the `specifiers` field, i.e. greater or greater equal the lowest
/// version allowed by `specifiers`.
bound: RequiresPythonBound,
/// The lower and upper bounds of `specifiers`.
range: RequiresPythonRange,
}
impl RequiresPython {
@ -44,22 +43,26 @@ impl RequiresPython {
specifiers: VersionSpecifiers::from(VersionSpecifier::greater_than_equal_version(
version.clone(),
)),
bound: RequiresPythonBound(Bound::Included(version)),
range: RequiresPythonRange(
RequiresPythonBound::new(Bound::Included(version.clone())),
RequiresPythonBound::new(Bound::Unbounded),
),
}
}
/// Returns a [`RequiresPython`] from a version specifier.
pub fn from_specifiers(specifiers: &VersionSpecifiers) -> Result<Self, RequiresPythonError> {
let bound = RequiresPythonBound(
let (lower_bound, upper_bound) =
crate::pubgrub::PubGrubSpecifier::from_release_specifiers(specifiers)?
.iter()
.next()
.map(|(lower, _)| lower.clone())
.unwrap_or(Bound::Unbounded),
);
.bounding_range()
.map(|(lower_bound, upper_bound)| (lower_bound.cloned(), upper_bound.cloned()))
.unwrap_or((Bound::Unbounded, Bound::Unbounded));
Ok(Self {
specifiers: specifiers.clone(),
bound,
range: RequiresPythonRange(
RequiresPythonBound(lower_bound),
RequiresPythonBound(upper_bound),
),
})
}
@ -85,14 +88,11 @@ impl RequiresPython {
return Ok(None);
};
// Extract the lower bound.
let bound = RequiresPythonBound(
range
.iter()
.next()
.map(|(lower, _)| lower.clone())
.unwrap_or(Bound::Unbounded),
);
// Extract the bounds.
let (lower_bound, upper_bound) = range
.bounding_range()
.map(|(lower_bound, upper_bound)| (lower_bound.cloned(), upper_bound.cloned()))
.unwrap_or((Bound::Unbounded, Bound::Unbounded));
// Convert back to PEP 440 specifiers.
let specifiers = range
@ -100,16 +100,24 @@ impl RequiresPython {
.flat_map(VersionSpecifier::from_release_only_bounds)
.collect();
Ok(Some(Self { specifiers, bound }))
Ok(Some(Self {
specifiers,
range: RequiresPythonRange(
RequiresPythonBound(lower_bound),
RequiresPythonBound(upper_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> {
if target > &self.bound {
pub fn narrow(&self, range: &RequiresPythonRange) -> Option<Self> {
if *range == self.range {
None
} else if range.0 >= self.range.0 && range.1 <= self.range.1 {
Some(Self {
specifiers: VersionSpecifiers::from(VersionSpecifier::from_lower_bound(target)?),
bound: target.clone(),
specifiers: self.specifiers.clone(),
range: range.clone(),
})
} else {
None
@ -142,7 +150,7 @@ impl RequiresPython {
// `>=3.7`.
//
// That is: `version_lower` should be less than or equal to `requires_python_lower`.
match (target, self.bound.as_ref()) {
match (target, self.range.lower().as_ref()) {
(Bound::Included(target_lower), Bound::Included(requires_python_lower)) => {
target_lower <= requires_python_lower
}
@ -170,17 +178,12 @@ impl RequiresPython {
/// Returns `true` if the `Requires-Python` specifier is unbounded.
pub fn is_unbounded(&self) -> bool {
self.bound.as_ref() == Bound::Unbounded
}
/// Returns the [`RequiresPythonBound`] for the `Requires-Python` specifier.
pub fn bound(&self) -> &RequiresPythonBound {
&self.bound
self.range.lower().as_ref() == Bound::Unbounded
}
/// Returns the [`RequiresPythonBound`] truncated to the major and minor version.
pub fn bound_major_minor(&self) -> RequiresPythonBound {
match self.bound.as_ref() {
match self.range.lower().as_ref() {
// Ex) `>=3.10.1` -> `>=3.10`
Bound::Included(version) => RequiresPythonBound(Bound::Included(Version::new(
version.release().iter().take(2),
@ -195,6 +198,11 @@ impl RequiresPython {
}
}
/// Returns the [`Range`] bounding the `Requires-Python` specifier.
pub fn range(&self) -> &RequiresPythonRange {
&self.range
}
/// Returns `false` if the wheel's tags state it can't be used in the given Python version
/// range.
///
@ -279,15 +287,76 @@ 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 = RequiresPythonBound(
let (lower_bound, upper_bound) =
crate::pubgrub::PubGrubSpecifier::from_release_specifiers(&specifiers)
.map_err(serde::de::Error::custom)?
.iter()
.next()
.map(|(lower, _)| lower.clone())
.unwrap_or(Bound::Unbounded),
);
Ok(Self { specifiers, bound })
.bounding_range()
.map(|(lower_bound, upper_bound)| (lower_bound.cloned(), upper_bound.cloned()))
.unwrap_or((Bound::Unbounded, Bound::Unbounded));
Ok(Self {
specifiers,
range: RequiresPythonRange(
RequiresPythonBound(lower_bound),
RequiresPythonBound(upper_bound),
),
})
}
}
#[derive(Debug, Clone, Eq, PartialEq, Hash)]
pub struct RequiresPythonRange(RequiresPythonBound, RequiresPythonBound);
impl RequiresPythonRange {
/// Initialize a [`RequiresPythonRange`] with the given bounds.
pub fn new(lower: RequiresPythonBound, upper: RequiresPythonBound) -> Self {
Self(lower, upper)
}
/// Returns the lower bound.
pub fn lower(&self) -> &RequiresPythonBound {
&self.0
}
/// Returns the upper bound.
pub fn upper(&self) -> &RequiresPythonBound {
&self.1
}
}
impl Default for RequiresPythonRange {
fn default() -> Self {
Self(
RequiresPythonBound(Bound::Unbounded),
RequiresPythonBound(Bound::Unbounded),
)
}
}
impl From<RequiresPythonRange> for Range<Version> {
fn from(value: RequiresPythonRange) -> Self {
match (value.0.as_ref(), value.1.as_ref()) {
(Bound::Included(lower), Bound::Included(upper)) => {
Range::from_range_bounds(lower.clone()..=upper.clone())
}
(Bound::Included(lower), Bound::Excluded(upper)) => {
Range::from_range_bounds(lower.clone()..upper.clone())
}
(Bound::Excluded(lower), Bound::Included(upper)) => {
Range::strictly_higher_than(lower.clone())
.intersection(&Range::lower_than(upper.clone()))
}
(Bound::Excluded(lower), Bound::Excluded(upper)) => {
Range::strictly_higher_than(lower.clone())
.intersection(&Range::strictly_lower_than(upper.clone()))
}
(Bound::Unbounded, Bound::Unbounded) => Range::full(),
(Bound::Unbounded, Bound::Included(upper)) => Range::lower_than(upper.clone()),
(Bound::Unbounded, Bound::Excluded(upper)) => Range::strictly_lower_than(upper.clone()),
(Bound::Included(lower), Bound::Unbounded) => Range::higher_than(lower.clone()),
(Bound::Excluded(lower), Bound::Unbounded) => {
Range::strictly_higher_than(lower.clone())
}
}
}
}
@ -301,6 +370,9 @@ impl Default for RequiresPythonBound {
}
impl RequiresPythonBound {
/// Initialize a [`RequiresPythonBound`] with the given bound.
///
/// These bounds use release-only semantics when comparing versions.
pub fn new(bound: Bound<Version>) -> Self {
Self(match bound {
Bound::Included(version) => Bound::Included(version.only_release()),
@ -310,16 +382,6 @@ impl RequiresPythonBound {
}
}
impl From<RequiresPythonBound> for Range<Version> {
fn from(value: RequiresPythonBound) -> Self {
match value.0 {
Bound::Included(version) => Range::higher_than(version),
Bound::Excluded(version) => Range::strictly_higher_than(version),
Bound::Unbounded => Range::full(),
}
}
}
impl Deref for RequiresPythonBound {
type Target = Bound<Version>;

View file

@ -1486,7 +1486,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
// supported by the root, skip it.
let requirement = if let Some(requires_python) = python_requirement.target().and_then(|target| target.as_requires_python()).filter(|_| !requirement.marker.is_true()) {
let marker = requirement.marker.clone().simplify_python_versions(
Range::from(requires_python.bound().clone()),
Range::from(requires_python.range().clone()),
);
if marker.is_false() {
@ -1552,7 +1552,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
// supported by the root, skip it.
let constraint = if let Some(requires_python) = python_requirement.target().and_then(|target| target.as_requires_python()).filter(|_| !constraint.marker.is_true()) {
let mut marker = constraint.marker.clone().simplify_python_versions(
Range::from(requires_python.bound().clone()),
Range::from(requires_python.range().clone()),
);
marker.and(requirement.marker.clone());
@ -2813,7 +2813,7 @@ impl Ord for Fork {
let self_bound = marker::requires_python(&self.markers).unwrap_or_default();
let other_bound = marker::requires_python(&other.markers).unwrap_or_default();
other_bound.cmp(&self_bound).then_with(|| {
other_bound.lower().cmp(self_bound.lower()).then_with(|| {
// If there's no difference, prioritize forks with upper bounds. We'd prefer to solve
// `numpy <= 2` before solving `numpy >= 1`, since the resolution produced by the former
// might work for the latter, but the inverse is unlikely to be true due to maximum
@ -2855,7 +2855,7 @@ fn simplify_python(marker: MarkerTree, python_requirement: &PythonRequirement) -
.target()
.and_then(|target| target.as_requires_python())
{
marker.simplify_python_versions(Range::from(requires_python.bound().clone()))
marker.simplify_python_versions(Range::from(requires_python.range().clone()))
} else {
marker
}