Improve handling of command arguments in uv run and uv tool run (#…

…4404)

Closes #4390

We no longer require `--` to disambiguate child command options that
overlap with uv options.

crates/uv/src/cli.rs

@@ -1,4 +1,5 @@
 use std::ffi::OsString;
+use std::ops::Deref;
 use std::path::PathBuf;
 use std::str::FromStr;
 
@@ -1457,6 +1458,31 @@ pub(crate) struct VenvArgs {
     pub(crate) compat_args: compat::VenvCompatArgs,
 }
 
+#[derive(Parser, Debug, Clone)]
+pub(crate) enum ExternalCommand {
+    #[command(external_subcommand)]
+    Cmd(Vec<OsString>),
+}
+
+impl Deref for ExternalCommand {
+    type Target = Vec<OsString>;
+
+    fn deref(&self) -> &Self::Target {
+        match self {
+            Self::Cmd(cmd) => cmd,
+        }
+    }
+}
+
+impl ExternalCommand {
+    pub(crate) fn split(&self) -> (Option<&OsString>, &[OsString]) {
+        match self.as_slice() {
+            [] => (None, &[]),
+            [cmd, args @ ..] => (Some(cmd), args),
+        }
+    }
+}
+
 #[derive(Args)]
 #[allow(clippy::struct_excessive_bools)]
 pub(crate) struct RunArgs {
@@ -1482,11 +1508,8 @@ pub(crate) struct RunArgs {
     pub(crate) no_dev: bool,
 
     /// The command to run.
-    pub(crate) target: Option<String>,
-
-    /// The arguments to the command.
-    #[arg(allow_hyphen_values = true)]
-    pub(crate) args: Vec<OsString>,
+    #[command(subcommand)]
+    pub(crate) command: ExternalCommand,
 
     /// Run with the given packages installed.
     #[arg(long)]
@@ -1684,11 +1707,8 @@ pub(crate) enum ToolCommand {
 #[allow(clippy::struct_excessive_bools)]
 pub(crate) struct ToolRunArgs {
     /// The command to run.
-    pub(crate) target: String,
-
-    /// The arguments to the command.
-    #[arg(allow_hyphen_values = true)]
-    pub(crate) args: Vec<OsString>,
+    #[command(subcommand)]
+    pub(crate) command: ExternalCommand,
 
     /// Use the given package to provide the command.
     ///

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

@@ -15,6 +15,7 @@ use uv_requirements::RequirementsSource;
 use uv_toolchain::{PythonEnvironment, SystemPython, Toolchain, ToolchainRequest};
 use uv_warnings::warn_user;
 
+use crate::cli::ExternalCommand;
 use crate::commands::pip::operations::Modifications;
 use crate::commands::{project, ExitStatus};
 use crate::printer::Printer;
@@ -25,8 +26,7 @@ use crate::settings::ResolverInstallerSettings;
 pub(crate) async fn run(
     extras: ExtrasSpecification,
     dev: bool,
-    target: Option<String>,
-    mut args: Vec<OsString>,
+    command: ExternalCommand,
     requirements: Vec<RequirementsSource>,
     python: Option<String>,
     package: Option<PackageName>,
@@ -179,25 +179,25 @@ pub(crate) async fn run(
         )
     };
 
-    // Construct the command
-    let command = if let Some(target) = target {
+    let (target, args) = command.split();
+    let (command, prefix_args) = if let Some(target) = target {
         let target_path = PathBuf::from(&target);
         if target_path
             .extension()
             .map_or(false, |ext| ext.eq_ignore_ascii_case("py"))
             && target_path.exists()
         {
-            args.insert(0, target_path.as_os_str().into());
-            "python".to_string()
+            (OsString::from("python"), vec![target_path])
         } else {
-            target
+            (target.clone(), vec![])
         }
     } else {
-        "python".to_string()
+        (OsString::from("python"), vec![])
     };
 
     let mut process = Command::new(&command);
-    process.args(&args);
+    process.args(prefix_args);
+    process.args(args);
 
     // Construct the `PATH` environment variable.
     let new_path = std::env::join_paths(
@@ -250,12 +250,13 @@ pub(crate) async fn run(
     // TODO(zanieb): Throw a nicer error message if the command is not found
     let space = if args.is_empty() { "" } else { " " };
     debug!(
-        "Running `{command}{space}{}`",
+        "Running `{}{space}{}`",
+        command.to_string_lossy(),
         args.iter().map(|arg| arg.to_string_lossy()).join(" ")
     );
     let mut handle = process
         .spawn()
-        .with_context(|| format!("Failed to spawn: `{command}`"))?;
+        .with_context(|| format!("Failed to spawn: `{}`", command.to_string_lossy()))?;
     let status = handle.wait().await.context("Child process disappeared")?;
 
     // Exit based on the result of the command

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

@@ -1,4 +1,3 @@
-use std::ffi::OsString;
 use std::path::PathBuf;
 
 use anyhow::{Context, Result};
@@ -13,6 +12,7 @@ use uv_requirements::RequirementsSource;
 use uv_toolchain::{PythonEnvironment, SystemPython, Toolchain, ToolchainRequest};
 use uv_warnings::warn_user;
 
+use crate::cli::ExternalCommand;
 use crate::commands::project::update_environment;
 use crate::commands::ExitStatus;
 use crate::printer::Printer;
@@ -21,8 +21,7 @@ use crate::settings::ResolverInstallerSettings;
 /// Run a command.
 #[allow(clippy::too_many_arguments)]
 pub(crate) async fn run(
-    target: String,
-    args: Vec<OsString>,
+    command: ExternalCommand,
     python: Option<String>,
     from: Option<String>,
     with: Vec<String>,
@@ -39,12 +38,24 @@ pub(crate) async fn run(
         warn_user!("`uv tool run` is experimental and may change without warning.");
     }
 
-    let requirements = [RequirementsSource::from_package(
-        from.unwrap_or_else(|| target.clone()),
-    )]
-    .into_iter()
-    .chain(with.into_iter().map(RequirementsSource::from_package))
-    .collect::<Vec<_>>();
+    let (target, args) = command.split();
+    let Some(target) = target else {
+        return Err(anyhow::anyhow!("No tool command provided"));
+    };
+
+    let from = if let Some(from) = from {
+        from
+    } else {
+        let Some(target) = 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.to_string()
+    };
+
+    let requirements = [RequirementsSource::from_package(from)]
+        .into_iter()
+        .chain(with.into_iter().map(RequirementsSource::from_package))
+        .collect::<Vec<_>>();
 
     // TODO(zanieb): When implementing project-level tools, discover the project and check if it has the tool.
     // TODO(zanieb): Determine if we should layer on top of the project environment if it is present.
@@ -92,8 +103,8 @@ pub(crate) async fn run(
     let command = target;
 
     // Construct the command
-    let mut process = Command::new(&command);
-    process.args(&args);
+    let mut process = Command::new(command);
+    process.args(args);
 
     // Construct the `PATH` environment variable.
     let new_path = std::env::join_paths(
@@ -133,12 +144,13 @@ pub(crate) async fn run(
     // TODO(zanieb): Throw a nicer error message if the command is not found
     let space = if args.is_empty() { "" } else { " " };
     debug!(
-        "Running `{command}{space}{}`",
+        "Running `{}{space}{}`",
+        command.to_string_lossy(),
         args.iter().map(|arg| arg.to_string_lossy()).join(" ")
     );
     let mut handle = process
         .spawn()
-        .with_context(|| format!("Failed to spawn: `{command}`"))?;
+        .with_context(|| format!("Failed to spawn: `{}`", command.to_string_lossy()))?;
     let status = handle.wait().await.context("Child process disappeared")?;
 
     // Exit based on the result of the command

crates/uv/src/main.rs

@@ -619,8 +619,7 @@ async fn run() -> Result<ExitStatus> {
             commands::run(
                 args.extras,
                 args.dev,
-                args.target,
-                args.args,
+                args.command,
                 requirements,
                 args.python,
                 args.package,
@@ -745,8 +744,7 @@ async fn run() -> Result<ExitStatus> {
             let cache = cache.init()?.with_refresh(args.refresh);
 
             commands::run_tool(
-                args.target,
-                args.args,
+                args.command,
                 args.python,
                 args.from,
                 args.with,

crates/uv/src/settings.rs

@@ -1,5 +1,4 @@
 use std::env::VarError;
-use std::ffi::OsString;
 use std::num::NonZeroUsize;
 use std::path::PathBuf;
 use std::process;
@@ -26,11 +25,11 @@ use uv_settings::{
 use uv_toolchain::{Prefix, PythonVersion, Target};
 
 use crate::cli::{
-    AddArgs, BuildArgs, ColorChoice, GlobalArgs, IndexArgs, InstallerArgs, LockArgs, Maybe,
-    PipCheckArgs, PipCompileArgs, PipFreezeArgs, PipInstallArgs, PipListArgs, PipShowArgs,
-    PipSyncArgs, PipUninstallArgs, RefreshArgs, RemoveArgs, ResolverArgs, ResolverInstallerArgs,
-    RunArgs, SyncArgs, ToolRunArgs, ToolchainFindArgs, ToolchainInstallArgs, ToolchainListArgs,
-    VenvArgs,
+    AddArgs, BuildArgs, ColorChoice, ExternalCommand, GlobalArgs, IndexArgs, InstallerArgs,
+    LockArgs, Maybe, PipCheckArgs, PipCompileArgs, PipFreezeArgs, PipInstallArgs, PipListArgs,
+    PipShowArgs, PipSyncArgs, PipUninstallArgs, RefreshArgs, RemoveArgs, ResolverArgs,
+    ResolverInstallerArgs, RunArgs, SyncArgs, ToolRunArgs, ToolchainFindArgs, ToolchainInstallArgs,
+    ToolchainListArgs, VenvArgs,
 };
 use crate::commands::pip::operations::Modifications;
 use crate::commands::ListFormat;
@@ -123,8 +122,7 @@ impl CacheSettings {
 pub(crate) struct RunSettings {
     pub(crate) extras: ExtrasSpecification,
     pub(crate) dev: bool,
-    pub(crate) target: Option<String>,
-    pub(crate) args: Vec<OsString>,
+    pub(crate) command: ExternalCommand,
     pub(crate) with: Vec<String>,
     pub(crate) python: Option<String>,
     pub(crate) package: Option<PackageName>,
@@ -142,8 +140,7 @@ impl RunSettings {
             no_all_extras,
             dev,
             no_dev,
-            target,
-            args,
+            command,
             with,
             installer,
             build,
@@ -158,8 +155,7 @@ impl RunSettings {
                 extra.unwrap_or_default(),
             ),
             dev: flag(dev, no_dev).unwrap_or(true),
-            target,
-            args,
+            command,
             with,
             python,
             package,
@@ -176,8 +172,7 @@ impl RunSettings {
 #[allow(clippy::struct_excessive_bools)]
 #[derive(Debug, Clone)]
 pub(crate) struct ToolRunSettings {
-    pub(crate) target: String,
-    pub(crate) args: Vec<OsString>,
+    pub(crate) command: ExternalCommand,
     pub(crate) from: Option<String>,
     pub(crate) with: Vec<String>,
     pub(crate) python: Option<String>,
@@ -190,8 +185,7 @@ impl ToolRunSettings {
     #[allow(clippy::needless_pass_by_value)]
     pub(crate) fn resolve(args: ToolRunArgs, filesystem: Option<FilesystemOptions>) -> Self {
         let ToolRunArgs {
-            target,
-            args,
+            command,
             from,
             with,
             installer,
@@ -201,8 +195,7 @@ impl ToolRunSettings {
         } = args;
 
         Self {
-            target,
-            args,
+            command,
             from,
             with,
             python,

crates/uv/tests/common/mod.rs

@@ -44,7 +44,7 @@ pub const INSTA_FILTERS: &[(&str, &str)] = &[
     (r"uv.exe", "uv"),
     // uv version display
     (
-        r"uv \d+\.\d+\.\d+( \(.*\))?",
+        r"uv(-.*)? \d+\.\d+\.\d+( \(.*\))?",
         r"uv [VERSION] ([COMMIT] DATE)",
     ),
     // The exact message is host language dependent
@@ -417,6 +417,41 @@ impl TestContext {
         command
     }
 
+    /// Create a `uv tool run` command with options shared across scenarios.
+    pub fn tool_run(&self) -> std::process::Command {
+        let mut command = self.tool_run_without_exclude_newer();
+        command.arg("--exclude-newer").arg(EXCLUDE_NEWER);
+        command
+    }
+
+    /// Create a `uv tool run` command with no `--exclude-newer` option.
+    ///
+    /// One should avoid using this in tests to the extent possible because
+    /// it can result in tests failing when the index state changes. Therefore,
+    /// if you use this, there should be some other kind of mitigation in place.
+    /// For example, pinning package versions.
+    pub fn tool_run_without_exclude_newer(&self) -> std::process::Command {
+        let mut command = std::process::Command::new(get_bin());
+        command
+            .arg("tool")
+            .arg("run")
+            .arg("--cache-dir")
+            .arg(self.cache_dir.path())
+            .env("VIRTUAL_ENV", self.venv.as_os_str())
+            .env("UV_NO_WRAP", "1")
+            .env("UV_TOOLCHAIN_DIR", "")
+            .env("UV_TEST_PYTHON_PATH", &self.python_path())
+            .current_dir(&self.temp_dir);
+
+        if cfg!(all(windows, debug_assertions)) {
+            // TODO(konstin): Reduce stack usage in debug mode enough that the tests pass with the
+            // default windows stack of 1MB
+            command.env("UV_STACK_SIZE", (4 * 1024 * 1024).to_string());
+        }
+
+        command
+    }
+
     /// Create a `uv add` command for the given requirements.
     pub fn add(&self, reqs: &[&str]) -> std::process::Command {
         let mut command = std::process::Command::new(get_bin());