Respect requires-python when prefetching (#4900)

## Summary

This is fallout from https://github.com/astral-sh/uv/pull/4705. We need
to respect `requires-python` in the prefetch code to avoid building
unsupported distributions.

Closes https://github.com/astral-sh/uv/issues/4898.
This commit is contained in:
Charlie Marsh 2024-07-08 11:32:09 -05:00 committed by GitHub
parent 71c6a9fad3
commit ac3a085084
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 113 additions and 8 deletions

View file

@ -6,13 +6,13 @@ use rustc_hash::FxHashMap;
use tokio::sync::mpsc::Sender;
use tracing::{debug, trace};
use distribution_types::DistributionMetadata;
use distribution_types::{CompatibleDist, DistributionMetadata};
use pep440_rs::Version;
use crate::candidate_selector::{CandidateDist, CandidateSelector};
use crate::candidate_selector::CandidateSelector;
use crate::pubgrub::{PubGrubPackage, PubGrubPackageInner};
use crate::resolver::Request;
use crate::{InMemoryIndex, ResolveError, VersionsResponse};
use crate::{InMemoryIndex, PythonRequirement, ResolveError, VersionsResponse};
enum BatchPrefetchStrategy {
/// Go through the next versions assuming the existing selection and its constraints
@ -49,6 +49,7 @@ impl BatchPrefetcher {
next: &PubGrubPackage,
version: &Version,
current_range: &Range<Version>,
python_requirement: &PythonRequirement,
request_sink: &Sender<Request>,
index: &InMemoryIndex,
selector: &CandidateSelector,
@ -128,7 +129,7 @@ impl BatchPrefetcher {
}
};
let CandidateDist::Compatible(dist) = candidate.dist() else {
let Some(dist) = candidate.compatible() else {
continue;
};
@ -136,6 +137,41 @@ impl BatchPrefetcher {
if !dist.prefetchable() {
continue;
}
// Avoid prefetching for distributions that don't satisfy the Python requirement.
match dist {
CompatibleDist::InstalledDist(_) => {}
CompatibleDist::SourceDist { sdist, .. }
| CompatibleDist::IncompatibleWheel { sdist, .. } => {
// Source distributions must meet both the _target_ Python version and the
// _installed_ Python version (to build successfully).
if let Some(requires_python) = sdist.file.requires_python.as_ref() {
if let Some(target) = python_requirement.target() {
if !target.is_compatible_with(requires_python) {
continue;
}
}
if !requires_python.contains(python_requirement.installed()) {
continue;
}
}
}
CompatibleDist::CompatibleWheel { wheel, .. } => {
// Wheels must meet the _target_ Python version.
if let Some(requires_python) = wheel.file.requires_python.as_ref() {
if let Some(target) = python_requirement.target() {
if !target.is_compatible_with(requires_python) {
continue;
}
} else {
if !requires_python.contains(python_requirement.installed()) {
continue;
}
}
}
}
};
let dist = dist.for_resolution();
// Emit a request to fetch the metadata for this version.

View file

@ -364,6 +364,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
Self::pre_visit(
state.pubgrub.partial_solution.prioritized_packages(),
&self.urls,
&state.python_requirement,
&request_sink,
)?;
}
@ -487,6 +488,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
&state.next,
&version,
term_intersection.unwrap_positive(),
&state.python_requirement,
&request_sink,
&self.index,
&self.selector,
@ -721,6 +723,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
fn pre_visit<'data>(
packages: impl Iterator<Item = (&'data PubGrubPackage, &'data Range<Version>)>,
urls: &Urls,
python_requirement: &PythonRequirement,
request_sink: &Sender<Request>,
) -> Result<(), ResolveError> {
// Iterate over the potential packages, and fetch file metadata for any of them. These
@ -740,7 +743,11 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
if urls.any_url(name) {
continue;
}
request_sink.blocking_send(Request::Prefetch(name.clone(), range.clone()))?;
request_sink.blocking_send(Request::Prefetch(
name.clone(),
range.clone(),
python_requirement.clone(),
))?;
}
Ok(())
}
@ -1656,7 +1663,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
}
// Pre-fetch the package and distribution metadata.
Request::Prefetch(package_name, range) => {
Request::Prefetch(package_name, range, python_requirement) => {
// Wait for the package metadata to become available.
let versions_response = self
.index
@ -1720,6 +1727,39 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
}
}
match dist {
CompatibleDist::InstalledDist(_) => {}
CompatibleDist::SourceDist { sdist, .. }
| CompatibleDist::IncompatibleWheel { sdist, .. } => {
// Source distributions must meet both the _target_ Python version and the
// _installed_ Python version (to build successfully).
if let Some(requires_python) = sdist.file.requires_python.as_ref() {
if let Some(target) = python_requirement.target() {
if !target.is_compatible_with(requires_python) {
return Ok(None);
}
}
if !requires_python.contains(python_requirement.installed()) {
return Ok(None);
}
}
}
CompatibleDist::CompatibleWheel { wheel, .. } => {
// Wheels must meet the _target_ Python version.
if let Some(requires_python) = wheel.file.requires_python.as_ref() {
if let Some(target) = python_requirement.target() {
if !target.is_compatible_with(requires_python) {
return Ok(None);
}
} else {
if !requires_python.contains(python_requirement.installed()) {
return Ok(None);
}
}
}
}
};
// Emit a request to fetch the metadata for this version.
if self.index.distributions().register(candidate.version_id()) {
let dist = dist.for_resolution().to_owned();
@ -2213,7 +2253,7 @@ pub(crate) enum Request {
/// A request to fetch the metadata from an already-installed distribution.
Installed(InstalledDist),
/// A request to pre-fetch the metadata for a package and the best-guess distribution.
Prefetch(PackageName, Range<Version>),
Prefetch(PackageName, Range<Version>, PythonRequirement),
}
impl<'a> From<ResolvedDistRef<'a>> for Request {
@ -2265,7 +2305,7 @@ impl Display for Request {
Self::Installed(dist) => {
write!(f, "Installed metadata {dist}")
}
Self::Prefetch(package_name, range) => {
Self::Prefetch(package_name, range, _) => {
write!(f, "Prefetch {package_name} {range}")
}
}