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

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

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<str>) -> Result<Self, ParseError> {
-        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<str>, working_dir: impl AsRef<Path>) -> 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<str>) -> Result<Self, VerbatimUrlError> {
         // 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
 ///   <http://pubs.opengroup.org/onlinepubs/9699919799/>`_ 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<String> = 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" => {
+            "PROJECT_ROOT" => match escape {
-                if escape {
+                Escape::Url => PROJECT_ROOT_FRAGMENT.replace(' ', "%20"),
-                    PROJECT_ROOT_FRAGMENT.replace(' ', "%20")
+                Escape::Path => PROJECT_ROOT_FRAGMENT.to_string(),
-                } else {
+            },
-                    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)> {
     /// <https://url.spec.whatwg.org/#c0-controls-and-space>

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

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(())
+}