uv-resolver: use Requires-Python to filter dependencies during universal resolution

In the time before universal resolving, we would always pass a
`MarkerEnvironment`, and this environment would capture any relevant
`Requires-Python` specifier (including if `-p/--python` was provided on
the CLI).

But in universal resolution, we very specifically do not use a
`MarkerEnvironment` because we want to produce a resolution that is
compatible across potentially multiple environments. This in turn meant
that we lost `Requires-Python` filtering.

This PR adds it back. We do this by converting our `PythonRequirement`
into a `MarkerTree` that encodes the version specifiers in a
`Requires-Python` specifier. We then ask whether that `MarkerTree` is
disjoint with a dependency specification's `MarkerTree`. If it is, then
we know it's impossible for that dependency specification to every be
true, and we can completely ignore it.
This commit is contained in:
Andrew Gallant 2024-06-11 09:57:10 -04:00 committed by Andrew Gallant
parent c32667caec
commit 75b323232d
8 changed files with 147 additions and 422 deletions

View file

@ -8,7 +8,7 @@ use tracing::warn;
use distribution_types::Verbatim;
use pep440_rs::Version;
use pep508_rs::MarkerEnvironment;
use pep508_rs::{MarkerEnvironment, MarkerTree};
use pypi_types::{
ParsedArchiveUrl, ParsedGitUrl, ParsedPathUrl, ParsedUrl, Requirement, RequirementSource,
};
@ -39,6 +39,7 @@ impl PubGrubDependencies {
locals: &Locals,
git: &GitResolver,
env: Option<&MarkerEnvironment>,
requires_python: Option<&MarkerTree>,
) -> Result<Self, ResolveError> {
let mut dependencies = Vec::default();
let mut seen = FxHashSet::default();
@ -55,6 +56,7 @@ impl PubGrubDependencies {
locals,
git,
env,
requires_python,
&mut dependencies,
&mut seen,
)?;
@ -87,6 +89,7 @@ fn add_requirements(
locals: &Locals,
git: &GitResolver,
env: Option<&MarkerEnvironment>,
requires_python: Option<&MarkerTree>,
dependencies: &mut Vec<(PubGrubPackage, Range<Version>)>,
seen: &mut FxHashSet<ExtraName>,
) -> Result<(), ResolveError> {
@ -96,6 +99,11 @@ fn add_requirements(
} else {
Either::Right(requirements.iter())
}) {
// If the requirement would not be selected with any Python version
// supported by the root, skip it.
if !satisfies_requires_python(requires_python, requirement) {
continue;
}
// If the requirement isn't relevant for the current platform, skip it.
match source_extra {
Some(source_extra) => {
@ -157,6 +165,7 @@ fn add_requirements(
locals,
git,
env,
requires_python,
dependencies,
seen,
)?;
@ -170,6 +179,11 @@ fn add_requirements(
// If the requirement was constrained, add those constraints.
for constraint in constraints.get(&requirement.name).into_iter().flatten() {
// If the requirement would not be selected with any Python
// version supported by the root, skip it.
if !satisfies_requires_python(requires_python, constraint) {
continue;
}
// If the requirement isn't relevant for the current platform, skip it.
match source_extra {
Some(source_extra) => {
@ -381,3 +395,22 @@ impl PubGrubRequirement {
Self::from_requirement(constraint, None, urls, locals, git)
}
}
/// Returns true if and only if the given requirement's marker expression has a
/// possible true value given the `requires_python` specifier given.
///
/// While this is always called, a `requires_python` is only non-None when in
/// universal resolution mode. In non-universal mode, `requires_python` is
/// `None` and this always returns `true`.
fn satisfies_requires_python(
requires_python: Option<&MarkerTree>,
requirement: &Requirement,
) -> bool {
let Some(requires_python) = requires_python else {
return true;
};
let Some(marker) = requirement.marker.as_ref() else {
return true;
};
!crate::marker::is_disjoint(requires_python, marker)
}

View file

@ -58,6 +58,12 @@ impl PythonRequirement {
pub fn target(&self) -> Option<&PythonTarget> {
self.target.as_ref()
}
/// Return the target version of Python as a "requires python" type,
/// if available.
pub(crate) fn requires_python(&self) -> Option<&RequiresPython> {
self.target().and_then(|target| target.as_requires_python())
}
}
#[derive(Debug, Clone, Eq, PartialEq)]

View file

@ -3,7 +3,8 @@ use std::collections::Bound;
use itertools::Itertools;
use pubgrub::range::Range;
use pep440_rs::{Version, VersionSpecifier, VersionSpecifiers};
use pep440_rs::{Operator, Version, VersionSpecifier, VersionSpecifiers};
use pep508_rs::{MarkerExpression, MarkerTree, MarkerValueVersion};
#[derive(thiserror::Error, Debug)]
pub enum RequiresPythonError {
@ -169,6 +170,66 @@ impl RequiresPython {
pub fn bound(&self) -> &Bound<Version> {
&self.bound
}
/// Returns this `Requires-Python` specifier as an equivalent marker
/// expression utilizing the `python_version` marker field.
///
/// This is useful for comparing a `Requires-Python` specifier with
/// arbitrary marker expressions. For example, one can ask whether the
/// returned marker expression is disjoint with another marker expression.
/// If it is, then one can conclude that the `Requires-Python` specifier
/// excludes the dependency with that other marker expression.
///
/// If this `Requires-Python` specifier has no constraints, then this
/// 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 {
// 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) => {
(Operator::GreaterThanEqual, version.clone().without_local())
}
};
// For the `python_version` marker expression, it specifically only
// supports truncate major/minor versions of Python. This means that
// a `Requires-Python: 3.10.1` is satisfied by `python_version ==
// '3.10'`. So for disjointness checking, we need to ensure that the
// marker expression we generate for `Requires-Python` doesn't try to
// be overly selective about the patch version. We do this by keeping
// this part of our marker limited to the major and minor version
// components only.
let version_major_minor_only = Version::new(version.release().iter().take(2));
let expr_python_version = MarkerExpression::Version {
key: MarkerValueVersion::PythonVersion,
// OK because a version specifier is only invalid when the
// version is local (which is impossible here because we
// strip it above) or if the operator is ~= (which is also
// impossible here).
specifier: VersionSpecifier::from_version(op, version_major_minor_only).unwrap(),
};
let expr_python_full_version = MarkerExpression::Version {
key: MarkerValueVersion::PythonFullVersion,
// For `python_full_version`, we can use the entire
// version as-is.
//
// OK because a version specifier is only invalid when the
// version is local (which is impossible here because we
// strip it above) or if the operator is ~= (which is also
// impossible here).
specifier: VersionSpecifier::from_version(op, version).unwrap(),
};
MarkerTree::And(vec![
MarkerTree::Expression(expr_python_version),
MarkerTree::Expression(expr_python_full_version),
])
}
}
impl std::fmt::Display for RequiresPython {

View file

@ -47,6 +47,7 @@ use crate::pubgrub::{
PubGrubPriorities, PubGrubPython, PubGrubSpecifier,
};
use crate::python_requirement::PythonRequirement;
use crate::requires_python::RequiresPython;
use crate::resolution::ResolutionGraph;
pub(crate) use crate::resolver::availability::{
IncompletePackage, ResolverVersion, UnavailablePackage, UnavailableReason, UnavailableVersion,
@ -94,6 +95,14 @@ struct ResolverState<InstalledPackages: InstalledPackagesProvider> {
/// When not set, the resolver is in "universal" mode.
markers: Option<MarkerEnvironment>,
python_requirement: PythonRequirement,
/// This is derived from `PythonRequirement` once at initialization
/// time. It's used in universal mode to filter our dependencies with
/// a `python_version` marker expression that has no overlap with the
/// `Requires-Python` specifier.
///
/// This is non-None if and only if the resolver is operating in
/// universal mode. (i.e., when `markers` is `None`.)
requires_python: Option<MarkerTree>,
selector: CandidateSelector,
index: InMemoryIndex,
installed_packages: InstalledPackages,
@ -181,6 +190,16 @@ impl<Provider: ResolverProvider, InstalledPackages: InstalledPackagesProvider>
provider: Provider,
installed_packages: InstalledPackages,
) -> Result<Self, ResolveError> {
let requires_python = if markers.is_some() {
None
} else {
Some(
python_requirement
.requires_python()
.map(RequiresPython::to_marker_tree)
.unwrap_or_else(|| MarkerTree::And(vec![])),
)
};
let state = ResolverState {
index: index.clone(),
git: git.clone(),
@ -200,6 +219,7 @@ impl<Provider: ResolverProvider, InstalledPackages: InstalledPackagesProvider>
hasher: hasher.clone(),
markers: markers.cloned(),
python_requirement: python_requirement.clone(),
requires_python,
reporter: None,
installed_packages,
};
@ -959,6 +979,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
&self.locals,
&self.git,
self.markers.as_ref(),
self.requires_python.as_ref(),
);
let dependencies = match dependencies {
@ -1109,6 +1130,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
&self.locals,
&self.git,
self.markers.as_ref(),
self.requires_python.as_ref(),
)?;
for (dep_package, dep_version) in dependencies.iter() {