Skip to content

Commit

Permalink
Always use release-only comparisons for requires-python (#4794)
Browse files Browse the repository at this point in the history
## Summary

There are a few ideas at play here:

1. pip always strips versions to the release when evaluating against a
`Requires-Python`, so we now do the same. That means, e.g., using
`3.13.0b0` will be accepted by a project with `Requires-Python: >=
3.13`, which does _not_ adhere to PEP 440 semantics but is somewhat
intuitive.
2. Because we know we'll only be evaluating against release-only
versions, we can use different semantics in PubGrub that let us collapse
ranges. For example, `python_version >= '3.10' or python_version <
'3.10'` can be collapsed to the truthy marker.

Closes #4714.
Closes #4272.
Closes #4719.
  • Loading branch information
charliermarsh authored Jul 4, 2024
1 parent 11cb005 commit b588054
Show file tree
Hide file tree
Showing 13 changed files with 403 additions and 173 deletions.
13 changes: 13 additions & 0 deletions PIP_COMPATIBILITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -402,3 +402,16 @@ By default, uv does not write any index URLs to the output file, while `pip-comp
`--index-url` or `--extra-index-url` that does not match the default (PyPI). To include index URLs
in the output file, pass the `--emit-index-url` flag to `uv pip compile`. Unlike `pip-compile`,
uv will include all index URLs when `--emit-index-url` is passed, including the default index URL.

## `requires-python` enforcement

When evaluating Python versions against `requires-python` specifiers, uv truncates the candidate
version to the major, minor, and patch components, ignoring (e.g.) pre-release and post-release
identifiers.

For example, a project that declares `requires-python: >=3.13` will accept Python 3.13.0b1. While
3.13.0b1 is not strictly greater than 3.13, it is greater than 3.13 when the pre-release identifier
is omitted.

While this is not strictly compliant with [PEP 440](https://peps.python.org/pep-0440/),
it _is_ consistent with [pip](https://github.com/pypa/pip/blob/24.1.1/src/pip/_internal/resolution/resolvelib/candidates.py#L540).
7 changes: 7 additions & 0 deletions crates/pep440-rs/src/version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
42 changes: 38 additions & 4 deletions crates/uv-resolver/src/marker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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()))
}
Expand Down Expand Up @@ -403,7 +437,7 @@ mod tests {
// a quirk of how pubgrub works, but this is considered part of normalization
assert_marker_equal(
"python_version > '3.17.post4' or python_version > '3.18.post4'",
"python_version >= '3.17.post5'",
"python_version > '3.17'",
);

assert_marker_equal(
Expand Down
6 changes: 4 additions & 2 deletions crates/uv-resolver/src/pubgrub/dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
124 changes: 112 additions & 12 deletions crates/uv-resolver/src/pubgrub/specifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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))
}
}
23 changes: 11 additions & 12 deletions crates/uv-resolver/src/python_requirement.rs
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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(),
)),
}
}

Expand All @@ -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,
}
}
Expand All @@ -63,7 +62,7 @@ impl PythonRequirement {
}

/// Return the installed version of Python.
pub fn installed(&self) -> &StringVersion {
pub fn installed(&self) -> &Version {
&self.installed
}

Expand Down Expand Up @@ -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),
Expand Down
Loading

0 comments on commit b588054

Please sign in to comment.