Skip to content

Commit

Permalink
Use hints for dotenv errors, rather than suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Oct 1, 2024
1 parent f0f2f89 commit b9bcb37
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 65 deletions.
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion crates/uv-build-frontend/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,16 @@ anyhow = { workspace = true }
fs-err = { workspace = true }
indoc = { workspace = true }
itertools = { workspace = true }
owo-colors = { workspace = true }
regex = { workspace = true }
rustc-hash = { workspace = true }
serde = { workspace = true }
serde_json = { workspace = true }
tempfile = { workspace = true }
thiserror = { workspace = true }
tokio = { workspace = true }
toml_edit = { workspace = true }
tracing = { workspace = true }
rustc-hash = { workspace = true }

[dev-dependencies]
insta = { version = "1.40.0" }
96 changes: 66 additions & 30 deletions crates/uv-build-frontend/src/error.rs
Original file line number Diff line number Diff line change
@@ -1,19 +1,37 @@
use crate::PythonRunnerOutput;
use itertools::Itertools;
use owo_colors::OwoColorize;
use pep440_rs::Version;
use pep508_rs::PackageName;
use regex::Regex;
use rustc_hash::FxHashMap;
use std::env;
use std::fmt::{Display, Formatter};
use std::io;
use std::path::PathBuf;
use std::process::ExitStatus;
use std::str::FromStr;
use std::sync::LazyLock;
use thiserror::Error;
use tracing::error;
use uv_configuration::BuildOutput;
use uv_fs::Simplified;

/// Static map of common package name typos or misconfigurations to their correct package names.
static SUGGESTIONS: LazyLock<FxHashMap<PackageName, PackageName>> = LazyLock::new(|| {
let suggestions: Vec<(String, String)> =
serde_json::from_str(include_str!("suggestions.json")).unwrap();
suggestions
.iter()
.map(|(k, v)| {
(
PackageName::from_str(k).unwrap(),
PackageName::from_str(v).unwrap(),
)
})
.collect()
});

