diff --git a/crates/uv-dispatch/src/lib.rs b/crates/uv-dispatch/src/lib.rs index b06d034cd..862030250 100644 --- a/crates/uv-dispatch/src/lib.rs +++ b/crates/uv-dispatch/src/lib.rs @@ -30,8 +30,8 @@ use uv_installer::{Installer, Plan, Planner, Preparer, SitePackages}; use uv_pypi_types::{Conflicts, Requirement}; use uv_python::{Interpreter, PythonEnvironment}; use uv_resolver::{ - DerivationChainBuilder, ExcludeNewer, FlatIndex, Flexibility, InMemoryIndex, Manifest, - OptionsBuilder, PythonRequirement, Resolver, ResolverEnvironment, + ExcludeNewer, FlatIndex, Flexibility, InMemoryIndex, Manifest, OptionsBuilder, + PythonRequirement, Resolver, ResolverEnvironment, }; use uv_types::{BuildContext, BuildIsolation, EmptyInstalledPackages, HashStrategy, InFlight}; @@ -274,18 +274,8 @@ impl<'a> BuildContext for BuildDispatch<'a> { ); preparer - .prepare(remote, &self.shared_state.in_flight) - .await - .map_err(|err| match err { - uv_installer::PrepareError::Dist(kind, dist, chain, err) => { - debug_assert!(chain.is_empty()); - let chain = - DerivationChainBuilder::from_resolution(resolution, (&*dist).into()) - .unwrap_or_default(); - uv_installer::PrepareError::Dist(kind, dist, chain, err) - } - _ => err, - })? + .prepare(remote, &self.shared_state.in_flight, resolution) + .await? }; // Remove any unnecessary packages. diff --git a/crates/uv-distribution-types/src/dist_error.rs b/crates/uv-distribution-types/src/dist_error.rs index d42de32ff..e26462a72 100644 --- a/crates/uv-distribution-types/src/dist_error.rs +++ b/crates/uv-distribution-types/src/dist_error.rs @@ -1,3 +1,8 @@ +use crate::{DistRef, Edge, Name, Node, Resolution, ResolvedDist}; +use petgraph::prelude::EdgeRef; +use petgraph::Direction; +use rustc_hash::FxHashSet; +use std::collections::VecDeque; use std::fmt::{Display, Formatter}; use uv_normalize::{ExtraName, GroupName, PackageName}; use uv_pep440::Version; @@ -37,6 +42,70 @@ impl FromIterator for DerivationChain { } impl DerivationChain { + /// Compute a [`DerivationChain`] from a resolution graph. + /// + /// This is used to construct a derivation chain upon install failure in the `uv pip` context, + /// where we don't have a lockfile describing the resolution. + pub fn from_resolution( + resolution: &Resolution, + target: DistRef<'_>, + ) -> Option { + // Find the target distribution in the resolution graph. + let target = resolution.graph().node_indices().find(|node| { + let Node::Dist { + dist: ResolvedDist::Installable { dist, .. }, + .. + } = &resolution.graph()[*node] + else { + return false; + }; + target == dist.as_ref() + })?; + + // Perform a BFS to find the shortest path to the root. + let mut queue = VecDeque::new(); + queue.push_back((target, None, None, Vec::new())); + + // TODO(charlie): Consider respecting markers here. + let mut seen = FxHashSet::default(); + while let Some((node, extra, group, mut path)) = queue.pop_front() { + if !seen.insert(node) { + continue; + } + match &resolution.graph()[node] { + Node::Root => { + path.reverse(); + path.pop(); + return Some(DerivationChain::from_iter(path)); + } + Node::Dist { dist, .. } => { + for edge in resolution.graph().edges_directed(node, Direction::Incoming) { + let mut path = path.clone(); + path.push(DerivationStep::new( + dist.name().clone(), + extra.clone(), + group.clone(), + dist.version().clone(), + Ranges::empty(), + )); + let target = edge.source(); + let extra = match edge.weight() { + Edge::Optional(extra, ..) => Some(extra.clone()), + _ => None, + }; + let group = match edge.weight() { + Edge::Dev(group, ..) => Some(group.clone()), + _ => None, + }; + queue.push_back((target, extra, group, path)); + } + } + } + } + + None + } + /// Returns the length of the derivation chain. pub fn len(&self) -> usize { self.0.len() diff --git a/crates/uv-installer/src/preparer.rs b/crates/uv-installer/src/preparer.rs index 28a24ed7d..2093162ac 100644 --- a/crates/uv-installer/src/preparer.rs +++ b/crates/uv-installer/src/preparer.rs @@ -10,7 +10,7 @@ use uv_configuration::BuildOptions; use uv_distribution::{DistributionDatabase, LocalWheel}; use uv_distribution_types::{ BuildableSource, CachedDist, DerivationChain, Dist, DistErrorKind, Hashed, Identifier, Name, - RemoteSource, + RemoteSource, Resolution, }; use uv_pep508::PackageName; use uv_platform_tags::Tags; @@ -65,11 +65,15 @@ impl<'a, Context: BuildContext> Preparer<'a, Context> { &'stream self, distributions: Vec, in_flight: &'stream InFlight, + resolution: &'stream Resolution, ) -> impl Stream> + 'stream { distributions .into_iter() .map(|dist| async { - let wheel = self.get_wheel(dist, in_flight).boxed_local().await?; + let wheel = self + .get_wheel(dist, in_flight, resolution) + .boxed_local() + .await?; if let Some(reporter) = self.reporter.as_ref() { reporter.on_progress(&wheel); } @@ -84,13 +88,14 @@ impl<'a, Context: BuildContext> Preparer<'a, Context> { &self, mut distributions: Vec, in_flight: &InFlight, + resolution: &Resolution, ) -> Result, Error> { // Sort the distributions by size. distributions .sort_unstable_by_key(|distribution| Reverse(distribution.size().unwrap_or(u64::MAX))); let wheels = self - .prepare_stream(distributions, in_flight) + .prepare_stream(distributions, in_flight, resolution) .try_collect() .await?; @@ -102,7 +107,12 @@ impl<'a, Context: BuildContext> Preparer<'a, Context> { } /// Download, build, and unzip a single wheel. #[instrument(skip_all, fields(name = % dist, size = ? dist.size(), url = dist.file().map(| file | file.url.to_string()).unwrap_or_default()))] - pub async fn get_wheel(&self, dist: Dist, in_flight: &InFlight) -> Result { + pub async fn get_wheel( + &self, + dist: Dist, + in_flight: &InFlight, + resolution: &Resolution, + ) -> Result { // Validate that the distribution is compatible with the build options. match dist { Dist::Built(ref dist) => { @@ -129,7 +139,7 @@ impl<'a, Context: BuildContext> Preparer<'a, Context> { .database .get_or_build_wheel(&dist, self.tags, policy) .boxed_local() - .map_err(|err| Error::from_dist(dist.clone(), err)) + .map_err(|err| Error::from_dist(dist.clone(), err, resolution)) .await .and_then(|wheel: LocalWheel| { if wheel.satisfies(policy) { @@ -140,7 +150,7 @@ impl<'a, Context: BuildContext> Preparer<'a, Context> { policy.digests(), wheel.hashes(), ); - Err(Error::from_dist(dist, err)) + Err(Error::from_dist(dist, err, resolution)) } }) .map(CachedDist::from); @@ -178,7 +188,7 @@ impl<'a, Context: BuildContext> Preparer<'a, Context> { given: dist.name().clone(), metadata: cached.filename().name.clone(), }; - return Err(Error::from_dist(dist, err)); + return Err(Error::from_dist(dist, err, resolution)); } if let Some(version) = dist.version() { if *version != cached.filename().version { @@ -186,7 +196,7 @@ impl<'a, Context: BuildContext> Preparer<'a, Context> { given: version.clone(), metadata: cached.filename().version.clone(), }; - return Err(Error::from_dist(dist, err)); + return Err(Error::from_dist(dist, err, resolution)); } } Ok(cached.clone()) @@ -216,7 +226,7 @@ pub enum Error { impl Error { /// Create an [`Error`] from a distribution error. - fn from_dist(dist: Dist, cause: uv_distribution::Error) -> Self { + fn from_dist(dist: Dist, cause: uv_distribution::Error, resolution: &Resolution) -> Self { let kind = match &dist { Dist::Built(_) => DistErrorKind::Download, Dist::Source(dist) => { @@ -227,7 +237,9 @@ impl Error { } } }; - Self::Dist(kind, Box::new(dist), DerivationChain::default(), cause) + let chain = + DerivationChain::from_resolution(resolution, (&dist).into()).unwrap_or_default(); + Self::Dist(kind, Box::new(dist), chain, cause) } } diff --git a/crates/uv-requirements/src/lib.rs b/crates/uv-requirements/src/lib.rs index bf1c46791..3c2c15f6a 100644 --- a/crates/uv-requirements/src/lib.rs +++ b/crates/uv-requirements/src/lib.rs @@ -5,7 +5,7 @@ pub use crate::sources::*; pub use crate::specification::*; pub use crate::unnamed::*; -use uv_distribution_types::{DerivationChain, Dist, DistErrorKind, GitSourceDist, SourceDist}; +use uv_distribution_types::{Dist, DistErrorKind, GitSourceDist, SourceDist}; use uv_git::GitUrl; use uv_pypi_types::{Requirement, RequirementSource}; @@ -20,12 +20,7 @@ pub mod upgrade; #[derive(Debug, thiserror::Error)] pub enum Error { #[error("{0} `{1}`")] - Dist( - DistErrorKind, - Box, - DerivationChain, - #[source] uv_distribution::Error, - ), + Dist(DistErrorKind, Box, #[source] uv_distribution::Error), #[error(transparent)] Distribution(#[from] uv_distribution::Error), @@ -41,24 +36,16 @@ impl Error { /// Create an [`Error`] from a distribution error. pub(crate) fn from_dist(dist: Dist, cause: uv_distribution::Error) -> Self { match dist { - Dist::Built(dist) => Self::Dist( - DistErrorKind::Download, - Box::new(Dist::Built(dist)), - DerivationChain::default(), - cause, - ), + Dist::Built(dist) => { + Self::Dist(DistErrorKind::Download, Box::new(Dist::Built(dist)), cause) + } Dist::Source(dist) => { let kind = if dist.is_local() { DistErrorKind::Build } else { DistErrorKind::DownloadAndBuild }; - Self::Dist( - kind, - Box::new(Dist::Source(dist)), - DerivationChain::default(), - cause, - ) + Self::Dist(kind, Box::new(Dist::Source(dist)), cause) } } } diff --git a/crates/uv-resolver/src/error.rs b/crates/uv-resolver/src/error.rs index 5c84c123c..c00dc9680 100644 --- a/crates/uv-resolver/src/error.rs +++ b/crates/uv-resolver/src/error.rs @@ -106,11 +106,7 @@ pub enum ResolveError { ), #[error("Failed to read metadata from installed package `{0}`")] - ReadInstalled( - Box, - DerivationChain, - #[source] InstalledDistError, - ), + ReadInstalled(Box, #[source] InstalledDistError), #[error(transparent)] NoSolution(#[from] NoSolutionError), diff --git a/crates/uv-resolver/src/resolver/derivation.rs b/crates/uv-resolver/src/resolver/derivation.rs index 42473ea2a..ece08e2f7 100644 --- a/crates/uv-resolver/src/resolver/derivation.rs +++ b/crates/uv-resolver/src/resolver/derivation.rs @@ -1,88 +1,18 @@ -use std::collections::VecDeque; +use pubgrub::{Id, Kind, State}; +use rustc_hash::FxHashMap; -use petgraph::visit::EdgeRef; -use petgraph::Direction; -use pubgrub::{Id, Kind, Range, State}; -use rustc_hash::{FxHashMap, FxHashSet}; - -use uv_distribution_types::{ - DerivationChain, DerivationStep, DistRef, Edge, Name, Node, Resolution, ResolvedDist, -}; +use uv_distribution_types::{DerivationChain, DerivationStep}; use uv_pep440::Version; use crate::dependency_provider::UvDependencyProvider; use crate::pubgrub::PubGrubPackage; -/// A chain of derivation steps from the root package to the current package, to explain why a -/// package is included in the resolution. +/// Build a [`DerivationChain`] from the pubgrub state, which is available in `uv-resolver`, but not +/// in `uv-distribution-types`. #[derive(Debug, Default, Clone, PartialEq, Eq, Hash)] pub struct DerivationChainBuilder; impl DerivationChainBuilder { - /// Compute a [`DerivationChain`] from a resolution graph. - /// - /// This is used to construct a derivation chain upon install failure in the `uv pip` context, - /// where we don't have a lockfile describing the resolution. - pub fn from_resolution( - resolution: &Resolution, - target: DistRef<'_>, - ) -> Option { - // Find the target distribution in the resolution graph. - let target = resolution.graph().node_indices().find(|node| { - let Node::Dist { - dist: ResolvedDist::Installable { dist, .. }, - .. - } = &resolution.graph()[*node] - else { - return false; - }; - target == dist.as_ref() - })?; - - // Perform a BFS to find the shortest path to the root. - let mut queue = VecDeque::new(); - queue.push_back((target, None, None, Vec::new())); - - // TODO(charlie): Consider respecting markers here. - let mut seen = FxHashSet::default(); - while let Some((node, extra, group, mut path)) = queue.pop_front() { - if !seen.insert(node) { - continue; - } - match &resolution.graph()[node] { - Node::Root => { - path.reverse(); - path.pop(); - return Some(DerivationChain::from_iter(path)); - } - Node::Dist { dist, .. } => { - for edge in resolution.graph().edges_directed(node, Direction::Incoming) { - let mut path = path.clone(); - path.push(DerivationStep::new( - dist.name().clone(), - extra.clone(), - group.clone(), - dist.version().clone(), - Range::empty(), - )); - let target = edge.source(); - let extra = match edge.weight() { - Edge::Optional(extra, ..) => Some(extra.clone()), - _ => None, - }; - let group = match edge.weight() { - Edge::Dev(group, ..) => Some(group.clone()), - _ => None, - }; - queue.push_back((target, extra, group, path)); - } - } - } - } - - None - } - /// Compute a [`DerivationChain`] from the current PubGrub state. /// /// This is used to construct a derivation chain upon resolution failure. diff --git a/crates/uv-resolver/src/resolver/mod.rs b/crates/uv-resolver/src/resolver/mod.rs index 6984ac5a8..f8ee9441f 100644 --- a/crates/uv-resolver/src/resolver/mod.rs +++ b/crates/uv-resolver/src/resolver/mod.rs @@ -1914,13 +1914,9 @@ impl ResolverState { // TODO(charlie): This should be return a `MetadataResponse`. - let metadata = dist.metadata().map_err(|err| { - ResolveError::ReadInstalled( - Box::new(dist.clone()), - DerivationChain::default(), - err, - ) - })?; + let metadata = dist + .metadata() + .map_err(|err| ResolveError::ReadInstalled(Box::new(dist.clone()), err))?; Ok(Some(Response::Installed { dist, metadata })) } @@ -2035,11 +2031,7 @@ impl ResolverState { let metadata = dist.metadata().map_err(|err| { - ResolveError::ReadInstalled( - Box::new(dist.clone()), - DerivationChain::default(), - err, - ) + ResolveError::ReadInstalled(Box::new(dist.clone()), err) })?; Response::Installed { dist, metadata } } diff --git a/crates/uv/src/commands/diagnostics.rs b/crates/uv/src/commands/diagnostics.rs index d851ca9e3..4767cff09 100644 --- a/crates/uv/src/commands/diagnostics.rs +++ b/crates/uv/src/commands/diagnostics.rs @@ -81,13 +81,8 @@ impl OperationDiagnostic { dist_error(kind, dist, &chain, err); None } - pip::operations::Error::Requirements(uv_requirements::Error::Dist( - kind, - dist, - chain, - err, - )) => { - dist_error(kind, dist, &chain, Arc::new(err)); + pip::operations::Error::Requirements(uv_requirements::Error::Dist(kind, dist, err)) => { + dist_error(kind, dist, &DerivationChain::default(), Arc::new(err)); None } pip::operations::Error::Prepare(uv_installer::PrepareError::Dist( diff --git a/crates/uv/src/commands/pip/operations.rs b/crates/uv/src/commands/pip/operations.rs index 42aa265e4..b9ad40512 100644 --- a/crates/uv/src/commands/pip/operations.rs +++ b/crates/uv/src/commands/pip/operations.rs @@ -36,9 +36,8 @@ use uv_requirements::{ SourceTreeResolver, }; use uv_resolver::{ - DependencyMode, DerivationChainBuilder, Exclusions, FlatIndex, InMemoryIndex, Manifest, - Options, Preference, Preferences, PythonRequirement, Resolver, ResolverEnvironment, - ResolverOutput, + DependencyMode, Exclusions, FlatIndex, InMemoryIndex, Manifest, Options, Preference, + Preferences, PythonRequirement, Resolver, ResolverEnvironment, ResolverOutput, }; use uv_types::{HashStrategy, InFlight, InstalledPackagesProvider}; use uv_warnings::warn_user; @@ -464,20 +463,8 @@ pub(crate) async fn install( .with_reporter(PrepareReporter::from(printer).with_length(remote.len() as u64)); let wheels = preparer - .prepare(remote.clone(), in_flight) - .await - .map_err(Error::from) - .map_err(|err| match err { - // Attach resolution context to the error. - Error::Prepare(uv_installer::PrepareError::Dist(kind, dist, chain, err)) => { - debug_assert!(chain.is_empty()); - let chain = - DerivationChainBuilder::from_resolution(resolution, (&*dist).into()) - .unwrap_or_default(); - Error::Prepare(uv_installer::PrepareError::Dist(kind, dist, chain, err)) - } - _ => err, - })?; + .prepare(remote.clone(), in_flight, resolution) + .await?; logger.on_prepare(wheels.len(), start, printer)?;