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.
This commit is contained in:
Charlie Marsh 2024-11-13 19:06:16 -05:00 committed by GitHub
parent 17181d9e59
commit 9339e55a11
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 107 additions and 80 deletions

View File

@ -442,7 +442,7 @@ impl<'a> CompatibleDist<'a> {
/// Return the [`ResolvedDistRef`] to use during resolution. /// Return the [`ResolvedDistRef`] to use during resolution.
pub fn for_resolution(&self) -> ResolvedDistRef<'a> { pub fn for_resolution(&self) -> ResolvedDistRef<'a> {
match *self { match *self {
CompatibleDist::InstalledDist(dist) => ResolvedDistRef::Installed(dist), CompatibleDist::InstalledDist(dist) => ResolvedDistRef::Installed { dist },
CompatibleDist::SourceDist { sdist, prioritized } => { CompatibleDist::SourceDist { sdist, prioritized } => {
ResolvedDistRef::InstallableRegistrySourceDist { sdist, prioritized } ResolvedDistRef::InstallableRegistrySourceDist { sdist, prioritized }
} }
@ -458,7 +458,7 @@ impl<'a> CompatibleDist<'a> {
/// Return the [`ResolvedDistRef`] to use during installation. /// Return the [`ResolvedDistRef`] to use during installation.
pub fn for_installation(&self) -> ResolvedDistRef<'a> { pub fn for_installation(&self) -> ResolvedDistRef<'a> {
match *self { match *self {
CompatibleDist::InstalledDist(dist) => ResolvedDistRef::Installed(dist), CompatibleDist::InstalledDist(dist) => ResolvedDistRef::Installed { dist },
CompatibleDist::SourceDist { sdist, prioritized } => { CompatibleDist::SourceDist { sdist, prioritized } => {
ResolvedDistRef::InstallableRegistrySourceDist { sdist, prioritized } ResolvedDistRef::InstallableRegistrySourceDist { sdist, prioritized }
} }

View File

@ -170,7 +170,7 @@ impl Diagnostic for ResolutionDiagnostic {
impl From<&ResolvedDist> for Requirement { impl From<&ResolvedDist> for Requirement {
fn from(resolved_dist: &ResolvedDist) -> Self { fn from(resolved_dist: &ResolvedDist) -> Self {
let source = match resolved_dist { let source = match resolved_dist {
ResolvedDist::Installable(dist) => match dist { ResolvedDist::Installable { dist, .. } => match dist {
Dist::Built(BuiltDist::Registry(wheels)) => RequirementSource::Registry { Dist::Built(BuiltDist::Registry(wheels)) => RequirementSource::Registry {
specifier: uv_pep440::VersionSpecifiers::from( specifier: uv_pep440::VersionSpecifiers::from(
uv_pep440::VersionSpecifier::equals_version( uv_pep440::VersionSpecifier::equals_version(
@ -229,7 +229,7 @@ impl From<&ResolvedDist> for Requirement {
r#virtual: sdist.r#virtual, r#virtual: sdist.r#virtual,
}, },
}, },
ResolvedDist::Installed(dist) => RequirementSource::Registry { ResolvedDist::Installed { dist } => RequirementSource::Registry {
specifier: uv_pep440::VersionSpecifiers::from( specifier: uv_pep440::VersionSpecifiers::from(
uv_pep440::VersionSpecifier::equals_version(dist.version().clone()), uv_pep440::VersionSpecifier::equals_version(dist.version().clone()),
), ),

View File

@ -1,5 +1,5 @@
use std::fmt::{Display, Formatter}; use std::fmt::{Display, Formatter};
use uv_pep440::Version;
use uv_pep508::PackageName; use uv_pep508::PackageName;
use uv_pypi_types::Yanked; use uv_pypi_types::Yanked;
@ -15,14 +15,16 @@ use crate::{
#[derive(Debug, Clone, Hash)] #[derive(Debug, Clone, Hash)]
#[allow(clippy::large_enum_variant)] #[allow(clippy::large_enum_variant)]
pub enum ResolvedDist { pub enum ResolvedDist {
Installed(InstalledDist), Installed { dist: InstalledDist },
Installable(Dist), Installable { dist: Dist, version: Version },
} }
/// A variant of [`ResolvedDist`] with borrowed inner distributions. /// A variant of [`ResolvedDist`] with borrowed inner distributions.
#[derive(Debug, Clone)] #[derive(Debug, Clone)]
pub enum ResolvedDistRef<'a> { pub enum ResolvedDistRef<'a> {
Installed(&'a InstalledDist), Installed {
dist: &'a InstalledDist,
},
InstallableRegistrySourceDist { InstallableRegistrySourceDist {
/// The source distribution that should be used. /// The source distribution that should be used.
sdist: &'a RegistrySourceDist, sdist: &'a RegistrySourceDist,
@ -41,36 +43,44 @@ impl ResolvedDist {
/// Return true if the distribution is editable. /// Return true if the distribution is editable.
pub fn is_editable(&self) -> bool { pub fn is_editable(&self) -> bool {
match self { match self {
Self::Installable(dist) => dist.is_editable(), Self::Installable { dist, .. } => dist.is_editable(),
Self::Installed(dist) => dist.is_editable(), Self::Installed { dist } => dist.is_editable(),
} }
} }
/// Return true if the distribution refers to a local file or directory. /// Return true if the distribution refers to a local file or directory.
pub fn is_local(&self) -> bool { pub fn is_local(&self) -> bool {
match self { match self {
Self::Installable(dist) => dist.is_local(), Self::Installable { dist, .. } => dist.is_local(),
Self::Installed(dist) => dist.is_local(), Self::Installed { dist } => dist.is_local(),
} }
} }
/// Returns the [`IndexUrl`], if the distribution is from a registry. /// Returns the [`IndexUrl`], if the distribution is from a registry.
pub fn index(&self) -> Option<&IndexUrl> { pub fn index(&self) -> Option<&IndexUrl> {
match self { match self {
Self::Installable(dist) => dist.index(), Self::Installable { dist, .. } => dist.index(),
Self::Installed(_) => None, Self::Installed { .. } => None,
} }
} }
/// Returns the [`Yanked`] status of the distribution, if available. /// Returns the [`Yanked`] status of the distribution, if available.
pub fn yanked(&self) -> Option<&Yanked> { pub fn yanked(&self) -> Option<&Yanked> {
match self { match self {
Self::Installable(dist) => match dist { Self::Installable { dist, .. } => match dist {
Dist::Source(SourceDist::Registry(sdist)) => sdist.file.yanked.as_ref(), Dist::Source(SourceDist::Registry(sdist)) => sdist.file.yanked.as_ref(),
Dist::Built(BuiltDist::Registry(wheel)) => wheel.best_wheel().file.yanked.as_ref(), Dist::Built(BuiltDist::Registry(wheel)) => wheel.best_wheel().file.yanked.as_ref(),
_ => None, _ => 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), (&source.name, &source.version),
"expected chosen sdist to match prioritized sdist" "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 { Self::InstallableRegistryBuiltDist {
wheel, prioritized, .. wheel, prioritized, ..
@ -100,9 +113,14 @@ impl ResolvedDistRef<'_> {
// This is okay because we're only here if the prioritized dist // This is okay because we're only here if the prioritized dist
// has at least one wheel, so this always succeeds. // has at least one wheel, so this always succeeds.
let built = prioritized.built_dist().expect("at least one wheel"); 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 { match self {
Self::InstallableRegistrySourceDist { sdist, .. } => Display::fmt(sdist, f), Self::InstallableRegistrySourceDist { sdist, .. } => Display::fmt(sdist, f),
Self::InstallableRegistryBuiltDist { wheel, .. } => Display::fmt(wheel, 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 { match self {
Self::InstallableRegistrySourceDist { sdist, .. } => sdist.name(), Self::InstallableRegistrySourceDist { sdist, .. } => sdist.name(),
Self::InstallableRegistryBuiltDist { wheel, .. } => wheel.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<'_> { impl DistributionMetadata for ResolvedDistRef<'_> {
fn version_or_url(&self) -> VersionOrUrlRef { fn version_or_url(&self) -> VersionOrUrlRef {
match self { match self {
Self::Installed(installed) => VersionOrUrlRef::Version(installed.version()), Self::Installed { dist } => VersionOrUrlRef::Version(dist.version()),
Self::InstallableRegistrySourceDist { sdist, .. } => sdist.version_or_url(), Self::InstallableRegistrySourceDist { sdist, .. } => sdist.version_or_url(),
Self::InstallableRegistryBuiltDist { wheel, .. } => wheel.version_or_url(), Self::InstallableRegistryBuiltDist { wheel, .. } => wheel.version_or_url(),
} }
@ -140,7 +158,7 @@ impl DistributionMetadata for ResolvedDistRef<'_> {
impl Identifier for ResolvedDistRef<'_> { impl Identifier for ResolvedDistRef<'_> {
fn distribution_id(&self) -> DistributionId { fn distribution_id(&self) -> DistributionId {
match self { match self {
Self::Installed(dist) => dist.distribution_id(), Self::Installed { dist } => dist.distribution_id(),
Self::InstallableRegistrySourceDist { sdist, .. } => sdist.distribution_id(), Self::InstallableRegistrySourceDist { sdist, .. } => sdist.distribution_id(),
Self::InstallableRegistryBuiltDist { wheel, .. } => wheel.distribution_id(), Self::InstallableRegistryBuiltDist { wheel, .. } => wheel.distribution_id(),
} }
@ -148,7 +166,7 @@ impl Identifier for ResolvedDistRef<'_> {
fn resource_id(&self) -> ResourceId { fn resource_id(&self) -> ResourceId {
match self { match self {
Self::Installed(dist) => dist.resource_id(), Self::Installed { dist } => dist.resource_id(),
Self::InstallableRegistrySourceDist { sdist, .. } => sdist.resource_id(), Self::InstallableRegistrySourceDist { sdist, .. } => sdist.resource_id(),
Self::InstallableRegistryBuiltDist { wheel, .. } => wheel.resource_id(), Self::InstallableRegistryBuiltDist { wheel, .. } => wheel.resource_id(),
} }
@ -158,8 +176,8 @@ impl Identifier for ResolvedDistRef<'_> {
impl Name for ResolvedDist { impl Name for ResolvedDist {
fn name(&self) -> &PackageName { fn name(&self) -> &PackageName {
match self { match self {
Self::Installable(dist) => dist.name(), Self::Installable { dist, .. } => dist.name(),
Self::Installed(dist) => dist.name(), Self::Installed { dist } => dist.name(),
} }
} }
} }
@ -167,8 +185,8 @@ impl Name for ResolvedDist {
impl DistributionMetadata for ResolvedDist { impl DistributionMetadata for ResolvedDist {
fn version_or_url(&self) -> VersionOrUrlRef { fn version_or_url(&self) -> VersionOrUrlRef {
match self { match self {
Self::Installed(installed) => installed.version_or_url(), Self::Installed { dist } => dist.version_or_url(),
Self::Installable(dist) => dist.version_or_url(), Self::Installable { dist, .. } => dist.version_or_url(),
} }
} }
} }
@ -176,30 +194,24 @@ impl DistributionMetadata for ResolvedDist {
impl Identifier for ResolvedDist { impl Identifier for ResolvedDist {
fn distribution_id(&self) -> DistributionId { fn distribution_id(&self) -> DistributionId {
match self { match self {
Self::Installed(dist) => dist.distribution_id(), Self::Installed { dist } => dist.distribution_id(),
Self::Installable(dist) => dist.distribution_id(), Self::Installable { dist, .. } => dist.distribution_id(),
} }
} }
fn resource_id(&self) -> ResourceId { fn resource_id(&self) -> ResourceId {
match self { match self {
Self::Installed(dist) => dist.resource_id(), Self::Installed { dist } => dist.resource_id(),
Self::Installable(dist) => dist.resource_id(), Self::Installable { dist, .. } => dist.resource_id(),
} }
} }
} }
impl From<Dist> for ResolvedDist {
fn from(value: Dist) -> Self {
ResolvedDist::Installable(value)
}
}
impl Display for ResolvedDist { impl Display for ResolvedDist {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
match self { match self {
Self::Installed(dist) => dist.fmt(f), Self::Installed { dist } => dist.fmt(f),
Self::Installable(dist) => dist.fmt(f), Self::Installable { dist, .. } => dist.fmt(f),
} }
} }
} }

View File

@ -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}"); unreachable!("Installed distribution could not be found in site-packages: {dist}");
}; };
if cache.must_revalidate(dist.name()) { if cache.must_revalidate(dist.name()) {
debug!("Must revalidate requirement: {}", dist.name()); debug!("Must revalidate requirement: {}", dist.name());
remote.push(installable.clone()); remote.push(dist.clone());
continue; continue;
} }
// Identify any cached distributions that satisfy the requirement. // Identify any cached distributions that satisfy the requirement.
match installable { match dist {
Dist::Built(BuiltDist::Registry(wheel)) => { Dist::Built(BuiltDist::Registry(wheel)) => {
if let Some(distribution) = registry_index.get(wheel.name()).find_map(|entry| { if let Some(distribution) = registry_index.get(wheel.name()).find_map(|entry| {
if *entry.index.url() != wheel.best_wheel().index { if *entry.index.url() != wheel.best_wheel().index {
@ -309,8 +309,8 @@ impl<'a> Planner<'a> {
} }
} }
debug!("Identified uncached distribution: {installable}"); debug!("Identified uncached distribution: {dist}");
remote.push(installable.clone()); remote.push(dist.clone());
} }
// Remove any unnecessary packages. // Remove any unnecessary packages.

View File

@ -2360,8 +2360,8 @@ impl Source {
fn from_resolved_dist(resolved_dist: &ResolvedDist, root: &Path) -> Result<Source, LockError> { fn from_resolved_dist(resolved_dist: &ResolvedDist, root: &Path) -> Result<Source, LockError> {
match *resolved_dist { match *resolved_dist {
// We pass empty installed packages for locking. // We pass empty installed packages for locking.
ResolvedDist::Installed(_) => unreachable!(), ResolvedDist::Installed { .. } => unreachable!(),
ResolvedDist::Installable(ref dist) => Source::from_dist(dist, root), ResolvedDist::Installable { ref dist, .. } => Source::from_dist(dist, root),
} }
} }
@ -2892,8 +2892,8 @@ impl SourceDist {
) -> Result<Option<SourceDist>, LockError> { ) -> Result<Option<SourceDist>, LockError> {
match annotated_dist.dist { match annotated_dist.dist {
// We pass empty installed packages for locking. // We pass empty installed packages for locking.
ResolvedDist::Installed(_) => unreachable!(), ResolvedDist::Installed { .. } => unreachable!(),
ResolvedDist::Installable(ref dist) => { ResolvedDist::Installable { ref dist, .. } => {
SourceDist::from_dist(id, dist, &annotated_dist.hashes, annotated_dist.index()) 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<Vec<Wheel>, LockError> { fn from_annotated_dist(annotated_dist: &AnnotatedDist) -> Result<Vec<Wheel>, LockError> {
match annotated_dist.dist { match annotated_dist.dist {
// We pass empty installed packages for locking. // We pass empty installed packages for locking.
ResolvedDist::Installed(_) => unreachable!(), ResolvedDist::Installed { .. } => unreachable!(),
ResolvedDist::Installable(ref dist) => { ResolvedDist::Installable { ref dist, .. } => {
Wheel::from_dist(dist, &annotated_dist.hashes, annotated_dist.index()) Wheel::from_dist(dist, &annotated_dist.hashes, annotated_dist.index())
} }
} }

View File

@ -266,11 +266,14 @@ impl<'env> InstallTarget<'env> {
) { ) {
map.insert( map.insert(
dist.id.name.clone(), dist.id.name.clone(),
ResolvedDist::Installable(dist.to_dist( ResolvedDist::Installable {
self.workspace().install_path(), dist: dist.to_dist(
TagPolicy::Required(tags), self.workspace().install_path(),
build_options, TagPolicy::Required(tags),
)?), build_options,
)?,
version: dist.id.version.clone(),
},
); );
hashes.insert(dist.id.name.clone(), dist.hashes()); hashes.insert(dist.id.name.clone(), dist.hashes());
} }

View File

@ -444,7 +444,14 @@ impl ResolutionGraph {
archive.metadata.clone() archive.metadata.clone()
}; };
(dist.into(), hashes, Some(metadata)) (
ResolvedDist::Installable {
dist,
version: version.clone(),
},
hashes,
Some(metadata),
)
} else { } else {
let dist = pins let dist = pins
.get(name, version) .get(name, version)

View File

@ -49,8 +49,8 @@ impl AnnotatedDist {
/// Returns the [`IndexUrl`] of the distribution, if it is from a registry. /// Returns the [`IndexUrl`] of the distribution, if it is from a registry.
pub(crate) fn index(&self) -> Option<&IndexUrl> { pub(crate) fn index(&self) -> Option<&IndexUrl> {
match &self.dist { match &self.dist {
ResolvedDist::Installed(_) => None, ResolvedDist::Installed { .. } => None,
ResolvedDist::Installable(dist) => match dist { ResolvedDist::Installable { dist, .. } => match dist {
Dist::Built(dist) => match dist { Dist::Built(dist) => match dist {
BuiltDist::Registry(dist) => Some(&dist.best_wheel().index), BuiltDist::Registry(dist) => Some(&dist.best_wheel().index),
BuiltDist::DirectUrl(_) => None, BuiltDist::DirectUrl(_) => None,

View File

@ -1151,7 +1151,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
ResolvedDistRef::InstallableRegistryBuiltDist { wheel, .. } => wheel ResolvedDistRef::InstallableRegistryBuiltDist { wheel, .. } => wheel
.filename() .filename()
.unwrap_or(Cow::Borrowed("unknown filename")), .unwrap_or(Cow::Borrowed("unknown filename")),
ResolvedDistRef::Installed(_) => Cow::Borrowed("installed"), ResolvedDistRef::Installed { .. } => Cow::Borrowed("installed"),
}; };
debug!( debug!(
@ -1958,7 +1958,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
let dist = dist.for_resolution().to_owned(); let dist = dist.for_resolution().to_owned();
let response = match dist { let response = match dist {
ResolvedDist::Installable(dist) => { ResolvedDist::Installable { dist, .. } => {
let metadata = provider let metadata = provider
.get_or_build_wheel_metadata(&dist) .get_or_build_wheel_metadata(&dist)
.boxed_local() .boxed_local()
@ -1966,7 +1966,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
Response::Dist { dist, metadata } Response::Dist { dist, metadata }
} }
ResolvedDist::Installed(dist) => { ResolvedDist::Installed { dist } => {
let metadata = dist.metadata().map_err(|err| { let metadata = dist.metadata().map_err(|err| {
ResolveError::ReadInstalled(Box::new(dist.clone()), err) ResolveError::ReadInstalled(Box::new(dist.clone()), err)
})?; })?;
@ -2621,7 +2621,7 @@ impl<'a> From<ResolvedDistRef<'a>> for Request {
let built = prioritized.built_dist().expect("at least one wheel"); let built = prioritized.built_dist().expect("at least one wheel");
Request::Dist(Dist::Built(BuiltDist::Registry(built))) Request::Dist(Dist::Built(BuiltDist::Registry(built)))
} }
ResolvedDistRef::Installed(dist) => Request::Installed(dist.clone()), ResolvedDistRef::Installed { dist } => Request::Installed(dist.clone()),
} }
} }
} }

View File

@ -454,7 +454,7 @@ fn apply_no_virtual_project(
resolution: uv_distribution_types::Resolution, resolution: uv_distribution_types::Resolution,
) -> uv_distribution_types::Resolution { ) -> uv_distribution_types::Resolution {
resolution.filter(|dist| { resolution.filter(|dist| {
let ResolvedDist::Installable(dist) = dist else { let ResolvedDist::Installable { dist, .. } = dist else {
return true; return true;
}; };
@ -481,26 +481,31 @@ fn apply_editable_mode(
// Filter out any editable distributions. // Filter out any editable distributions.
EditableMode::NonEditable => resolution.map(|dist| { EditableMode::NonEditable => resolution.map(|dist| {
let ResolvedDist::Installable(Dist::Source(SourceDist::Directory( let ResolvedDist::Installable {
DirectorySourceDist { dist:
name, Dist::Source(SourceDist::Directory(DirectorySourceDist {
install_path, name,
editable: true, install_path,
r#virtual: false, editable: true,
url, r#virtual: false,
}, url,
))) = dist })),
version,
} = dist
else { else {
return dist; return dist;
}; };
ResolvedDist::Installable(Dist::Source(SourceDist::Directory(DirectorySourceDist { ResolvedDist::Installable {
name, dist: Dist::Source(SourceDist::Directory(DirectorySourceDist {
install_path, name,
editable: false, install_path,
r#virtual: false, editable: false,
url, r#virtual: false,
}))) url,
})),
version,
}
}), }),
} }
} }