/// e.g. `pygraphviz/graphviz_wrap.c:3020:10: fatal error: graphviz/cgraph.h: No such file or directory`
static MISSING_HEADER_RE_GCC: LazyLock<Regex> = LazyLock::new(|| {
Regex::new(
Expand Down Expand Up @@ -77,25 +95,23 @@ pub enum Error {
stderr: String,
},
/// Nudge the user towards installing the missing dev library
#[error("{message} with {exit_code}\n--- stdout:\n{stdout}\n--- stderr:\n{stderr}\n---")]
#[error("{message} with {exit_code}\n--- stdout:\n{stdout}\n--- stderr:\n{stderr}\n---\n\n{hint}{colon} {missing_header_cause}", hint = "hint".cyan().bold(), colon = ":".bold())]
MissingHeaderOutput {
message: String,
exit_code: ExitStatus,
stdout: String,
stderr: String,
#[source]
missing_header_cause: MissingHeaderCause,
},
#[error("{message} with {exit_code}")]
BuildBackend {
message: String,
exit_code: ExitStatus,
},
#[error("{message} with {exit_code}")]
#[error("{message} with {exit_code}\n\n{hint}{colon} {missing_header_cause}", hint = "hint".cyan().bold(), colon = ":".bold())]
MissingHeader {
message: String,
exit_code: ExitStatus,
#[source]
missing_header_cause: MissingHeaderCause,
},
#[error("Failed to build PATH for build script")]
Expand All @@ -108,6 +124,7 @@ enum MissingLibrary {
Linker(String),
BuildDependency(String),
DeprecatedModule(String, Version),
SuggestedPackage(String, String),
}

#[derive(Debug, Error)]
Expand Down Expand Up @@ -196,6 +213,15 @@ impl Display for MissingHeaderCause {
)
}
}
MissingLibrary::SuggestedPackage(package, suggestion) => {
write!(
f,
"`{}` is often confused for `{}` Did you mean to install `{}` instead?",
package.cyan(),
suggestion.cyan(),
suggestion.cyan(),
)
}
}
}
}
Expand All @@ -210,32 +236,42 @@ impl Error {
version: Option<&Version>,
version_id: Option<&str>,
) -> Self {
// In the cases I've seen it was the 5th and 3rd last line (see test case), 10 seems like a reasonable cutoff.
let missing_library = output.stderr.iter().rev().take(10).find_map(|line| {
if let Some((_, [header])) = MISSING_HEADER_RE_GCC
.captures(line.trim())
.or(MISSING_HEADER_RE_CLANG.captures(line.trim()))
.or(MISSING_HEADER_RE_MSVC.captures(line.trim()))
.map(|c| c.extract())
{
Some(MissingLibrary::Header(header.to_string()))
} else if let Some((_, [library])) =
LD_NOT_FOUND_RE.captures(line.trim()).map(|c| c.extract())
{
Some(MissingLibrary::Linker(library.to_string()))
} else if WHEEL_NOT_FOUND_RE.is_match(line.trim()) {
Some(MissingLibrary::BuildDependency("wheel".to_string()))
} else if TORCH_NOT_FOUND_RE.is_match(line.trim()) {
Some(MissingLibrary::BuildDependency("torch".to_string()))
} else if DISTUTILS_NOT_FOUND_RE.is_match(line.trim()) {
Some(MissingLibrary::DeprecatedModule(
"distutils".to_string(),
Version::new([3, 12]),
))
} else {
None
}
});
// Check if we failed to install a known package.
let missing_library = if let Some((name, suggestion)) =
name.and_then(|name| SUGGESTIONS.get(name).map(|suggestion| (name, suggestion)))
{
Some(MissingLibrary::SuggestedPackage(
name.to_string(),
suggestion.to_string(),
))
} else {
// Limit search to the last 10 lines, which typically contains the relevant error.
output.stderr.iter().rev().take(10).find_map(|line| {
if let Some((_, [header])) = MISSING_HEADER_RE_GCC
.captures(line.trim())
.or(MISSING_HEADER_RE_CLANG.captures(line.trim()))
.or(MISSING_HEADER_RE_MSVC.captures(line.trim()))
.map(|c| c.extract())
{
Some(MissingLibrary::Header(header.to_string()))
} else if let Some((_, [library])) =
LD_NOT_FOUND_RE.captures(line.trim()).map(|c| c.extract())
{
Some(MissingLibrary::Linker(library.to_string()))
} else if WHEEL_NOT_FOUND_RE.is_match(line.trim()) {
Some(MissingLibrary::BuildDependency("wheel".to_string()))
} else if TORCH_NOT_FOUND_RE.is_match(line.trim()) {
Some(MissingLibrary::BuildDependency("torch".to_string()))
} else if DISTUTILS_NOT_FOUND_RE.is_match(line.trim()) {
Some(MissingLibrary::DeprecatedModule(
"distutils".to_string(),
Version::new([3, 12]),
))
} else {
None
}
})
};

if let Some(missing_library) = missing_library {
return match level {
Expand Down
4 changes: 4 additions & 0 deletions crates/uv-build-frontend/src/suggestions.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
[
["sklearn", "scikit-learn"],
["dotenv", "python-dotenv"]
]
1 change: 0 additions & 1 deletion crates/uv/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ uv-cache-info = { workspace = true }
uv-cli = { workspace = true }
uv-client = { workspace = true }
uv-configuration = { workspace = true }
uv-console = { workspace = true }
uv-dispatch = { workspace = true }
uv-distribution = { workspace = true }
uv-extract = { workspace = true }
Expand Down
32 changes: 0 additions & 32 deletions crates/uv/src/commands/project/add.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,10 @@
use anyhow::{bail, Context, Result};
use console::Term;
use itertools::Itertools;
use owo_colors::OwoColorize;
use rustc_hash::{FxBuildHasher, FxHashMap};
use std::collections::hash_map::Entry;
use std::fmt::Write;
use std::path::{Path, PathBuf};
use std::str::FromStr;
use std::sync::LazyLock;
use tracing::debug;

use cache_key::RepositoryUrl;
Expand Down Expand Up @@ -50,18 +47,6 @@ use crate::commands::{pip, project, ExitStatus, SharedState};
use crate::printer::Printer;
use crate::settings::{ResolverInstallerSettings, ResolverInstallerSettingsRef};

static CORRECTIONS: LazyLock<FxHashMap<PackageName, PackageName>> = LazyLock::new(|| {
[("dotenv", "python-dotenv"), ("sklearn", "scikit-learn")]
.iter()
.map(|(k, v)| {
(
PackageName::from_str(k).unwrap(),
PackageName::from_str(v).unwrap(),
)
})
.collect()
});

/// Add one or more packages to the project requirements.
#[allow(clippy::fn_params_excessive_bools)]
pub(crate) async fn add(
Expand Down Expand Up @@ -385,23 +370,6 @@ pub(crate) async fn add(
}?;
let mut edits = Vec::<DependencyEdit>::with_capacity(requirements.len());
for mut requirement in requirements {
// If the user requested a package that is often confused for another package, prompt them.
if let Some(correction) = CORRECTIONS.get(&requirement.name) {
let term = Term::stderr();
if term.is_term() {
let prompt = format!(
"`{}` is often confused for `{}`. Did you mean `{}`?",
requirement.name.cyan(),
correction.cyan(),
format!("uv add {correction}").green()
);
let confirmation = uv_console::confirm(&prompt, &term, true)?;
if confirmation {
requirement.name = correction.clone();
}
}
}

// Add the specified extras.
requirement.extras.extend(extras.iter().cloned());
requirement.extras.sort_unstable();
Expand Down

0 comments on commit b9bcb37

Please sign in to comment.