Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use rich diagnostic formatting for install failures #9043

Merged
merged 1 commit into from
Nov 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions crates/uv-dispatch/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,10 +275,7 @@ impl<'a> BuildContext for BuildDispatch<'a> {
remote.iter().map(ToString::to_string).join(", ")
);

preparer
.prepare(remote, self.in_flight)
.await
.context("Failed to prepare distributions")?
preparer.prepare(remote, self.in_flight).await?
};

// Remove any unnecessary packages.
Expand Down
2 changes: 1 addition & 1 deletion crates/uv-installer/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
pub use compile::{compile_tree, CompileError};
pub use installer::{Installer, Reporter as InstallReporter};
pub use plan::{Plan, Planner};
pub use preparer::{Preparer, Reporter as PrepareReporter};
pub use preparer::{Error as PrepareError, Preparer, Reporter as PrepareReporter};
pub use site_packages::{SatisfiesResult, SitePackages, SitePackagesDiagnostic};
pub use uninstall::{uninstall, UninstallError};

Expand Down
18 changes: 9 additions & 9 deletions crates/uv-installer/src/preparer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,11 @@ pub enum Error {
#[error("Using pre-built wheels is disabled, but attempted to use `{0}`")]
NoBinary(PackageName),
#[error("Failed to download `{0}`")]
Download(Box<BuiltDist>, #[source] Box<uv_distribution::Error>),
Download(Box<BuiltDist>, #[source] uv_distribution::Error),
#[error("Failed to download and build `{0}`")]
DownloadAndBuild(Box<SourceDist>, #[source] Box<uv_distribution::Error>),
DownloadAndBuild(Box<SourceDist>, #[source] uv_distribution::Error),
#[error("Failed to build `{0}`")]
Build(Box<SourceDist>, #[source] Box<uv_distribution::Error>),
Build(Box<SourceDist>, #[source] uv_distribution::Error),
#[error("Unzip failed in another thread: {0}")]
Thread(String),
}
Expand Down Expand Up @@ -146,12 +146,12 @@ impl<'a, Context: BuildContext> Preparer<'a, Context> {
.get_or_build_wheel(&dist, self.tags, policy)
.boxed_local()
.map_err(|err| match dist.clone() {
Dist::Built(dist) => Error::Download(Box::new(dist), Box::new(err)),
Dist::Built(dist) => Error::Download(Box::new(dist), err),
Dist::Source(dist) => {
if dist.is_local() {
Error::Build(Box::new(dist), Box::new(err))
Error::Build(Box::new(dist), err)
} else {
Error::DownloadAndBuild(Box::new(dist), Box::new(err))
Error::DownloadAndBuild(Box::new(dist), err)
}
}
})
Expand All @@ -166,12 +166,12 @@ impl<'a, Context: BuildContext> Preparer<'a, Context> {
wheel.hashes(),
);
Err(match dist {
Dist::Built(dist) => Error::Download(Box::new(dist), Box::new(err)),
Dist::Built(dist) => Error::Download(Box::new(dist), err),
Dist::Source(dist) => {
if dist.is_local() {
Error::Build(Box::new(dist), Box::new(err))
Error::Build(Box::new(dist), err)
} else {
Error::DownloadAndBuild(Box::new(dist), Box::new(err))
Error::DownloadAndBuild(Box::new(dist), err)
}
}
})
Expand Down
30 changes: 29 additions & 1 deletion crates/uv/src/commands/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use owo_colors::OwoColorize;
use rustc_hash::FxHashMap;
use std::str::FromStr;
use std::sync::LazyLock;
use uv_distribution_types::{Name, SourceDist};
use uv_distribution_types::{BuiltDist, Name, SourceDist};
use uv_normalize::PackageName;

/// Static map of common package name typos or misconfigurations to their correct package names.
Expand Down Expand Up @@ -48,6 +48,34 @@ pub(crate) fn download_and_build(sdist: Box<SourceDist>, cause: uv_distribution:
anstream::eprint!("{report:?}");
}

/// Render a remote binary distribution download failure with a help message.
pub(crate) fn download(sdist: Box<BuiltDist>, cause: uv_distribution::Error) {
#[derive(Debug, miette::Diagnostic, thiserror::Error)]
#[error("Failed to download `{sdist}`")]
#[diagnostic()]
struct Error {
sdist: Box<BuiltDist>,
#[source]
cause: uv_distribution::Error,
#[help]
help: Option<String>,
}

let report = miette::Report::new(Error {
help: SUGGESTIONS.get(sdist.name()).map(|suggestion| {
format!(
"`{}` is often confused for `{}` Did you mean to install `{}` instead?",
sdist.name().cyan(),
suggestion.cyan(),
suggestion.cyan(),
)
}),
sdist,
cause,
});
anstream::eprint!("{report:?}");
}

/// Render a local source distribution build failure with a help message.
pub(crate) fn build(sdist: Box<SourceDist>, cause: uv_distribution::Error) {
#[derive(Debug, miette::Diagnostic, thiserror::Error)]
Expand Down
23 changes: 21 additions & 2 deletions crates/uv/src/commands/pip/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,7 @@ pub(crate) async fn pip_install(
};

// Sync the environment.
operations::install(
match operations::install(
&resolution,
site_packages,
modifications,
Expand All @@ -461,7 +461,26 @@ pub(crate) async fn pip_install(
dry_run,
printer,
)
.await?;
.await
{
Ok(_) => {}
Err(operations::Error::Prepare(uv_installer::PrepareError::Build(dist, err))) => {
diagnostics::build(dist, err);
return Ok(ExitStatus::Failure);
}
Err(operations::Error::Prepare(uv_installer::PrepareError::DownloadAndBuild(
dist,
err,
))) => {
diagnostics::download_and_build(dist, err);
return Ok(ExitStatus::Failure);
}
Err(operations::Error::Prepare(uv_installer::PrepareError::Download(dist, err))) => {
diagnostics::download(dist, err);
return Ok(ExitStatus::Failure);
}
Err(err) => return Err(err.into()),
}

// Notify the user of any resolution diagnostics.
operations::diagnose_resolution(resolution.diagnostics(), printer)?;
Expand Down
8 changes: 4 additions & 4 deletions crates/uv/src/commands/pip/operations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -457,10 +457,7 @@ 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
.context("Failed to prepare distributions")?;
let wheels = preparer.prepare(remote.clone(), in_flight).await?;

logger.on_prepare(wheels.len(), start, printer)?;

Expand Down Expand Up @@ -751,6 +748,9 @@ pub(crate) fn diagnose_environment(

#[derive(thiserror::Error, Debug)]
pub(crate) enum Error {
#[error("Failed to prepare distributions")]
Prepare(#[from] uv_installer::PrepareError),

#[error(transparent)]
Resolve(#[from] uv_resolver::ResolveError),

Expand Down
23 changes: 21 additions & 2 deletions crates/uv/src/commands/pip/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,7 @@ pub(crate) async fn pip_sync(
};

// Sync the environment.
operations::install(
match operations::install(
&resolution,
site_packages,
Modifications::Exact,
Expand All @@ -405,7 +405,26 @@ pub(crate) async fn pip_sync(
dry_run,
printer,
)
.await?;
.await
{
Ok(_) => {}
Err(operations::Error::Prepare(uv_installer::PrepareError::Build(dist, err))) => {
diagnostics::build(dist, err);
return Ok(ExitStatus::Failure);
}
Err(operations::Error::Prepare(uv_installer::PrepareError::DownloadAndBuild(
dist,
err,
))) => {
diagnostics::download_and_build(dist, err);
return Ok(ExitStatus::Failure);
}
Err(operations::Error::Prepare(uv_installer::PrepareError::Download(dist, err))) => {
diagnostics::download(dist, err);
return Ok(ExitStatus::Failure);
}
Err(err) => return Err(err.into()),
}

// Notify the user of any resolution diagnostics.
operations::diagnose_resolution(resolution.diagnostics(), printer)?;
Expand Down
18 changes: 18 additions & 0 deletions crates/uv/src/commands/project/add.rs
Original file line number Diff line number Diff line change
Expand Up @@ -716,6 +716,24 @@ pub(crate) async fn add(
diagnostics::build(dist, err);
Ok(ExitStatus::Failure)
}
ProjectError::Operation(pip::operations::Error::Prepare(
uv_installer::PrepareError::Build(dist, err),
)) => {
diagnostics::build(dist, err);
Ok(ExitStatus::Failure)
}
ProjectError::Operation(pip::operations::Error::Prepare(
uv_installer::PrepareError::DownloadAndBuild(dist, err),
)) => {
diagnostics::download_and_build(dist, err);
Ok(ExitStatus::Failure)
}
ProjectError::Operation(pip::operations::Error::Prepare(
uv_installer::PrepareError::Download(dist, err),
)) => {
diagnostics::download(dist, err);
Ok(ExitStatus::Failure)
}
err => Err(err.into()),
}
}
Expand Down
70 changes: 63 additions & 7 deletions crates/uv/src/commands/project/remove.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,11 @@ use uv_workspace::pyproject_mut::{DependencyTarget, PyProjectTomlMut};
use uv_workspace::{DiscoveryOptions, VirtualProject, Workspace};

use crate::commands::pip::loggers::{DefaultInstallLogger, DefaultResolveLogger};
use crate::commands::pip::operations;
use crate::commands::pip::operations::Modifications;
use crate::commands::project::default_dependency_groups;
use crate::commands::project::lock::LockMode;
use crate::commands::{project, ExitStatus, SharedState};
use crate::commands::project::{default_dependency_groups, ProjectError};
use crate::commands::{diagnostics, project, ExitStatus, SharedState};
use crate::printer::Printer;
use crate::settings::ResolverInstallerSettings;

Expand Down Expand Up @@ -213,7 +214,7 @@ pub(crate) async fn remove(
let state = SharedState::default();

// Lock and sync the environment, if necessary.
let lock = project::lock::do_safe_lock(
let lock = match project::lock::do_safe_lock(
mode,
project.workspace(),
settings.as_ref().into(),
Expand All @@ -227,8 +228,41 @@ pub(crate) async fn remove(
cache,
printer,
)
.await?
.into_lock();
.await
{
Ok(result) => result.into_lock(),
Err(ProjectError::Operation(operations::Error::Resolve(
uv_resolver::ResolveError::NoSolution(err),
))) => {
diagnostics::no_solution(&err);
return Ok(ExitStatus::Failure);
}
Err(ProjectError::Operation(operations::Error::Resolve(
uv_resolver::ResolveError::DownloadAndBuild(dist, err),
))) => {
diagnostics::download_and_build(dist, err);
return Ok(ExitStatus::Failure);
}
Err(ProjectError::Operation(operations::Error::Resolve(
uv_resolver::ResolveError::Build(dist, err),
))) => {
diagnostics::build(dist, err);
return Ok(ExitStatus::Failure);
}
Err(ProjectError::Operation(operations::Error::Requirements(
uv_requirements::Error::DownloadAndBuild(dist, err),
))) => {
diagnostics::download_and_build(dist, err);
return Ok(ExitStatus::Failure);
}
Err(ProjectError::Operation(operations::Error::Requirements(
uv_requirements::Error::Build(dist, err),
))) => {
diagnostics::build(dist, err);
return Ok(ExitStatus::Failure);
}
Err(err) => return Err(err.into()),
};

if no_sync {
return Ok(ExitStatus::Success);
Expand All @@ -255,7 +289,7 @@ pub(crate) async fn remove(
},
};

project::sync::do_sync(
match project::sync::do_sync(
target,
&venv,
&extras,
Expand All @@ -272,7 +306,29 @@ pub(crate) async fn remove(
cache,
printer,
)
.await?;
.await
{
Ok(()) => {}
Err(ProjectError::Operation(operations::Error::Prepare(
uv_installer::PrepareError::Build(dist, err),
))) => {
diagnostics::build(dist, err);
return Ok(ExitStatus::Failure);
}
Err(ProjectError::Operation(operations::Error::Prepare(
uv_installer::PrepareError::DownloadAndBuild(dist, err),
))) => {
diagnostics::download_and_build(dist, err);
return Ok(ExitStatus::Failure);
}
Err(ProjectError::Operation(operations::Error::Prepare(
uv_installer::PrepareError::Download(dist, err),
))) => {
diagnostics::download(dist, err);
return Ok(ExitStatus::Failure);
}
Err(err) => return Err(err.into()),
}

Ok(ExitStatus::Success)
}
Expand Down
26 changes: 24 additions & 2 deletions crates/uv/src/commands/project/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -697,7 +697,7 @@ pub(crate) async fn run(

let install_options = InstallOptions::default();

project::sync::do_sync(
match project::sync::do_sync(
target,
&venv,
&extras,
Expand All @@ -718,7 +718,29 @@ pub(crate) async fn run(
cache,
printer,
)
.await?;
.await
{
Ok(()) => {}
Err(ProjectError::Operation(operations::Error::Prepare(
uv_installer::PrepareError::Build(dist, err),
))) => {
diagnostics::build(dist, err);
return Ok(ExitStatus::Failure);
}
Err(ProjectError::Operation(operations::Error::Prepare(
uv_installer::PrepareError::DownloadAndBuild(dist, err),
))) => {
diagnostics::download_and_build(dist, err);
return Ok(ExitStatus::Failure);
}
Err(ProjectError::Operation(operations::Error::Prepare(
uv_installer::PrepareError::Download(dist, err),
))) => {
diagnostics::download(dist, err);
return Ok(ExitStatus::Failure);
}
Err(err) => return Err(err.into()),
}

lock = Some(result.into_lock());
}
Expand Down
Loading
Loading