Skip to content

Commit

Permalink
Show full derivation chain when encountering build failures (#9108)
Browse files Browse the repository at this point in the history
## Summary

This PR adds context to our error messages to explain _why_ a given
package was included, if we fail to download or build it.

It's quite a large change, but it motivated some good refactors and
improvements along the way.

Closes #8962.
  • Loading branch information
charliermarsh authored Nov 14, 2024
1 parent a552f74 commit fe477c3
Show file tree
Hide file tree
Showing 20 changed files with 1,147 additions and 172 deletions.
32 changes: 29 additions & 3 deletions crates/uv-dispatch/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ use uv_installer::{Installer, Plan, Planner, Preparer, SitePackages};
use uv_pypi_types::{Conflicts, Requirement};
use uv_python::{Interpreter, PythonEnvironment};
use uv_resolver::{
ExcludeNewer, FlatIndex, Flexibility, InMemoryIndex, Manifest, OptionsBuilder,
PythonRequirement, Resolver, ResolverEnvironment,
DerivationChainBuilder, ExcludeNewer, FlatIndex, Flexibility, InMemoryIndex, Manifest,
OptionsBuilder, PythonRequirement, Resolver, ResolverEnvironment,
};
use uv_types::{BuildContext, BuildIsolation, EmptyInstalledPackages, HashStrategy, InFlight};

Expand Down Expand Up @@ -278,7 +278,33 @@ impl<'a> BuildContext for BuildDispatch<'a> {
remote.iter().map(ToString::to_string).join(", ")
);

preparer.prepare(remote, self.in_flight).await?
preparer
.prepare(remote, self.in_flight)
.await
.map_err(|err| match err {
uv_installer::PrepareError::DownloadAndBuild(dist, chain, err) => {
debug_assert!(chain.is_empty());
let chain =
DerivationChainBuilder::from_resolution(resolution, (&*dist).into())
.unwrap_or_default();
uv_installer::PrepareError::DownloadAndBuild(dist, chain, err)
}
uv_installer::PrepareError::Download(dist, chain, err) => {
debug_assert!(chain.is_empty());
let chain =
DerivationChainBuilder::from_resolution(resolution, (&*dist).into())
.unwrap_or_default();
uv_installer::PrepareError::Download(dist, chain, err)
}
uv_installer::PrepareError::Build(dist, chain, err) => {
debug_assert!(chain.is_empty());
let chain =
DerivationChainBuilder::from_resolution(resolution, (&*dist).into())
.unwrap_or_default();
uv_installer::PrepareError::Build(dist, chain, err)
}
_ => err,
})?
};

// Remove any unnecessary packages.
Expand Down
82 changes: 82 additions & 0 deletions crates/uv-distribution-types/src/derivation.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
use uv_normalize::PackageName;
use uv_pep440::Version;

/// A chain of derivation steps from the root package to the current package, to explain why a
/// package is included in the resolution.
#[derive(Debug, Default, Clone, PartialEq, Eq, Hash)]
pub struct DerivationChain(Vec<DerivationStep>);

impl FromIterator<DerivationStep> for DerivationChain {
fn from_iter<T: IntoIterator<Item = DerivationStep>>(iter: T) -> Self {
Self(iter.into_iter().collect())
}
}

impl DerivationChain {
/// Returns the length of the derivation chain.
pub fn len(&self) -> usize {
self.0.len()
}

/// Returns `true` if the derivation chain is empty.
pub fn is_empty(&self) -> bool {
self.0.is_empty()
}

/// Returns an iterator over the steps in the derivation chain.
pub fn iter(&self) -> std::slice::Iter<DerivationStep> {
self.0.iter()
}
}

impl std::fmt::Display for DerivationChain {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
for (idx, step) in self.0.iter().enumerate() {
if idx > 0 {
write!(f, " -> ")?;
}
write!(f, "{}=={}", step.name, step.version)?;
}
Ok(())
}
}

impl<'chain> IntoIterator for &'chain DerivationChain {
type Item = &'chain DerivationStep;
type IntoIter = std::slice::Iter<'chain, DerivationStep>;

fn into_iter(self) -> Self::IntoIter {
self.0.iter()
}
}

