From dcd96a83aa5254d6b5068d2c68b074a0376d4100 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Mon, 30 Dec 2024 12:47:06 -0500 Subject: [PATCH] Respect static metadata for already-installed distributions (#10242) ## Summary Closes https://github.com/astral-sh/uv/issues/10239#issuecomment-2565663046 --- .../uv-distribution-types/src/dist_error.rs | 22 ++- crates/uv-distribution-types/src/lib.rs | 2 + crates/uv-distribution-types/src/requested.rs | 71 ++++++++++ .../src/distribution_database.rs | 29 +++- crates/uv-distribution/src/error.rs | 4 +- crates/uv-installer/src/preparer.rs | 2 +- crates/uv-requirements/src/lib.rs | 6 +- crates/uv-resolver/src/error.rs | 8 +- crates/uv-resolver/src/resolver/mod.rs | 37 ++--- crates/uv-resolver/src/resolver/provider.rs | 28 +++- crates/uv/src/commands/diagnostics.rs | 51 ++++++- crates/uv/tests/it/pip_install.rs | 134 ++++++++++++++++++ 12 files changed, 347 insertions(+), 47 deletions(-) create mode 100644 crates/uv-distribution-types/src/requested.rs diff --git a/crates/uv-distribution-types/src/dist_error.rs b/crates/uv-distribution-types/src/dist_error.rs index 9bfcf8e974bb..fcbb13dde83b 100644 --- a/crates/uv-distribution-types/src/dist_error.rs +++ b/crates/uv-distribution-types/src/dist_error.rs @@ -1,12 +1,17 @@ -use crate::{BuiltDist, Dist, DistRef, Edge, Name, Node, Resolution, ResolvedDist, SourceDist}; +use std::collections::VecDeque; +use std::fmt::{Debug, Display, Formatter}; + use petgraph::prelude::EdgeRef; use petgraph::Direction; use rustc_hash::FxHashSet; -use std::collections::VecDeque; -use std::fmt::{Debug, Display, Formatter}; +use version_ranges::Ranges; + use uv_normalize::{ExtraName, GroupName, PackageName}; use uv_pep440::Version; -use version_ranges::Ranges; + +use crate::{ + BuiltDist, Dist, DistRef, Edge, Name, Node, RequestedDist, Resolution, ResolvedDist, SourceDist, +}; /// Inspect whether an error type is a build error. pub trait IsBuildBackendError: std::error::Error + Send + Sync + 'static { @@ -25,7 +30,14 @@ pub enum DistErrorKind { } impl DistErrorKind { - pub fn from_dist_and_err(dist: &Dist, err: &impl IsBuildBackendError) -> Self { + pub fn from_requested_dist(dist: &RequestedDist, err: &impl IsBuildBackendError) -> Self { + match dist { + RequestedDist::Installed(_) => DistErrorKind::Read, + RequestedDist::Installable(dist) => Self::from_dist(dist, err), + } + } + + pub fn from_dist(dist: &Dist, err: &impl IsBuildBackendError) -> Self { if err.is_build_backend_error() { DistErrorKind::BuildBackend } else { diff --git a/crates/uv-distribution-types/src/lib.rs b/crates/uv-distribution-types/src/lib.rs index 9aba9e5c5f14..c9a0c48b50cd 100644 --- a/crates/uv-distribution-types/src/lib.rs +++ b/crates/uv-distribution-types/src/lib.rs @@ -67,6 +67,7 @@ pub use crate::installed::*; pub use crate::origin::*; pub use crate::pip_index::*; pub use crate::prioritized_distribution::*; +pub use crate::requested::*; pub use crate::resolution::*; pub use crate::resolved::*; pub use crate::specified_requirement::*; @@ -90,6 +91,7 @@ mod installed; mod origin; mod pip_index; mod prioritized_distribution; +mod requested; mod resolution; mod resolved; mod specified_requirement; diff --git a/crates/uv-distribution-types/src/requested.rs b/crates/uv-distribution-types/src/requested.rs new file mode 100644 index 000000000000..b804a16adb88 --- /dev/null +++ b/crates/uv-distribution-types/src/requested.rs @@ -0,0 +1,71 @@ +use std::fmt::{Display, Formatter}; + +use crate::{ + Dist, DistributionId, DistributionMetadata, Identifier, InstalledDist, Name, ResourceId, + VersionOrUrlRef, +}; +use uv_normalize::PackageName; +use uv_pep440::Version; + +/// A distribution that can be requested during resolution. +/// +/// Either an already-installed distribution or a distribution that can be installed. +#[derive(Debug, Clone)] +#[allow(clippy::large_enum_variant)] +pub enum RequestedDist { + Installed(InstalledDist), + Installable(Dist), +} + +impl RequestedDist { + /// Returns the version of the distribution, if it is known. + pub fn version(&self) -> Option<&Version> { + match self { + Self::Installed(dist) => Some(dist.version()), + Self::Installable(dist) => dist.version(), + } + } +} + +impl Name for RequestedDist { + fn name(&self) -> &PackageName { + match self { + Self::Installable(dist) => dist.name(), + Self::Installed(dist) => dist.name(), + } + } +} + +impl DistributionMetadata for RequestedDist { + fn version_or_url(&self) -> VersionOrUrlRef { + match self { + Self::Installed(dist) => dist.version_or_url(), + Self::Installable(dist) => dist.version_or_url(), + } + } +} + +impl Identifier for RequestedDist { + 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 Display for RequestedDist { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + match self { + Self::Installed(dist) => dist.fmt(f), + Self::Installable(dist) => dist.fmt(f), + } + } +} diff --git a/crates/uv-distribution/src/distribution_database.rs b/crates/uv-distribution/src/distribution_database.rs index 95b52b516051..d1ab5a5d0519 100644 --- a/crates/uv-distribution/src/distribution_database.rs +++ b/crates/uv-distribution/src/distribution_database.rs @@ -21,7 +21,8 @@ use uv_client::{ }; use uv_distribution_filename::WheelFilename; use uv_distribution_types::{ - BuildableSource, BuiltDist, Dist, FileLocation, HashPolicy, Hashed, Name, SourceDist, + BuildableSource, BuiltDist, Dist, FileLocation, HashPolicy, Hashed, InstalledDist, Name, + SourceDist, }; use uv_extract::hash::Hasher; use uv_fs::write_atomic; @@ -115,6 +116,32 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> { } } + /// Either fetch the only wheel metadata (directly from the index or with range requests) or + /// fetch and build the source distribution. + /// + /// While hashes will be generated in some cases, hash-checking is only enforced for source + /// distributions, and should be enforced by the caller for wheels. + #[instrument(skip_all, fields(%dist))] + pub async fn get_installed_metadata( + &self, + dist: &InstalledDist, + ) -> Result { + // If the metadata was provided by the user directly, prefer it. + if let Some(metadata) = self + .build_context + .dependency_metadata() + .get(dist.name(), Some(dist.version())) + { + return Ok(ArchiveMetadata::from_metadata23(metadata.clone())); + } + + let metadata = dist + .metadata() + .map_err(|err| Error::ReadInstalled(Box::new(dist.clone()), err))?; + + Ok(ArchiveMetadata::from_metadata23(metadata)) + } + /// Either fetch the only wheel metadata (directly from the index or with range requests) or /// fetch and build the source distribution. /// diff --git a/crates/uv-distribution/src/error.rs b/crates/uv-distribution/src/error.rs index 217682501bd8..84315658e42e 100644 --- a/crates/uv-distribution/src/error.rs +++ b/crates/uv-distribution/src/error.rs @@ -8,7 +8,7 @@ use zip::result::ZipError; use crate::metadata::MetadataError; use uv_client::WrappedReqwestError; use uv_distribution_filename::WheelFilenameError; -use uv_distribution_types::IsBuildBackendError; +use uv_distribution_types::{InstalledDist, InstalledDistError, IsBuildBackendError}; use uv_fs::Simplified; use uv_normalize::PackageName; use uv_pep440::{Version, VersionSpecifiers}; @@ -82,6 +82,8 @@ pub enum Error { Metadata(#[from] uv_pypi_types::MetadataError), #[error("Failed to read metadata: `{}`", _0.user_display())] WheelMetadata(PathBuf, #[source] Box), + #[error("Failed to read metadata from installed package `{0}`")] + ReadInstalled(Box, #[source] InstalledDistError), #[error("Failed to read zip archive from built wheel")] Zip(#[from] ZipError), #[error("Source distribution directory contains neither readable `pyproject.toml` nor `setup.py`: `{}`", _0.user_display())] diff --git a/crates/uv-installer/src/preparer.rs b/crates/uv-installer/src/preparer.rs index fac49057025c..541383de10da 100644 --- a/crates/uv-installer/src/preparer.rs +++ b/crates/uv-installer/src/preparer.rs @@ -230,7 +230,7 @@ impl Error { let chain = DerivationChain::from_resolution(resolution, (&dist).into()).unwrap_or_default(); Self::Dist( - DistErrorKind::from_dist_and_err(&dist, &err), + DistErrorKind::from_dist(&dist, &err), Box::new(dist), chain, err, diff --git a/crates/uv-requirements/src/lib.rs b/crates/uv-requirements/src/lib.rs index adb342e133c6..a6bff1998a61 100644 --- a/crates/uv-requirements/src/lib.rs +++ b/crates/uv-requirements/src/lib.rs @@ -35,11 +35,7 @@ pub enum Error { impl Error { /// Create an [`Error`] from a distribution error. pub(crate) fn from_dist(dist: Dist, err: uv_distribution::Error) -> Self { - Self::Dist( - DistErrorKind::from_dist_and_err(&dist, &err), - Box::new(dist), - err, - ) + Self::Dist(DistErrorKind::from_dist(&dist, &err), Box::new(dist), err) } } diff --git a/crates/uv-resolver/src/error.rs b/crates/uv-resolver/src/error.rs index 70ef20f60e0d..2cd333c40a82 100644 --- a/crates/uv-resolver/src/error.rs +++ b/crates/uv-resolver/src/error.rs @@ -10,8 +10,7 @@ use rustc_hash::FxHashMap; use tracing::trace; use uv_distribution_types::{ - DerivationChain, Dist, DistErrorKind, IndexCapabilities, IndexLocations, IndexUrl, - InstalledDist, InstalledDistError, + DerivationChain, DistErrorKind, IndexCapabilities, IndexLocations, IndexUrl, RequestedDist, }; use uv_normalize::{ExtraName, PackageName}; use uv_pep440::{LocalVersionSlice, Version}; @@ -98,14 +97,11 @@ pub enum ResolveError { #[error("{0} `{1}`")] Dist( DistErrorKind, - Box, + Box, DerivationChain, #[source] Arc, ), - #[error("Failed to read metadata from installed package `{0}`")] - ReadInstalled(Box, #[source] InstalledDistError), - #[error(transparent)] NoSolution(#[from] NoSolutionError), diff --git a/crates/uv-resolver/src/resolver/mod.rs b/crates/uv-resolver/src/resolver/mod.rs index d27f0df1b76b..7873a44e0a89 100644 --- a/crates/uv-resolver/src/resolver/mod.rs +++ b/crates/uv-resolver/src/resolver/mod.rs @@ -21,7 +21,7 @@ use tokio_stream::wrappers::ReceiverStream; use tracing::{debug, info, instrument, trace, warn, Level}; use uv_configuration::{Constraints, Overrides}; -use uv_distribution::{ArchiveMetadata, DistributionDatabase}; +use uv_distribution::DistributionDatabase; use uv_distribution_types::{ BuiltDist, CompatibleDist, DerivationChain, Dist, DistErrorKind, DistributionMetadata, IncompatibleDist, IncompatibleSource, IncompatibleWheel, IndexCapabilities, IndexLocations, @@ -33,9 +33,7 @@ use uv_normalize::{ExtraName, GroupName, PackageName}; use uv_pep440::{release_specifiers_to_ranges, Version, VersionSpecifiers, MIN_VERSION}; use uv_pep508::MarkerTree; use uv_platform_tags::Tags; -use uv_pypi_types::{ - ConflictItem, ConflictItemRef, Conflicts, Requirement, ResolutionMetadata, VerbatimParsedUrl, -}; +use uv_pypi_types::{ConflictItem, ConflictItemRef, Conflicts, Requirement, VerbatimParsedUrl}; use uv_types::{BuildContext, HashStrategy, InstalledPackagesProvider}; use uv_warnings::warn_user_once; @@ -1097,11 +1095,11 @@ impl ResolverState { - // TODO(charlie): Add derivation chain for URL dependencies. In practice, this isn't - // critical since we fetch URL dependencies _prior_ to invoking the resolver. return Err(ResolveError::Dist( - DistErrorKind::from_dist_and_err(dist, &**err), + DistErrorKind::from_requested_dist(dist, &**err), dist.clone(), DerivationChain::default(), err.clone(), @@ -1642,7 +1640,7 @@ impl ResolverState ResolverState { trace!("Received installed distribution metadata for: {dist}"); - self.index.distributions().done( - dist.version_id(), - Arc::new(MetadataResponse::Found(ArchiveMetadata::from_metadata23( - metadata, - ))), - ); + self.index + .distributions() + .done(dist.version_id(), Arc::new(metadata)); } Some(Response::Dist { dist, metadata }) => { let dist_kind = match dist { @@ -2134,10 +2129,8 @@ impl ResolverState { - // TODO(charlie): This should be return a `MetadataResponse`. - let metadata = dist - .metadata() - .map_err(|err| ResolveError::ReadInstalled(Box::new(dist.clone()), err))?; + let metadata = provider.get_installed_metadata(&dist).boxed_local().await?; + Ok(Some(Response::Installed { dist, metadata })) } @@ -2251,9 +2244,9 @@ impl ResolverState { - let metadata = dist.metadata().map_err(|err| { - ResolveError::ReadInstalled(Box::new(dist.clone()), err) - })?; + let metadata = + provider.get_installed_metadata(&dist).boxed_local().await?; + Response::Installed { dist, metadata } } }; @@ -3079,7 +3072,7 @@ enum Response { /// The returned metadata for an already-installed distribution. Installed { dist: InstalledDist, - metadata: ResolutionMetadata, + metadata: MetadataResponse, }, } diff --git a/crates/uv-resolver/src/resolver/provider.rs b/crates/uv-resolver/src/resolver/provider.rs index 29e02384699e..03cda439d294 100644 --- a/crates/uv-resolver/src/resolver/provider.rs +++ b/crates/uv-resolver/src/resolver/provider.rs @@ -3,7 +3,7 @@ use std::sync::Arc; use uv_configuration::BuildOptions; use uv_distribution::{ArchiveMetadata, DistributionDatabase}; -use uv_distribution_types::{Dist, IndexCapabilities, IndexUrl}; +use uv_distribution_types::{Dist, IndexCapabilities, IndexUrl, InstalledDist, RequestedDist}; use uv_normalize::PackageName; use uv_pep440::{Version, VersionSpecifiers}; use uv_platform_tags::Tags; @@ -37,7 +37,7 @@ pub enum MetadataResponse { /// A non-fatal error. Unavailable(MetadataUnavailable), /// The distribution could not be built or downloaded, a fatal error. - Error(Box, Arc), + Error(Box, Arc), } /// Non-fatal metadata fetching error. @@ -83,7 +83,7 @@ pub trait ResolverProvider { /// Get the metadata for a distribution. /// - /// For a wheel, this is done by querying it's (remote) metadata, for a source dist we + /// For a wheel, this is done by querying it (remote) metadata. For a source distribution, we /// (fetch and) build the source distribution and return the metadata from the built /// distribution. fn get_or_build_wheel_metadata<'io>( @@ -91,6 +91,12 @@ pub trait ResolverProvider { dist: &'io Dist, ) -> impl Future + 'io; + /// Get the metadata for an installed distribution. + fn get_installed_metadata<'io>( + &'io self, + dist: &'io InstalledDist, + ) -> impl Future + 'io; + /// Set the [`uv_distribution::Reporter`] to use for this installer. #[must_use] fn with_reporter(self, reporter: impl uv_distribution::Reporter + 'static) -> Self; @@ -246,13 +252,27 @@ impl<'a, Context: BuildContext> ResolverProvider for DefaultResolverProvider<'a, )) } err => Ok(MetadataResponse::Error( - Box::new(dist.clone()), + Box::new(RequestedDist::Installable(dist.clone())), Arc::new(err), )), }, } } + /// Return the metadata for an installed distribution. + async fn get_installed_metadata<'io>( + &'io self, + dist: &'io InstalledDist, + ) -> WheelMetadataResult { + match self.fetcher.get_installed_metadata(dist).await { + Ok(metadata) => Ok(MetadataResponse::Found(metadata)), + Err(err) => Ok(MetadataResponse::Error( + Box::new(RequestedDist::Installed(dist.clone())), + Arc::new(err), + )), + } + } + /// Set the [`uv_distribution::Reporter`] to use for this installer. #[must_use] fn with_reporter(self, reporter: impl uv_distribution::Reporter + 'static) -> Self { diff --git a/crates/uv/src/commands/diagnostics.rs b/crates/uv/src/commands/diagnostics.rs index 4767cff090b3..2d375e6f7201 100644 --- a/crates/uv/src/commands/diagnostics.rs +++ b/crates/uv/src/commands/diagnostics.rs @@ -5,7 +5,9 @@ use owo_colors::OwoColorize; use rustc_hash::FxHashMap; use version_ranges::Ranges; -use uv_distribution_types::{DerivationChain, DerivationStep, Dist, DistErrorKind, Name}; +use uv_distribution_types::{ + DerivationChain, DerivationStep, Dist, DistErrorKind, Name, RequestedDist, +}; use uv_normalize::PackageName; use uv_pep440::Version; use uv_resolver::SentinelRange; @@ -78,7 +80,7 @@ impl OperationDiagnostic { chain, err, )) => { - dist_error(kind, dist, &chain, err); + requested_dist_error(kind, dist, &chain, err); None } pip::operations::Error::Requirements(uv_requirements::Error::Dist(kind, dist, err)) => { @@ -154,6 +156,51 @@ pub(crate) fn dist_error( anstream::eprint!("{report:?}"); } +/// Render a requested distribution failure (read, download or build) with a help message. +pub(crate) fn requested_dist_error( + kind: DistErrorKind, + dist: Box, + chain: &DerivationChain, + cause: Arc, +) { + #[derive(Debug, miette::Diagnostic, thiserror::Error)] + #[error("{kind} `{dist}`")] + #[diagnostic()] + struct Diagnostic { + kind: DistErrorKind, + dist: Box, + #[source] + cause: Arc, + #[help] + help: Option, + } + + let help = SUGGESTIONS + .get(dist.name()) + .map(|suggestion| { + format!( + "`{}` is often confused for `{}` Did you mean to install `{}` instead?", + dist.name().cyan(), + suggestion.cyan(), + suggestion.cyan(), + ) + }) + .or_else(|| { + if chain.is_empty() { + None + } else { + Some(format_chain(dist.name(), dist.version(), chain)) + } + }); + let report = miette::Report::new(Diagnostic { + kind, + dist, + cause, + help, + }); + anstream::eprint!("{report:?}"); +} + /// Render a [`uv_resolver::NoSolutionError`]. pub(crate) fn no_solution(err: &uv_resolver::NoSolutionError) { let report = miette::Report::msg(format!("{err}")).context(err.header()); diff --git a/crates/uv/tests/it/pip_install.rs b/crates/uv/tests/it/pip_install.rs index 1a8df7a7a68c..c51e3819f3e3 100644 --- a/crates/uv/tests/it/pip_install.rs +++ b/crates/uv/tests/it/pip_install.rs @@ -7724,3 +7724,137 @@ fn missing_subdirectory_url() -> Result<()> { Ok(()) } + +#[test] +fn static_metadata_pyproject_toml() -> Result<()> { + let context = TestContext::new("3.12"); + + context.temp_dir.child("pyproject.toml").write_str( + r#" + [project] + name = "example" + version = "0.0.0" + dependencies = [ + "anyio==3.7.0" + ] + + [[tool.uv.dependency-metadata]] + name = "anyio" + version = "3.7.0" + requires-dist = ["typing-extensions"] + "#, + )?; + + uv_snapshot!(context.filters(), context.pip_install() + .arg("-r") + .arg("pyproject.toml"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 2 packages in [TIME] + Prepared 2 packages in [TIME] + Installed 2 packages in [TIME] + + anyio==3.7.0 + + typing-extensions==4.10.0 + "### + ); + + Ok(()) +} + +#[test] +fn static_metadata_source_tree() -> Result<()> { + let context = TestContext::new("3.12"); + + context.temp_dir.child("pyproject.toml").write_str( + r#" + [project] + name = "example" + version = "0.0.0" + dependencies = [ + "anyio==3.7.0" + ] + + [[tool.uv.dependency-metadata]] + name = "anyio" + version = "3.7.0" + requires-dist = ["typing-extensions"] + "#, + )?; + + uv_snapshot!(context.filters(), context.pip_install() + .arg("-e") + .arg("."), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 3 packages in [TIME] + Prepared 3 packages in [TIME] + Installed 3 packages in [TIME] + + anyio==3.7.0 + + example==0.0.0 (from file://[TEMP_DIR]/) + + typing-extensions==4.10.0 + "### + ); + + Ok(()) +} + +/// Regression test for: +#[test] +fn static_metadata_already_installed() -> Result<()> { + let context = TestContext::new("3.12"); + + context.temp_dir.child("pyproject.toml").write_str( + r#" + [project] + name = "example" + version = "0.0.0" + dependencies = [ + "anyio==3.7.0" + ] + + [[tool.uv.dependency-metadata]] + name = "anyio" + version = "3.7.0" + requires-dist = ["typing-extensions"] + "#, + )?; + + uv_snapshot!(context.filters(), context.pip_install() + .arg("-r") + .arg("pyproject.toml"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 2 packages in [TIME] + Prepared 2 packages in [TIME] + Installed 2 packages in [TIME] + + anyio==3.7.0 + + typing-extensions==4.10.0 + "### + ); + + uv_snapshot!(context.filters(), context.pip_install() + .arg("-e") + .arg("."), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 3 packages in [TIME] + Prepared 1 package in [TIME] + Installed 1 package in [TIME] + + example==0.0.0 (from file://[TEMP_DIR]/) + "### + ); + + Ok(()) +}