Add support for subdirectories in direct URLs in uv.lock (#10068)

## Summary There were some subtle bugs here and no test coverage.
astral-sh · Dec 21, 2024 · ddc290f · ddc290f
1 parent 2c68dfd
commit ddc290f
Show file tree

Hide file tree

Showing 5 changed files with 177 additions and 25 deletions.
diff --git a/crates/uv-distribution/src/metadata/lowering.rs b/crates/uv-distribution/src/metadata/lowering.rs
@@ -622,7 +622,7 @@ fn url_source(
         let subdirectory = subdirectory
             .to_str()
             .ok_or_else(|| LoweringError::NonUtf8Path(subdirectory.clone()))?;
-        verbatim_url.set_fragment(Some(subdirectory));
+        verbatim_url.set_fragment(Some(&format!("subdirectory={subdirectory}")));
     }
 
     let ext = match DistExtension::from_path(url.path()) {

diff --git a/crates/uv-pypi-types/src/requirement.rs b/crates/uv-pypi-types/src/requirement.rs
@@ -853,13 +853,23 @@ impl TryFrom<RequirementSourceWire> for RequirementSource {
                     url,
                 })
             }
-            RequirementSourceWire::Direct { url, subdirectory } => Ok(Self::Url {
-                url: VerbatimUrl::from_url(url.clone()),
-                location: url.clone(),
-                subdirectory: subdirectory.map(PathBuf::from),
-                ext: DistExtension::from_path(url.path())
-                    .map_err(|err| ParsedUrlError::MissingExtensionUrl(url.to_string(), err))?,
-            }),
+            RequirementSourceWire::Direct { url, subdirectory } => {
+                let location = url.clone();
+
+                // Create a PEP 508-compatible URL.
+                let mut url = url.clone();
+                if let Some(subdirectory) = &subdirectory {
+                    url.set_fragment(Some(&format!("subdirectory={subdirectory}")));
+                }
+
+                Ok(Self::Url {
+                    location,
+                    subdirectory: subdirectory.map(PathBuf::from),
+                    ext: DistExtension::from_path(url.path())
+                        .map_err(|err| ParsedUrlError::MissingExtensionUrl(url.to_string(), err))?,
+                    url: VerbatimUrl::from_url(url.clone()),
+                })
+            }
             // TODO(charlie): The use of `CWD` here is incorrect. These should be resolved relative
             // to the workspace root, but we don't have access to it here. When comparing these
             // sources in the lockfile, we replace the URL anyway.

diff --git a/crates/uv-resolver/src/lock/mod.rs b/crates/uv-resolver/src/lock/mod.rs
@@ -1864,15 +1864,16 @@ impl Package {
                 let DistExtension::Source(ext) = DistExtension::from_path(url.as_ref())? else {
                     return Ok(None);
                 };
+                let location = url.to_url();
                 let subdirectory = direct.subdirectory.as_ref().map(PathBuf::from);
                 let url = Url::from(ParsedArchiveUrl {
-                    url: url.to_url(),
+                    url: location.clone(),
                     subdirectory: subdirectory.clone(),
                     ext: DistExtension::Source(ext),
                 });
                 let direct_dist = DirectUrlSourceDist {
                     name: self.id.name.clone(),
-                    location: url.clone(),
+                    location,
                     subdirectory: subdirectory.clone(),
                     ext,
                     url: VerbatimUrl::from_url(url),
@@ -3822,23 +3823,33 @@ fn normalize_requirement(
             })
         }
         RequirementSource::Url {
-            location,
+            mut location,
             subdirectory,
             ext,
             url,
-        } => Ok(Requirement {
-            name: requirement.name,
-            extras: requirement.extras,
-            groups: requirement.groups,
-            marker: requirement.marker,
-            source: RequirementSource::Url {
-                location,
-                subdirectory,
-                ext,
-                url,
-            },
-            origin: None,
-        }),
+        } => {
+            // Redact the credentials.
+            redact_credentials(&mut location);
+
+            // Redact the PEP 508 URL.
+            let mut url = url.to_url();
+            redact_credentials(&mut url);
+            let url = VerbatimUrl::from_url(url);
+
+            Ok(Requirement {
+                name: requirement.name,
+                extras: requirement.extras,
+                groups: requirement.groups,
+                marker: requirement.marker,
+                source: RequirementSource::Url {
+                    location,
+                    subdirectory,
+                    ext,
+                    url,
+                },
+                origin: None,
+            })
+        }
     }
 }
 

