From 4aff917ea953b63990e689c93ff904ae5e0821e0 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 2 Jan 2024 19:20:50 -0500 Subject: [PATCH] Avoid building incompatible distributions --- crates/puffin-resolver/src/resolver.rs | 54 ++++++++++++- crates/puffin-resolver/src/version_map.rs | 98 ++++++++--------------- requirements.in | 1 + 3 files changed, 87 insertions(+), 66 deletions(-) create mode 100644 requirements.in diff --git a/crates/puffin-resolver/src/resolver.rs b/crates/puffin-resolver/src/resolver.rs index 5140281ad4202..af82c09778410 100644 --- a/crates/puffin-resolver/src/resolver.rs +++ b/crates/puffin-resolver/src/resolver.rs @@ -22,6 +22,7 @@ use distribution_types::{ BuiltDist, Dist, DistributionMetadata, IndexUrl, LocalEditable, Name, PackageId, SourceDist, VersionOrUrl, }; +use pep440_rs::VersionSpecifiers; use pep508_rs::{MarkerEnvironment, Requirement}; use platform_tags::Tags; use puffin_client::RegistryClient; @@ -494,6 +495,20 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> { return Ok(()); }; + // If the version is incompatible, short-circuit. + if let Some(requires_python) = candidate.resolve().requires_python.as_ref() { + let interpreter_version = self.interpreter.version(); + let marker_version = &self.markers.python_version.version; + if !requires_python.contains(interpreter_version) + || !requires_python.contains(marker_version) + { + self.index + .incompatibilities + .done(candidate.package_id(), requires_python.clone()); + continue; + } + } + // Emit a request to fetch the metadata for this version. if self.index.distributions.register(&candidate.package_id()) { let distribution = candidate.into_distribution(index.clone(), base.clone()); @@ -592,6 +607,20 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> { return Ok(None); }; + // If the version is incompatible, short-circuit. + if let Some(requires_python) = candidate.resolve().requires_python.as_ref() { + let interpreter_version = self.interpreter.version(); + let marker_version = &self.markers.python_version.version; + if !requires_python.contains(interpreter_version) + || !requires_python.contains(marker_version) + { + self.index + .incompatibilities + .done(candidate.package_id(), requires_python.clone()); + return Ok(Some(candidate.version().clone())); + } + } + if let Some(extra) = extra { debug!( "Selecting: {}[{}]=={} ({})", @@ -684,7 +713,25 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> { Some(url) => PubGrubDistribution::from_url(package_name, url), None => PubGrubDistribution::from_registry(package_name, version), }; - let entry = self.index.distributions.wait(&dist.package_id()).await; + let package_id = dist.package_id(); + + // If the package is known to be incompatible, return the Python version as an + // incompatibility, and skip fetching the metadata. + if let Some(entry) = self.index.incompatibilities.get(&package_id) { + let requires_python = entry.value(); + let version = requires_python + .iter() + .map(PubGrubSpecifier::try_from) + .fold_ok(Range::full(), |range, specifier| { + range.intersection(&specifier.into()) + })?; + + let mut constraints = DependencyConstraints::default(); + constraints.insert(PubGrubPackage::Python, version.clone()); + return Ok(Dependencies::Known(constraints)); + } + + let entry = self.index.distributions.wait(&package_id).await; let metadata = entry.value(); let mut constraints = PubGrubDependencies::from_requirements( @@ -901,9 +948,12 @@ pub(crate) struct Index { /// came from. pub(crate) packages: OnceMap, - /// A map from distribution SHA to metadata for that distribution. + /// A map from package ID to metadata for that distribution. pub(crate) distributions: OnceMap, + /// A map from package ID to required Python version. + pub(crate) incompatibilities: OnceMap, + /// A map from source URL to precise URL. pub(crate) redirects: OnceMap, } diff --git a/crates/puffin-resolver/src/version_map.rs b/crates/puffin-resolver/src/version_map.rs index ba88fab0be922..28cac8547c572 100644 --- a/crates/puffin-resolver/src/version_map.rs +++ b/crates/puffin-resolver/src/version_map.rs @@ -67,24 +67,24 @@ impl VersionMap { } } - // When resolving, include `requires_python` when determining version compatibility. - let is_compatible = file - .requires_python - .as_ref() - .map_or(true, |requires_python| { - // The interpreter and marker version are often the same, but can differ. - // For example, if the user is resolving against a target Python version - // passed in via the command line, that version will differ from the - // interpreter version. - let interpreter_version = interpreter.version(); - let marker_version = &markers.python_version.version; - requires_python.contains(interpreter_version) - && requires_python.contains(marker_version) - }); - match filename { DistFilename::WheelFilename(filename) => { - let priority = filename.compatibility(tags).filter(|_| is_compatible); + // To be compatible, the wheel must both have compatible tags _and_ have a + // compatible Python version marker. + let priority = filename.compatibility(tags).filter(|_| { + file.requires_python + .as_ref() + .map_or(true, |requires_python| { + // The interpreter and marker version are often the same, but can differ. + // For example, if the user is resolving against a target Python version + // passed in via the command line, that version will differ from the + // interpreter version. + let interpreter_version = interpreter.version(); + let marker_version = &markers.python_version.version; + requires_python.contains(interpreter_version) + && requires_python.contains(marker_version) + }) + }); match version_map.entry(version.clone().into()) { Entry::Occupied(mut entry) => { entry.get_mut().insert_built(WheelFile(file), priority); @@ -100,15 +100,10 @@ impl VersionMap { DistFilename::SourceDistFilename(_) => { match version_map.entry(version.clone().into()) { Entry::Occupied(mut entry) => { - entry - .get_mut() - .insert_source(SdistFile(file), is_compatible); + entry.get_mut().insert_source(SdistFile(file)); } Entry::Vacant(entry) => { - entry.insert(PrioritizedDistribution::from_source( - SdistFile(file), - is_compatible, - )); + entry.insert(PrioritizedDistribution::from_source(SdistFile(file))); } } } @@ -137,9 +132,7 @@ impl VersionMap { #[derive(Debug)] struct PrioritizedDistribution { /// An arbitrary source distribution for the package version. - compatible_source: Option, - /// An arbitrary source distribution for the package version. - incompatible_source: Option, + source: Option, /// The highest-priority, platform-compatible wheel for the package version. compatible_wheel: Option<(DistFile, TagPriority)>, /// An arbitrary, platform-incompatible wheel for the package version. @@ -151,15 +144,13 @@ impl PrioritizedDistribution { fn from_built(dist: WheelFile, priority: Option) -> Self { if let Some(priority) = priority { Self { - compatible_source: None, - incompatible_source: None, + source: None, compatible_wheel: Some((dist.into(), priority)), incompatible_wheel: None, } } else { Self { - compatible_source: None, - incompatible_source: None, + source: None, compatible_wheel: None, incompatible_wheel: Some(dist.into()), } @@ -167,21 +158,11 @@ impl PrioritizedDistribution { } /// Create a new [`PrioritizedDistribution`] from the given source distribution. - fn from_source(dist: SdistFile, is_compatible: bool) -> Self { - if is_compatible { - Self { - compatible_source: Some(dist.into()), - incompatible_source: None, - compatible_wheel: None, - incompatible_wheel: None, - } - } else { - Self { - compatible_source: None, - incompatible_source: Some(dist.into()), - compatible_wheel: None, - incompatible_wheel: None, - } + fn from_source(dist: SdistFile) -> Self { + Self { + source: Some(dist.into()), + compatible_wheel: None, + incompatible_wheel: None, } } @@ -202,13 +183,9 @@ impl PrioritizedDistribution { } /// Insert the given source distribution into the [`PrioritizedDistribution`]. - fn insert_source(&mut self, file: SdistFile, is_compatible: bool) { - if is_compatible { - if self.compatible_source.is_none() { - self.compatible_source = Some(file.into()); - } - } else if self.incompatible_source.is_none() { - self.incompatible_source = Some(file.into()); + fn insert_source(&mut self, file: SdistFile) { + if self.source.is_none() { + self.source = Some(file.into()); } } @@ -216,25 +193,18 @@ impl PrioritizedDistribution { fn get(&self) -> Option { match ( &self.compatible_wheel, - &self.compatible_source, + &self.source, &self.incompatible_wheel, - &self.incompatible_source, ) { // Prefer the highest-priority, platform-compatible wheel. - (Some((wheel, _)), _, _, _) => Some(ResolvableFile::CompatibleWheel(wheel)), + (Some((wheel, _)), _, _) => Some(ResolvableFile::CompatibleWheel(wheel)), // If we have a compatible source distribution and an incompatible wheel, return the // wheel. We assume that all distributions have the same metadata for a given package // version. If a compatible source distribution exists, we assume we can build it, but // using the wheel is faster. - (_, Some(sdist), Some(wheel), _) => { - Some(ResolvableFile::IncompatibleWheel(sdist, wheel)) - } - // Otherwise, if we have a compatible source distribution, return it. - (_, Some(sdist), _, _) => Some(ResolvableFile::SourceDist(sdist)), - // Otherwise, if we have an incompatible source distribution, return it. We should - // ultimately reject it when resolving, since the incompatibility _must_ be due to a - // mismatch in Python version. - (_, _, _, Some(sdist)) => Some(ResolvableFile::SourceDist(sdist)), + (_, Some(sdist), Some(wheel)) => Some(ResolvableFile::IncompatibleWheel(sdist, wheel)), + // Otherwise, if we have a source distribution, return it. + (_, Some(sdist), _) => Some(ResolvableFile::SourceDist(sdist)), _ => None, } } diff --git a/requirements.in b/requirements.in new file mode 100644 index 0000000000000..24ce15ab7ead3 --- /dev/null +++ b/requirements.in @@ -0,0 +1 @@ +numpy