Use Dist in VersionMap (#851)

Refactoring split out from find links support: Find links files can be
represented as `Dist`, but not really as `File`, they don't have url nor
hashes.

`DistRequiresPython` is somewhat odd as an in between type.
This commit is contained in:
konsti 2024-01-10 00:14:42 +01:00 committed by GitHub
parent 1203f8f9e8
commit 858d5584cc
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 158 additions and 200 deletions

View file

@ -1,18 +1,16 @@
use pubgrub::range::Range;
use rustc_hash::FxHashMap;
use distribution_types::{Dist, DistributionMetadata, IndexUrl, Name};
use distribution_types::{Dist, DistributionMetadata, Name};
use pep440_rs::VersionSpecifiers;
use pep508_rs::{Requirement, VersionOrUrl};
use puffin_normalize::PackageName;
use pypi_types::BaseUrl;
use crate::file::DistFile;
use crate::prerelease_mode::PreReleaseStrategy;
use crate::pubgrub::PubGrubVersion;
use crate::python_requirement::PythonRequirement;
use crate::resolution_mode::ResolutionStrategy;
use crate::version_map::{ResolvableFile, VersionMap};
use crate::version_map::{DistRequiresPython, ResolvableFile, VersionMap};
use crate::{Manifest, ResolutionOptions};
#[derive(Debug, Clone)]
@ -247,12 +245,12 @@ impl<'a> Candidate<'a> {
}
/// Return the [`DistFile`] to use when resolving the package.
pub(crate) fn resolve(&self) -> &DistFile {
pub(crate) fn resolve(&self) -> &DistRequiresPython {
self.file.resolve()
}
/// Return the [`DistFile`] to use when installing the package.
pub(crate) fn install(&self) -> &DistFile {
pub(crate) fn install(&self) -> &DistRequiresPython {
self.file.install()
}
@ -271,7 +269,7 @@ impl<'a> Candidate<'a> {
// 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 self.install().is_sdist() {
if matches!(self.install().dist, Dist::Source(_)) {
if !requires_python.contains(requirement.installed()) {
return Some(requires_python);
}
@ -279,17 +277,6 @@ impl<'a> Candidate<'a> {
None
}
/// Return the [`Dist`] to use when resolving the candidate.
pub(crate) fn into_distribution(self, index: IndexUrl, base: BaseUrl) -> Dist {
Dist::from_registry(
self.name().clone(),
self.version().clone().into(),
self.resolve().clone().into(),
index,
base,
)
}
}
impl Name for Candidate<'_> {

View file

@ -7,12 +7,11 @@ use rustc_hash::FxHashMap;
use thiserror::Error;
use url::Url;
use distribution_types::{BuiltDist, IndexUrl, PathBuiltDist, PathSourceDist, SourceDist};
use distribution_types::{BuiltDist, PathBuiltDist, PathSourceDist, SourceDist};
use pep508_rs::Requirement;
use puffin_distribution::DistributionDatabaseError;
use puffin_normalize::PackageName;
use puffin_traits::OnceMap;
use pypi_types::BaseUrl;
use crate::candidate_selector::CandidateSelector;
use crate::pubgrub::{PubGrubPackage, PubGrubPython, PubGrubReportFormatter, PubGrubVersion};
@ -161,7 +160,7 @@ impl NoSolutionError {
pub(crate) fn with_available_versions(
mut self,
python_requirement: &PythonRequirement,
package_versions: &OnceMap<PackageName, (IndexUrl, BaseUrl, VersionMap)>,
package_versions: &OnceMap<PackageName, VersionMap>,
) -> Self {
let mut available_versions = FxHashMap::default();
for package in self.derivation_tree.packages() {
@ -181,7 +180,7 @@ impl NoSolutionError {
}
PubGrubPackage::Package(name, ..) => {
if let Some(entry) = package_versions.get(name) {
let (_, _, version_map) = entry.value();
let version_map = entry.value();
available_versions.insert(
package.clone(),
version_map

View file

@ -1,92 +0,0 @@
use std::ops::Deref;
use distribution_types::File;
/// A distribution can either be a wheel or a source distribution.
#[derive(Debug, Clone)]
pub(crate) struct WheelFile(pub(crate) File);
#[derive(Debug, Clone)]
pub(crate) struct SdistFile(pub(crate) File);
#[derive(Debug, Clone)]
pub(crate) enum DistFile {
Wheel(WheelFile),
Sdist(SdistFile),
}
impl Deref for WheelFile {
type Target = File;
fn deref(&self) -> &Self::Target {
&self.0
}
}
impl Deref for SdistFile {
type Target = File;
fn deref(&self) -> &Self::Target {
&self.0
}
}
impl From<WheelFile> for File {
fn from(wheel: WheelFile) -> Self {
wheel.0
}
}
impl From<SdistFile> for File {
fn from(sdist: SdistFile) -> Self {
sdist.0
}
}
impl From<WheelFile> for DistFile {
fn from(wheel: WheelFile) -> Self {
Self::Wheel(wheel)
}
}
impl From<SdistFile> for DistFile {
fn from(sdist: SdistFile) -> Self {
Self::Sdist(sdist)
}
}
impl DistFile {
pub(crate) fn filename(&self) -> &str {
match self {
Self::Wheel(wheel) => wheel.filename.as_str(),
Self::Sdist(sdist) => sdist.filename.as_str(),
}
}
pub(crate) fn is_sdist(&self) -> bool {
match self {
Self::Wheel(_) => false,
Self::Sdist(_) => true,
}
}
}
impl From<DistFile> for File {
fn from(file: DistFile) -> Self {
match file {
DistFile::Wheel(wheel) => wheel.into(),
DistFile::Sdist(sdist) => sdist.into(),
}
}
}
impl Deref for DistFile {
type Target = File;
fn deref(&self) -> &Self::Target {
match self {
DistFile::Wheel(file) => &file.0,
DistFile::Sdist(file) => &file.0,
}
}
}

View file

@ -9,7 +9,6 @@ pub use resolver::{BuildId, Reporter as ResolverReporter, Resolver, ResolverProv
mod candidate_selector;
mod error;
mod file;
mod finder;
mod manifest;
mod overrides;

View file

@ -1,8 +1,7 @@
use rustc_hash::FxHashMap;
use distribution_types::{File, IndexUrl};
use distribution_types::Dist;
use puffin_normalize::PackageName;
use pypi_types::BaseUrl;
use crate::candidate_selector::Candidate;
@ -11,29 +10,19 @@ use crate::candidate_selector::Candidate;
/// For example, given `Flask==3.0.0`, the [`FilePins`] would contain a mapping from `Flask` to
/// `3.0.0` to the specific wheel or source distribution archive that was pinned for that version.
#[derive(Debug, Default)]
pub(crate) struct FilePins(
FxHashMap<PackageName, FxHashMap<pep440_rs::Version, (IndexUrl, BaseUrl, File)>>,
);
pub(crate) struct FilePins(FxHashMap<PackageName, FxHashMap<pep440_rs::Version, Dist>>);
impl FilePins {
/// Pin a candidate package.
pub(crate) fn insert(&mut self, candidate: &Candidate, index: &IndexUrl, base: &BaseUrl) {
pub(crate) fn insert(&mut self, candidate: &Candidate) {
self.0.entry(candidate.name().clone()).or_default().insert(
candidate.version().clone().into(),
(
index.clone(),
base.clone(),
candidate.install().clone().into(),
),
candidate.install().dist.clone(),
);
}
/// Return the pinned file for the given package name and version, if it exists.
pub(crate) fn get(
&self,
name: &PackageName,
version: &pep440_rs::Version,
) -> Option<&(IndexUrl, BaseUrl, File)> {
pub(crate) fn get(&self, name: &PackageName, version: &pep440_rs::Version) -> Option<&Dist> {
self.0.get(name)?.get(version)
}
}

View file

@ -55,12 +55,10 @@ impl ResolutionGraph {
match package {
PubGrubPackage::Package(package_name, None, None) => {
let version = Version::from(version.clone());
let (index, base, file) = pins
let pinned_package = pins
.get(package_name, &version)
.expect("Every package should be pinned")
.clone();
let pinned_package =
Dist::from_registry(package_name.clone(), version, file, index, base);
let index = petgraph.add_node(pinned_package);
inverse.insert(package_name, index);
@ -89,12 +87,10 @@ impl ResolutionGraph {
if !metadata.provides_extras.contains(extra) {
let version = Version::from(version.clone());
let (index, base, file) = pins
let pinned_package = pins
.get(package_name, &version)
.expect("Every package should be pinned")
.clone();
let pinned_package =
Dist::from_registry(package_name.clone(), version, file, index, base);
diagnostics.push(Diagnostic::MissingExtra {
dist: pinned_package,

View file

@ -1,10 +1,10 @@
use url::Url;
use distribution_types::{IndexUrl, PackageId};
use distribution_types::PackageId;
use pep440_rs::VersionSpecifiers;
use puffin_normalize::PackageName;
use puffin_traits::OnceMap;
use pypi_types::{BaseUrl, Metadata21};
use pypi_types::Metadata21;
use crate::version_map::VersionMap;
@ -13,7 +13,7 @@ use crate::version_map::VersionMap;
pub(crate) struct Index {
/// A map from package name to the metadata for that package and the index where the metadata
/// came from.
pub(crate) packages: OnceMap<PackageName, (IndexUrl, BaseUrl, VersionMap)>,
pub(crate) packages: OnceMap<PackageName, VersionMap>,
/// A map from package ID to metadata for that distribution.
pub(crate) distributions: OnceMap<PackageId, Metadata21>,

View file

@ -17,7 +17,8 @@ use url::Url;
use distribution_filename::WheelFilename;
use distribution_types::{
BuiltDist, Dist, DistributionMetadata, IndexUrl, LocalEditable, Name, SourceDist, VersionOrUrl,
BuiltDist, Dist, DistributionMetadata, LocalEditable, Name, RemoteSource, SourceDist,
VersionOrUrl,
};
use pep508_rs::{MarkerEnvironment, Requirement};
use platform_tags::Tags;
@ -26,7 +27,7 @@ use puffin_distribution::DistributionDatabase;
use puffin_interpreter::Interpreter;
use puffin_normalize::PackageName;
use puffin_traits::BuildContext;
use pypi_types::{BaseUrl, Metadata21};
use pypi_types::Metadata21;
use crate::candidate_selector::CandidateSelector;
use crate::error::ResolveError;
@ -472,7 +473,7 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
PubGrubPackage::Package(package_name, extra, None) => {
// Wait for the metadata to be available.
let entry = self.index.packages.wait(package_name).await;
let (index, base, version_map) = entry.value();
let version_map = entry.value();
if let Some(extra) = extra {
debug!(
@ -502,20 +503,28 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
candidate.name(),
extra,
candidate.version(),
candidate.resolve().filename()
candidate
.resolve()
.dist
.filename()
.unwrap_or("unknown filename")
);
} else {
debug!(
"Selecting: {}=={} ({})",
candidate.name(),
candidate.version(),
candidate.resolve().filename()
candidate
.resolve()
.dist
.filename()
.unwrap_or("unknown filename")
);
}
// 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, index, base);
pins.insert(&candidate);
let version = candidate.version().clone();
@ -525,7 +534,7 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
.distributions
.register_owned(candidate.package_id())
{
let distribution = candidate.into_distribution(index.clone(), base.clone());
let distribution = candidate.resolve().dist.clone();
request_sink.unbounded_send(Request::Dist(distribution))?;
}
@ -670,11 +679,9 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
while let Some(response) = response_stream.next().await {
match response? {
Some(Response::Package(package_name, index, base, version_map)) => {
Some(Response::Package(package_name, version_map)) => {
trace!("Received package metadata for: {package_name}");
self.index
.packages
.done(package_name, (index, base, version_map));
self.index.packages.done(package_name, version_map);
}
Some(Response::Dist(Dist::Built(distribution), metadata, ..)) => {
trace!("Received built distribution metadata for: {distribution}");
@ -713,12 +720,12 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
match request {
// Fetch package metadata from the registry.
Request::Package(package_name) => {
let (index, base, metadata) = self
let version_map = self
.provider
.get_version_map(&package_name)
.await
.map_err(ResolveError::Client)?;
Ok(Some(Response::Package(package_name, index, base, metadata)))
Ok(Some(Response::Package(package_name, version_map)))
}
// Fetch distribution metadata from the distribution database.
@ -746,7 +753,7 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
Request::Prefetch(package_name, range) => {
// Wait for the package metadata to become available.
let entry = self.index.packages.wait(&package_name).await;
let (index, base, version_map) = entry.value();
let version_map = entry.value();
// Try to find a compatible version. If there aren't any compatible versions,
// short-circuit and return `None`.
@ -769,7 +776,7 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
.distributions
.register_owned(candidate.package_id())
{
let dist = candidate.into_distribution(index.clone(), base.clone());
let dist = candidate.resolve().dist.clone();
drop(entry);
let (metadata, precise) = self
@ -837,7 +844,7 @@ enum Request {
#[allow(clippy::large_enum_variant)]
enum Response {
/// The returned metadata for a package hosted on a registry.
Package(PackageName, IndexUrl, BaseUrl, VersionMap),
Package(PackageName, VersionMap),
/// The returned metadata for a distribution.
Dist(Dist, Metadata21, Option<Url>),
}

View file

@ -5,19 +5,19 @@ use chrono::{DateTime, Utc};
use futures::TryFutureExt;
use url::Url;
use distribution_types::{Dist, IndexUrl};
use distribution_types::Dist;
use platform_tags::Tags;
use puffin_client::RegistryClient;
use puffin_distribution::{DistributionDatabase, DistributionDatabaseError};
use puffin_normalize::PackageName;
use puffin_traits::BuildContext;
use pypi_types::{BaseUrl, Metadata21};
use pypi_types::Metadata21;
use crate::python_requirement::PythonRequirement;
use crate::version_map::VersionMap;
use crate::yanks::AllowedYanks;
type VersionMapResponse = Result<(IndexUrl, BaseUrl, VersionMap), puffin_client::Error>;
type VersionMapResponse = Result<VersionMap, puffin_client::Error>;
type WheelMetadataResponse = Result<(Metadata21, Option<Url>), DistributionDatabaseError>;
pub trait ResolverProvider: Send + Sync {
@ -83,17 +83,15 @@ impl<'a, Context: BuildContext + Send + Sync> ResolverProvider
self.client
.simple(package_name)
.map_ok(move |(index, base, metadata)| {
(
index,
base,
VersionMap::from_metadata(
metadata,
package_name,
self.tags,
&self.python_requirement,
&self.allowed_yanks,
self.exclude_newer.as_ref(),
),
VersionMap::from_metadata(
metadata,
package_name,
&index,
&base,
self.tags,
&self.python_requirement,
&self.allowed_yanks,
self.exclude_newer.as_ref(),
)
})
}

View file

@ -5,13 +5,14 @@ use chrono::{DateTime, Utc};
use tracing::{instrument, warn};
use distribution_filename::DistFilename;
use distribution_types::{Dist, IndexUrl};
use pep440_rs::VersionSpecifiers;
use platform_tags::{TagPriority, Tags};
use puffin_client::SimpleMetadata;
use puffin_normalize::PackageName;
use puffin_warnings::warn_user_once;
use pypi_types::Yanked;
use pypi_types::{BaseUrl, Yanked};
use crate::file::{DistFile, SdistFile, WheelFile};
use crate::pubgrub::PubGrubVersion;
use crate::python_requirement::PythonRequirement;
use crate::yanks::AllowedYanks;
@ -23,9 +24,12 @@ pub struct VersionMap(BTreeMap<PubGrubVersion, PrioritizedDistribution>);
impl VersionMap {
/// Initialize a [`VersionMap`] from the given metadata.
#[instrument(skip_all, fields(package_name = % package_name))]
#[allow(clippy::too_many_arguments)]
pub(crate) fn from_metadata(
metadata: SimpleMetadata,
package_name: &PackageName,
index: &IndexUrl,
base: &BaseUrl,
tags: &Tags,
python_requirement: &PythonRequirement,
allowed_yanks: &AllowedYanks,
@ -65,6 +69,7 @@ impl VersionMap {
}
}
let requires_python = file.requires_python.clone();
match filename {
DistFilename::WheelFilename(filename) => {
// To be compatible, the wheel must both have compatible tags _and_ have a
@ -78,25 +83,45 @@ impl VersionMap {
.all(|version| requires_python.contains(version))
})
});
let dist = Dist::from_registry(
filename.name.clone(),
filename.version.clone(),
file,
index.clone(),
base.clone(),
);
match version_map.entry(version.clone().into()) {
Entry::Occupied(mut entry) => {
entry.get_mut().insert_built(WheelFile(file), priority);
entry
.get_mut()
.insert_built(dist, requires_python, priority);
}
Entry::Vacant(entry) => {
entry.insert(PrioritizedDistribution::from_built(
WheelFile(file),
dist,
requires_python,
priority,
));
}
}
}
DistFilename::SourceDistFilename(_) => {
DistFilename::SourceDistFilename(filename) => {
let dist = Dist::from_registry(
filename.name.clone(),
filename.version.clone(),
file,
index.clone(),
base.clone(),
);
match version_map.entry(version.clone().into()) {
Entry::Occupied(mut entry) => {
entry.get_mut().insert_source(SdistFile(file));
entry.get_mut().insert_source(dist, requires_python);
}
Entry::Vacant(entry) => {
entry.insert(PrioritizedDistribution::from_source(SdistFile(file)));
entry.insert(PrioritizedDistribution::from_source(
dist,
requires_python,
));
}
}
}
@ -122,63 +147,111 @@ impl VersionMap {
}
}
/// Attach its requires-python to a [`Dist`], since downstream needs this information to filter
/// [`PrioritizedDistribution`].
#[derive(Debug)]
pub(crate) struct DistRequiresPython {
pub(crate) dist: Dist,
pub(crate) requires_python: Option<VersionSpecifiers>,
}
#[derive(Debug)]
struct PrioritizedDistribution {
/// An arbitrary source distribution for the package version.
source: Option<DistFile>,
source: Option<DistRequiresPython>,
/// The highest-priority, platform-compatible wheel for the package version.
compatible_wheel: Option<(DistFile, TagPriority)>,
compatible_wheel: Option<(DistRequiresPython, TagPriority)>,
/// An arbitrary, platform-incompatible wheel for the package version.
incompatible_wheel: Option<DistFile>,
incompatible_wheel: Option<DistRequiresPython>,
}
impl PrioritizedDistribution {
/// Create a new [`PrioritizedDistribution`] from the given wheel distribution.
fn from_built(dist: WheelFile, priority: Option<TagPriority>) -> Self {
fn from_built(
dist: Dist,
requires_python: Option<VersionSpecifiers>,
priority: Option<TagPriority>,
) -> Self {
if let Some(priority) = priority {
Self {
source: None,
compatible_wheel: Some((dist.into(), priority)),
compatible_wheel: Some((
DistRequiresPython {
dist,
requires_python,
},
priority,
)),
incompatible_wheel: None,
}
} else {
Self {
source: None,
compatible_wheel: None,
incompatible_wheel: Some(dist.into()),
incompatible_wheel: Some(DistRequiresPython {
dist,
requires_python,
}),
}
}
}
/// Create a new [`PrioritizedDistribution`] from the given source distribution.
fn from_source(dist: SdistFile) -> Self {
fn from_source(dist: Dist, requires_python: Option<VersionSpecifiers>) -> Self {
Self {
source: Some(dist.into()),
source: Some(DistRequiresPython {
dist,
requires_python,
}),
compatible_wheel: None,
incompatible_wheel: None,
}
}
/// Insert the given built distribution into the [`PrioritizedDistribution`].
fn insert_built(&mut self, file: WheelFile, priority: Option<TagPriority>) {
fn insert_built(
&mut self,
dist: Dist,
requires_python: Option<VersionSpecifiers>,
priority: Option<TagPriority>,
) {
// Prefer the highest-priority, platform-compatible wheel.
if let Some(priority) = priority {
if let Some((.., existing_priority)) = &self.compatible_wheel {
if priority > *existing_priority {
self.compatible_wheel = Some((file.into(), priority));
self.compatible_wheel = Some((
DistRequiresPython {
dist,
requires_python,
},
priority,
));
}
} else {
self.compatible_wheel = Some((file.into(), priority));
self.compatible_wheel = Some((
DistRequiresPython {
dist,
requires_python,
},
priority,
));
}
} else if self.incompatible_wheel.is_none() {
self.incompatible_wheel = Some(file.into());
self.incompatible_wheel = Some(DistRequiresPython {
dist,
requires_python,
});
}
}
/// Insert the given source distribution into the [`PrioritizedDistribution`].
fn insert_source(&mut self, file: SdistFile) {
fn insert_source(&mut self, dist: Dist, requires_python: Option<VersionSpecifiers>) {
if self.source.is_none() {
self.source = Some(file.into());
self.source = Some(DistRequiresPython {
dist,
requires_python,
});
}
}
@ -195,9 +268,11 @@ impl PrioritizedDistribution {
// 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)),
(_, Some(source_dist), Some(wheel)) => {
Some(ResolvableFile::IncompatibleWheel(source_dist, wheel))
}
// Otherwise, if we have a source distribution, return it.
(_, Some(sdist), _) => Some(ResolvableFile::SourceDist(sdist)),
(_, Some(source_dist), _) => Some(ResolvableFile::SourceDist(source_dist)),
_ => None,
}
}
@ -206,18 +281,18 @@ impl PrioritizedDistribution {
#[derive(Debug, Clone)]
pub(crate) enum ResolvableFile<'a> {
/// The distribution should be resolved and installed using a source distribution.
SourceDist(&'a DistFile),
SourceDist(&'a DistRequiresPython),
/// The distribution should be resolved and installed using a wheel distribution.
CompatibleWheel(&'a DistFile),
CompatibleWheel(&'a DistRequiresPython),
/// The distribution should be resolved using an incompatible wheel distribution, but
/// installed using a source distribution.
IncompatibleWheel(&'a DistFile, &'a DistFile),
IncompatibleWheel(&'a DistRequiresPython, &'a DistRequiresPython),
}
impl<'a> ResolvableFile<'a> {
/// Return the [`DistFile`] to use during resolution.
pub(crate) fn resolve(&self) -> &DistFile {
match self {
pub(crate) fn resolve(&self) -> &DistRequiresPython {
match *self {
ResolvableFile::SourceDist(sdist) => sdist,
ResolvableFile::CompatibleWheel(wheel) => wheel,
ResolvableFile::IncompatibleWheel(_, wheel) => wheel,
@ -225,8 +300,8 @@ impl<'a> ResolvableFile<'a> {
}
/// Return the [`DistFile`] to use during installation.
pub(crate) fn install(&self) -> &DistFile {
match self {
pub(crate) fn install(&self) -> &DistRequiresPython {
match *self {
ResolvableFile::SourceDist(sdist) => sdist,
ResolvableFile::CompatibleWheel(wheel) => wheel,
ResolvableFile::IncompatibleWheel(sdist, _) => sdist,