From 9339e55a11a0ab3861f94842fded054c2bad1f76 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 13 Nov 2024 19:06:16 -0500 Subject: [PATCH] Add `version` to `ResolvedDist` (#9102) ## Summary I need this for the derivation chain work (https://github.com/astral-sh/uv/issues/8962), but it just seems generally useful. You can't always get a version from a `Dist` (it could be URL-based!), but when we create a `ResolvedDist`, we _do_ know the version (and not just the URL). This PR preserves it. --- .../src/prioritized_distribution.rs | 4 +- .../uv-distribution-types/src/resolution.rs | 4 +- crates/uv-distribution-types/src/resolved.rs | 84 +++++++++++-------- crates/uv-installer/src/plan.rs | 10 +-- crates/uv-resolver/src/lock/mod.rs | 12 +-- crates/uv-resolver/src/lock/target.rs | 13 +-- crates/uv-resolver/src/resolution/graph.rs | 9 +- crates/uv-resolver/src/resolution/mod.rs | 4 +- crates/uv-resolver/src/resolver/mod.rs | 8 +- crates/uv/src/commands/project/sync.rs | 39 +++++---- 10 files changed, 107 insertions(+), 80 deletions(-) diff --git a/crates/uv-distribution-types/src/prioritized_distribution.rs b/crates/uv-distribution-types/src/prioritized_distribution.rs index c03317245..a232b334b 100644 --- a/crates/uv-distribution-types/src/prioritized_distribution.rs +++ b/crates/uv-distribution-types/src/prioritized_distribution.rs @@ -442,7 +442,7 @@ impl<'a> CompatibleDist<'a> { /// Return the [`ResolvedDistRef`] to use during resolution. pub fn for_resolution(&self) -> ResolvedDistRef<'a> { match *self { - CompatibleDist::InstalledDist(dist) => ResolvedDistRef::Installed(dist), + CompatibleDist::InstalledDist(dist) => ResolvedDistRef::Installed { dist }, CompatibleDist::SourceDist { sdist, prioritized } => { ResolvedDistRef::InstallableRegistrySourceDist { sdist, prioritized } } @@ -458,7 +458,7 @@ impl<'a> CompatibleDist<'a> { /// Return the [`ResolvedDistRef`] to use during installation. pub fn for_installation(&self) -> ResolvedDistRef<'a> { match *self { - CompatibleDist::InstalledDist(dist) => ResolvedDistRef::Installed(dist), + CompatibleDist::InstalledDist(dist) => ResolvedDistRef::Installed { dist }, CompatibleDist::SourceDist { sdist, prioritized } => { ResolvedDistRef::InstallableRegistrySourceDist { sdist, prioritized } } diff --git a/crates/uv-distribution-types/src/resolution.rs b/crates/uv-distribution-types/src/resolution.rs index 7c97223c1..f122cf263 100644 --- a/crates/uv-distribution-types/src/resolution.rs +++ b/crates/uv-distribution-types/src/resolution.rs @@ -170,7 +170,7 @@ impl Diagnostic for ResolutionDiagnostic { impl From<&ResolvedDist> for Requirement { fn from(resolved_dist: &ResolvedDist) -> Self { let source = match resolved_dist { - ResolvedDist::Installable(dist) => match dist { + ResolvedDist::Installable { dist, .. } => match dist { Dist::Built(BuiltDist::Registry(wheels)) => RequirementSource::Registry { specifier: uv_pep440::VersionSpecifiers::from( uv_pep440::VersionSpecifier::equals_version( @@ -229,7 +229,7 @@ impl From<&ResolvedDist> for Requirement { r#virtual: sdist.r#virtual, }, }, - ResolvedDist::Installed(dist) => RequirementSource::Registry { + ResolvedDist::Installed { dist } => RequirementSource::Registry { specifier: uv_pep440::VersionSpecifiers::from( uv_pep440::VersionSpecifier::equals_version(dist.version().clone()), ), diff --git a/crates/uv-distribution-types/src/resolved.rs b/crates/uv-distribution-types/src/resolved.rs index b5bb587f3..dcbbb8269 100644 --- a/crates/uv-distribution-types/src/resolved.rs +++ b/crates/uv-distribution-types/src/resolved.rs @@ -1,5 +1,5 @@ use std::fmt::{Display, Formatter}; - +use uv_pep440::Version; use uv_pep508::PackageName; use uv_pypi_types::Yanked; @@ -15,14 +15,16 @@ use crate::{ #[derive(Debug, Clone, Hash)] #[allow(clippy::large_enum_variant)] pub enum ResolvedDist { - Installed(InstalledDist), - Installable(Dist), + Installed { dist: InstalledDist }, + Installable { dist: Dist, version: Version }, } /// A variant of [`ResolvedDist`] with borrowed inner distributions. #[derive(Debug, Clone)] pub enum ResolvedDistRef<'a> { - Installed(&'a InstalledDist), + Installed { + dist: &'a InstalledDist, + }, InstallableRegistrySourceDist { /// The source distribution that should be used. sdist: &'a RegistrySourceDist, @@ -41,36 +43,44 @@ impl ResolvedDist { /// Return true if the distribution is editable. pub fn is_editable(&self) -> bool { match self { - Self::Installable(dist) => dist.is_editable(), - Self::Installed(dist) => dist.is_editable(), + Self::Installable { dist, .. } => dist.is_editable(), + Self::Installed { dist } => dist.is_editable(), } } /// Return true if the distribution refers to a local file or directory. pub fn is_local(&self) -> bool { match self { - Self::Installable(dist) => dist.is_local(), - Self::Installed(dist) => dist.is_local(), + Self::Installable { dist, .. } => dist.is_local(), + Self::Installed { dist } => dist.is_local(), } } /// Returns the [`IndexUrl`], if the distribution is from a registry. pub fn index(&self) -> Option<&IndexUrl> { match self { - Self::Installable(dist) => dist.index(), - Self::Installed(_) => None, + Self::Installable { dist, .. } => dist.index(), + Self::Installed { .. } => None, } } /// Returns the [`Yanked`] status of the distribution, if available. pub fn yanked(&self) -> Option<&Yanked> { match self { - Self::Installable(dist) => match dist { + Self::Installable { dist, .. } => match dist { Dist::Source(SourceDist::Registry(sdist)) => sdist.file.yanked.as_ref(), Dist::Built(BuiltDist::Registry(wheel)) => wheel.best_wheel().file.yanked.as_ref(), _ => None, }, - Self::Installed(_) => None, + Self::Installed { .. } => None, + } + } + + /// Returns the version of the distribution. + pub fn version(&self) -> &Version { + match self { + Self::Installable { version, .. } => version, + Self::Installed { dist } => dist.version(), } } } @@ -87,7 +97,10 @@ impl ResolvedDistRef<'_> { (&source.name, &source.version), "expected chosen sdist to match prioritized sdist" ); - ResolvedDist::Installable(Dist::Source(SourceDist::Registry(source))) + ResolvedDist::Installable { + dist: Dist::Source(SourceDist::Registry(source)), + version: sdist.version.clone(), + } } Self::InstallableRegistryBuiltDist { wheel, prioritized, .. @@ -100,9 +113,14 @@ impl ResolvedDistRef<'_> { // This is okay because we're only here if the prioritized dist // has at least one wheel, so this always succeeds. let built = prioritized.built_dist().expect("at least one wheel"); - ResolvedDist::Installable(Dist::Built(BuiltDist::Registry(built))) + ResolvedDist::Installable { + dist: Dist::Built(BuiltDist::Registry(built)), + version: wheel.filename.version.clone(), + } } - Self::Installed(dist) => ResolvedDist::Installed((*dist).clone()), + Self::Installed { dist } => ResolvedDist::Installed { + dist: (*dist).clone(), + }, } } } @@ -112,7 +130,7 @@ impl Display for ResolvedDistRef<'_> { match self { Self::InstallableRegistrySourceDist { sdist, .. } => Display::fmt(sdist, f), Self::InstallableRegistryBuiltDist { wheel, .. } => Display::fmt(wheel, f), - Self::Installed(dist) => Display::fmt(dist, f), + Self::Installed { dist } => Display::fmt(dist, f), } } } @@ -122,7 +140,7 @@ impl Name for ResolvedDistRef<'_> { match self { Self::InstallableRegistrySourceDist { sdist, .. } => sdist.name(), Self::InstallableRegistryBuiltDist { wheel, .. } => wheel.name(), - Self::Installed(dist) => dist.name(), + Self::Installed { dist } => dist.name(), } } } @@ -130,7 +148,7 @@ impl Name for ResolvedDistRef<'_> { impl DistributionMetadata for ResolvedDistRef<'_> { fn version_or_url(&self) -> VersionOrUrlRef { match self { - Self::Installed(installed) => VersionOrUrlRef::Version(installed.version()), + Self::Installed { dist } => VersionOrUrlRef::Version(dist.version()), Self::InstallableRegistrySourceDist { sdist, .. } => sdist.version_or_url(), Self::InstallableRegistryBuiltDist { wheel, .. } => wheel.version_or_url(), } @@ -140,7 +158,7 @@ impl DistributionMetadata for ResolvedDistRef<'_> { impl Identifier for ResolvedDistRef<'_> { fn distribution_id(&self) -> DistributionId { match self { - Self::Installed(dist) => dist.distribution_id(), + Self::Installed { dist } => dist.distribution_id(), Self::InstallableRegistrySourceDist { sdist, .. } => sdist.distribution_id(), Self::InstallableRegistryBuiltDist { wheel, .. } => wheel.distribution_id(), } @@ -148,7 +166,7 @@ impl Identifier for ResolvedDistRef<'_> { fn resource_id(&self) -> ResourceId { match self { - Self::Installed(dist) => dist.resource_id(), + Self::Installed { dist } => dist.resource_id(), Self::InstallableRegistrySourceDist { sdist, .. } => sdist.resource_id(), Self::InstallableRegistryBuiltDist { wheel, .. } => wheel.resource_id(), } @@ -158,8 +176,8 @@ impl Identifier for ResolvedDistRef<'_> { impl Name for ResolvedDist { fn name(&self) -> &PackageName { match self { - Self::Installable(dist) => dist.name(), - Self::Installed(dist) => dist.name(), + Self::Installable { dist, .. } => dist.name(), + Self::Installed { dist } => dist.name(), } } } @@ -167,8 +185,8 @@ impl Name for ResolvedDist { impl DistributionMetadata for ResolvedDist { fn version_or_url(&self) -> VersionOrUrlRef { match self { - Self::Installed(installed) => installed.version_or_url(), - Self::Installable(dist) => dist.version_or_url(), + Self::Installed { dist } => dist.version_or_url(), + Self::Installable { dist, .. } => dist.version_or_url(), } } } @@ -176,30 +194,24 @@ impl DistributionMetadata for ResolvedDist { impl Identifier for ResolvedDist { fn distribution_id(&self) -> DistributionId { match self { - Self::Installed(dist) => dist.distribution_id(), - Self::Installable(dist) => dist.distribution_id(), + 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(), + 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 Display for ResolvedDist { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { match self { - Self::Installed(dist) => dist.fmt(f), - Self::Installable(dist) => dist.fmt(f), + Self::Installed { dist } => dist.fmt(f), + Self::Installable { dist, .. } => dist.fmt(f), } } } diff --git a/crates/uv-installer/src/plan.rs b/crates/uv-installer/src/plan.rs index cd654206c..4694a0e08 100644 --- a/crates/uv-installer/src/plan.rs +++ b/crates/uv-installer/src/plan.rs @@ -107,18 +107,18 @@ impl<'a> Planner<'a> { } } - let ResolvedDist::Installable(installable) = dist else { + let ResolvedDist::Installable { dist, .. } = dist else { unreachable!("Installed distribution could not be found in site-packages: {dist}"); }; if cache.must_revalidate(dist.name()) { debug!("Must revalidate requirement: {}", dist.name()); - remote.push(installable.clone()); + remote.push(dist.clone()); continue; } // Identify any cached distributions that satisfy the requirement. - match installable { + match dist { Dist::Built(BuiltDist::Registry(wheel)) => { if let Some(distribution) = registry_index.get(wheel.name()).find_map(|entry| { if *entry.index.url() != wheel.best_wheel().index { @@ -309,8 +309,8 @@ impl<'a> Planner<'a> { } } - debug!("Identified uncached distribution: {installable}"); - remote.push(installable.clone()); + debug!("Identified uncached distribution: {dist}"); + remote.push(dist.clone()); } // Remove any unnecessary packages. diff --git a/crates/uv-resolver/src/lock/mod.rs b/crates/uv-resolver/src/lock/mod.rs index ac6756e5b..0b49444e8 100644 --- a/crates/uv-resolver/src/lock/mod.rs +++ b/crates/uv-resolver/src/lock/mod.rs @@ -2360,8 +2360,8 @@ impl Source { fn from_resolved_dist(resolved_dist: &ResolvedDist, root: &Path) -> Result { match *resolved_dist { // We pass empty installed packages for locking. - ResolvedDist::Installed(_) => unreachable!(), - ResolvedDist::Installable(ref dist) => Source::from_dist(dist, root), + ResolvedDist::Installed { .. } => unreachable!(), + ResolvedDist::Installable { ref dist, .. } => Source::from_dist(dist, root), } } @@ -2892,8 +2892,8 @@ impl SourceDist { ) -> Result, LockError> { match annotated_dist.dist { // We pass empty installed packages for locking. - ResolvedDist::Installed(_) => unreachable!(), - ResolvedDist::Installable(ref dist) => { + ResolvedDist::Installed { .. } => unreachable!(), + ResolvedDist::Installable { ref dist, .. } => { SourceDist::from_dist(id, dist, &annotated_dist.hashes, annotated_dist.index()) } } @@ -3174,8 +3174,8 @@ impl Wheel { fn from_annotated_dist(annotated_dist: &AnnotatedDist) -> Result, LockError> { match annotated_dist.dist { // We pass empty installed packages for locking. - ResolvedDist::Installed(_) => unreachable!(), - ResolvedDist::Installable(ref dist) => { + ResolvedDist::Installed { .. } => unreachable!(), + ResolvedDist::Installable { ref dist, .. } => { Wheel::from_dist(dist, &annotated_dist.hashes, annotated_dist.index()) } } diff --git a/crates/uv-resolver/src/lock/target.rs b/crates/uv-resolver/src/lock/target.rs index 4d6e7e46c..d0fdd4ea4 100644 --- a/crates/uv-resolver/src/lock/target.rs +++ b/crates/uv-resolver/src/lock/target.rs @@ -266,11 +266,14 @@ impl<'env> InstallTarget<'env> { ) { map.insert( dist.id.name.clone(), - ResolvedDist::Installable(dist.to_dist( - self.workspace().install_path(), - TagPolicy::Required(tags), - build_options, - )?), + ResolvedDist::Installable { + dist: dist.to_dist( + self.workspace().install_path(), + TagPolicy::Required(tags), + build_options, + )?, + version: dist.id.version.clone(), + }, ); hashes.insert(dist.id.name.clone(), dist.hashes()); } diff --git a/crates/uv-resolver/src/resolution/graph.rs b/crates/uv-resolver/src/resolution/graph.rs index b19abab78..bee82a74b 100644 --- a/crates/uv-resolver/src/resolution/graph.rs +++ b/crates/uv-resolver/src/resolution/graph.rs @@ -444,7 +444,14 @@ impl ResolutionGraph { archive.metadata.clone() }; - (dist.into(), hashes, Some(metadata)) + ( + ResolvedDist::Installable { + dist, + version: version.clone(), + }, + hashes, + Some(metadata), + ) } else { let dist = pins .get(name, version) diff --git a/crates/uv-resolver/src/resolution/mod.rs b/crates/uv-resolver/src/resolution/mod.rs index 0e553edfd..5b75baa3f 100644 --- a/crates/uv-resolver/src/resolution/mod.rs +++ b/crates/uv-resolver/src/resolution/mod.rs @@ -49,8 +49,8 @@ impl AnnotatedDist { /// Returns the [`IndexUrl`] of the distribution, if it is from a registry. pub(crate) fn index(&self) -> Option<&IndexUrl> { match &self.dist { - ResolvedDist::Installed(_) => None, - ResolvedDist::Installable(dist) => match dist { + ResolvedDist::Installed { .. } => None, + ResolvedDist::Installable { dist, .. } => match dist { Dist::Built(dist) => match dist { BuiltDist::Registry(dist) => Some(&dist.best_wheel().index), BuiltDist::DirectUrl(_) => None, diff --git a/crates/uv-resolver/src/resolver/mod.rs b/crates/uv-resolver/src/resolver/mod.rs index 2242e36f7..d7c131b8d 100644 --- a/crates/uv-resolver/src/resolver/mod.rs +++ b/crates/uv-resolver/src/resolver/mod.rs @@ -1151,7 +1151,7 @@ impl ResolverState wheel .filename() .unwrap_or(Cow::Borrowed("unknown filename")), - ResolvedDistRef::Installed(_) => Cow::Borrowed("installed"), + ResolvedDistRef::Installed { .. } => Cow::Borrowed("installed"), }; debug!( @@ -1958,7 +1958,7 @@ impl ResolverState { + ResolvedDist::Installable { dist, .. } => { let metadata = provider .get_or_build_wheel_metadata(&dist) .boxed_local() @@ -1966,7 +1966,7 @@ impl ResolverState { + ResolvedDist::Installed { dist } => { let metadata = dist.metadata().map_err(|err| { ResolveError::ReadInstalled(Box::new(dist.clone()), err) })?; @@ -2621,7 +2621,7 @@ impl<'a> From> for Request { let built = prioritized.built_dist().expect("at least one wheel"); Request::Dist(Dist::Built(BuiltDist::Registry(built))) } - ResolvedDistRef::Installed(dist) => Request::Installed(dist.clone()), + ResolvedDistRef::Installed { dist } => Request::Installed(dist.clone()), } } } diff --git a/crates/uv/src/commands/project/sync.rs b/crates/uv/src/commands/project/sync.rs index 015604931..90b4f2717 100644 --- a/crates/uv/src/commands/project/sync.rs +++ b/crates/uv/src/commands/project/sync.rs @@ -454,7 +454,7 @@ fn apply_no_virtual_project( resolution: uv_distribution_types::Resolution, ) -> uv_distribution_types::Resolution { resolution.filter(|dist| { - let ResolvedDist::Installable(dist) = dist else { + let ResolvedDist::Installable { dist, .. } = dist else { return true; }; @@ -481,26 +481,31 @@ fn apply_editable_mode( // Filter out any editable distributions. EditableMode::NonEditable => resolution.map(|dist| { - let ResolvedDist::Installable(Dist::Source(SourceDist::Directory( - DirectorySourceDist { - name, - install_path, - editable: true, - r#virtual: false, - url, - }, - ))) = dist + let ResolvedDist::Installable { + dist: + Dist::Source(SourceDist::Directory(DirectorySourceDist { + name, + install_path, + editable: true, + r#virtual: false, + url, + })), + version, + } = dist else { return dist; }; - ResolvedDist::Installable(Dist::Source(SourceDist::Directory(DirectorySourceDist { - name, - install_path, - editable: false, - r#virtual: false, - url, - }))) + ResolvedDist::Installable { + dist: Dist::Source(SourceDist::Directory(DirectorySourceDist { + name, + install_path, + editable: false, + r#virtual: false, + url, + })), + version, + } }), } }