impl IntoIterator for DerivationChain {
type Item = DerivationStep;
type IntoIter = std::vec::IntoIter<DerivationStep>;

fn into_iter(self) -> Self::IntoIter {
self.0.into_iter()
}
}

/// A step in a derivation chain.
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub struct DerivationStep {
/// The name of the package.
name: PackageName,
/// The version of the package.
version: Version,
}

impl DerivationStep {
/// Create a [`DerivationStep`] from a package name and version.
pub fn new(name: PackageName, version: Version) -> Self {
Self { name, version }
}
}

impl std::fmt::Display for DerivationStep {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "{}=={}", self.name, self.version)
}
}
4 changes: 2 additions & 2 deletions crates/uv-distribution-types/src/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ pub enum FileConversionError {
}

/// Internal analog to [`uv_pypi_types::File`].
#[derive(Debug, Clone, Hash, rkyv::Archive, rkyv::Deserialize, rkyv::Serialize)]
#[derive(Debug, Clone, PartialEq, Eq, Hash, rkyv::Archive, rkyv::Deserialize, rkyv::Serialize)]
#[rkyv(derive(Debug))]
pub struct File {
pub dist_info_metadata: bool,
Expand Down Expand Up @@ -66,7 +66,7 @@ impl File {
}

/// While a registry file is generally a remote URL, it can also be a file if it comes from a directory flat indexes.
#[derive(Debug, Clone, Hash, rkyv::Archive, rkyv::Deserialize, rkyv::Serialize)]
#[derive(Debug, Clone, PartialEq, Eq, Hash, rkyv::Archive, rkyv::Deserialize, rkyv::Serialize)]
#[rkyv(derive(Debug))]
pub enum FileLocation {
/// URL relative to the base URL.
Expand Down
54 changes: 42 additions & 12 deletions crates/uv-distribution-types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ pub use crate::any::*;
pub use crate::buildable::*;
pub use crate::cached::*;
pub use crate::dependency_metadata::*;
pub use crate::derivation::*;
pub use crate::diagnostic::*;
pub use crate::error::*;
pub use crate::file::*;
Expand All @@ -74,6 +75,7 @@ mod any;
mod buildable;
mod cached;
mod dependency_metadata;
mod derivation;
mod diagnostic;
mod error;
mod file;
Expand Down Expand Up @@ -166,14 +168,21 @@ impl std::fmt::Display for InstalledVersion<'_> {
/// Either a built distribution, a wheel, or a source distribution that exists at some location.
///
/// The location can be an index, URL or path (wheel), or index, URL, path or Git repository (source distribution).
#[derive(Debug, Clone, Hash)]
#[derive(Debug, Clone, Hash, PartialEq, Eq)]
pub enum Dist {
Built(BuiltDist),
Source(SourceDist),
}

/// A reference to a built or source distribution.
#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)]
pub enum DistRef<'a> {
Built(&'a BuiltDist),
Source(&'a SourceDist),
}

/// A wheel, with its three possible origins (index, url, path)
#[derive(Debug, Clone, Hash)]
#[derive(Debug, Clone, Hash, PartialEq, Eq)]
#[allow(clippy::large_enum_variant)]
pub enum BuiltDist {
Registry(RegistryBuiltDist),
Expand All @@ -182,7 +191,7 @@ pub enum BuiltDist {
}

/// A source distribution, with its possible origins (index, url, path, git)
#[derive(Debug, Clone, Hash)]
#[derive(Debug, Clone, Hash, PartialEq, Eq)]
#[allow(clippy::large_enum_variant)]
pub enum SourceDist {
Registry(RegistrySourceDist),
Expand All @@ -193,15 +202,15 @@ pub enum SourceDist {
}

/// A built distribution (wheel) that exists in a registry, like `PyPI`.
#[derive(Debug, Clone, Hash)]
#[derive(Debug, Clone, Hash, PartialEq, Eq)]
pub struct RegistryBuiltWheel {
pub filename: WheelFilename,
pub file: Box<File>,
pub index: IndexUrl,
}

/// A built distribution (wheel) that exists in a registry, like `PyPI`.
#[derive(Debug, Clone, Hash)]
#[derive(Debug, Clone, Hash, PartialEq, Eq)]
pub struct RegistryBuiltDist {
/// All wheels associated with this distribution. It is guaranteed
/// that there is at least one wheel.
Expand Down Expand Up @@ -231,7 +240,7 @@ pub struct RegistryBuiltDist {
}

/// A built distribution (wheel) that exists at an arbitrary URL.
#[derive(Debug, Clone, Hash)]
#[derive(Debug, Clone, Hash, PartialEq, Eq)]
pub struct DirectUrlBuiltDist {
/// We require that wheel urls end in the full wheel filename, e.g.
/// `https://example.org/packages/flask-3.0.0-py3-none-any.whl`
Expand All @@ -243,7 +252,7 @@ pub struct DirectUrlBuiltDist {
}

/// A built distribution (wheel) that exists in a local directory.
#[derive(Debug, Clone, Hash)]
#[derive(Debug, Clone, Hash, PartialEq, Eq)]
pub struct PathBuiltDist {
pub filename: WheelFilename,
/// The absolute path to the wheel which we use for installing.
Expand All @@ -253,7 +262,7 @@ pub struct PathBuiltDist {
}

/// A source distribution that exists in a registry, like `PyPI`.
#[derive(Debug, Clone, Hash)]
#[derive(Debug, Clone, Hash, PartialEq, Eq)]
pub struct RegistrySourceDist {
pub name: PackageName,
pub version: Version,
Expand All @@ -272,7 +281,7 @@ pub struct RegistrySourceDist {
}

/// A source distribution that exists at an arbitrary URL.
#[derive(Debug, Clone, Hash)]
#[derive(Debug, Clone, Hash, PartialEq, Eq)]
pub struct DirectUrlSourceDist {
/// Unlike [`DirectUrlBuiltDist`], we can't require a full filename with a version here, people
/// like using e.g. `foo @ https://github.com/org/repo/archive/master.zip`
Expand All @@ -288,7 +297,7 @@ pub struct DirectUrlSourceDist {
}

/// A source distribution that exists in a Git repository.
#[derive(Debug, Clone, Hash)]
#[derive(Debug, Clone, Hash, PartialEq, Eq)]
pub struct GitSourceDist {
pub name: PackageName,
/// The URL without the revision and subdirectory fragment.
Expand All @@ -300,7 +309,7 @@ pub struct GitSourceDist {
}

/// A source distribution that exists in a local archive (e.g., a `.tar.gz` file).
#[derive(Debug, Clone, Hash)]
#[derive(Debug, Clone, Hash, PartialEq, Eq)]
pub struct PathSourceDist {
pub name: PackageName,
/// The absolute path to the distribution which we use for installing.
Expand All @@ -312,7 +321,7 @@ pub struct PathSourceDist {
}

/// A source distribution that exists in a local directory.
#[derive(Debug, Clone, Hash)]
#[derive(Debug, Clone, Hash, PartialEq, Eq)]
pub struct DirectorySourceDist {
pub name: PackageName,
/// The absolute path to the distribution which we use for installing.
Expand Down Expand Up @@ -512,12 +521,33 @@ impl Dist {
}
}

/// Returns the version of the distribution, if it is known.
pub fn version(&self) -> Option<&Version> {
match self {
Self::Built(wheel) => Some(wheel.version()),
Self::Source(source_dist) => source_dist.version(),
}
}

/// Convert this distribution into a reference.
pub fn as_ref(&self) -> DistRef {
match self {
Self::Built(dist) => DistRef::Built(dist),
Self::Source(dist) => DistRef::Source(dist),
}
}
}

impl<'a> From<&'a SourceDist> for DistRef<'a> {
fn from(dist: &'a SourceDist) -> Self {
DistRef::Source(dist)
}
}

impl<'a> From<&'a BuiltDist> for DistRef<'a> {
fn from(dist: &'a BuiltDist) -> Self {
DistRef::Built(dist)
}
}

impl BuiltDist {
Expand Down
Loading

0 comments on commit fe477c3

Please sign in to comment.