From 836b90c7604519ad46c9cadd167e15be7138ddbd Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 3 Mar 2024 15:47:04 -0800 Subject: [PATCH] Expand environment variables in `-r` and `-c` subfile paths (#2143) ## Summary This PR expands environment variables in `-r` and `-c` paths _within_ requirements files. We already do this for `@` URL references and others. Closes https://github.com/astral-sh/uv/issues/1473. --- crates/pep508-rs/src/lib.rs | 2 +- crates/pep508-rs/src/verbatim_url.rs | 33 ++++++++++++++++++---------- crates/requirements-txt/src/lib.rs | 7 +++--- crates/uv/tests/pip_compile.rs | 32 +++++++++++++++++++++++++++ 4 files changed, 59 insertions(+), 15 deletions(-) diff --git a/crates/pep508-rs/src/lib.rs b/crates/pep508-rs/src/lib.rs index 0ca61add8..f4d8d64a6 100644 --- a/crates/pep508-rs/src/lib.rs +++ b/crates/pep508-rs/src/lib.rs @@ -45,7 +45,7 @@ use pep440_rs::{Version, VersionSpecifier, VersionSpecifiers}; use uv_fs::normalize_url_path; // Parity with the crates.io version of pep508_rs pub use uv_normalize::{ExtraName, InvalidNameError, PackageName}; -pub use verbatim_url::{split_scheme, Scheme, VerbatimUrl}; +pub use verbatim_url::{expand_path_vars, split_scheme, Scheme, VerbatimUrl}; mod marker; mod verbatim_url; diff --git a/crates/pep508-rs/src/verbatim_url.rs b/crates/pep508-rs/src/verbatim_url.rs index f35c6d776..32123639d 100644 --- a/crates/pep508-rs/src/verbatim_url.rs +++ b/crates/pep508-rs/src/verbatim_url.rs @@ -32,7 +32,7 @@ pub struct VerbatimUrl { impl VerbatimUrl { /// Parse a URL from a string, expanding any environment variables. pub fn parse(given: impl AsRef) -> Result { - let url = Url::parse(&expand_env_vars(given.as_ref(), true))?; + let url = Url::parse(&expand_env_vars(given.as_ref(), Escape::Url))?; Ok(Self { url, given: None }) } @@ -52,7 +52,7 @@ impl VerbatimUrl { #[cfg(feature = "non-pep508-extensions")] // PEP 508 arguably only allows absolute file URLs. pub fn parse_path(path: impl AsRef, working_dir: impl AsRef) -> Self { // Expand any environment variables. - let path = PathBuf::from(expand_env_vars(path.as_ref(), false).as_ref()); + let path = PathBuf::from(expand_env_vars(path.as_ref(), Escape::Path).as_ref()); // Convert the path to an absolute path, if necessary. let path = if path.is_absolute() { @@ -73,7 +73,7 @@ impl VerbatimUrl { /// Parse a URL from an absolute path. pub fn parse_absolute_path(path: impl AsRef) -> Result { // Expand any environment variables. - let path = PathBuf::from(expand_env_vars(path.as_ref(), false).as_ref()); + let path = PathBuf::from(expand_env_vars(path.as_ref(), Escape::Path).as_ref()); // Convert the path to an absolute path, if necessary. let path = if path.is_absolute() { @@ -160,6 +160,15 @@ pub enum VerbatimUrlError { RelativePath(PathBuf), } +/// Whether to apply percent-encoding when expanding environment variables. +#[derive(Debug, Clone, PartialEq, Eq)] +enum Escape { + /// Apply percent-encoding. + Url, + /// Do not apply percent-encoding. + Path, +} + /// Expand all available environment variables. /// /// This is modeled off of pip's environment variable expansion, which states: @@ -175,7 +184,7 @@ pub enum VerbatimUrlError { /// Valid characters in variable names follow the `POSIX standard /// `_ and are limited /// to uppercase letter, digits and the `_` (underscore). -fn expand_env_vars(s: &str, escape: bool) -> Cow<'_, str> { +fn expand_env_vars(s: &str, escape: Escape) -> Cow<'_, str> { // Generate the project root, to be used via the `${PROJECT_ROOT}` // environment variable. static PROJECT_ROOT_FRAGMENT: Lazy = Lazy::new(|| { @@ -190,18 +199,20 @@ fn expand_env_vars(s: &str, escape: bool) -> Cow<'_, str> { let name = caps.name("name").unwrap().as_str(); std::env::var(name).unwrap_or_else(|_| match name { // Ensure that the variable is URL-escaped, if necessary. - "PROJECT_ROOT" => { - if escape { - PROJECT_ROOT_FRAGMENT.replace(' ', "%20") - } else { - PROJECT_ROOT_FRAGMENT.to_string() - } - } + "PROJECT_ROOT" => match escape { + Escape::Url => PROJECT_ROOT_FRAGMENT.replace(' ', "%20"), + Escape::Path => PROJECT_ROOT_FRAGMENT.to_string(), + }, _ => caps["var"].to_owned(), }) }) } +/// Expand all available environment variables in a path-like string. +pub fn expand_path_vars(path: &str) -> Cow<'_, str> { + expand_env_vars(path, Escape::Path) +} + /// Like [`Url::parse`], but only splits the scheme. Derived from the `url` crate. pub fn split_scheme(s: &str) -> Option<(&str, &str)> { /// diff --git a/crates/requirements-txt/src/lib.rs b/crates/requirements-txt/src/lib.rs index 0ab290994..83a6dbeec 100644 --- a/crates/requirements-txt/src/lib.rs +++ b/crates/requirements-txt/src/lib.rs @@ -46,7 +46,8 @@ use url::Url; use uv_warnings::warn_user; use pep508_rs::{ - split_scheme, Extras, Pep508Error, Pep508ErrorSource, Requirement, Scheme, VerbatimUrl, + expand_path_vars, split_scheme, Extras, Pep508Error, Pep508ErrorSource, Requirement, Scheme, + VerbatimUrl, }; use uv_fs::{normalize_url_path, Simplified}; use uv_normalize::ExtraName; @@ -369,7 +370,7 @@ impl RequirementsTxt { start, end, } => { - let sub_file = requirements_dir.join(filename); + let sub_file = requirements_dir.join(expand_path_vars(&filename).as_ref()); let sub_requirements = Self::parse(&sub_file, working_dir).map_err(|err| { RequirementsTxtParserError::Subfile { source: Box::new(err), @@ -385,7 +386,7 @@ impl RequirementsTxt { start, end, } => { - let sub_file = requirements_dir.join(filename); + let sub_file = requirements_dir.join(expand_path_vars(&filename).as_ref()); let sub_constraints = Self::parse(&sub_file, working_dir).map_err(|err| { RequirementsTxtParserError::Subfile { source: Box::new(err), diff --git a/crates/uv/tests/pip_compile.rs b/crates/uv/tests/pip_compile.rs index 7ae410042..e4e433d14 100644 --- a/crates/uv/tests/pip_compile.rs +++ b/crates/uv/tests/pip_compile.rs @@ -4638,3 +4638,35 @@ fn index_url_env_var_override() -> Result<()> { Ok(()) } + +/// Expand an environment variable in a `-r` path within a `requirements.in` file. +#[test] +fn expand_env_var_requirements_txt() -> Result<()> { + let context = TestContext::new("3.12"); + + let requirements_in = context.temp_dir.child("requirements.in"); + requirements_in.write_str("-r ${PROJECT_ROOT}/requirements-dev.in")?; + + let requirements_dev_in = context.temp_dir.child("requirements-dev.in"); + requirements_dev_in.write_str("anyio")?; + + uv_snapshot!(context.compile() + .arg("requirements.in"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + # This file was autogenerated by uv via the following command: + # uv pip compile --cache-dir [CACHE_DIR] --exclude-newer 2023-11-18T12:00:00Z requirements.in + anyio==4.0.0 + idna==3.4 + # via anyio + sniffio==1.3.0 + # via anyio + + ----- stderr ----- + Resolved 3 packages in [TIME] + "### + ); + + Ok(()) +}