From 4bd3f166df0a8a70e325df28b7449175afa06898 Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Mon, 25 Mar 2024 13:24:32 -0500 Subject: [PATCH] Add `ResolvedDist` instead of `Dist::Installed` --- crates/distribution-types/src/cached.rs | 13 +- crates/distribution-types/src/installed.rs | 9 +- crates/distribution-types/src/lib.rs | 11 +- .../src/prioritized_distribution.rs | 28 ++-- crates/distribution-types/src/resolution.rs | 85 +++++++----- crates/distribution-types/src/resolved.rs | 122 ++++++++++++++++++ crates/uv-dev/src/install_many.rs | 22 +++- crates/uv-dispatch/src/lib.rs | 3 +- .../src/distribution_database.rs | 5 - crates/uv-installer/src/plan.rs | 15 +-- crates/uv-resolver/src/candidate_selector.rs | 6 +- crates/uv-resolver/src/finder.rs | 18 +-- crates/uv-resolver/src/pins.rs | 18 ++- crates/uv-resolver/src/resolution.rs | 35 +++-- crates/uv-resolver/src/resolver/mod.rs | 114 ++++++++++------ crates/uv/src/commands/pip_install.rs | 6 +- crates/uv/src/commands/pip_sync.rs | 13 +- crates/uv/src/commands/reporters.rs | 6 +- crates/uv/src/commands/venv.rs | 6 +- 19 files changed, 369 insertions(+), 166 deletions(-) create mode 100644 crates/distribution-types/src/resolved.rs diff --git a/crates/distribution-types/src/cached.rs b/crates/distribution-types/src/cached.rs index 90af713bf3ea..2a0da761d2ed 100644 --- a/crates/distribution-types/src/cached.rs +++ b/crates/distribution-types/src/cached.rs @@ -8,8 +8,8 @@ use uv_normalize::PackageName; use crate::direct_url::{DirectUrl, LocalFileUrl}; use crate::{ - BuiltDist, Dist, DistributionMetadata, InstalledDist, InstalledMetadata, InstalledVersion, - Name, SourceDist, VersionOrUrl, + BuiltDist, Dist, DistributionMetadata, InstalledMetadata, InstalledVersion, Name, SourceDist, + VersionOrUrl, }; /// A built distribution (wheel) that exists in the local cache. @@ -39,15 +39,6 @@ impl CachedDist { /// Initialize a [`CachedDist`] from a [`Dist`]. pub fn from_remote(remote: Dist, filename: WheelFilename, path: PathBuf) -> Self { match remote { - Dist::Installed(InstalledDist::Registry(_dist)) => { - Self::Registry(CachedRegistryDist { filename, path }) - } - Dist::Installed(InstalledDist::Url(dist)) => Self::Url(CachedDirectUrlDist { - filename, - url: VerbatimUrl::from_url(dist.url), - path, - editable: dist.editable, - }), Dist::Built(BuiltDist::Registry(_dist)) => { Self::Registry(CachedRegistryDist { filename, path }) } diff --git a/crates/distribution-types/src/installed.rs b/crates/distribution-types/src/installed.rs index 05b821f81582..25b9484ee8e6 100644 --- a/crates/distribution-types/src/installed.rs +++ b/crates/distribution-types/src/installed.rs @@ -10,7 +10,7 @@ use pep440_rs::Version; use uv_fs::Simplified; use uv_normalize::PackageName; -use crate::{InstalledMetadata, InstalledVersion, Name}; +use crate::{DistributionMetadata, InstalledMetadata, InstalledVersion, Name, VersionOrUrl}; /// A built distribution (wheel) that is installed in a virtual environment. #[derive(Debug, Clone)] @@ -114,6 +114,7 @@ impl InstalledDist { pub fn metadata(&self) -> Result { let path = self.path().join("METADATA"); let contents = fs::read(&path)?; + // TODO(zanieb): Update this to use thiserror so we can unpack parse errors downstream pypi_types::Metadata23::parse_metadata(&contents) .with_context(|| format!("Failed to parse METADATA file at: {}", path.user_display())) } @@ -145,6 +146,12 @@ impl InstalledDist { } } +impl DistributionMetadata for InstalledDist { + fn version_or_url(&self) -> VersionOrUrl { + VersionOrUrl::Version(self.version()) + } +} + impl Name for InstalledRegistryDist { fn name(&self) -> &PackageName { &self.name diff --git a/crates/distribution-types/src/lib.rs b/crates/distribution-types/src/lib.rs index 9f36b4c9b397..2ed82cf34221 100644 --- a/crates/distribution-types/src/lib.rs +++ b/crates/distribution-types/src/lib.rs @@ -56,6 +56,7 @@ pub use crate::index_url::*; pub use crate::installed::*; pub use crate::prioritized_distribution::*; pub use crate::resolution::*; +pub use crate::resolved::*; pub use crate::traits::*; mod any; @@ -70,6 +71,7 @@ mod index_url; mod installed; mod prioritized_distribution; mod resolution; +mod resolved; mod traits; #[derive(Debug, Clone)] @@ -121,7 +123,6 @@ impl std::fmt::Display for InstalledVersion<'_> { /// The location can be index, url or path (wheel) or index, url, path or git (source distribution) #[derive(Debug, Clone)] pub enum Dist { - Installed(InstalledDist), Built(BuiltDist), Source(SourceDist), } @@ -365,7 +366,6 @@ impl Dist { /// Returns the [`File`] instance, if this dist is from a registry with simple json api support pub fn file(&self) -> Option<&File> { match self { - Self::Installed(_) => None, Self::Built(built) => built.file(), Self::Source(source) => source.file(), } @@ -373,7 +373,6 @@ impl Dist { pub fn version(&self) -> Option<&Version> { match self { - Self::Installed(installed) => Some(installed.version()), Self::Built(wheel) => Some(wheel.version()), Self::Source(source_dist) => source_dist.version(), } @@ -496,7 +495,6 @@ impl Name for BuiltDist { impl Name for Dist { fn name(&self) -> &PackageName { match self { - Self::Installed(dist) => dist.name(), Self::Built(dist) => dist.name(), Self::Source(dist) => dist.name(), } @@ -569,7 +567,6 @@ impl DistributionMetadata for BuiltDist { impl DistributionMetadata for Dist { fn version_or_url(&self) -> VersionOrUrl { match self { - Self::Installed(installed) => VersionOrUrl::Version(installed.version()), Self::Built(dist) => dist.version_or_url(), Self::Source(dist) => dist.version_or_url(), } @@ -732,7 +729,6 @@ impl RemoteSource for BuiltDist { impl RemoteSource for Dist { fn filename(&self) -> Result, Error> { match self { - Self::Installed(_) => Ok(Cow::from("foo")), Self::Built(dist) => dist.filename(), Self::Source(dist) => dist.filename(), } @@ -740,7 +736,6 @@ impl RemoteSource for Dist { fn size(&self) -> Option { match self { - Self::Installed(_) => None, Self::Built(dist) => dist.size(), Self::Source(dist) => dist.size(), } @@ -964,7 +959,6 @@ impl Identifier for InstalledDist { impl Identifier for Dist { fn distribution_id(&self) -> DistributionId { match self { - Self::Installed(dist) => dist.distribution_id(), Self::Built(dist) => dist.distribution_id(), Self::Source(dist) => dist.distribution_id(), } @@ -972,7 +966,6 @@ impl Identifier for Dist { fn resource_id(&self) -> ResourceId { match self { - Self::Installed(dist) => dist.resource_id(), Self::Built(dist) => dist.resource_id(), Self::Source(dist) => dist.resource_id(), } diff --git a/crates/distribution-types/src/prioritized_distribution.rs b/crates/distribution-types/src/prioritized_distribution.rs index 888051c99208..e039eb311c6c 100644 --- a/crates/distribution-types/src/prioritized_distribution.rs +++ b/crates/distribution-types/src/prioritized_distribution.rs @@ -4,7 +4,7 @@ use pep440_rs::VersionSpecifiers; use platform_tags::{IncompatibleTag, TagCompatibility, TagPriority}; use pypi_types::{Hashes, Yanked}; -use crate::Dist; +use crate::{Dist, InstalledDist, ResolvedDistRef}; /// A collection of distributions that have been filtered by relevance. #[derive(Debug, Default, Clone)] @@ -25,7 +25,7 @@ struct PrioritizedDistInner { #[derive(Debug, Clone)] pub enum CompatibleDist<'a> { /// The distribution is already installed and can be used. - InstalledDist(Dist), + InstalledDist(&'a InstalledDist), /// The distribution should be resolved and installed using a source distribution. SourceDist(&'a Dist), /// The distribution should be resolved and installed using a wheel distribution. @@ -286,29 +286,29 @@ impl PrioritizedDist { } impl<'a> CompatibleDist<'a> { - /// Return the [`Dist`] to use during resolution. - pub fn for_resolution(&self) -> &Dist { + /// Return the [`ResolvedDistRef`] to use during resolution. + pub fn for_resolution(&self) -> ResolvedDistRef<'a> { match *self { - CompatibleDist::InstalledDist(ref dist) => dist, - CompatibleDist::SourceDist(sdist) => sdist, - CompatibleDist::CompatibleWheel(wheel, _) => wheel, + CompatibleDist::InstalledDist(dist) => ResolvedDistRef::Installed(dist), + CompatibleDist::SourceDist(sdist) => ResolvedDistRef::Installable(sdist), + CompatibleDist::CompatibleWheel(wheel, _) => ResolvedDistRef::Installable(wheel), CompatibleDist::IncompatibleWheel { source_dist: _, wheel, - } => wheel, + } => ResolvedDistRef::Installable(wheel), } } - /// Return the [`Dist`] to use during installation. - pub fn for_installation(&self) -> &Dist { + /// Return the [`ResolvedDistRef`] to use during installation. + pub fn for_installation(&self) -> ResolvedDistRef<'a> { match *self { - CompatibleDist::InstalledDist(ref dist) => dist, - CompatibleDist::SourceDist(sdist) => sdist, - CompatibleDist::CompatibleWheel(wheel, _) => wheel, + CompatibleDist::InstalledDist(dist) => ResolvedDistRef::Installed(dist), + CompatibleDist::SourceDist(sdist) => ResolvedDistRef::Installable(sdist), + CompatibleDist::CompatibleWheel(wheel, _) => ResolvedDistRef::Installable(wheel), CompatibleDist::IncompatibleWheel { source_dist, wheel: _, - } => source_dist, + } => ResolvedDistRef::Installable(source_dist), } } } diff --git a/crates/distribution-types/src/resolution.rs b/crates/distribution-types/src/resolution.rs index d0b81d11c0b0..118fd407814b 100644 --- a/crates/distribution-types/src/resolution.rs +++ b/crates/distribution-types/src/resolution.rs @@ -3,35 +3,46 @@ use rustc_hash::FxHashMap; use pep508_rs::Requirement; use uv_normalize::PackageName; -use crate::{BuiltDist, Dist, Name, PathSourceDist, SourceDist}; +use crate::{BuiltDist, Dist, InstalledDist, Name, PathSourceDist, ResolvedDist, SourceDist}; /// A set of packages pinned at specific versions. #[derive(Debug, Default, Clone)] -pub struct Resolution(FxHashMap); +pub struct Resolution(FxHashMap); impl Resolution { /// Create a new resolution from the given pinned packages. - pub fn new(packages: FxHashMap) -> Self { + pub fn new(packages: FxHashMap) -> Self { Self(packages) } /// Return the distribution for the given package name, if it exists. - pub fn get(&self, package_name: &PackageName) -> Option<&Dist> { + pub fn get(&self, package_name: &PackageName) -> Option<&ResolvedDist> { self.0.get(package_name) } + /// Return the remote distribution for the given package name, if it exists. + pub fn get_remote(&self, package_name: &PackageName) -> Option<&Dist> { + match self.0.get(package_name) { + Some(dist) => match dist { + ResolvedDist::Installable(dist) => Some(dist), + ResolvedDist::Installed(_) => None, + }, + None => None, + } + } + /// Iterate over the [`PackageName`] entities in this resolution. pub fn packages(&self) -> impl Iterator { self.0.keys() } - /// Iterate over the [`Dist`] entities in this resolution. - pub fn distributions(&self) -> impl Iterator { + /// Iterate over the [`ResolvedDist`] entities in this resolution. + pub fn distributions(&self) -> impl Iterator { self.0.values() } - /// Iterate over the [`Dist`] entities in this resolution. - pub fn into_distributions(self) -> impl Iterator { + /// Iterate over the [`ResolvedDist`] entities in this resolution. + pub fn into_distributions(self) -> impl Iterator { self.0.into_values() } @@ -48,17 +59,19 @@ impl Resolution { /// Return the set of [`Requirement`]s that this resolution represents, exclusive of any /// editable requirements. pub fn requirements(&self) -> Vec { - let mut requirements = self - .0 - .values() - .filter_map(|dist| match dist { - // Remove editable requirements - Dist::Source(SourceDist::Path(PathSourceDist { editable: true, .. })) => None, - // Remove already-installed distributions - Dist::Installed(_) => None, - dist => Some(Requirement::from(dist.clone())), - }) - .collect::>(); + let mut requirements = + self.0 + .values() + .filter_map(|dist| match dist { + // Remove editable requirements + &ResolvedDist::Installable(Dist::Source(SourceDist::Path( + PathSourceDist { editable: true, .. }, + ))) => None, + // Remove already-installed distributions + &ResolvedDist::Installed(_) => None, + dist => Some(Requirement::from(dist.clone())), + }) + .collect::>(); requirements.sort_unstable_by(|a, b| a.name.cmp(&b.name)); requirements } @@ -67,16 +80,6 @@ impl Resolution { impl From for Requirement { fn from(dist: Dist) -> Self { match dist { - Dist::Installed(dist) => Self { - name: dist.name().clone(), - extras: vec![], - version_or_url: Some(pep508_rs::VersionOrUrl::VersionSpecifier( - pep440_rs::VersionSpecifiers::from( - pep440_rs::VersionSpecifier::equals_version(dist.version().clone()), - ), - )), - marker: None, - }, Dist::Built(BuiltDist::Registry(wheel)) => Self { name: wheel.filename.name, extras: vec![], @@ -131,3 +134,27 @@ impl From for Requirement { } } } + +impl From for Requirement { + fn from(dist: InstalledDist) -> Self { + Self { + name: dist.name().clone(), + extras: vec![], + version_or_url: Some(pep508_rs::VersionOrUrl::VersionSpecifier( + pep440_rs::VersionSpecifiers::from(pep440_rs::VersionSpecifier::equals_version( + dist.version().clone(), + )), + )), + marker: None, + } + } +} + +impl From for Requirement { + fn from(dist: ResolvedDist) -> Self { + match dist { + ResolvedDist::Installable(dist) => dist.clone().into(), + ResolvedDist::Installed(dist) => dist.clone().into(), + } + } +} diff --git a/crates/distribution-types/src/resolved.rs b/crates/distribution-types/src/resolved.rs new file mode 100644 index 000000000000..7c0c9935596b --- /dev/null +++ b/crates/distribution-types/src/resolved.rs @@ -0,0 +1,122 @@ +use std::fmt::Display; + +use pep508_rs::PackageName; + +use crate::{ + Dist, DistributionId, DistributionMetadata, Identifier, InstalledDist, Name, ResourceId, + VersionOrUrl, +}; + +/// A distribution that can be used for resolution and installation. +/// +/// Either an already-installed distribution or a distribution that can be installed. +#[derive(Debug, Clone)] +pub enum ResolvedDist { + Installed(InstalledDist), + Installable(Dist), +} + +/// A variant of [`ResolvedDist`] with borrowed inner distributions. +#[derive(Debug, Clone)] +pub enum ResolvedDistRef<'a> { + Installed(&'a InstalledDist), + Installable(&'a Dist), +} + +impl ResolvedDistRef<'_> { + pub fn as_owned(&self) -> ResolvedDist { + match self { + Self::Installable(dist) => ResolvedDist::Installable((*dist).clone()), + Self::Installed(dist) => ResolvedDist::Installed((*dist).clone()), + } + } +} + +impl Name for ResolvedDistRef<'_> { + fn name(&self) -> &PackageName { + match self { + Self::Installable(dist) => dist.name(), + Self::Installed(dist) => dist.name(), + } + } +} + +impl DistributionMetadata for ResolvedDistRef<'_> { + fn version_or_url(&self) -> VersionOrUrl { + match self { + Self::Installed(installed) => VersionOrUrl::Version(installed.version()), + Self::Installable(dist) => dist.version_or_url(), + } + } +} + +impl Identifier for ResolvedDistRef<'_> { + fn distribution_id(&self) -> DistributionId { + match self { + Self::Installed(dist) => dist.distribution_id(), + Self::Installable(dist) => dist.distribution_id(), + } + } + + fn resource_id(&self) -> ResourceId { + match self { + Self::Installed(dist) => dist.resource_id(), + Self::Installable(dist) => dist.resource_id(), + } + } +} + +impl Name for ResolvedDist { + fn name(&self) -> &PackageName { + match self { + Self::Installable(dist) => dist.name(), + Self::Installed(dist) => dist.name(), + } + } +} + +impl DistributionMetadata for ResolvedDist { + fn version_or_url(&self) -> VersionOrUrl { + match self { + Self::Installed(installed) => installed.version_or_url(), + Self::Installable(dist) => dist.version_or_url(), + } + } +} + +impl Identifier for ResolvedDist { + fn distribution_id(&self) -> DistributionId { + match self { + Self::Installed(dist) => dist.distribution_id(), + Self::Installable(dist) => dist.distribution_id(), + } + } + + fn resource_id(&self) -> ResourceId { + match self { + Self::Installed(dist) => dist.resource_id(), + Self::Installable(dist) => dist.resource_id(), + } + } +} + +impl From for ResolvedDist { + fn from(value: Dist) -> Self { + ResolvedDist::Installable(value) + } +} + +impl From for ResolvedDist { + fn from(value: InstalledDist) -> Self { + ResolvedDist::Installed(value) + } +} + +impl Display for ResolvedDist { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Installed(dist) => dist.fmt(f), + Self::Installable(dist) => dist.fmt(f), + } + } +} diff --git a/crates/uv-dev/src/install_many.rs b/crates/uv-dev/src/install_many.rs index 5885ee18903a..1045b29b5a12 100644 --- a/crates/uv-dev/src/install_many.rs +++ b/crates/uv-dev/src/install_many.rs @@ -10,7 +10,8 @@ use rustc_hash::FxHashMap; use tracing::info; use distribution_types::{ - CachedDist, Dist, DistributionMetadata, IndexLocations, Name, Resolution, VersionOrUrl, + CachedDist, Dist, DistributionMetadata, IndexLocations, Name, Resolution, ResolvedDist, + VersionOrUrl, }; use install_wheel_rs::linker::LinkMode; use pep508_rs::Requirement; @@ -125,7 +126,7 @@ async fn install_chunk( .resolve_stream(requirements) .collect() .await; - let (resolution, failures): (FxHashMap, Vec<_>) = + let (resolution, failures): (FxHashMap, Vec<_>) = resolution.into_iter().partition_result(); for failure in &failures { info!("Failed to find wheel: {failure}"); @@ -140,9 +141,11 @@ async fn install_chunk( let only_wheels: FxHashMap<_, _> = resolution .into_iter() .filter(|(_, dist)| match dist { - Dist::Installed(_) => true, - Dist::Built(_) => true, - Dist::Source(_) => false, + ResolvedDist::Installable(dist) => match dist { + Dist::Built(_) => true, + Dist::Source(_) => false, + }, + ResolvedDist::Installed(_) => true, }) .collect(); info!( @@ -171,9 +174,16 @@ async fn install_chunk( }); info!("Cached: {}, Uncached {}", cached.len(), uncached.len()); + let (_installed, remote): (Vec<_>, Vec<_>) = + dists.into_iter().partition_map(|dist| match dist { + ResolvedDist::Installed(dist) => Either::Left(dist), + ResolvedDist::Installable(dist) => Either::Right(dist), + }); + info!("Cached: {}, Uncached {}", cached.len(), uncached.len()); + let downloader = Downloader::new(build_dispatch.cache(), tags, client, build_dispatch); let in_flight = InFlight::default(); - let fetches: Vec<_> = futures::stream::iter(uncached) + let fetches: Vec<_> = futures::stream::iter(remote) .map(|dist| downloader.get_wheel(dist, &in_flight)) .buffer_unordered(50) .collect() diff --git a/crates/uv-dispatch/src/lib.rs b/crates/uv-dispatch/src/lib.rs index 56a2b858be55..304f4336eae9 100644 --- a/crates/uv-dispatch/src/lib.rs +++ b/crates/uv-dispatch/src/lib.rs @@ -189,6 +189,7 @@ impl<'a> BuildContext for BuildDispatch<'a> { let Plan { local, remote, + installed: _, reinstalls, extraneous: _, } = Planner::with_requirements(&resolution.requirements()).build( @@ -212,7 +213,7 @@ impl<'a> BuildContext for BuildDispatch<'a> { .iter() .map(|dist| { resolution - .get(&dist.name) + .get_remote(&dist.name) .cloned() .expect("Resolution should contain all packages") }) diff --git a/crates/uv-distribution/src/distribution_database.rs b/crates/uv-distribution/src/distribution_database.rs index ebf3cd7013c9..ead8afb97a41 100644 --- a/crates/uv-distribution/src/distribution_database.rs +++ b/crates/uv-distribution/src/distribution_database.rs @@ -103,9 +103,6 @@ impl<'a, Context: BuildContext + Send + Sync> DistributionDatabase<'a, Context> NoBinary::Packages(packages) => packages.contains(dist.name()), }; match &dist { - Dist::Installed(_) => { - unreachable!("Wheels should not be retrieved for already installed distributions") - } Dist::Built(BuiltDist::Registry(wheel)) => { if no_binary { return Err(Error::NoBinary); @@ -330,8 +327,6 @@ impl<'a, Context: BuildContext + Send + Sync> DistributionDatabase<'a, Context> dist: &Dist, ) -> Result<(Metadata23, Option), Error> { match dist { - // TODO(zanieb): `dist.metadata()` should return us the right error kind so we can use `Error::Metadata` here - Dist::Installed(dist) => Ok(dist.metadata().map(|metadata| (metadata, None)).unwrap()), Dist::Built(built_dist) => { match self.client.wheel_metadata(built_dist).boxed().await { Ok(metadata) => Ok((metadata, None)), diff --git a/crates/uv-installer/src/plan.rs b/crates/uv-installer/src/plan.rs index e71f87fd8f58..58061c89839e 100644 --- a/crates/uv-installer/src/plan.rs +++ b/crates/uv-installer/src/plan.rs @@ -71,6 +71,7 @@ impl<'a> Planner<'a> { let mut local = vec![]; let mut remote = vec![]; let mut reinstalls = vec![]; + let installed = vec![]; let mut extraneous = vec![]; let mut seen = FxHashMap::with_capacity_and_hasher( self.requirements.len(), @@ -252,9 +253,6 @@ impl<'a> Planner<'a> { } Some(VersionOrUrl::Url(url)) => { match Dist::from_url(requirement.name.clone(), url.clone())? { - Dist::Installed(_) => { - // Nothing to do. - } Dist::Built(BuiltDist::Registry(_)) => { // Nothing to do. } @@ -411,12 +409,7 @@ impl<'a> Planner<'a> { } } - Ok(Plan { - local, - remote, - reinstalls, - extraneous, - }) + Ok(Plan { local, remote, reinstalls, installed, extraneous }) } } @@ -442,6 +435,10 @@ pub struct Plan { /// re-installed (including upgraded) to satisfy the requirements. pub reinstalls: Vec, + /// Any distributions that are already installed in the current environment, and can be used + /// to satisfy the requirements without reinstallation. + pub installed: Vec, + /// Any distributions that are already installed in the current environment, and are /// _not_ necessary to satisfy the requirements. pub extraneous: Vec, diff --git a/crates/uv-resolver/src/candidate_selector.rs b/crates/uv-resolver/src/candidate_selector.rs index 0a46776965c1..96dd015fda45 100644 --- a/crates/uv-resolver/src/candidate_selector.rs +++ b/crates/uv-resolver/src/candidate_selector.rs @@ -1,6 +1,6 @@ use pubgrub::range::Range; -use distribution_types::{CompatibleDist, Dist, IncompatibleDist, IncompatibleSource}; +use distribution_types::{CompatibleDist, IncompatibleDist, IncompatibleSource}; use distribution_types::{DistributionMetadata, IncompatibleWheel, Name, PrioritizedDist}; use pep440_rs::Version; use pep508_rs::MarkerEnvironment; @@ -91,9 +91,7 @@ impl CandidateSelector { return Some(Candidate { name: package_name, version, - dist: CandidateDist::Compatible(CompatibleDist::InstalledDist( - Dist::Installed(dist.clone()), - )), + dist: CandidateDist::Compatible(CompatibleDist::InstalledDist(dist)), }); } } diff --git a/crates/uv-resolver/src/finder.rs b/crates/uv-resolver/src/finder.rs index c6f6770a3477..07ca8032ff9b 100644 --- a/crates/uv-resolver/src/finder.rs +++ b/crates/uv-resolver/src/finder.rs @@ -8,7 +8,7 @@ use rustc_hash::FxHashMap; use uv_traits::{NoBinary, NoBuild}; use distribution_filename::DistFilename; -use distribution_types::{Dist, IndexUrl, Resolution}; +use distribution_types::{Dist, IndexUrl, Resolution, ResolvedDist}; use pep508_rs::{Requirement, VersionOrUrl}; use platform_tags::{TagCompatibility, Tags}; use uv_client::{ @@ -66,7 +66,7 @@ impl<'a> DistFinder<'a> { &self, requirement: &Requirement, flat_index: Option<&FlatDistributions>, - ) -> Result<(PackageName, Dist), ResolveError> { + ) -> Result<(PackageName, ResolvedDist), ResolveError> { match requirement.version_or_url.as_ref() { None | Some(VersionOrUrl::VersionSpecifier(_)) => { // Query the index(es) (cached) to get the URLs for the available files. @@ -89,7 +89,7 @@ impl<'a> DistFinder<'a> { // We have a URL; fetch the distribution directly. let package_name = requirement.name.clone(); let package = Dist::from_url(package_name.clone(), url.clone())?; - Ok((package_name, package)) + Ok((package_name, ResolvedDist::Installable(package))) } } } @@ -98,7 +98,7 @@ impl<'a> DistFinder<'a> { pub fn resolve_stream<'data>( &'data self, requirements: &'data [Requirement], - ) -> impl Stream> + 'data { + ) -> impl Stream> + 'data { stream::iter(requirements) .map(move |requirement| { self.resolve_requirement(requirement, self.flat_index.get(&requirement.name)) @@ -112,7 +112,7 @@ impl<'a> DistFinder<'a> { return Ok(Resolution::default()); } - let resolution: FxHashMap = + let resolution: FxHashMap = self.resolve_stream(requirements).try_collect().await?; if let Some(reporter) = self.reporter.as_ref() { @@ -132,7 +132,7 @@ impl<'a> DistFinder<'a> { metadata: SimpleMetadata, index: &IndexUrl, flat_index: Option<&FlatDistributions>, - ) -> Option { + ) -> Option { let no_binary = match self.no_binary { NoBinary::None => false, NoBinary::All => true, @@ -248,13 +248,15 @@ impl<'a> DistFinder<'a> { } } - best_wheel.map_or(best_sdist, |(wheel, ..)| Some(wheel)) + best_wheel + .map_or(best_sdist, |(wheel, ..)| Some(wheel)) + .map(ResolvedDist::Installable) } } pub trait Reporter: Send + Sync { /// Callback to invoke when a package is resolved to a specific distribution. - fn on_progress(&self, dist: &Dist); + fn on_progress(&self, dist: &ResolvedDist); /// Callback to invoke when the resolution is complete. fn on_complete(&self); diff --git a/crates/uv-resolver/src/pins.rs b/crates/uv-resolver/src/pins.rs index 70e055ae8518..480901bb5349 100644 --- a/crates/uv-resolver/src/pins.rs +++ b/crates/uv-resolver/src/pins.rs @@ -1,6 +1,6 @@ use rustc_hash::FxHashMap; -use distribution_types::{CompatibleDist, Dist}; +use distribution_types::{CompatibleDist, ResolvedDist}; use uv_normalize::PackageName; use crate::candidate_selector::Candidate; @@ -10,19 +10,23 @@ 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>); +pub(crate) struct FilePins(FxHashMap>); impl FilePins { /// Pin a candidate package. pub(crate) fn insert(&mut self, candidate: &Candidate, dist: &CompatibleDist) { - self.0 - .entry(candidate.name().clone()) - .or_default() - .insert(candidate.version().clone(), dist.for_installation().clone()); + self.0.entry(candidate.name().clone()).or_default().insert( + candidate.version().clone(), + dist.for_installation().as_owned(), + ); } /// 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<&Dist> { + pub(crate) fn get( + &self, + name: &PackageName, + version: &pep440_rs::Version, + ) -> Option<&ResolvedDist> { self.0.get(name)?.get(version) } } diff --git a/crates/uv-resolver/src/resolution.rs b/crates/uv-resolver/src/resolution.rs index 59430bb0c3dd..1d5e54538669 100644 --- a/crates/uv-resolver/src/resolution.rs +++ b/crates/uv-resolver/src/resolution.rs @@ -14,7 +14,10 @@ use rustc_hash::FxHashMap; use url::Url; use crate::dependency_provider::UvDependencyProvider; -use distribution_types::{Dist, DistributionMetadata, LocalEditable, Name, PackageId, Verbatim}; +use distribution_types::{ + Dist, DistributionMetadata, LocalEditable, Name, PackageId, ResolvedDist, ResolvedDist, + Verbatim, +}; use once_map::OnceMap; use pep440_rs::Version; use pypi_types::{Hashes, Metadata23}; @@ -45,7 +48,7 @@ pub enum AnnotationStyle { #[derive(Debug)] pub struct ResolutionGraph { /// The underlying graph. - petgraph: petgraph::graph::Graph, petgraph::Directed>, + petgraph: petgraph::graph::Graph, petgraph::Directed>, /// The metadata for every distribution in this resolution. hashes: FxHashMap>, /// The enabled extras for every distribution in this resolution. @@ -85,7 +88,10 @@ impl ResolutionGraph { PubGrubPackage::Package(package_name, None, None) => { // Create the distribution. let pinned_package = if let Some((editable, _)) = editables.get(package_name) { - Dist::from_editable(package_name.clone(), editable.clone())? + ResolvedDist::Installable(Dist::from_editable( + package_name.clone(), + editable.clone(), + )?) } else { pins.get(package_name, version) .expect("Every package should be pinned") @@ -137,7 +143,7 @@ impl ResolutionGraph { } // Add the distribution to the graph. - let index = petgraph.add_node(pinned_package); + let index = petgraph.add_node(ResolvedDist::Installable(pinned_package)); inverse.insert(package_name, index); } PubGrubPackage::Package(package_name, Some(extra), None) => { @@ -155,7 +161,7 @@ impl ResolutionGraph { Dist::from_editable(package_name.clone(), editable.clone())?; diagnostics.push(Diagnostic::MissingExtra { - dist: pinned_package, + dist: ResolvedDist::Installable(pinned_package), extra: extra.clone(), }); } @@ -198,8 +204,10 @@ impl ResolutionGraph { .or_insert_with(Vec::new) .push(extra.clone()); } else { - let pinned_package = - Dist::from_editable(package_name.clone(), editable.clone())?; + let pinned_package = ResolvedDist::Installable(Dist::from_editable( + package_name.clone(), + editable.clone(), + )?); diagnostics.push(Diagnostic::MissingExtra { dist: pinned_package, @@ -224,7 +232,10 @@ impl ResolutionGraph { || url.clone(), |precise| apply_redirect(url, precise.value()), ); - let pinned_package = Dist::from_url(package_name.clone(), url)?; + let pinned_package = ResolvedDist::Installable(Dist::from_url( + package_name.clone(), + url, + )?); diagnostics.push(Diagnostic::MissingExtra { dist: pinned_package, @@ -313,7 +324,9 @@ impl ResolutionGraph { } /// Return the underlying graph. - pub fn petgraph(&self) -> &petgraph::graph::Graph, petgraph::Directed> { + pub fn petgraph( + &self, + ) -> &petgraph::graph::Graph, petgraph::Directed> { &self.petgraph } } @@ -376,7 +389,7 @@ enum Node<'a> { /// A node linked to an editable distribution. Editable(&'a PackageName, &'a LocalEditable), /// A node linked to a non-editable distribution. - Distribution(&'a PackageName, &'a Dist, &'a [ExtraName]), + Distribution(&'a PackageName, &'a ResolvedDist, &'a [ExtraName]), } #[derive(Debug, PartialEq, Eq, PartialOrd, Ord)] @@ -571,7 +584,7 @@ pub enum Diagnostic { MissingExtra { /// The distribution that was requested with an non-existent extra. For example, /// `black==23.10.0`. - dist: Dist, + dist: ResolvedDist, /// The extra that was requested. For example, `colorama` in `black[colorama]`. extra: ExtraName, }, diff --git a/crates/uv-resolver/src/resolver/mod.rs b/crates/uv-resolver/src/resolver/mod.rs index fafb1332a034..2bc9d792a467 100644 --- a/crates/uv-resolver/src/resolver/mod.rs +++ b/crates/uv-resolver/src/resolver/mod.rs @@ -19,7 +19,7 @@ use url::Url; use distribution_types::{ BuiltDist, Dist, DistributionMetadata, IncompatibleDist, IncompatibleSource, IncompatibleWheel, - Name, RemoteSource, SourceDist, VersionOrUrl, + InstalledDist, Name, RemoteSource, ResolvedDist, ResolvedDistRef, SourceDist, VersionOrUrl, }; pub(crate) use locals::Locals; use pep440_rs::{Version, MIN_VERSION}; @@ -687,24 +687,26 @@ impl< } }; + let filename = match dist.for_installation() { + ResolvedDistRef::Installable(dist) => { + dist.filename().unwrap_or(Cow::Borrowed("unknown filename")) + } + ResolvedDistRef::Installed(_) => Cow::Borrowed("installed"), + }; if let Some(extra) = extra { debug!( "Selecting: {}[{}]=={} ({})", candidate.name(), extra, candidate.version(), - dist.for_resolution() - .filename() - .unwrap_or(Cow::Borrowed("unknown filename")) + filename, ); } else { debug!( "Selecting: {}=={} ({})", candidate.name(), candidate.version(), - dist.for_resolution() - .filename() - .unwrap_or(Cow::Borrowed("unknown filename")) + filename, ); } @@ -715,11 +717,14 @@ impl< 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 = dist.for_resolution().clone(); - request_sink.send(Request::Dist(dist)).await?; + let request = match dist.for_resolution() { + ResolvedDistRef::Installable(dist) => Request::Dist(dist.clone()), + ResolvedDistRef::Installed(dist) => Request::Installed(dist.clone()), + }; + request_sink.send(request).await?; } - Ok(Some(ResolverVersion::Available(version))) } } @@ -922,6 +927,10 @@ impl< trace!("Received package metadata for: {package_name}"); self.index.packages.done(package_name, version_map); } + Some(Response::Installed { dist, metadata }) => { + trace!("Received installed distribution metadata for: {dist}"); + self.index.distributions.done(dist.package_id(), metadata); + } Some(Response::Dist { dist: Dist::Built(dist), metadata, @@ -954,11 +963,6 @@ impl< } } } - Some(Response::Dist { - dist: dist @ Dist::Installed(_), - metadata, - .. - }) => self.index.distributions.done(dist.package_id(), metadata), None => {} } } @@ -999,7 +1003,6 @@ impl< Dist::Source(source_dist) => { ResolveError::FetchAndBuild(Box::new(source_dist), err) } - Dist::Installed(_) => unreachable!(), })?; Ok(Some(Response::Dist { dist, @@ -1008,6 +1011,12 @@ impl< })) } + Request::Installed(dist) => { + // TODO(zanieb): Forward the error from `metadata` + let metadata = dist.metadata().unwrap(); + Ok(Some(Response::Installed { dist, metadata })) + } + // Pre-fetch the package and distribution metadata. Request::Prefetch(package_name, range) => { // Wait for the package metadata to become available. @@ -1060,34 +1069,43 @@ impl< // Emit a request to fetch the metadata for this version. if self.index.distributions.register(candidate.package_id()) { - let dist = dist.for_resolution().clone(); - - let (metadata, precise) = self - .provider - .get_or_build_wheel_metadata(&dist) - .boxed() - .await - .map_err(|err| match dist.clone() { - Dist::Built(BuiltDist::Path(built_dist)) => { - ResolveError::Read(Box::new(built_dist), err) - } - Dist::Source(SourceDist::Path(source_dist)) => { - ResolveError::Build(Box::new(source_dist), err) - } - Dist::Built(built_dist) => { - ResolveError::Fetch(Box::new(built_dist), err) - } - Dist::Source(source_dist) => { - ResolveError::FetchAndBuild(Box::new(source_dist), err) + let dist = dist.for_resolution().as_owned(); + + let response = match dist { + ResolvedDist::Installable(dist) => { + let (metadata, precise) = self + .provider + .get_or_build_wheel_metadata(&dist) + .boxed() + .await + .map_err(|err| match dist.clone() { + Dist::Built(BuiltDist::Path(built_dist)) => { + ResolveError::Read(Box::new(built_dist), err) + } + Dist::Source(SourceDist::Path(source_dist)) => { + ResolveError::Build(Box::new(source_dist), err) + } + Dist::Built(built_dist) => { + ResolveError::Fetch(Box::new(built_dist), err) + } + Dist::Source(source_dist) => { + ResolveError::FetchAndBuild(Box::new(source_dist), err) + } + })?; + Response::Dist { + dist, + metadata, + precise, } - Dist::Installed(_) => unreachable!(), - })?; - - Ok(Some(Response::Dist { - dist, - metadata, - precise, - })) + } + ResolvedDist::Installed(dist) => { + // TODO(zanieb): Forward the error from `metadata` + let metadata = dist.metadata().unwrap(); + Response::Installed { dist, metadata } + } + }; + + Ok(Some(response)) } else { Ok(None) } @@ -1125,6 +1143,8 @@ pub(crate) enum Request { Package(PackageName), /// A request to fetch the metadata for a built or source distribution. Dist(Dist), + /// 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), } @@ -1138,6 +1158,9 @@ impl Display for Request { Self::Dist(dist) => { write!(f, "Metadata {dist}") } + Self::Installed(dist) => { + write!(f, "Installed metadata {dist}") + } Self::Prefetch(package_name, range) => { write!(f, "Prefetch {package_name} {range}") } @@ -1156,6 +1179,11 @@ enum Response { metadata: Metadata23, precise: Option, }, + /// The returned metadata for an already-installed distribution. + Installed { + dist: InstalledDist, + metadata: Metadata23, + }, } /// An enum used by [`DependencyProvider`] that holds information about package dependencies. diff --git a/crates/uv/src/commands/pip_install.rs b/crates/uv/src/commands/pip_install.rs index cf9457b579de..a3e6bbc47830 100644 --- a/crates/uv/src/commands/pip_install.rs +++ b/crates/uv/src/commands/pip_install.rs @@ -618,6 +618,7 @@ async fn install( local, remote, reinstalls, + installed: _, extraneous: _, } = plan; @@ -642,7 +643,7 @@ async fn install( .iter() .map(|dist| { resolution - .get(&dist.name) + .get_remote(&dist.name) .cloned() .expect("Resolution should contain all packages") }) @@ -781,6 +782,7 @@ async fn install( local, remote, reinstalls, + installed: _, extraneous: _, } = plan; @@ -806,7 +808,7 @@ async fn install( .iter() .map(|dist| { resolution - .get(&dist.name) + .get_remote(&dist.name) .cloned() .expect("Resolution should contain all packages") }) diff --git a/crates/uv/src/commands/pip_sync.rs b/crates/uv/src/commands/pip_sync.rs index 5f6722494663..89a1e15c5aed 100644 --- a/crates/uv/src/commands/pip_sync.rs +++ b/crates/uv/src/commands/pip_sync.rs @@ -5,7 +5,9 @@ use itertools::Itertools; use owo_colors::OwoColorize; use tracing::debug; -use distribution_types::{IndexLocations, InstalledMetadata, LocalDist, LocalEditable, Name}; +use distribution_types::{ + IndexLocations, InstalledMetadata, LocalDist, LocalEditable, Name, ResolvedDist, +}; use install_wheel_rs::linker::LinkMode; use platform_tags::Tags; use pypi_types::Yanked; @@ -206,6 +208,7 @@ pub(crate) async fn pip_sync( local, remote, reinstalls, + installed: _, extraneous, } = Planner::with_requirements(&requirements) .with_editable_requirements(&resolved_editables.editables) @@ -266,7 +269,13 @@ pub(crate) async fn pip_sync( .dimmed() )?; - resolution.into_distributions().collect::>() + resolution + .into_distributions() + .filter_map(|dist| match dist { + ResolvedDist::Installable(dist) => Some(dist), + ResolvedDist::Installed(_) => None, + }) + .collect::>() }; // Download, build, and unzip any missing distributions. diff --git a/crates/uv/src/commands/reporters.rs b/crates/uv/src/commands/reporters.rs index b6d28194fa02..0bfaf0b3acf0 100644 --- a/crates/uv/src/commands/reporters.rs +++ b/crates/uv/src/commands/reporters.rs @@ -6,8 +6,8 @@ use owo_colors::OwoColorize; use url::Url; use distribution_types::{ - BuildableSource, CachedDist, Dist, DistributionMetadata, LocalEditable, Name, SourceDist, - VersionOrUrl, + BuildableSource, CachedDist, DistributionMetadata, LocalEditable, Name, ResolvedDist, + SourceDist, VersionOrUrl, }; use uv_normalize::PackageName; @@ -38,7 +38,7 @@ impl FinderReporter { } impl uv_resolver::FinderReporter for FinderReporter { - fn on_progress(&self, dist: &Dist) { + fn on_progress(&self, dist: &ResolvedDist) { self.progress.set_message(format!("{dist}")); self.progress.inc(1); } diff --git a/crates/uv/src/commands/venv.rs b/crates/uv/src/commands/venv.rs index fd64d90215f6..8c39b438d5cb 100644 --- a/crates/uv/src/commands/venv.rs +++ b/crates/uv/src/commands/venv.rs @@ -11,7 +11,7 @@ use miette::{Diagnostic, IntoDiagnostic}; use owo_colors::OwoColorize; use thiserror::Error; -use distribution_types::{DistributionMetadata, IndexLocations, Name}; +use distribution_types::{DistributionMetadata, IndexLocations, Name, ResolvedDist}; use pep508_rs::Requirement; use uv_auth::{KeyringProvider, GLOBAL_AUTH_STORE}; use uv_cache::Cache; @@ -213,6 +213,10 @@ async fn venv_impl( for distribution in resolution .distributions() + .filter_map(|dist| match dist { + ResolvedDist::Installable(dist) => Some(dist), + ResolvedDist::Installed(_) => None, + }) .sorted_unstable_by(|a, b| a.name().cmp(b.name()).then(a.version().cmp(&b.version()))) { writeln!(