From c9e09de794f1bb7d236352b07c75c1ac85b79e9c Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 5 Mar 2025 18:15:19 -0800 Subject: [PATCH] Warn user on `uvx run` command (#11992) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary If a user invokes `uvx run ...`, we hint them towards `uvx`. Otherwise, this invokes the `run` package, which is unmaintained on PyPI. If the user is _only_ using PyPI, we show an interactive prompt here; otherwise, we just show a dedicated warning on error. Closes https://github.com/astral-sh/uv/issues/11982. ## Test Plan Prompting to success: ![Screenshot 2025-03-05 at 5 00 47 PM](https://github.com/user-attachments/assets/d8180606-94e1-41df-b799-19b8ba57e662) If you use `--from`, we avoid the prompt and hint: ![Screenshot 2025-03-05 at 5 03 26 PM](https://github.com/user-attachments/assets/59919390-01d3-4ddf-97bc-bb857ae9f8b0) If you provide another index, we don't prompt, but we do warn on failure: ![Screenshot 2025-03-05 at 5 03 43 PM](https://github.com/user-attachments/assets/0cc72c36-5744-48f1-aeff-4a214190d6fd) --- crates/uv/src/commands/diagnostics.rs | 88 +++++++++++++++------------ crates/uv/src/commands/tool/run.rs | 61 +++++++++++++++++-- crates/uv/tests/it/edit.rs | 10 +-- 3 files changed, 111 insertions(+), 48 deletions(-) diff --git a/crates/uv/src/commands/diagnostics.rs b/crates/uv/src/commands/diagnostics.rs index 93ea0cf15..5ddff0737 100644 --- a/crates/uv/src/commands/diagnostics.rs +++ b/crates/uv/src/commands/diagnostics.rs @@ -77,8 +77,6 @@ impl OperationDiagnostic { if let Some(context) = self.context { no_solution_context(&err, context); } else if let Some(hint) = self.hint { - // TODO(charlie): The `hint` should be shown on all diagnostics, not just - // `NoSolutionError`. no_solution_hint(err, hint); } else { no_solution(&err); @@ -91,11 +89,17 @@ impl OperationDiagnostic { chain, err, )) => { - requested_dist_error(kind, dist, &chain, err); + requested_dist_error(kind, dist, &chain, err, self.hint); None } pip::operations::Error::Requirements(uv_requirements::Error::Dist(kind, dist, err)) => { - dist_error(kind, dist, &DerivationChain::default(), Arc::new(err)); + dist_error( + kind, + dist, + &DerivationChain::default(), + Arc::new(err), + self.hint, + ); None } pip::operations::Error::Prepare(uv_installer::PrepareError::Dist( @@ -104,7 +108,7 @@ impl OperationDiagnostic { chain, err, )) => { - dist_error(kind, dist, &chain, Arc::new(err)); + dist_error(kind, dist, &chain, Arc::new(err), self.hint); None } pip::operations::Error::Requirements(err) => { @@ -134,6 +138,7 @@ pub(crate) fn dist_error( dist: Box, chain: &DerivationChain, cause: Arc, + help: Option, ) { #[derive(Debug, miette::Diagnostic, thiserror::Error)] #[error("{kind} `{dist}`")] @@ -147,23 +152,25 @@ pub(crate) fn dist_error( help: Option, } - let help = SUGGESTIONS - .get(dist.name()) - .map(|suggestion| { - format!( - "`{}` is often confused for `{}` Did you mean to install `{}` instead?", - dist.name().cyan(), - suggestion.cyan(), - suggestion.cyan(), - ) - }) - .or_else(|| { - if chain.is_empty() { - None - } else { - Some(format_chain(dist.name(), dist.version(), chain)) - } - }); + let help = help.or_else(|| { + SUGGESTIONS + .get(dist.name()) + .map(|suggestion| { + format!( + "`{}` is often confused for `{}` Did you mean to install `{}` instead?", + dist.name().cyan(), + suggestion.cyan(), + suggestion.cyan(), + ) + }) + .or_else(|| { + if chain.is_empty() { + None + } else { + Some(format_chain(dist.name(), dist.version(), chain)) + } + }) + }); let report = miette::Report::new(Diagnostic { kind, dist, @@ -179,6 +186,7 @@ pub(crate) fn requested_dist_error( dist: Box, chain: &DerivationChain, cause: Arc, + help: Option, ) { #[derive(Debug, miette::Diagnostic, thiserror::Error)] #[error("{kind} `{dist}`")] @@ -192,23 +200,25 @@ pub(crate) fn requested_dist_error( help: Option, } - let help = SUGGESTIONS - .get(dist.name()) - .map(|suggestion| { - format!( - "`{}` is often confused for `{}` Did you mean to install `{}` instead?", - dist.name().cyan(), - suggestion.cyan(), - suggestion.cyan(), - ) - }) - .or_else(|| { - if chain.is_empty() { - None - } else { - Some(format_chain(dist.name(), dist.version(), chain)) - } - }); + let help = help.or_else(|| { + SUGGESTIONS + .get(dist.name()) + .map(|suggestion| { + format!( + "`{}` is often confused for `{}` Did you mean to install `{}` instead?", + dist.name().cyan(), + suggestion.cyan(), + suggestion.cyan(), + ) + }) + .or_else(|| { + if chain.is_empty() { + None + } else { + Some(format_chain(dist.name(), dist.version(), chain)) + } + }) + }); let report = miette::Report::new(Diagnostic { kind, dist, diff --git a/crates/uv/src/commands/tool/run.rs b/crates/uv/src/commands/tool/run.rs index 573a1f7c3..3e36e2461 100644 --- a/crates/uv/src/commands/tool/run.rs +++ b/crates/uv/src/commands/tool/run.rs @@ -6,6 +6,7 @@ use std::str::FromStr; use anstream::eprint; use anyhow::{bail, Context}; +use console::Term; use itertools::Itertools; use owo_colors::OwoColorize; use tokio::process::Command; @@ -17,7 +18,8 @@ use uv_cli::ExternalCommand; use uv_client::BaseClientBuilder; use uv_configuration::{Concurrency, PreviewMode}; use uv_distribution_types::{ - Name, NameRequirementSpecification, UnresolvedRequirement, UnresolvedRequirementSpecification, + IndexUrl, Name, NameRequirementSpecification, UnresolvedRequirement, + UnresolvedRequirementSpecification, }; use uv_fs::Simplified; use uv_installer::{SatisfiesResult, SitePackages}; @@ -124,11 +126,13 @@ pub(crate) async fn run( "hint".bold().cyan(), ":".bold(), package_name.cyan(), - format!("{} --from {} {}", invocation_source, package_name.cyan(), target), + format!("{} --from {} {}", invocation_source, package_name.cyan(), target).green(), )); } } else { let target_path = Path::new(target); + + // If the user tries to invoke `uvx script.py`, hint them towards `uv run`. if has_python_script_ext(target_path) { return if target_path.try_exists()? { Err(anyhow::anyhow!( @@ -147,12 +151,46 @@ pub(crate) async fn run( "hint".bold().cyan(), ":".bold(), package_name.cyan(), - format!("{} --from {} {}", invocation_source, package_name, target), + format!("{invocation_source} --from {package_name} {target}").green(), )) }; } } + // If the user tries to invoke `uvx run ruff`, hint them towards `uvx ruff`, but only if + // the `run` package is guaranteed to come from PyPI. + let (mut target, mut args) = (target, args); + if from.is_none() + && invocation_source == ToolRunCommand::Uvx + && target == "run" + && settings + .index_locations + .indexes() + .all(|index| matches!(index.url, IndexUrl::Pypi(..))) + { + let term = Term::stderr(); + if term.is_term() { + let rest = args.iter().map(|s| s.to_string_lossy()).join(" "); + let prompt = format!( + "`{}` invokes the `{}` package. Did you mean `{}`?", + format!("uvx run {rest}").green(), + "run".cyan(), + format!("uvx {rest}").green() + ); + let confirmation = uv_console::confirm(&prompt, &term, true)?; + if confirmation { + let Some((next_target, next_args)) = args.split_first() else { + return Err(anyhow::anyhow!("No tool command provided")); + }; + let Some(next_target) = next_target.to_str() else { + return Err(anyhow::anyhow!("Tool command could not be parsed as UTF-8 string. Use `--from` to specify the package name")); + }; + target = next_target; + args = next_args; + } + } + } + let request = ToolRequest::parse(target, from.as_deref()); // If the user passed, e.g., `ruff@latest`, refresh the cache. @@ -188,10 +226,25 @@ pub(crate) async fn run( let (from, environment) = match result { Ok(resolution) => resolution, Err(ProjectError::Operation(err)) => { + // If the user ran `uvx run ...`, the `run` is likely a mistake. Show a dedicated hint. + if from.is_none() && invocation_source == ToolRunCommand::Uvx && target == "run" { + let rest = args.iter().map(|s| s.to_string_lossy()).join(" "); + return diagnostics::OperationDiagnostic::native_tls(network_settings.native_tls) + .with_hint(format!( + "`{}` invokes the `{}` package. Did you mean `{}`?", + format!("uvx run {rest}").green(), + "run".cyan(), + format!("uvx {rest}").green() + )) + .with_context("tool") + .report(err) + .map_or(Ok(ExitStatus::Failure), |err| Err(err.into())); + } + return diagnostics::OperationDiagnostic::native_tls(network_settings.native_tls) .with_context("tool") .report(err) - .map_or(Ok(ExitStatus::Failure), |err| Err(err.into())) + .map_or(Ok(ExitStatus::Failure), |err| Err(err.into())); } Err(ProjectError::Requirements(err)) => { let err = miette::Report::msg(format!("{err}")) diff --git a/crates/uv/tests/it/edit.rs b/crates/uv/tests/it/edit.rs index 0eafe0192..8d7d01bbf 100644 --- a/crates/uv/tests/it/edit.rs +++ b/crates/uv/tests/it/edit.rs @@ -6646,7 +6646,7 @@ fn fail_to_add_revert_project() -> Result<()> { ZeroDivisionError: division by zero hint: This usually indicates a problem with the package or the build environment. - help: `child` was included because `parent` (v0.1.0) depends on `child` + help: If you want to add the package regardless of the failed resolution, provide the `--frozen` flag to skip locking and syncing. "###); let pyproject_toml = fs_err::read_to_string(context.temp_dir.join("pyproject.toml"))?; @@ -6757,7 +6757,7 @@ fn fail_to_edit_revert_project() -> Result<()> { ZeroDivisionError: division by zero hint: This usually indicates a problem with the package or the build environment. - help: `child` was included because `parent` (v0.1.0) depends on `child` + help: If you want to add the package regardless of the failed resolution, provide the `--frozen` flag to skip locking and syncing. "###); let pyproject_toml = fs_err::read_to_string(context.temp_dir.join("pyproject.toml"))?; @@ -10135,7 +10135,7 @@ fn add_with_build_constraints() -> Result<()> { build-constraint-dependencies = ["setuptools==1"] "#})?; - uv_snapshot!(context.filters(), context.add().arg("requests==1.2"), @r" + uv_snapshot!(context.filters(), context.add().arg("requests==1.2"), @r###" success: false exit_code: 1 ----- stdout ----- @@ -10145,8 +10145,8 @@ fn add_with_build_constraints() -> Result<()> { ├─▶ Failed to resolve requirements from `setup.py` build ├─▶ No solution found when resolving: `setuptools>=40.8.0` ╰─▶ Because you require setuptools>=40.8.0 and setuptools==1, we can conclude that your requirements are unsatisfiable. - help: `requests` (v1.2.0) was included because `project` (v0.1.0) depends on `requests==1.2` - "); + help: If you want to add the package regardless of the failed resolution, provide the `--frozen` flag to skip locking and syncing. + "###); let pyproject_toml = context.temp_dir.child("pyproject.toml"); pyproject_toml.write_str(indoc! {r#"