From 084ecb1a5008fe42d938d6eba9aa20b8906a2d33 Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Wed, 12 Aug 2026 11:41:18 -0500 Subject: [PATCH 1/2] Report invalid tool directories during tool upgrades --- crates/uv/src/commands/diagnostics.rs | 2 ++ crates/uv/src/commands/tool/upgrade.rs | 29 ++++++++++++++++++++++++-- crates/uv/tests/tool/tool_upgrade.rs | 16 +++++++------- 3 files changed, 38 insertions(+), 9 deletions(-) diff --git a/crates/uv/src/commands/diagnostics.rs b/crates/uv/src/commands/diagnostics.rs index 7811b65b1bb..b941f7f1612 100644 --- a/crates/uv/src/commands/diagnostics.rs +++ b/crates/uv/src/commands/diagnostics.rs @@ -21,6 +21,7 @@ use crate::commands::project::run::RecursionLimitError; use crate::commands::project::version::MissingProjectVersionError; use crate::commands::tool::common::NoExecutablesError; use crate::commands::tool::run::ToolRunScriptError; +use crate::commands::tool::upgrade::ToolEnumerationError; use crate::printer::Printer; static SUGGESTIONS: LazyLock> = LazyLock::new(|| { @@ -237,6 +238,7 @@ pub(crate) fn hints_for_error(err: &anyhow::Error) -> Hints<'static> { collect_hint::(cause, &mut hints); collect_hint::(cause, &mut hints); collect_hint::(cause, &mut hints); + collect_hint::(cause, &mut hints); collect_hint::(cause, &mut hints); collect_hint::(cause, &mut hints); collect_hint::(cause, &mut hints); diff --git a/crates/uv/src/commands/tool/upgrade.rs b/crates/uv/src/commands/tool/upgrade.rs index e93598b32eb..f5896ced4ea 100644 --- a/crates/uv/src/commands/tool/upgrade.rs +++ b/crates/uv/src/commands/tool/upgrade.rs @@ -3,6 +3,7 @@ use itertools::Itertools; use owo_colors::OwoColorize; use std::collections::BTreeMap; use std::fmt::Write; +use std::path::PathBuf; use std::str::FromStr; use tracing::{debug, trace}; @@ -13,7 +14,7 @@ use uv_configuration::{Concurrency, Constraints, DryRun, HashCheckingMode, Targe use uv_distribution::LoweredExtraBuildDependencies; use uv_distribution_types::{ExtraBuildRequires, Index, Name, Requirement, RequirementSource}; use uv_errors::{ErrorOptions, Hints, write_error_chain_with_options}; -use uv_fs::CWD; +use uv_fs::{CWD, Simplified}; use uv_installer::{InstallationStrategy, Planner, SitePackages}; use uv_normalize::PackageName; use uv_pep440::{Operator, Version}; @@ -68,7 +69,10 @@ pub(crate) async fn upgrade( if names.is_empty() { installed_tools .tools() - .unwrap_or_default() + .map_err(|source| ToolEnumerationError { + root: installed_tools.root().to_path_buf(), + source, + })? .into_iter() .map(|(name, _)| (name, Vec::new())) .collect() @@ -215,6 +219,27 @@ pub(crate) async fn upgrade( Ok(ExitStatus::Success) } +/// An installed tool could not be discovered from the configured tool directory. +#[derive(Debug, thiserror::Error)] +#[error("Failed to inspect installed tools in `{}`", root.user_display())] +pub(crate) struct ToolEnumerationError { + root: PathBuf, + #[source] + source: uv_tool::Error, +} + +impl uv_errors::Hint for ToolEnumerationError { + fn hints(&self) -> Hints<'_> { + if matches!(&self.source, uv_tool::Error::ToolName(_)) { + Hints::from( + "Move directories with invalid package names out of the tool directory, or remove them if they are no longer needed", + ) + } else { + Hints::none() + } + } +} + #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum UpgradeOutcome { /// The tool itself was upgraded. diff --git a/crates/uv/tests/tool/tool_upgrade.rs b/crates/uv/tests/tool/tool_upgrade.rs index 422d704e921..8815aa51b02 100644 --- a/crates/uv/tests/tool/tool_upgrade.rs +++ b/crates/uv/tests/tool/tool_upgrade.rs @@ -87,24 +87,26 @@ fn tool_upgrade_empty() { } #[test] -fn tool_upgrade_all_ignores_invalid_tool_name() -> Result<()> { +fn tool_upgrade_all_reports_invalid_tool_name() -> Result<()> { let context = uv_test::test_context!("3.12").with_filtered_exe_suffix(); let tool_dir = context.temp_dir.child("tools"); let bin_dir = context.temp_dir.child("bin"); tool_dir.child("tool backup").create_dir_all()?; - // Silently treating an enumeration error as an empty tool directory hides a broken - // installation and exits successfully; see astral-sh/uv#21058. + // Invalid tool directories must not be mistaken for an empty tool inventory. uv_snapshot!(context.filters(), context.tool_upgrade() .arg("--all") .env(EnvVars::UV_TOOL_DIR, tool_dir.as_os_str()) .env(EnvVars::XDG_BIN_HOME, bin_dir.as_os_str()) - .env(EnvVars::PATH, bin_dir.as_os_str()), @" - exit_code: 0 (success) + .env(EnvVars::PATH, bin_dir.as_os_str()), @r#" + exit_code: 2 (failure) ----- stderr ----- - Nothing to upgrade - "); + error: Failed to inspect installed tools in `tools` + Caused by: Not a valid package or extra name: "tool backup". Names must start and end with a letter or digit and may only contain -, _, ., and alphanumeric characters. + + hint: Move directories with invalid package names out of the tool directory, or remove them if they are no longer needed + "#); Ok(()) } From 53f8402a09b9da00298ff432dac20003feda0fea Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Mon, 31 Aug 2026 12:49:08 -0400 Subject: [PATCH 2/2] Warn and skip invalid tool directories --- Cargo.lock | 1 + crates/uv-tool/Cargo.toml | 1 + crates/uv-tool/src/lib.rs | 16 +++++++---- crates/uv/src/commands/diagnostics.rs | 2 -- crates/uv/src/commands/tool/upgrade.rs | 32 ++++----------------- crates/uv/tests/tool/tool_upgrade.rs | 40 ++++++++++++++++++++------ 6 files changed, 51 insertions(+), 41 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index df54850a893..98962e01375 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7569,6 +7569,7 @@ dependencies = [ "uv-state", "uv-static", "uv-virtualenv", + "uv-warnings", ] [[package]] diff --git a/crates/uv-tool/Cargo.toml b/crates/uv-tool/Cargo.toml index 875d11e2c16..531445aa042 100644 --- a/crates/uv-tool/Cargo.toml +++ b/crates/uv-tool/Cargo.toml @@ -33,6 +33,7 @@ uv-settings = { workspace = true } uv-state = { workspace = true } uv-static = { workspace = true } uv-virtualenv = { workspace = true } +uv-warnings = { workspace = true } fs-err = { workspace = true } owo-colors = { workspace = true } diff --git a/crates/uv-tool/src/lib.rs b/crates/uv-tool/src/lib.rs index e97b0b18097..314197b4d88 100644 --- a/crates/uv-tool/src/lib.rs +++ b/crates/uv-tool/src/lib.rs @@ -13,11 +13,12 @@ use uv_dirs::user_executable_directory; use uv_fs::{LockedFile, LockedFileError, LockedFileMode, Simplified}; use uv_install_wheel::read_record; use uv_installer::SitePackages; -use uv_normalize::{InvalidNameError, PackageName}; +use uv_normalize::PackageName; use uv_pep440::Version; use uv_python::{BrokenLink, Interpreter, PythonEnvironment}; use uv_state::{StateBucket, StateStore}; use uv_static::EnvVars; +use uv_warnings::warn_user; pub(crate) use receipt::ToolReceipt; pub use tool::{Tool, ToolEntrypoint}; @@ -77,8 +78,6 @@ pub enum Error { #[error("Failed to find a directory to install executables into")] NoExecutableDirectory, #[error(transparent)] - ToolName(#[from] InvalidNameError), - #[error(transparent)] EnvironmentError(#[from] uv_python::Error), #[error("Failed to find a receipt for tool `{0}` at {1}")] MissingToolReceipt(String, PathBuf), @@ -101,7 +100,6 @@ impl Error { | Self::VirtualEnvError(_) | Self::EntrypointRead(_) | Self::NoExecutableDirectory - | Self::ToolName(_) | Self::EnvironmentError(_) | Self::MissingToolReceipt(_, _) | Self::EnvironmentRead(_, _) @@ -148,6 +146,8 @@ impl InstalledTools { /// Return the metadata for all installed tools. /// + /// Directories with invalid package names are skipped with a warning. + /// /// If a tool is present, but is missing a receipt or the receipt is invalid, the tool will be /// included with an error. /// @@ -162,7 +162,13 @@ impl InstalledTools { else { continue; }; - let name = PackageName::from_str(name)?; + let Ok(name) = PackageName::from_str(name) else { + warn_user!( + "Ignoring tool directory `{}` with an invalid package name; move it outside the tool directory, or remove it if no longer needed", + directory.user_display() + ); + continue; + }; let path = directory.join("uv-receipt.toml"); let contents = match fs_err::read_to_string(&path) { Ok(contents) => contents, diff --git a/crates/uv/src/commands/diagnostics.rs b/crates/uv/src/commands/diagnostics.rs index b941f7f1612..7811b65b1bb 100644 --- a/crates/uv/src/commands/diagnostics.rs +++ b/crates/uv/src/commands/diagnostics.rs @@ -21,7 +21,6 @@ use crate::commands::project::run::RecursionLimitError; use crate::commands::project::version::MissingProjectVersionError; use crate::commands::tool::common::NoExecutablesError; use crate::commands::tool::run::ToolRunScriptError; -use crate::commands::tool::upgrade::ToolEnumerationError; use crate::printer::Printer; static SUGGESTIONS: LazyLock> = LazyLock::new(|| { @@ -238,7 +237,6 @@ pub(crate) fn hints_for_error(err: &anyhow::Error) -> Hints<'static> { collect_hint::(cause, &mut hints); collect_hint::(cause, &mut hints); collect_hint::(cause, &mut hints); - collect_hint::(cause, &mut hints); collect_hint::(cause, &mut hints); collect_hint::(cause, &mut hints); collect_hint::(cause, &mut hints); diff --git a/crates/uv/src/commands/tool/upgrade.rs b/crates/uv/src/commands/tool/upgrade.rs index f5896ced4ea..577220e3de7 100644 --- a/crates/uv/src/commands/tool/upgrade.rs +++ b/crates/uv/src/commands/tool/upgrade.rs @@ -1,9 +1,8 @@ -use anyhow::Result; +use anyhow::{Context, Result}; use itertools::Itertools; use owo_colors::OwoColorize; use std::collections::BTreeMap; use std::fmt::Write; -use std::path::PathBuf; use std::str::FromStr; use tracing::{debug, trace}; @@ -69,9 +68,11 @@ pub(crate) async fn upgrade( if names.is_empty() { installed_tools .tools() - .map_err(|source| ToolEnumerationError { - root: installed_tools.root().to_path_buf(), - source, + .with_context(|| { + format!( + "Failed to inspect installed tools in `{}`", + installed_tools.root().user_display() + ) })? .into_iter() .map(|(name, _)| (name, Vec::new())) @@ -219,27 +220,6 @@ pub(crate) async fn upgrade( Ok(ExitStatus::Success) } -/// An installed tool could not be discovered from the configured tool directory. -#[derive(Debug, thiserror::Error)] -#[error("Failed to inspect installed tools in `{}`", root.user_display())] -pub(crate) struct ToolEnumerationError { - root: PathBuf, - #[source] - source: uv_tool::Error, -} - -impl uv_errors::Hint for ToolEnumerationError { - fn hints(&self) -> Hints<'_> { - if matches!(&self.source, uv_tool::Error::ToolName(_)) { - Hints::from( - "Move directories with invalid package names out of the tool directory, or remove them if they are no longer needed", - ) - } else { - Hints::none() - } - } -} - #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum UpgradeOutcome { /// The tool itself was upgraded. diff --git a/crates/uv/tests/tool/tool_upgrade.rs b/crates/uv/tests/tool/tool_upgrade.rs index 8815aa51b02..22f315e9bb5 100644 --- a/crates/uv/tests/tool/tool_upgrade.rs +++ b/crates/uv/tests/tool/tool_upgrade.rs @@ -87,26 +87,44 @@ fn tool_upgrade_empty() { } #[test] -fn tool_upgrade_all_reports_invalid_tool_name() -> Result<()> { +fn tool_upgrade_all_ignores_invalid_tool_name() -> Result<()> { let context = uv_test::test_context!("3.12").with_filtered_exe_suffix(); let tool_dir = context.temp_dir.child("tools"); let bin_dir = context.temp_dir.child("bin"); tool_dir.child("tool backup").create_dir_all()?; - // Invalid tool directories must not be mistaken for an empty tool inventory. uv_snapshot!(context.filters(), context.tool_upgrade() .arg("--all") .env(EnvVars::UV_TOOL_DIR, tool_dir.as_os_str()) .env(EnvVars::XDG_BIN_HOME, bin_dir.as_os_str()) - .env(EnvVars::PATH, bin_dir.as_os_str()), @r#" + .env(EnvVars::PATH, bin_dir.as_os_str()), @" + exit_code: 0 (success) + ----- stderr ----- + warning: Ignoring tool directory `tools/tool backup` with an invalid package name; move it outside the tool directory, or remove it if no longer needed + Nothing to upgrade + "); + + Ok(()) +} + +#[test] +fn tool_upgrade_all_unreadable_receipt() -> Result<()> { + let context = uv_test::test_context!("3.12").with_tool_dirs(); + let tool_dir = context.temp_dir.child("tools"); + + tool_dir.child("babel").create_dir_all()?; + tool_dir + .child("babel") + .child("uv-receipt.toml") + .write_binary(&[0xff])?; + + uv_snapshot!(context.filters(), context.tool_upgrade().arg("--all"), @" exit_code: 2 (failure) ----- stderr ----- error: Failed to inspect installed tools in `tools` - Caused by: Not a valid package or extra name: "tool backup". Names must start and end with a letter or digit and may only contain -, _, ., and alphanumeric characters. - - hint: Move directories with invalid package names out of the tool directory, or remove them if they are no longer needed - "#); + Caused by: failed to read from file `[TEMP_DIR]/tools/babel/uv-receipt.toml`: stream did not contain valid UTF-8 + "); Ok(()) } @@ -756,7 +774,7 @@ fn tool_upgrade_pinned_hint_with_mixed_constraint() { } #[test] -fn tool_upgrade_all() { +fn tool_upgrade_all() -> Result<()> { let context = uv_test::test_context!("3.12") .with_filtered_counts() .with_filtered_exe_suffix(); @@ -798,6 +816,9 @@ fn tool_upgrade_all() { Installed 1 executable: pybabel "); + // An invalid directory must not prevent valid tools from being upgraded. + tool_dir.child("tool backup").create_dir_all()?; + // Upgrade all from PyPI. uv_snapshot!(context.filters(), context.tool_upgrade() .arg("--all") @@ -808,6 +829,7 @@ fn tool_upgrade_all() { .env(EnvVars::PATH, bin_dir.as_os_str()), @" exit_code: 0 (success) ----- stderr ----- + warning: Ignoring tool directory `tools/tool backup` with an invalid package name; move it outside the tool directory, or remove it if no longer needed Updated babel v2.6.0 -> v2.14.0 - babel==2.6.0 + babel==2.14.0 @@ -818,6 +840,8 @@ fn tool_upgrade_all() { + python-dotenv==1.0.1 Installed 1 executable: dotenv "); + + Ok(()) } #[test]