Skip to content

Warn user on uvx run command #11992

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.

Sign up for GitHub

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

Merged
charliermarsh merged 1 commit into from
Mar 6, 2025
Merged

Warn user on uvx run command #11992

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
88 changes: 49 additions & 39 deletions crates/uv/src/commands/diagnostics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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(
Expand All @@ -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) => {
Expand Down Expand Up @@ -134,6 +138,7 @@ pub(crate) fn dist_error(
dist: Box<Dist>,
chain: &DerivationChain,
cause: Arc<uv_distribution::Error>,
help: Option<String>,
) {
#[derive(Debug, miette::Diagnostic, thiserror::Error)]
#[error("{kind} `{dist}`")]
Expand All @@ -147,23 +152,25 @@ pub(crate) fn dist_error(
help: Option<String>,
}

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,
Expand All @@ -179,6 +186,7 @@ pub(crate) fn requested_dist_error(
dist: Box<RequestedDist>,
chain: &DerivationChain,
cause: Arc<uv_distribution::Error>,
help: Option<String>,
) {
#[derive(Debug, miette::Diagnostic, thiserror::Error)]
#[error("{kind} `{dist}`")]
Expand All @@ -192,23 +200,25 @@ pub(crate) fn requested_dist_error(
help: Option<String>,
}

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,
Expand Down
61 changes: 57 additions & 4 deletions crates/uv/src/commands/tool/run.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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};
Expand Down Expand Up @@ -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!(
Expand All @@ -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.
Expand Down Expand Up @@ -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}"))
Expand Down
10 changes: 5 additions & 5 deletions crates/uv/tests/it/edit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"))?;
Expand Down Expand Up @@ -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"))?;
Expand Down Expand Up @@ -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 -----
Expand All @@ -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#"
Expand Down
Loading