Return error when running uvx with a .py script by jtfmumm · Pull Request #11623 · astral-sh/uv

jtfmumm · 2025-02-19T15:32:02Z

If we see uvx script.py, we exit early, giving a hint to use uv run script.py if the script exists. If it does not exist, we suggest running uv run with a normalized package name.

This PR includes a snapshot test for each of these scenarios.

An alternative approach would be to wait until we encounter an error, and then add the hint. But if there happens to be a malicious package called script-py, this would be run unintentionally (a point raised by @zanieb).

Closes #10784

crates/uv/src/commands/tool/run.rs

zanieb · 2025-02-19T15:42:12Z

crates/uv/src/commands/tool/run.rs

+        .extension()
+        .is_some_and(|ext| ext.eq_ignore_ascii_case("py"))
+    {
+        if possible_path.try_exists()? {


Should we error if we can't read the path? (earnestly not sure here)

It's a good question. We could treat that as the "doesn't exist case" if we'd prefer.

crates/uv/src/commands/tool/run.rs

zanieb · 2025-02-20T20:26:16Z

This looks good to me, thanks for addressing the feedback. I want to do one final pass on the messaging, since this is ostensibly breaking (though I really hope nobody is relying on this behavior). I'll do that on Monday.

zanieb

Sorry again about the delay here. I pushed a commit (41bbf8c) with some tweaks and left some notes via review for context.

My focus here was being a bit more verbose and consistent with the error messages, but I made some nit-changes at the same time.

zanieb · 2025-03-03T19:14:19Z

crates/uv/src/commands/tool/run.rs

        return Err(anyhow::anyhow!("Tool command could not be parsed as UTF-8 string. Use `--from` to specify the package name."));
    };

+    if let Some(ref f) = from {


We try to avoid single letter variables names

zanieb · 2025-03-03T19:14:54Z

crates/uv/src/commands/tool/run.rs

+                "It looks you have passed a script instead of a package into `--from`. \n\n{}{} Did you mean to run a tool with `{}`?",
+                    "hint".bold().cyan(),
+                    ":".bold(),
+                    format!("{} --from {} {}", invocation_source, PackageName::new(possible_script.to_string_lossy().to_string())?.cyan(), target),


I think we can probably just the original from string instead of going from String -> Path -> String.

zanieb · 2025-03-03T19:16:21Z

crates/uv/src/commands/tool/run.rs

+            return if possible_script.try_exists()? {
+                Err(anyhow::anyhow!(
+                    "It looks like you are trying to run a script. Did you mean `{}`?",
+                    format!("uv run {}", possible_script.display()).cyan()


Did you mean to display the absolute path here? user_display is generally preferable, which handles display of paths in the working directory.

zanieb · 2025-03-03T19:17:31Z

crates/uv/tests/it/tool_run.rs

+    let context = TestContext::new("3.12").with_filtered_counts();
+    context.temp_dir.child("script.py").touch()?;
+
+    // We treat arguments before the command as uv arguments


This looks superfluous

zanieb · 2025-03-03T19:22:30Z

crates/uv/src/commands/tool/run.rs

+                    "hint".bold().cyan(),
+                    ":".bold(),
+                    format!("{} --from {} {}", invocation_source, PackageName::new(possible_script.to_string_lossy().to_string())?.cyan(), target),


Minor note, rustfmt chokes on macros so you need to format those manually sometimes.

zanieb · 2025-03-03T19:51:07Z

If you think any of the error messages are worse for some reason please let me know so we can align on the best message

jtfmumm · 2025-03-04T13:14:58Z

If you think any of the error messages are worse for some reason please let me know so we can align on the best message

Your changes look good to me.

This MR contains the following updates: | Package | Update | Change | |---|---|---| | [astral-sh/uv](https://github.com/astral-sh/uv) | patch | `0.6.4` -> `0.6.5` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>astral-sh/uv (astral-sh/uv)</summary> ### [`v0.6.5`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#065) [Compare Source](astral-sh/uv@0.6.4...0.6.5) ##### Enhancements - Allow `--constraints` and `--overrides` in `uvx` ([#10207](astral-sh/uv#10207)) - Allow overrides in `satisfies` check for `uv tool run` ([#11994](astral-sh/uv#11994)) - Allow users to set `package = true` on `tool.uv.sources` ([#12014](astral-sh/uv#12014)) - Add support for Windows legacy scripts via `uv run` ([#11888](astral-sh/uv#11888)) - Return error when running uvx with a `.py` script ([#11623](astral-sh/uv#11623)) - Warn user on use of `uvx run` ([#11992](astral-sh/uv#11992)) ##### Configuration - Add `NO_BUILD` and `NO_BUILD_PACKAGE` environment variables ([#11968](astral-sh/uv#11968)) ##### Performance - Allow overrides in all satisfies checks ([#11995](astral-sh/uv#11995)) - Respect markers on constraints when validating current environment ([#11976](astral-sh/uv#11976)) ##### Bug fixes - Compare major-minor specifiers when filtering interpreters ([#11952](astral-sh/uv#11952)) - Fix system site packages detection default ([#11956](astral-sh/uv#11956)) - Invalidate lockfile when empty dependency groups are added or removed ([#12010](astral-sh/uv#12010)) - Remove prepended sys.path ([#11954](astral-sh/uv#11954)) - Fix PyPy Python version label ([#11965](astral-sh/uv#11965)) - Fix error message suggesting `--user` instead of `--username` ([#11947](astral-sh/uv#11947)) ##### Preview - Move the uv build backend into a separate, minimal `uv_build` package ([#11446](astral-sh/uv#11446)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

Return error when running uvx with a .py script

7a5208f

jtfmumm force-pushed the jtfm/uvx-run-hint branch from d477bb2 to 7a5208f Compare February 19, 2025 15:35

zanieb reviewed Feb 19, 2025

View reviewed changes