From 3e04fdb8ae7be7e99b2bb683454ed580c5da6d44 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 20 Feb 2025 12:13:09 -0800 Subject: [PATCH] Allow quotes around command-line options in `requirement.txt files` (#11644) ## Summary Closes #11592. --- Cargo.lock | 11 +- Cargo.toml | 1 + crates/uv-requirements-txt/Cargo.toml | 1 + crates/uv-requirements-txt/src/lib.rs | 36 +++++-- crates/uv/tests/it/pip_install.rs | 140 ++++++++++++++++++++++++++ 5 files changed, 178 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ff094d6e8..1f634d5ae 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -697,7 +697,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "117725a109d387c937a1533ce01b450cbde6b88abceea8473c4d7a85853cda3c" dependencies = [ "lazy_static", - "windows-sys 0.48.0", + "windows-sys 0.59.0", ] [[package]] @@ -2854,6 +2854,12 @@ version = "0.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "640c9bd8497b02465aeef5375144c26062e0dcd5939dfcbb0f5db76cb8c17c73" +[[package]] +name = "r-shquote" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d76b7a332c240b0b30ad8b52cc9aecf8ec96878ccb927ce1d2feb03920e0f711" + [[package]] name = "rancor" version = "0.1.0" @@ -5541,6 +5547,7 @@ dependencies = [ "indoc", "insta", "itertools 0.14.0", + "r-shquote", "regex", "reqwest", "reqwest-middleware", @@ -6072,7 +6079,7 @@ version = "0.1.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cf221c93e13a30d793f7645a0e7762c55d169dbb0a49671918a2319d289b10bb" dependencies = [ - "windows-sys 0.48.0", + "windows-sys 0.59.0", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index fbdaf7908..aa524ceab 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -135,6 +135,7 @@ proc-macro2 = { version = "1.0.86" } procfs = { version = "0.17.0", default-features = false, features = ["flate2"] } pubgrub = { git = "https://github.com/astral-sh/pubgrub", rev = "b70cf707aa43f21b32f3a61b8a0889b15032d5c4" } quote = { version = "1.0.37" } +r-shquote = { version = "0.1.1" } rayon = { version = "1.10.0" } reflink-copy = { version = "0.1.19" } regex = { version = "1.10.6" } diff --git a/crates/uv-requirements-txt/Cargo.toml b/crates/uv-requirements-txt/Cargo.toml index 7dcc2964e..0f7a4f0fa 100644 --- a/crates/uv-requirements-txt/Cargo.toml +++ b/crates/uv-requirements-txt/Cargo.toml @@ -26,6 +26,7 @@ uv-pypi-types = { workspace = true } uv-warnings = { workspace = true } fs-err = { workspace = true } +r-shquote = { workspace = true } regex = { workspace = true } reqwest = { workspace = true, optional = true } reqwest-middleware = { workspace = true, optional = true } diff --git a/crates/uv-requirements-txt/src/lib.rs b/crates/uv-requirements-txt/src/lib.rs index a1e171c71..8953b82ab 100644 --- a/crates/uv-requirements-txt/src/lib.rs +++ b/crates/uv-requirements-txt/src/lib.rs @@ -40,6 +40,7 @@ use std::io; use std::path::{Path, PathBuf}; use std::str::FromStr; +use r_shquote::unquote; use tracing::instrument; use unscanny::{Pattern, Scanner}; use url::Url; @@ -520,18 +521,20 @@ fn parse_entry( let start = s.cursor(); Ok(Some(if s.eat_if("-r") || s.eat_if("--requirement") { - let requirements_file = parse_value(content, s, |c: char| !is_terminal(c))?; + let filename = parse_value(content, s, |c: char| !is_terminal(c))?; + let filename = unquote(filename).unwrap_or_else(|_| filename.to_string()); let end = s.cursor(); RequirementsTxtStatement::Requirements { - filename: requirements_file.to_string(), + filename, start, end, } } else if s.eat_if("-c") || s.eat_if("--constraint") { - let constraints_file = parse_value(content, s, |c: char| !is_terminal(c))?; + let filename = parse_value(content, s, |c: char| !is_terminal(c))?; + let filename = unquote(filename).unwrap_or_else(|_| filename.to_string()); let end = s.cursor(); RequirementsTxtStatement::Constraint { - filename: constraints_file.to_string(), + filename, start, end, } @@ -572,7 +575,10 @@ fn parse_entry( }) } else if s.eat_if("-i") || s.eat_if("--index-url") { let given = parse_value(content, s, |c: char| !is_terminal(c))?; - let expanded = expand_env_vars(given); + let given = unquote(given) + .map(Cow::Owned) + .unwrap_or(Cow::Borrowed(given)); + let expanded = expand_env_vars(given.as_ref()); let url = if let Some(path) = std::path::absolute(expanded.as_ref()) .ok() .filter(|path| path.exists()) @@ -598,7 +604,10 @@ fn parse_entry( RequirementsTxtStatement::IndexUrl(url.with_given(given)) } else if s.eat_if("--extra-index-url") { let given = parse_value(content, s, |c: char| !is_terminal(c))?; - let expanded = expand_env_vars(given); + let given = unquote(given) + .map(Cow::Owned) + .unwrap_or(Cow::Borrowed(given)); + let expanded = expand_env_vars(given.as_ref()); let url = if let Some(path) = std::path::absolute(expanded.as_ref()) .ok() .filter(|path| path.exists()) @@ -626,7 +635,10 @@ fn parse_entry( RequirementsTxtStatement::NoIndex } else if s.eat_if("--find-links") || s.eat_if("-f") { let given = parse_value(content, s, |c: char| !is_terminal(c))?; - let expanded = expand_env_vars(given); + let given = unquote(given) + .map(Cow::Owned) + .unwrap_or(Cow::Borrowed(given)); + let expanded = expand_env_vars(given.as_ref()); let url = if let Some(path) = std::path::absolute(expanded.as_ref()) .ok() .filter(|path| path.exists()) @@ -652,7 +664,10 @@ fn parse_entry( RequirementsTxtStatement::FindLinks(url.with_given(given)) } else if s.eat_if("--no-binary") { let given = parse_value(content, s, |c: char| !is_terminal(c))?; - let specifier = PackageNameSpecifier::from_str(given).map_err(|err| { + let given = unquote(given) + .map(Cow::Owned) + .unwrap_or(Cow::Borrowed(given)); + let specifier = PackageNameSpecifier::from_str(given.as_ref()).map_err(|err| { RequirementsTxtParserError::NoBinary { source: err, specifier: given.to_string(), @@ -663,7 +678,10 @@ fn parse_entry( RequirementsTxtStatement::NoBinary(NoBinary::from_pip_arg(specifier)) } else if s.eat_if("--only-binary") { let given = parse_value(content, s, |c: char| !is_terminal(c))?; - let specifier = PackageNameSpecifier::from_str(given).map_err(|err| { + let given = unquote(given) + .map(Cow::Owned) + .unwrap_or(Cow::Borrowed(given)); + let specifier = PackageNameSpecifier::from_str(given.as_ref()).map_err(|err| { RequirementsTxtParserError::NoBinary { source: err, specifier: given.to_string(), diff --git a/crates/uv/tests/it/pip_install.rs b/crates/uv/tests/it/pip_install.rs index 42111acac..bf12678e9 100644 --- a/crates/uv/tests/it/pip_install.rs +++ b/crates/uv/tests/it/pip_install.rs @@ -7015,6 +7015,146 @@ fn verify_hashes_editable() -> Result<()> { Ok(()) } +/// Allow arguments within a `requirements.txt` file to be quoted or unquoted, as in the CLI. +#[test] +fn double_quoted_arguments() -> Result<()> { + let context = TestContext::new("3.12"); + + let constraints_in = context.temp_dir.child("constraints.in"); + constraints_in.write_str(indoc::indoc! {r" + iniconfig==1.0.0 + "})?; + + let requirements_in = context.temp_dir.child("requirements.in"); + requirements_in.write_str(indoc::indoc! {r#" + --constraint "./constraints.in" + + iniconfig + "#})?; + + uv_snapshot!(context.pip_install() + .arg("-r") + .arg("requirements.in"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 1 package in [TIME] + Prepared 1 package in [TIME] + Installed 1 package in [TIME] + + iniconfig==1.0.0 + "### + ); + + Ok(()) +} + +/// Allow arguments within a `requirements.txt` file to be quoted or unquoted, as in the CLI. +#[test] +fn single_quoted_arguments() -> Result<()> { + let context = TestContext::new("3.12"); + + let constraints_in = context.temp_dir.child("constraints.in"); + constraints_in.write_str(indoc::indoc! {r" + iniconfig==1.0.0 + "})?; + + let requirements_in = context.temp_dir.child("requirements.in"); + requirements_in.write_str(indoc::indoc! {r" + --constraint './constraints.in' + + iniconfig + "})?; + + uv_snapshot!(context.pip_install() + .arg("-r") + .arg("requirements.in"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 1 package in [TIME] + Prepared 1 package in [TIME] + Installed 1 package in [TIME] + + iniconfig==1.0.0 + "### + ); + + Ok(()) +} + +/// Allow arguments within a `requirements.txt` file to be quoted or unquoted, as in the CLI. +#[test] +fn unquoted_arguments() -> Result<()> { + let context = TestContext::new("3.12"); + + let constraints_in = context.temp_dir.child("constraints.in"); + constraints_in.write_str(indoc::indoc! {r" + iniconfig==1.0.0 + "})?; + + let requirements_in = context.temp_dir.child("requirements.in"); + requirements_in.write_str(indoc::indoc! {r" + --constraint ./constraints.in + + iniconfig + "})?; + + uv_snapshot!(context.pip_install() + .arg("-r") + .arg("requirements.in"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 1 package in [TIME] + Prepared 1 package in [TIME] + Installed 1 package in [TIME] + + iniconfig==1.0.0 + "### + ); + + Ok(()) +} + +/// Allow arguments within a `requirements.txt` file to be quoted or unquoted, as in the CLI. +#[test] +fn concatenated_quoted_arguments() -> Result<()> { + let context = TestContext::new("3.12"); + + let constraints_in = context.temp_dir.child("constraints.in"); + constraints_in.write_str(indoc::indoc! {r" + iniconfig==1.0.0 + "})?; + + let requirements_in = context.temp_dir.child("requirements.in"); + requirements_in.write_str(indoc::indoc! {r#" + --constraint "./constr""aints.in" + + iniconfig + "#})?; + + uv_snapshot!(context.pip_install() + .arg("-r") + .arg("requirements.in"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 1 package in [TIME] + Prepared 1 package in [TIME] + Installed 1 package in [TIME] + + iniconfig==1.0.0 + "### + ); + + Ok(()) +} + #[test] #[cfg(feature = "git")] fn tool_uv_sources() -> Result<()> {