From e9e3e573a28a4d4f4f30021d981f33e69ab5d8ea Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Thu, 15 Feb 2024 10:48:15 -0600 Subject: [PATCH] Report incompatible distributions to users (#1293) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of dropping versions without a compatible distribution, we track them as incompatibilities in the solver. This implementation follows patterns established in https://github.com/astral-sh/puffin/pull/1290. This required some significant refactoring of how we track incompatible distributions. Notably: - `Option` is now `WheelCompatibility` which allows us to track the reason a wheel is incompatible instead of just `None`. - `Candidate` now has a `CandidateDist` with `Compatible` and `Incompatibile` variants instead of just `ResolvableDist`; candidates are not strictly compatible anymore - `ResolvableDist` was renamed to `CompatibleDist` - `IncompatibleWheel` was given an ordering implementation so we can track the "most compatible" (but still incompatible) wheel. This allows us to collapse the reason a version cannot be used to a single incompatibility. - The filtering in the `VersionMap` is retained, we still only store one incompatible wheel per version. This is sufficient for error reporting. - A `TagCompatibility` type was added for tracking which part of a wheel tag is incompatible - `Candidate::validate_python` moved to `PythonRequirement::validate_dist` I am doing more refactoring in #1298 — I think a couple passes will be necessary to clarify the relationships of these types. Includes improved error message snapshots for multiple incompatible Python tag types from #1285 — we should add more scenarios for coverage of behavior when multiple tags with different levels are present. --- crates/distribution-filename/src/wheel.rs | 7 +- .../src/prioritized_distribution.rs | 269 ++++++++++++------ crates/platform-tags/src/lib.rs | 64 ++++- crates/puffin-client/src/flat_index.rs | 40 +-- .../src/index/built_wheel_index.rs | 2 +- .../src/index/registry_wheel_index.rs | 2 +- .../puffin-resolver/src/candidate_selector.rs | 130 ++++----- crates/puffin-resolver/src/finder.rs | 6 +- crates/puffin-resolver/src/pins.rs | 6 +- .../puffin-resolver/src/python_requirement.rs | 43 ++- crates/puffin-resolver/src/resolver/mod.rs | 80 ++++-- crates/puffin-resolver/src/version_map.rs | 132 ++++----- crates/puffin/tests/pip_compile_scenarios.rs | 2 +- crates/puffin/tests/pip_install_scenarios.rs | 24 +- scripts/scenarios/update.py | 2 +- 15 files changed, 517 insertions(+), 292 deletions(-) diff --git a/crates/distribution-filename/src/wheel.rs b/crates/distribution-filename/src/wheel.rs index 5d2f343c9..403bba011 100644 --- a/crates/distribution-filename/src/wheel.rs +++ b/crates/distribution-filename/src/wheel.rs @@ -7,7 +7,7 @@ use thiserror::Error; use url::Url; use pep440_rs::{Version, VersionParseError}; -use platform_tags::{TagPriority, Tags}; +use platform_tags::{TagCompatibility, Tags}; use puffin_normalize::{InvalidNameError, PackageName}; #[derive(Debug, Clone, Eq, PartialEq, Hash)] @@ -57,9 +57,8 @@ impl WheelFilename { compatible_tags.is_compatible(&self.python_tag, &self.abi_tag, &self.platform_tag) } - /// Return the [`TagPriority`] score of the wheel with the given tags, or `None` if the wheel is - /// incompatible. - pub fn compatibility(&self, compatible_tags: &Tags) -> Option { + /// Return the [`TagCompatibility`] of the wheel with the given tags + pub fn compatibility(&self, compatible_tags: &Tags) -> TagCompatibility { compatible_tags.compatibility(&self.python_tag, &self.abi_tag, &self.platform_tag) } diff --git a/crates/distribution-types/src/prioritized_distribution.rs b/crates/distribution-types/src/prioritized_distribution.rs index daaedc450..fff9ea2de 100644 --- a/crates/distribution-types/src/prioritized_distribution.rs +++ b/crates/distribution-types/src/prioritized_distribution.rs @@ -1,48 +1,78 @@ use pep440_rs::VersionSpecifiers; -use platform_tags::TagPriority; +use platform_tags::{IncompatibleTag, TagCompatibility, TagPriority}; use pypi_types::{Hashes, Yanked}; use crate::Dist; +/// A collection of distributions that have been filtered by relevance. +#[derive(Debug, Default, Clone)] +pub struct PrioritizedDist(Box); + +/// [`PrioritizedDist`] is boxed because [`Dist`] is large. +#[derive(Debug, Default, Clone)] +struct PrioritizedDistInner { + /// An arbitrary source distribution for the package version. + source: Option, + /// The highest-priority, installable wheel for the package version. + compatible_wheel: Option<(DistMetadata, TagPriority)>, + /// The most-relevant, incompatible wheel for the package version. + incompatible_wheel: Option<(DistMetadata, IncompatibleWheel)>, + /// The hashes for each distribution. + hashes: Vec, + /// If exclude newer filtered files from this distribution + exclude_newer: bool, +} + +/// A distribution that can be used for both resolution and installation. +#[derive(Debug, Clone)] +pub enum CompatibleDist<'a> { + /// The distribution should be resolved and installed using a source distribution. + SourceDist(&'a DistMetadata), + /// The distribution should be resolved and installed using a wheel distribution. + CompatibleWheel(&'a DistMetadata, TagPriority), + /// The distribution should be resolved using an incompatible wheel distribution, but + /// installed using a source distribution. + IncompatibleWheel { + source_dist: &'a DistMetadata, + wheel: &'a DistMetadata, + }, +} + +#[derive(Debug, PartialEq, Eq)] +pub enum WheelCompatibility { + Incompatible(IncompatibleWheel), + Compatible(TagPriority), +} + +#[derive(Debug, PartialEq, Eq, Ord, PartialOrd, Clone)] +pub enum IncompatibleWheel { + Tag(IncompatibleTag), + RequiresPython, + NoBinary, +} + /// A [`Dist`] and metadata about it required for downstream filtering. #[derive(Debug, Clone)] pub struct DistMetadata { + /// The distribution. pub dist: Dist, - - /// The version of Python required by the distribution + /// The version of Python required by the distribution. pub requires_python: Option, - - /// Is the distribution file yanked? + /// If the distribution file is yanked. pub yanked: Yanked, } -// Boxed because `Dist` is large. -#[derive(Debug, Default, Clone)] -pub struct PrioritizedDistribution(Box); - -#[derive(Debug, Default, Clone)] -struct PrioritizedDistributionInner { - /// An arbitrary source distribution for the package version. - source: Option, - /// The highest-priority, platform-compatible wheel for the package version. - compatible_wheel: Option<(DistMetadata, TagPriority)>, - /// An arbitrary, platform-incompatible wheel for the package version. - incompatible_wheel: Option, - /// The hashes for each distribution. - hashes: Vec, -} - -impl PrioritizedDistribution { - /// Create a new [`PrioritizedDistribution`] from the given wheel distribution. +impl PrioritizedDist { + /// Create a new [`PrioritizedDist`] from the given wheel distribution. pub fn from_built( dist: Dist, requires_python: Option, yanked: Yanked, hash: Option, - priority: Option, + compatibility: WheelCompatibility, ) -> Self { - if let Some(priority) = priority { - Self(Box::new(PrioritizedDistributionInner { + match compatibility { + WheelCompatibility::Compatible(priority) => Self(Box::new(PrioritizedDistInner { source: None, compatible_wheel: Some(( DistMetadata { @@ -54,29 +84,35 @@ impl PrioritizedDistribution { )), incompatible_wheel: None, hashes: hash.map(|hash| vec![hash]).unwrap_or_default(), - })) - } else { - Self(Box::new(PrioritizedDistributionInner { - source: None, - compatible_wheel: None, - incompatible_wheel: Some(DistMetadata { - dist, - requires_python, - yanked, - }), - hashes: hash.map(|hash| vec![hash]).unwrap_or_default(), - })) + exclude_newer: false, + })), + WheelCompatibility::Incompatible(incompatibility) => { + Self(Box::new(PrioritizedDistInner { + source: None, + compatible_wheel: None, + incompatible_wheel: Some(( + DistMetadata { + dist, + requires_python, + yanked, + }, + incompatibility, + )), + hashes: hash.map(|hash| vec![hash]).unwrap_or_default(), + exclude_newer: false, + })) + } } } - /// Create a new [`PrioritizedDistribution`] from the given source distribution. + /// Create a new [`PrioritizedDist`] from the given source distribution. pub fn from_source( dist: Dist, requires_python: Option, yanked: Yanked, hash: Option, ) -> Self { - Self(Box::new(PrioritizedDistributionInner { + Self(Box::new(PrioritizedDistInner { source: Some(DistMetadata { dist, requires_python, @@ -85,22 +121,34 @@ impl PrioritizedDistribution { compatible_wheel: None, incompatible_wheel: None, hashes: hash.map(|hash| vec![hash]).unwrap_or_default(), + exclude_newer: false, })) } - /// Insert the given built distribution into the [`PrioritizedDistribution`]. + /// Insert the given built distribution into the [`PrioritizedDist`]. pub fn insert_built( &mut self, dist: Dist, requires_python: Option, yanked: Yanked, hash: Option, - priority: Option, + compatibility: WheelCompatibility, ) { - // Prefer the highest-priority, platform-compatible wheel. - if let Some(priority) = priority { - if let Some((.., existing_priority)) = &self.0.compatible_wheel { - if priority > *existing_priority { + match compatibility { + // Prefer the highest-priority, compatible wheel. + WheelCompatibility::Compatible(priority) => { + if let Some((.., existing_priority)) = &self.0.compatible_wheel { + if priority > *existing_priority { + self.0.compatible_wheel = Some(( + DistMetadata { + dist, + requires_python, + yanked, + }, + priority, + )); + } + } else { self.0.compatible_wheel = Some(( DistMetadata { dist, @@ -110,22 +158,31 @@ impl PrioritizedDistribution { priority, )); } - } else { - self.0.compatible_wheel = Some(( - DistMetadata { - dist, - requires_python, - yanked, - }, - priority, - )); } - } else if self.0.incompatible_wheel.is_none() { - self.0.incompatible_wheel = Some(DistMetadata { - dist, - requires_python, - yanked, - }); + // Track the most relevant incompatible wheel + WheelCompatibility::Incompatible(incompatibility) => { + if let Some((.., existing_incompatibility)) = &self.0.incompatible_wheel { + if incompatibility > *existing_incompatibility { + self.0.incompatible_wheel = Some(( + DistMetadata { + dist, + requires_python, + yanked, + }, + incompatibility, + )); + } + } else { + self.0.incompatible_wheel = Some(( + DistMetadata { + dist, + requires_python, + yanked, + }, + incompatibility, + )); + } + } } if let Some(hash) = hash { @@ -133,7 +190,7 @@ impl PrioritizedDistribution { } } - /// Insert the given source distribution into the [`PrioritizedDistribution`]. + /// Insert the given source distribution into the [`PrioritizedDist`]. pub fn insert_source( &mut self, dist: Dist, @@ -155,7 +212,7 @@ impl PrioritizedDistribution { } /// Return the highest-priority distribution for the package version, if any. - pub fn get(&self) -> Option { + pub fn get(&self) -> Option { match ( &self.0.compatible_wheel, &self.0.source, @@ -163,17 +220,17 @@ impl PrioritizedDistribution { ) { // Prefer the highest-priority, platform-compatible wheel. (Some((wheel, tag_priority)), _, _) => { - Some(ResolvableDist::CompatibleWheel(wheel, *tag_priority)) + Some(CompatibleDist::CompatibleWheel(wheel, *tag_priority)) } // 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(source_dist), Some(wheel)) => { - Some(ResolvableDist::IncompatibleWheel { source_dist, wheel }) + (_, Some(source_dist), Some((wheel, _))) => { + Some(CompatibleDist::IncompatibleWheel { source_dist, wheel }) } // Otherwise, if we have a source distribution, return it. - (_, Some(source_dist), _) => Some(ResolvableDist::SourceDist(source_dist)), + (_, Some(source_dist), _) => Some(CompatibleDist::SourceDist(source_dist)), _ => None, } } @@ -188,6 +245,21 @@ impl PrioritizedDistribution { self.0.compatible_wheel.as_ref() } + /// Return the incompatible built distribution, if any. + pub fn incompatible_wheel(&self) -> Option<&(DistMetadata, IncompatibleWheel)> { + self.0.incompatible_wheel.as_ref() + } + + /// Set the `exclude_newer` flag + pub fn set_exclude_newer(&mut self) { + self.0.exclude_newer = true; + } + + /// Check if any distributions were excluded by the `exclude_newer` option + pub fn exclude_newer(&self) -> bool { + self.0.exclude_newer + } + /// Return the hashes for each distribution. pub fn hashes(&self) -> &[Hashes] { &self.0.hashes @@ -202,28 +274,13 @@ impl PrioritizedDistribution { } } -/// A collection of distributions ([`Dist`]) that can be used for resolution and installation. -#[derive(Debug, Clone)] -pub enum ResolvableDist<'a> { - /// The distribution should be resolved and installed using a source distribution. - SourceDist(&'a DistMetadata), - /// The distribution should be resolved and installed using a wheel distribution. - CompatibleWheel(&'a DistMetadata, TagPriority), - /// The distribution should be resolved using an incompatible wheel distribution, but - /// installed using a source distribution. - IncompatibleWheel { - source_dist: &'a DistMetadata, - wheel: &'a DistMetadata, - }, -} - -impl<'a> ResolvableDist<'a> { +impl<'a> CompatibleDist<'a> { /// Return the [`DistMetadata`] to use during resolution. pub fn for_resolution(&self) -> &DistMetadata { match *self { - ResolvableDist::SourceDist(sdist) => sdist, - ResolvableDist::CompatibleWheel(wheel, _) => wheel, - ResolvableDist::IncompatibleWheel { + CompatibleDist::SourceDist(sdist) => sdist, + CompatibleDist::CompatibleWheel(wheel, _) => wheel, + CompatibleDist::IncompatibleWheel { source_dist: _, wheel, } => wheel, @@ -233,9 +290,9 @@ impl<'a> ResolvableDist<'a> { /// Return the [`DistMetadata`] to use during installation. pub fn for_installation(&self) -> &DistMetadata { match *self { - ResolvableDist::SourceDist(sdist) => sdist, - ResolvableDist::CompatibleWheel(wheel, _) => wheel, - ResolvableDist::IncompatibleWheel { + CompatibleDist::SourceDist(sdist) => sdist, + CompatibleDist::CompatibleWheel(wheel, _) => wheel, + CompatibleDist::IncompatibleWheel { source_dist, wheel: _, } => source_dist, @@ -255,3 +312,37 @@ impl<'a> ResolvableDist<'a> { &self.for_installation().yanked } } + +impl Ord for WheelCompatibility { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + match (self, other) { + (Self::Compatible(p_self), Self::Compatible(p_other)) => p_self.cmp(p_other), + (Self::Incompatible(_), Self::Compatible(_)) => std::cmp::Ordering::Less, + (Self::Compatible(_), Self::Incompatible(_)) => std::cmp::Ordering::Greater, + (Self::Incompatible(t_self), Self::Incompatible(t_other)) => t_self.cmp(t_other), + } + } +} + +impl PartialOrd for WheelCompatibility { + fn partial_cmp(&self, other: &Self) -> Option { + Some(WheelCompatibility::cmp(self, other)) + } +} + +impl WheelCompatibility { + pub fn is_compatible(&self) -> bool { + matches!(self, Self::Compatible(_)) + } +} + +impl From for WheelCompatibility { + fn from(value: TagCompatibility) -> Self { + match value { + TagCompatibility::Compatible(priority) => WheelCompatibility::Compatible(priority), + TagCompatibility::Incompatible(tag) => { + WheelCompatibility::Incompatible(IncompatibleWheel::Tag(tag)) + } + } + } +} diff --git a/crates/platform-tags/src/lib.rs b/crates/platform-tags/src/lib.rs index 6bbd4c285..d9e98a9fc 100644 --- a/crates/platform-tags/src/lib.rs +++ b/crates/platform-tags/src/lib.rs @@ -1,6 +1,6 @@ -use std::num::NonZeroU32; use std::str::FromStr; use std::sync::Arc; +use std::{cmp, num::NonZeroU32}; use rustc_hash::FxHashMap; @@ -18,6 +18,43 @@ pub enum TagsError { InvalidPriority(usize, #[source] std::num::TryFromIntError), } +#[derive(Debug, Eq, Ord, PartialEq, PartialOrd, Clone)] +pub enum IncompatibleTag { + Invalid, + Python, + Abi, + Platform, +} + +#[derive(Debug, PartialEq, Eq)] +pub enum TagCompatibility { + Incompatible(IncompatibleTag), + Compatible(TagPriority), +} + +impl Ord for TagCompatibility { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + match (self, other) { + (Self::Compatible(p_self), Self::Compatible(p_other)) => p_self.cmp(p_other), + (Self::Incompatible(_), Self::Compatible(_)) => cmp::Ordering::Less, + (Self::Compatible(_), Self::Incompatible(_)) => cmp::Ordering::Greater, + (Self::Incompatible(t_self), Self::Incompatible(t_other)) => t_self.cmp(t_other), + } + } +} + +impl PartialOrd for TagCompatibility { + fn partial_cmp(&self, other: &Self) -> Option { + Some(TagCompatibility::cmp(self, other)) + } +} + +impl TagCompatibility { + pub fn is_compatible(&self) -> bool { + matches!(self, Self::Compatible(_)) + } +} + /// A set of compatible tags for a given Python version and platform. /// /// Its principle function is to determine whether the tags for a particular @@ -157,30 +194,43 @@ impl Tags { false } - /// Returns the [`TagPriority`] of the most-compatible platform tag, or `None` if there is no - /// compatible tag. + /// Returns the [`TagCompatiblity`] of the given tags. + /// + /// If compatible, includes the score of the most-compatible platform tag. + /// If incompatible, includes the tag part which was a closest match. pub fn compatibility( &self, wheel_python_tags: &[String], wheel_abi_tags: &[String], wheel_platform_tags: &[String], - ) -> Option { - let mut max_priority = None; + ) -> TagCompatibility { + let mut max_compatibility = TagCompatibility::Incompatible(IncompatibleTag::Invalid); + for wheel_py in wheel_python_tags { let Some(abis) = self.map.get(wheel_py) else { + max_compatibility = + max_compatibility.max(TagCompatibility::Incompatible(IncompatibleTag::Python)); continue; }; for wheel_abi in wheel_abi_tags { let Some(platforms) = abis.get(wheel_abi) else { + max_compatibility = + max_compatibility.max(TagCompatibility::Incompatible(IncompatibleTag::Abi)); continue; }; for wheel_platform in wheel_platform_tags { let priority = platforms.get(wheel_platform).copied(); - max_priority = max_priority.max(priority); + if let Some(priority) = priority { + max_compatibility = + max_compatibility.max(TagCompatibility::Compatible(priority)); + } else { + max_compatibility = max_compatibility + .max(TagCompatibility::Incompatible(IncompatibleTag::Platform)); + } } } } - max_priority + max_compatibility } } diff --git a/crates/puffin-client/src/flat_index.rs b/crates/puffin-client/src/flat_index.rs index 95bc64f2c..bb36a55df 100644 --- a/crates/puffin-client/src/flat_index.rs +++ b/crates/puffin-client/src/flat_index.rs @@ -10,7 +10,7 @@ use url::Url; use distribution_filename::DistFilename; use distribution_types::{ - BuiltDist, Dist, File, FileLocation, FlatIndexLocation, IndexUrl, PrioritizedDistribution, + BuiltDist, Dist, File, FileLocation, FlatIndexLocation, IndexUrl, PrioritizedDist, RegistryBuiltDist, RegistrySourceDist, SourceDist, }; use pep440_rs::Version; @@ -249,7 +249,7 @@ impl<'a> FlatIndexClient<'a> { } } -/// A set of [`PrioritizedDistribution`] from a `--find-links` entry, indexed by [`PackageName`] +/// A set of [`PrioritizedDist`] from a `--find-links` entry, indexed by [`PackageName`] /// and [`Version`]. #[derive(Debug, Clone, Default)] pub struct FlatIndex { @@ -288,7 +288,7 @@ impl FlatIndex { // for wheels, we read it lazily only when selected. match filename { DistFilename::WheelFilename(filename) => { - let priority = filename.compatibility(tags); + let compatibility = filename.compatibility(tags); let version = filename.version.clone(); let dist = Dist::Built(BuiltDist::Registry(RegistryBuiltDist { @@ -298,17 +298,21 @@ impl FlatIndex { })); match distributions.0.entry(version) { Entry::Occupied(mut entry) => { - entry - .get_mut() - .insert_built(dist, None, Yanked::default(), None, priority); - } - Entry::Vacant(entry) => { - entry.insert(PrioritizedDistribution::from_built( + entry.get_mut().insert_built( dist, None, Yanked::default(), None, - priority, + compatibility.into(), + ); + } + Entry::Vacant(entry) => { + entry.insert(PrioritizedDist::from_built( + dist, + None, + Yanked::default(), + None, + compatibility.into(), )); } } @@ -326,7 +330,7 @@ impl FlatIndex { .insert_source(dist, None, Yanked::default(), None); } Entry::Vacant(entry) => { - entry.insert(PrioritizedDistribution::from_source( + entry.insert(PrioritizedDist::from_source( dist, None, Yanked::default(), @@ -349,31 +353,31 @@ impl FlatIndex { } } -/// A set of [`PrioritizedDistribution`] from a `--find-links` entry for a single package, indexed +/// A set of [`PrioritizedDist`] from a `--find-links` entry for a single package, indexed /// by [`Version`]. #[derive(Debug, Clone, Default)] -pub struct FlatDistributions(BTreeMap); +pub struct FlatDistributions(BTreeMap); impl FlatDistributions { - pub fn iter(&self) -> impl Iterator { + pub fn iter(&self) -> impl Iterator { self.0.iter() } - pub fn remove(&mut self, version: &Version) -> Option { + pub fn remove(&mut self, version: &Version) -> Option { self.0.remove(version) } } impl IntoIterator for FlatDistributions { - type Item = (Version, PrioritizedDistribution); - type IntoIter = std::collections::btree_map::IntoIter; + type Item = (Version, PrioritizedDist); + type IntoIter = std::collections::btree_map::IntoIter; fn into_iter(self) -> Self::IntoIter { self.0.into_iter() } } -impl From for BTreeMap { +impl From for BTreeMap { fn from(distributions: FlatDistributions) -> Self { distributions.0 } diff --git a/crates/puffin-distribution/src/index/built_wheel_index.rs b/crates/puffin-distribution/src/index/built_wheel_index.rs index 5beabaafe..b644cd4d5 100644 --- a/crates/puffin-distribution/src/index/built_wheel_index.rs +++ b/crates/puffin-distribution/src/index/built_wheel_index.rs @@ -106,7 +106,7 @@ impl BuiltWheelIndex { let compatibility = dist_info.filename.compatibility(tags); // Only consider wheels that are compatible with our tags. - if compatibility.is_none() { + if !compatibility.is_compatible() { continue; } diff --git a/crates/puffin-distribution/src/index/registry_wheel_index.rs b/crates/puffin-distribution/src/index/registry_wheel_index.rs index e2524b5bc..ccd87c463 100644 --- a/crates/puffin-distribution/src/index/registry_wheel_index.rs +++ b/crates/puffin-distribution/src/index/registry_wheel_index.rs @@ -139,7 +139,7 @@ impl<'a> RegistryWheelIndex<'a> { if compatibility > existing.filename.compatibility(tags) { *existing = dist_info; } - } else if compatibility.is_some() { + } else if compatibility.is_compatible() { versions.insert(dist_info.filename.version.clone(), dist_info); } } diff --git a/crates/puffin-resolver/src/candidate_selector.rs b/crates/puffin-resolver/src/candidate_selector.rs index a18dbf89b..10e93e3ce 100644 --- a/crates/puffin-resolver/src/candidate_selector.rs +++ b/crates/puffin-resolver/src/candidate_selector.rs @@ -1,15 +1,15 @@ use pubgrub::range::Range; -use pypi_types::Yanked; + use rustc_hash::FxHashMap; -use distribution_types::{Dist, DistributionMetadata, Name}; -use distribution_types::{DistMetadata, ResolvableDist}; -use pep440_rs::{Version, VersionSpecifiers}; +use distribution_types::CompatibleDist; +use distribution_types::{DistributionMetadata, IncompatibleWheel, Name, PrioritizedDist}; +use pep440_rs::Version; use pep508_rs::{Requirement, VersionOrUrl}; use puffin_normalize::PackageName; use crate::prerelease_mode::PreReleaseStrategy; -use crate::python_requirement::PythonRequirement; + use crate::resolution_mode::ResolutionStrategy; use crate::version_map::{VersionMap, VersionMapDistHandle}; use crate::{Manifest, Options}; @@ -177,18 +177,19 @@ impl CandidateSelector { #[derive(Debug)] enum PreReleaseCandidate<'a> { NotNecessary, - IfNecessary(&'a Version, ResolvableDist<'a>), + IfNecessary(&'a Version, &'a PrioritizedDist), } let mut prerelease = None; let mut steps = 0; for (version, maybe_dist) in versions { steps += 1; - if version.any_prerelease() { + + let dist = if version.any_prerelease() { if range.contains(version) { match allow_prerelease { AllowPreRelease::Yes => { - let Some(dist) = maybe_dist.resolvable_dist() else { + let Some(dist) = maybe_dist.prioritized_dist() else { continue; }; tracing::trace!( @@ -201,10 +202,10 @@ impl CandidateSelector { ); // If pre-releases are allowed, treat them equivalently // to stable distributions. - return Some(Candidate::new(package_name, version, dist)); + dist } AllowPreRelease::IfNecessary => { - let Some(dist) = maybe_dist.resolvable_dist() else { + let Some(dist) = maybe_dist.prioritized_dist() else { continue; }; // If pre-releases are allowed as a fallback, store the @@ -212,11 +213,14 @@ impl CandidateSelector { if prerelease.is_none() { prerelease = Some(PreReleaseCandidate::IfNecessary(version, dist)); } + continue; } AllowPreRelease::No => { continue; } } + } else { + continue; } } else { // If we have at least one stable release, we shouldn't allow the "if-necessary" @@ -226,7 +230,7 @@ impl CandidateSelector { // Always return the first-matching stable distribution. if range.contains(version) { - let Some(dist) = maybe_dist.resolvable_dist() else { + let Some(dist) = maybe_dist.prioritized_dist() else { continue; }; tracing::trace!( @@ -237,9 +241,18 @@ impl CandidateSelector { steps, version, ); - return Some(Candidate::new(package_name, version, dist)); + dist + } else { + continue; } + }; + + // Skip empty candidates due to exclude newer + if dist.exclude_newer() && dist.incompatible_wheel().is_none() && dist.get().is_none() { + continue; } + + return Some(Candidate::new(package_name, version, dist)); } tracing::trace!( "exhausted all candidates for package {:?} with range {:?} \ @@ -258,22 +271,48 @@ impl CandidateSelector { } } +#[derive(Debug, Clone)] +pub(crate) enum CandidateDist<'a> { + Compatible(CompatibleDist<'a>), + Incompatible(Option<&'a IncompatibleWheel>), + ExcludeNewer, +} + +impl<'a> From<&'a PrioritizedDist> for CandidateDist<'a> { + fn from(value: &'a PrioritizedDist) -> Self { + if let Some(dist) = value.get() { + CandidateDist::Compatible(dist) + } else { + if value.exclude_newer() && value.incompatible_wheel().is_none() { + // If empty because of exclude-newer, mark as a special case + CandidateDist::ExcludeNewer + } else { + CandidateDist::Incompatible( + value + .incompatible_wheel() + .map(|(_, incompatibility)| incompatibility), + ) + } + } + } +} + #[derive(Debug, Clone)] pub(crate) struct Candidate<'a> { /// The name of the package. name: &'a PackageName, /// The version of the package. version: &'a Version, - /// The file to use for resolving and installing the package. - dist: ResolvableDist<'a>, + /// The distributions to use for resolving and installing the package. + dist: CandidateDist<'a>, } impl<'a> Candidate<'a> { - fn new(name: &'a PackageName, version: &'a Version, dist: ResolvableDist<'a>) -> Self { + fn new(name: &'a PackageName, version: &'a Version, dist: &'a PrioritizedDist) -> Self { Self { name, version, - dist, + dist: CandidateDist::from(dist), } } @@ -287,57 +326,18 @@ impl<'a> Candidate<'a> { self.version } - /// Return the [`DistFile`] to use when resolving the package. - pub(crate) fn resolution_dist(&self) -> &DistMetadata { - self.dist.for_resolution() + /// Return the distribution for the package, if compatible. + pub(crate) fn compatible(&self) -> Option<&CompatibleDist<'a>> { + if let CandidateDist::Compatible(ref dist) = self.dist { + Some(dist) + } else { + None + } } - /// Return the [`DistFile`] to use when installing the package. - pub(crate) fn installation_dist(&self) -> &DistMetadata { - self.dist.for_installation() - } - - /// If the candidate doesn't match the given Python requirement, return the version specifiers. - pub(crate) fn validate_python( - &self, - requirement: &PythonRequirement, - ) -> Option<&VersionSpecifiers> { - // Validate the _installed_ file. - let requires_python = self.installation_dist().requires_python.as_ref()?; - - // If the candidate doesn't support the target Python version, return the failing version - // specifiers. - if !requires_python.contains(requirement.target()) { - return Some(requires_python); - } - - // If the candidate is a source distribution, and doesn't support the installed Python - // version, return the failing version specifiers, since we won't be able to build it. - if matches!(self.installation_dist().dist, Dist::Source(_)) { - if !requires_python.contains(requirement.installed()) { - return Some(requires_python); - } - } - - // Validate the resolved file. - let requires_python = self.resolution_dist().requires_python.as_ref()?; - - // If the candidate is a source distribution, and doesn't support the installed Python - // version, return the failing version specifiers, since we won't be able to build it. - // This isn't strictly necessary, since if `self.resolve()` is a source distribution, it - // should be the same file as `self.install()` (validated above). - if matches!(self.resolution_dist().dist, Dist::Source(_)) { - if !requires_python.contains(requirement.installed()) { - return Some(requires_python); - } - } - - None - } - - /// If the distribution that would be installed is yanked. - pub(crate) fn yanked(&self) -> &Yanked { - self.dist.yanked() + /// Return the distribution for the candidate. + pub(crate) fn dist(&self) -> &CandidateDist<'a> { + &self.dist } } diff --git a/crates/puffin-resolver/src/finder.rs b/crates/puffin-resolver/src/finder.rs index 93bb8ac77..97305056c 100644 --- a/crates/puffin-resolver/src/finder.rs +++ b/crates/puffin-resolver/src/finder.rs @@ -10,7 +10,7 @@ use rustc_hash::FxHashMap; use distribution_filename::DistFilename; use distribution_types::{Dist, IndexUrl, Resolution}; use pep508_rs::{Requirement, VersionOrUrl}; -use platform_tags::Tags; +use platform_tags::{TagCompatibility, Tags}; use puffin_client::{ FlatDistributions, FlatIndex, OwnedArchive, RegistryClient, SimpleMetadata, SimpleMetadatum, }; @@ -192,7 +192,9 @@ impl<'a> DistFinder<'a> { } best_version = Some(version.clone()); - if let Some(priority) = version_wheel.name.compatibility(self.tags) { + if let TagCompatibility::Compatible(priority) = + version_wheel.name.compatibility(self.tags) + { if best_wheel .as_ref() .map_or(true, |(.., existing)| priority > *existing) diff --git a/crates/puffin-resolver/src/pins.rs b/crates/puffin-resolver/src/pins.rs index 49e83cbc6..428f5074f 100644 --- a/crates/puffin-resolver/src/pins.rs +++ b/crates/puffin-resolver/src/pins.rs @@ -1,6 +1,6 @@ use rustc_hash::FxHashMap; -use distribution_types::Dist; +use distribution_types::{CompatibleDist, Dist}; use puffin_normalize::PackageName; use crate::candidate_selector::Candidate; @@ -14,10 +14,10 @@ pub(crate) struct FilePins(FxHashMap &Version { &self.target } + + /// If the dist doesn't match the given Python requirement, return the version specifiers. + pub(crate) fn validate_dist<'a>( + &self, + dist: &'a CompatibleDist, + ) -> Option<&'a VersionSpecifiers> { + // Validate the _installed_ file. + let requires_python = dist.for_installation().requires_python.as_ref()?; + + // If the dist doesn't support the target Python version, return the failing version + // specifiers. + if !requires_python.contains(self.target()) { + return Some(requires_python); + } + + // If the dist is a source distribution, and doesn't support the installed Python + // version, return the failing version specifiers, since we won't be able to build it. + if matches!(dist.for_installation().dist, Dist::Source(_)) { + if !requires_python.contains(self.installed()) { + return Some(requires_python); + } + } + + // Validate the resolved file. + let requires_python = dist.for_resolution().requires_python.as_ref()?; + + // If the dist is a source distribution, and doesn't support the installed Python + // version, return the failing version specifiers, since we won't be able to build it. + // This isn't strictly necessary, since if `dist.resolve_metadata()` is a source distribution, it + // should be the same file as `dist.install_metadata()` (validated above). + if matches!(dist.for_resolution().dist, Dist::Source(_)) { + if !requires_python.contains(self.installed()) { + return Some(requires_python); + } + } + + None + } } diff --git a/crates/puffin-resolver/src/resolver/mod.rs b/crates/puffin-resolver/src/resolver/mod.rs index 731e97ccf..d202b57e4 100644 --- a/crates/puffin-resolver/src/resolver/mod.rs +++ b/crates/puffin-resolver/src/resolver/mod.rs @@ -20,12 +20,12 @@ use url::Url; use distribution_filename::WheelFilename; use distribution_types::{ - BuiltDist, Dist, DistributionMetadata, LocalEditable, Name, RemoteSource, SourceDist, - VersionOrUrl, + BuiltDist, Dist, DistributionMetadata, IncompatibleWheel, LocalEditable, Name, RemoteSource, + SourceDist, VersionOrUrl, }; use pep440_rs::{Version, VersionSpecifiers, MIN_VERSION}; use pep508_rs::{MarkerEnvironment, Requirement}; -use platform_tags::Tags; +use platform_tags::{IncompatibleTag, Tags}; use puffin_client::{FlatIndex, RegistryClient}; use puffin_distribution::DistributionDatabase; use puffin_interpreter::Interpreter; @@ -33,7 +33,7 @@ use puffin_normalize::PackageName; use puffin_traits::BuildContext; use pypi_types::{Metadata21, Yanked}; -use crate::candidate_selector::CandidateSelector; +use crate::candidate_selector::{CandidateDist, CandidateSelector}; use crate::error::ResolveError; use crate::manifest::Manifest; use crate::overrides::Overrides; @@ -67,6 +67,8 @@ pub(crate) enum UnavailableVersion { RequiresPython(VersionSpecifiers), /// Version is incompatible because it is yanked Yanked(Yanked), + /// Version is incompatible because it has no usable distributions + NoDistributions(Option), } /// The package is unavailable and cannot be used @@ -413,6 +415,25 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> { reason.trim().trim_end_matches('.') ), }, + UnavailableVersion::NoDistributions(best_incompatible) => { + if let Some(best_incompatible) = best_incompatible { + match best_incompatible { + IncompatibleWheel::NoBinary => "no source distribution is available and using wheels is disabled".to_string(), + IncompatibleWheel::RequiresPython => "no wheels are available that meet your required Python version".to_string(), + IncompatibleWheel::Tag(tag) => { + match tag { + IncompatibleTag::Invalid => "no wheels are available with valid tags".to_string(), + IncompatibleTag::Python => "no wheels are available with a matching Python implementation".to_string(), + IncompatibleTag::Abi => "no wheels are available with a matching Python ABI".to_string(), + IncompatibleTag::Platform => "no wheels are available with a matching platform".to_string(), + } + } + } + } else { + // TODO(zanieb): It's unclear why we would encounter this case still + "no wheels are available for your system".to_string() + } + } }; state.add_incompatibility(Incompatibility::unavailable( next.clone(), @@ -654,14 +675,29 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> { debug!("Searching for a compatible version of {package_name} ({range})"); } - // Find a compatible version. + // Find a version. let Some(candidate) = self.selector.select(package_name, range, version_map) else { - // Short circuit: we couldn't find _any_ compatible versions for a package. + // Short circuit: we couldn't find _any_ versions for a package. return Ok(None); }; - // If the version is incompatible because it was yanked - if candidate.yanked().is_yanked() { + let dist = match candidate.dist() { + CandidateDist::Compatible(dist) => dist, + CandidateDist::ExcludeNewer => { + // If the version is incomatible because of `exclude_newer`, pretend the versions do not exist + return Ok(None); + } + CandidateDist::Incompatible(incompatibility) => { + // If the version is incompatible because no distributions match, exit early. + return Ok(Some(ResolverVersion::Unavailable( + candidate.version().clone(), + UnavailableVersion::NoDistributions(incompatibility.cloned()), + ))); + } + }; + + // If the version is incompatible because it was yanked, exit early. + if dist.yanked().is_yanked() { if self .allowed_yanks .allowed(package_name, candidate.version()) @@ -670,13 +706,13 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> { } else { return Ok(Some(ResolverVersion::Unavailable( candidate.version().clone(), - UnavailableVersion::Yanked(candidate.yanked().clone()), + UnavailableVersion::Yanked(dist.yanked().clone()), ))); } } // If the version is incompatible because of its Python requirement - if let Some(requires_python) = candidate.validate_python(&self.python_requirement) { + if let Some(requires_python) = self.python_requirement.validate_dist(dist) { return Ok(Some(ResolverVersion::Unavailable( candidate.version().clone(), UnavailableVersion::RequiresPython(requires_python.clone()), @@ -689,8 +725,7 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> { candidate.name(), extra, candidate.version(), - candidate - .resolution_dist() + dist.for_resolution() .dist .filename() .unwrap_or("unknown filename") @@ -700,8 +735,7 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> { "Selecting: {}=={} ({})", candidate.name(), candidate.version(), - candidate - .resolution_dist() + dist.for_resolution() .dist .filename() .unwrap_or("unknown filename") @@ -710,13 +744,13 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> { // We want to return a package pinned to a specific version; but we _also_ want to // store the exact file that we selected to satisfy that version. - pins.insert(&candidate); + pins.insert(&candidate, dist); let version = candidate.version().clone(); // Emit a request to fetch the metadata for this version. if self.index.distributions.register(candidate.package_id()) { - let dist = candidate.resolution_dist().dist.clone(); + let dist = dist.for_resolution().dist.clone(); request_sink.send(Request::Dist(dist)).await?; } @@ -989,17 +1023,19 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> { return Ok(None); }; - // If the version is incompatible, short-circuit. - if candidate - .validate_python(&self.python_requirement) - .is_some() - { + // If there is not a compatible distribution, short-circuit. + let Some(dist) = candidate.compatible() else { + return Ok(None); + }; + + // If the Python version is incompatible, short-circuit. + if self.python_requirement.validate_dist(dist).is_some() { return Ok(None); } // Emit a request to fetch the metadata for this version. if self.index.distributions.register(candidate.package_id()) { - let dist = candidate.resolution_dist().dist.clone(); + let dist = dist.for_resolution().dist.clone(); let (metadata, precise) = self .provider diff --git a/crates/puffin-resolver/src/version_map.rs b/crates/puffin-resolver/src/version_map.rs index 1a0b08591..1d2793c84 100644 --- a/crates/puffin-resolver/src/version_map.rs +++ b/crates/puffin-resolver/src/version_map.rs @@ -5,7 +5,7 @@ use chrono::{DateTime, Utc}; use tracing::{instrument, warn}; use distribution_filename::DistFilename; -use distribution_types::{Dist, IndexUrl, PrioritizedDistribution, ResolvableDist}; +use distribution_types::{Dist, IncompatibleWheel, IndexUrl, PrioritizedDist, WheelCompatibility}; use pep440_rs::Version; use platform_tags::Tags; use puffin_client::{FlatDistributions, OwnedArchive, SimpleMetadata, VersionFiles}; @@ -39,7 +39,7 @@ impl VersionMap { ) -> Self { let mut map = BTreeMap::new(); // Create stubs for each entry in simple metadata. The full conversion - // from a `VersionFiles` to a PrioritizedDistribution for each version + // from a `VersionFiles` to a PrioritizedDist for each version // isn't done until that specific version is requested. for (datum_index, datum) in simple_metadata.iter().enumerate() { let version: Version = datum @@ -48,7 +48,7 @@ impl VersionMap { .expect("archived version always deserializes"); map.insert( version, - LazyPrioritizedDistribution::OnlySimple(SimplePrioritizedDistribution { + LazyPrioritizedDist::OnlySimple(SimplePrioritizedDist { datum_index, dist: OnceLock::new(), }), @@ -59,17 +59,17 @@ impl VersionMap { for (version, prioritized_dist) in flat_index.into_iter().flatten() { match map.entry(version) { Entry::Vacant(e) => { - e.insert(LazyPrioritizedDistribution::OnlyFlat(prioritized_dist)); + e.insert(LazyPrioritizedDist::OnlyFlat(prioritized_dist)); } // When there is both a `VersionFiles` (from the "simple" // metadata) and a flat distribution for the same version of // a package, we store both and "merge" them into a single - // `PrioritizedDistribution` upon access later. + // `PrioritizedDist` upon access later. Entry::Occupied(e) => match e.remove_entry() { - (version, LazyPrioritizedDistribution::OnlySimple(simple_dist)) => { + (version, LazyPrioritizedDist::OnlySimple(simple_dist)) => { map.insert( version, - LazyPrioritizedDistribution::Both { + LazyPrioritizedDist::Both { flat: prioritized_dist, simple: simple_dist, }, @@ -99,9 +99,8 @@ impl VersionMap { } /// Return the [`DistFile`] for the given version, if any. - pub(crate) fn get(&self, version: &Version) -> Option { - self.get_with_version(version) - .map(|(_, resolvable_dist)| resolvable_dist) + pub(crate) fn get(&self, version: &Version) -> Option<&PrioritizedDist> { + self.get_with_version(version).map(|(_version, dist)| dist) } /// Return the [`DistFile`] and the `Version` from the map for the given @@ -114,21 +113,17 @@ impl VersionMap { pub(crate) fn get_with_version<'a>( &'a self, version: &Version, - ) -> Option<(&'a Version, ResolvableDist)> { + ) -> Option<(&'a Version, &'a PrioritizedDist)> { match self.inner { - VersionMapInner::Eager(ref map) => map - .get_key_value(version) - .and_then(|(version, dist)| Some((version, dist.get()?))), - VersionMapInner::Lazy(ref lazy) => lazy - .get_with_version(version) - .and_then(|(version, dist)| Some((version, dist.get()?))), + VersionMapInner::Eager(ref map) => map.get_key_value(version), + VersionMapInner::Lazy(ref lazy) => lazy.get_with_version(version), } } /// Return an iterator over the versions and distributions. /// /// Note that the value returned in this iterator is a [`VersionMapDist`], - /// which can be used to lazily request a [`ResolvableDist`]. This is + /// which can be used to lazily request a [`CompatibleDist`]. This is /// useful in cases where one can skip materializing a full distribution /// for each version. pub(crate) fn iter(&self) -> impl DoubleEndedIterator { @@ -197,25 +192,25 @@ impl From for VersionMap { /// Note that because of laziness, not all such items can be turned into /// a valid distribution. For example, if in the process of building a /// distribution no compatible wheel or source distribution could be found, -/// then building a `ResolvableDist` will fail. +/// then building a `CompatibleDist` will fail. pub(crate) struct VersionMapDistHandle<'a> { inner: VersionMapDistHandleInner<'a>, } enum VersionMapDistHandleInner<'a> { - Eager(&'a PrioritizedDistribution), + Eager(&'a PrioritizedDist), Lazy { lazy: &'a VersionMapLazy, - dist: &'a LazyPrioritizedDistribution, + dist: &'a LazyPrioritizedDist, }, } impl<'a> VersionMapDistHandle<'a> { - /// Returns a resolvable distribution from this handle. - pub(crate) fn resolvable_dist(&self) -> Option> { + /// Returns a prioritized distribution from this handle. + pub(crate) fn prioritized_dist(&self) -> Option<&'a PrioritizedDist> { match self.inner { - VersionMapDistHandleInner::Eager(dist) => dist.get(), - VersionMapDistHandleInner::Lazy { lazy, dist } => Some(lazy.get_lazy(dist)?.get()?), + VersionMapDistHandleInner::Eager(dist) => Some(dist), + VersionMapDistHandleInner::Lazy { lazy, dist } => Some(lazy.get_lazy(dist)?), } } } @@ -227,11 +222,11 @@ enum VersionMapInner { /// /// This usually happens when one needs a `VersionMap` from a /// `FlatDistributions`. - Eager(BTreeMap), + Eager(BTreeMap), /// Some distributions might be fully materialized (i.e., by initializing /// a `VersionMap` with a `FlatDistributions`), but some distributions /// might still be in their "raw" `SimpleMetadata` format. In this case, a - /// `PrioritizedDistribution` isn't actually created in memory until the + /// `PrioritizedDist` isn't actually created in memory until the /// specific version has been requested. Lazy(VersionMapLazy), } @@ -247,8 +242,8 @@ enum VersionMapInner { #[derive(Debug)] struct VersionMapLazy { /// A map from version to possibly-initialized distribution. - map: BTreeMap, - /// The raw simple metadata from which `PrioritizedDistribution`s should + map: BTreeMap, + /// The raw simple metadata from which `PrioritizedDist`s should /// be constructed. simple_metadata: OwnedArchive, /// When true, wheels aren't allowed. @@ -268,14 +263,14 @@ struct VersionMapLazy { impl VersionMapLazy { /// Returns the distribution for the given version, if it exists. - fn get(&self, version: &Version) -> Option<&PrioritizedDistribution> { + fn get(&self, version: &Version) -> Option<&PrioritizedDist> { self.get_with_version(version) .map(|(_, prioritized_dist)| prioritized_dist) } /// Returns the distribution for the given version along with the version /// in this map, if it exists. - fn get_with_version(&self, version: &Version) -> Option<(&Version, &PrioritizedDistribution)> { + fn get_with_version(&self, version: &Version) -> Option<(&Version, &PrioritizedDist)> { let (version, lazy_dist) = self.map.get_key_value(version)?; let priority_dist = self.get_lazy(lazy_dist)?; Some((version, priority_dist)) @@ -286,14 +281,11 @@ impl VersionMapLazy { /// /// When both a flat and simple distribution are present internally, they /// are merged automatically. - fn get_lazy<'p>( - &'p self, - lazy_dist: &'p LazyPrioritizedDistribution, - ) -> Option<&'p PrioritizedDistribution> { + fn get_lazy<'p>(&'p self, lazy_dist: &'p LazyPrioritizedDist) -> Option<&'p PrioritizedDist> { match *lazy_dist { - LazyPrioritizedDistribution::OnlyFlat(ref dist) => Some(dist), - LazyPrioritizedDistribution::OnlySimple(ref dist) => self.get_simple(None, dist), - LazyPrioritizedDistribution::Both { + LazyPrioritizedDist::OnlyFlat(ref dist) => Some(dist), + LazyPrioritizedDist::OnlySimple(ref dist) => self.get_simple(None, dist), + LazyPrioritizedDist::Both { ref flat, ref simple, } => self.get_simple(Some(flat), simple), @@ -306,9 +298,9 @@ impl VersionMapLazy { /// returns `None`. fn get_simple<'p>( &'p self, - init: Option<&'p PrioritizedDistribution>, - simple: &'p SimplePrioritizedDistribution, - ) -> Option<&'p PrioritizedDistribution> { + init: Option<&'p PrioritizedDist>, + simple: &'p SimplePrioritizedDist, + ) -> Option<&'p PrioritizedDist> { let get_or_init = || { let files: VersionFiles = self .simple_metadata @@ -322,6 +314,7 @@ impl VersionMapLazy { if let Some(exclude_newer) = self.exclude_newer { match file.upload_time_utc_ms.as_ref() { Some(&upload_time) if upload_time >= exclude_newer.timestamp_millis() => { + priority_dist.set_exclude_newer(); continue; } None => { @@ -329,6 +322,7 @@ impl VersionMapLazy { "{} is missing an upload date, but user provided: {exclude_newer}", file.filename, ); + priority_dist.set_exclude_newer(); continue; } _ => {} @@ -339,21 +333,27 @@ impl VersionMapLazy { let hash = file.hashes.clone(); match filename { DistFilename::WheelFilename(filename) => { - // If pre-built binaries are disabled, skip this wheel - if self.no_binary { - continue; - } + // Determine a compatibility for the wheel based on tags + let mut compatibility = + WheelCompatibility::from(filename.compatibility(&self.tags)); + + if compatibility.is_compatible() { + // Check for Python version incompatibility + if let Some(ref requires_python) = file.requires_python { + if !requires_python.contains(self.python_requirement.target()) { + compatibility = WheelCompatibility::Incompatible( + IncompatibleWheel::RequiresPython, + ); + } + } + + // Mark all wheels as incompatibility when binaries are disabled + if self.no_binary { + compatibility = + WheelCompatibility::Incompatible(IncompatibleWheel::NoBinary); + } + }; - // To be compatible, the wheel must both have - // compatible tags _and_ have a compatible Python - // requirement. - let priority = filename.compatibility(&self.tags).filter(|_| { - file.requires_python - .as_ref() - .map_or(true, |requires_python| { - requires_python.contains(self.python_requirement.target()) - }) - }); let dist = Dist::from_registry( DistFilename::WheelFilename(filename), file, @@ -364,7 +364,7 @@ impl VersionMapLazy { requires_python, yanked, Some(hash), - priority, + compatibility, ); } DistFilename::SourceDistFilename(filename) => { @@ -387,30 +387,30 @@ impl VersionMapLazy { } } -/// Represents a possibly initialized `PrioritizedDistribution` for +/// Represents a possibly initialized [`PrioritizedDist`] for /// a single version of a package. #[derive(Debug)] -enum LazyPrioritizedDistribution { +enum LazyPrioritizedDist { /// Represents a eagerly constructed distribution from a /// `FlatDistributions`. - OnlyFlat(PrioritizedDistribution), + OnlyFlat(PrioritizedDist), /// Represents a lazyily constructed distribution from an index into a /// `VersionFiles` from `SimpleMetadata`. - OnlySimple(SimplePrioritizedDistribution), + OnlySimple(SimplePrioritizedDist), /// Combines the above. This occurs when we have data from both a flat /// distribution and a simple distribution. Both { - flat: PrioritizedDistribution, - simple: SimplePrioritizedDistribution, + flat: PrioritizedDist, + simple: SimplePrioritizedDist, }, } -/// Represents a lazily initialized `PrioritizedDistribution`. +/// Represents a lazily initialized `PrioritizedDist`. #[derive(Debug)] -struct SimplePrioritizedDistribution { +struct SimplePrioritizedDist { /// An offset into `SimpleMetadata` corresponding to a `SimpleMetadatum`. /// This provides access to a `VersionFiles` that is used to construct a - /// `PrioritizedDistribution`. + /// `PrioritizedDist`. datum_index: usize, /// A lazily initialized distribution. /// @@ -419,5 +419,5 @@ struct SimplePrioritizedDistribution { /// if initialization could not find any usable files from which to /// construct a distribution. (One easy way to effect this, at the time /// of writing, is to use `--exclude-newer 1900-01-01`.) - dist: OnceLock>, + dist: OnceLock>, } diff --git a/crates/puffin/tests/pip_compile_scenarios.rs b/crates/puffin/tests/pip_compile_scenarios.rs index 5ba4344bd..0b7920f65 100644 --- a/crates/puffin/tests/pip_compile_scenarios.rs +++ b/crates/puffin/tests/pip_compile_scenarios.rs @@ -1,7 +1,7 @@ //! DO NOT EDIT //! //! Generated with ./scripts/scenarios/update.py -//! Scenarios from +//! Scenarios from //! #![cfg(all(feature = "python", feature = "pypi"))] diff --git a/crates/puffin/tests/pip_install_scenarios.rs b/crates/puffin/tests/pip_install_scenarios.rs index 7e9bd94c3..fed29ba9b 100644 --- a/crates/puffin/tests/pip_install_scenarios.rs +++ b/crates/puffin/tests/pip_install_scenarios.rs @@ -1,7 +1,7 @@ //! DO NOT EDIT //! //! Generated with ./scripts/scenarios/update.py -//! Scenarios from +//! Scenarios from //! #![cfg(all(feature = "python", feature = "pypi"))] @@ -2460,7 +2460,7 @@ fn no_wheels_with_matching_platform() { /// distributions available /// /// ```text -/// af6bcec1 +/// 94e293e5 /// ├── environment /// │ └── python3.8 /// ├── root @@ -2475,11 +2475,11 @@ fn no_sdist_no_wheels_with_matching_platform() { // In addition to the standard filters, swap out package names for more realistic messages let mut filters = INSTA_FILTERS.to_vec(); - filters.push((r"a-af6bcec1", "albatross")); - filters.push((r"-af6bcec1", "")); + filters.push((r"a-94e293e5", "albatross")); + filters.push((r"-94e293e5", "")); puffin_snapshot!(filters, command(&context) - .arg("a-af6bcec1") + .arg("a-94e293e5") , @r###" success: false exit_code: 1 @@ -2487,10 +2487,11 @@ fn no_sdist_no_wheels_with_matching_platform() { ----- stderr ----- × No solution found when resolving dependencies: - ╰─▶ Because there are no versions of albatross and you require albatross, we can conclude that the requirements are unsatisfiable. + ╰─▶ Because only albatross==1.0.0 is available and albatross==1.0.0 is unusable because no wheels are available with a matching platform, we can conclude that all versions of albatross cannot be used. + And because you require albatross, we can conclude that the requirements are unsatisfiable. "###); - assert_not_installed(&context.venv, "a_af6bcec1", &context.temp_dir); + assert_not_installed(&context.venv, "a_94e293e5", &context.temp_dir); } /// no-sdist-no-wheels-with-matching-python @@ -2526,7 +2527,8 @@ fn no_sdist_no_wheels_with_matching_python() { ----- stderr ----- × No solution found when resolving dependencies: - ╰─▶ Because there are no versions of albatross and you require albatross, we can conclude that the requirements are unsatisfiable. + ╰─▶ Because only albatross==1.0.0 is available and albatross==1.0.0 is unusable because no wheels are available with a matching Python implementation, we can conclude that all versions of albatross cannot be used. + And because you require albatross, we can conclude that the requirements are unsatisfiable. "###); assert_not_installed(&context.venv, "a_40fe677d", &context.temp_dir); @@ -2565,7 +2567,8 @@ fn no_sdist_no_wheels_with_matching_abi() { ----- stderr ----- × No solution found when resolving dependencies: - ╰─▶ Because there are no versions of albatross and you require albatross, we can conclude that the requirements are unsatisfiable. + ╰─▶ Because only albatross==1.0.0 is available and albatross==1.0.0 is unusable because no wheels are available with a matching Python ABI, we can conclude that all versions of albatross cannot be used. + And because you require albatross, we can conclude that the requirements are unsatisfiable. "###); assert_not_installed(&context.venv, "a_8727a9b9", &context.temp_dir); @@ -2647,7 +2650,8 @@ fn only_wheels_no_binary() { ----- stderr ----- × No solution found when resolving dependencies: - ╰─▶ Because there are no versions of albatross and you require albatross, we can conclude that the requirements are unsatisfiable. + ╰─▶ Because only albatross==1.0.0 is available and albatross==1.0.0 is unusable because no source distribution is available and using wheels is disabled, we can conclude that all versions of albatross cannot be used. + And because you require albatross, we can conclude that the requirements are unsatisfiable. "###); assert_not_installed(&context.venv, "a_dd137625", &context.temp_dir); diff --git a/scripts/scenarios/update.py b/scripts/scenarios/update.py index 91d85b4a0..8015a2f4b 100755 --- a/scripts/scenarios/update.py +++ b/scripts/scenarios/update.py @@ -45,7 +45,7 @@ import textwrap from pathlib import Path -PACKSE_COMMIT = "c35c57f5b4ab3381658661edbd0cd955680f9cda" +PACKSE_COMMIT = "de58b3e3f998486b6c0f3dd67b7341c880eb54b2" TOOL_ROOT = Path(__file__).parent TEMPLATES = TOOL_ROOT / "templates" INSTALL_TEMPLATE = TEMPLATES / "install.mustache"