diff --git a/crates/uv/tests/it/lock.rs b/crates/uv/tests/it/lock.rs
@@ -1222,6 +1222,137 @@ fn lock_sdist_url() -> Result<()> {
     Ok(())
 }
 
+/// Lock a requirement from a direct URL to a source distribution, with a subdirectory.
+#[test]
+fn lock_sdist_url_subdirectory() -> Result<()> {
+    let context = TestContext::new("3.12");
+
+    let pyproject_toml = context.temp_dir.child("pyproject.toml");
+    pyproject_toml.write_str(
+        r#"
+        [project]
+        name = "project"
+        version = "0.1.0"
+        requires-python = ">=3.12"
+        dependencies = ["root"]
+
+        [build-system]
+        requires = ["setuptools>=42"]
+        build-backend = "setuptools.build_meta"
+
+        [tool.uv.sources]
+        root = { url = "https://github.com/user-attachments/files/18216295/subdirectory-test.tar.gz", subdirectory = "packages/root" }
+        "#,
+    )?;
+
+    uv_snapshot!(context.filters(), context.lock(), @r###"
+    success: true
+    exit_code: 0
+    ----- stdout -----
+
+    ----- stderr -----
+    Resolved 5 packages in [TIME]
+    "###);
+
+    let lock = context.read("uv.lock");
+
+    insta::with_settings!({
+        filters => context.filters(),
+    }, {
+        assert_snapshot!(
+            lock, @r###"
+        version = 1
+        requires-python = ">=3.12"
+
+        [options]
+        exclude-newer = "2024-03-25T00:00:00Z"
+
+        [[package]]
+        name = "anyio"
+        version = "4.3.0"
+        source = { registry = "https://pypi.org/simple" }
+        dependencies = [
+            { name = "idna" },
+            { name = "sniffio" },
+        ]
+        sdist = { url = "https://files.pythonhosted.org/packages/db/4d/3970183622f0330d3c23d9b8a5f52e365e50381fd484d08e3285104333d3/anyio-4.3.0.tar.gz", hash = "sha256:f75253795a87df48568485fd18cdd2a3fa5c4f7c5be8e5e36637733fce06fed6", size = 159642 }
+        wheels = [
+            { url = "https://files.pythonhosted.org/packages/14/fd/2f20c40b45e4fb4324834aea24bd4afdf1143390242c0b33774da0e2e34f/anyio-4.3.0-py3-none-any.whl", hash = "sha256:048e05d0f6caeed70d731f3db756d35dcc1f35747c8c403364a8332c630441b8", size = 85584 },
+        ]
+
+        [[package]]
+        name = "idna"
+        version = "3.6"
+        source = { registry = "https://pypi.org/simple" }
+        sdist = { url = "https://files.pythonhosted.org/packages/bf/3f/ea4b9117521a1e9c50344b909be7886dd00a519552724809bb1f486986c2/idna-3.6.tar.gz", hash = "sha256:9ecdbbd083b06798ae1e86adcbfe8ab1479cf864e4ee30fe4e46a003d12491ca", size = 175426 }
+        wheels = [
+            { url = "https://files.pythonhosted.org/packages/c2/e7/a82b05cf63a603df6e68d59ae6a68bf5064484a0718ea5033660af4b54a9/idna-3.6-py3-none-any.whl", hash = "sha256:c05567e9c24a6b9faaa835c4821bad0590fbb9d5779e7caa6e1cc4978e7eb24f", size = 61567 },
+        ]
+
+        [[package]]
+        name = "project"
+        version = "0.1.0"
+        source = { editable = "." }
+        dependencies = [
+            { name = "root" },
+        ]
+
+        [package.metadata]
+        requires-dist = [{ name = "root", url = "https://github.com/user-attachments/files/18216295/subdirectory-test.tar.gz", subdirectory = "packages/root" }]
+
+        [[package]]
+        name = "root"
+        version = "0.0.1"
+        source = { url = "https://github.com/user-attachments/files/18216295/subdirectory-test.tar.gz", subdirectory = "packages/root" }
+        dependencies = [
+            { name = "anyio" },
+        ]
+        sdist = { url = "https://github.com/user-attachments/files/18216295/subdirectory-test.tar.gz", hash = "sha256:24b55efee28d08ad3cdc58903e359e820601baa6a4a4b3424311541ebcfb09d3" }
+
+        [package.metadata]
+        requires-dist = [{ name = "anyio" }]
+
+        [[package]]
+        name = "sniffio"
+        version = "1.3.1"
+        source = { registry = "https://pypi.org/simple" }
+        sdist = { url = "https://files.pythonhosted.org/packages/a2/87/a6771e1546d97e7e041b6ae58d80074f81b7d5121207425c964ddf5cfdbd/sniffio-1.3.1.tar.gz", hash = "sha256:f4324edc670a0f49750a81b895f35c3adb843cca46f0530f79fc1babb23789dc", size = 20372 }
+        wheels = [
+            { url = "https://files.pythonhosted.org/packages/e9/44/75a9c9421471a6c4805dbf2356f7c181a29c1879239abab1ea2cc8f38b40/sniffio-1.3.1-py3-none-any.whl", hash = "sha256:2f6da418d1f1e0fddd844478f41680e794e6051915791a034ff65e5f100525a2", size = 10235 },
+        ]
+        "###
+        );
+    });
+
+    // Re-run with `--locked`.
+    uv_snapshot!(context.filters(), context.lock().arg("--locked"), @r###"
+    success: true
+    exit_code: 0
+    ----- stdout -----
+
+    ----- stderr -----
+    Resolved 5 packages in [TIME]
+    "###);
+
+    // Install from the lockfile.
+    uv_snapshot!(context.filters(), context.sync().arg("--frozen"), @r###"
+    success: true
+    exit_code: 0
+    ----- stdout -----
+
+    ----- stderr -----
+    Prepared 5 packages in [TIME]
+    Installed 5 packages in [TIME]
+     + anyio==4.3.0
+     + idna==3.6
+     + project==0.1.0 (from file://[TEMP_DIR]/)
+     + root==0.0.1 (from https://github.com/user-attachments/files/18216295/subdirectory-test.tar.gz#subdirectory=packages/root)
+     + sniffio==1.3.1
+    "###);
+
+    Ok(())
+}
+
 /// Lock a project with an extra. When resolving, all extras should be included.
 #[test]
 fn lock_project_extra() -> Result<()> {
@@ -7754,7 +7885,6 @@ fn lock_redact_index_sources() -> Result<()> {
     Ok(())
 }
 
-/// We don't currently redact credentials from direct URLs, though.
 #[test]
 fn lock_redact_url_sources() -> Result<()> {
     let context = TestContext::new("3.12").with_filtered_link_mode_warning();

diff --git a/crates/uv/tests/it/sync.rs b/crates/uv/tests/it/sync.rs
@@ -5448,6 +5448,7 @@ fn sync_git_repeated_member_backwards_path() -> Result<()> {
         version = "0.1.0"
         requires-python = ">=3.13"
         dependencies = ["package", "dependency"]
+
         [tool.uv.sources]
         package = { git = "https://github.com/astral-sh/uv-backwards-path-test", subdirectory = "root" }
         dependency = { git = "https://github.com/astral-sh/uv-backwards-path-test", subdirectory = "dependency" }