mirrors/uv
1
0
Fork 0
mirror of https://github.com/astral-sh/uv.git synced 2025-07-07 21:35:00 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

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

Browse source 
Closes https://github.com/astral-sh/uv/issues/4390

We no longer require `--` to disambiguate child command options that
overlap with uv options.
This commit is contained in:
Zanie Blue 2024-06-19 10:55:21 -04:00 • committed by GitHub
parent 9a3b8511f1
commit 39da3917e5
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 226 additions and 57 deletions
Download patch file Download diff file Expand all files Collapse all files

40
crates/uv/src/cli.rs
View file

@ -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.
///

23
crates/uv/src/commands/project/run.rs
View file

@ -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

38
crates/uv/src/commands/tool/run.rs
View file

@ -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

6
crates/uv/src/main.rs
View file

@ -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,

29
crates/uv/src/settings.rs
View file

@ -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,

37
crates/uv/tests/common/mod.rs
View file

@ -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());

55
crates/uv/tests/run.rs
View file

@ -116,3 +116,58 @@ fn run_with_python_version() -> Result<()> {
Ok(())
}
#[test]
fn run_args() -> Result<()> {
let context = TestContext::new("3.12");
let pyproject_toml = context.temp_dir.child("pyproject.toml");
pyproject_toml.write_str(indoc! { r#"
[project]
name = "foo"
version = "1.0.0"
requires-python = ">=3.8"
dependencies = []
"#
})?;
// We treat arguments before the command as uv arguments
uv_snapshot!(context.filters(), context.run().arg("--version").arg("python"), @r###"
success: true
exit_code: 0
----- stdout -----
uv [VERSION] ([COMMIT] DATE)
----- stderr -----
"###);
// We don't treat arguments after the command as uv arguments
uv_snapshot!(context.filters(), context.run().arg("python").arg("--version"), @r###"
success: true
exit_code: 0
----- stdout -----
Python 3.12.[X]
----- stderr -----
warning: `uv run` is experimental and may change without warning.
Resolved 1 package in [TIME]
Prepared 1 package in [TIME]
Installed 1 package in [TIME]
+ foo==1.0.0 (from file://[TEMP_DIR]/)
"###);
// Can use `--` to separate uv arguments from the command arguments.
uv_snapshot!(context.filters(), context.run().arg("--").arg("python").arg("--version"), @r###"
success: true
exit_code: 0
----- stdout -----
Python 3.12.[X]
----- stderr -----
warning: `uv run` is experimental and may change without warning.
Resolved 1 package in [TIME]
Audited 1 package in [TIME]
"###);
Ok(())
}

55
crates/uv/tests/tool_run.rs Normal file
View file

@ -0,0 +1,55 @@
#![cfg(all(feature = "python", feature = "pypi"))]
use common::{uv_snapshot, TestContext};
mod common;
#[test]
fn tool_run_args() {
let context = TestContext::new("3.12");
// We treat arguments before the command as uv arguments
uv_snapshot!(context.filters(), context.tool_run().arg("--version").arg("pytest"), @r###"
success: true
exit_code: 0
----- stdout -----
uv [VERSION] ([COMMIT] DATE)
----- stderr -----
"###);
// We don't treat arguments after the command as uv arguments
uv_snapshot!(context.filters(), context.tool_run().arg("pytest").arg("--version"), @r###"
success: true
exit_code: 0
----- stdout -----
pytest 8.1.1
----- stderr -----
warning: `uv tool run` is experimental and may change without warning.
Resolved 4 packages in [TIME]
Prepared 4 packages in [TIME]
Installed 4 packages in [TIME]
+ iniconfig==2.0.0
+ packaging==24.0
+ pluggy==1.4.0
+ pytest==8.1.1
"###);
// Can use `--` to separate uv arguments from the command arguments.
uv_snapshot!(context.filters(), context.tool_run().arg("--").arg("pytest").arg("--version"), @r###"
success: true
exit_code: 0
----- stdout -----
pytest 8.1.1
----- stderr -----
warning: `uv tool run` is experimental and may change without warning.
Resolved 4 packages in [TIME]
Installed 4 packages in [TIME]
+ iniconfig==2.0.0
+ packaging==24.0
+ pluggy==1.4.0
+ pytest==8.1.1
"###);
}