Strip fragment when storing direct URL (astral-sh#10093)

## Summary Closes astral-sh#10088 (comment).
flyaroundme · Dec 22, 2024 · 33cb349 · 33cb349
1 parent ad92aaf
commit 33cb349
Show file tree

Hide file tree

Showing 5 changed files with 242 additions and 20 deletions.
diff --git a/crates/uv-distribution-types/src/specified_requirement.rs b/crates/uv-distribution-types/src/specified_requirement.rs
@@ -3,7 +3,7 @@ use std::fmt::{Display, Formatter};
 
 use uv_normalize::ExtraName;
 use uv_pep508::{MarkerEnvironment, UnnamedRequirement};
-use uv_pypi_types::{Hashes, ParsedUrl, Requirement, RequirementSource};
+use uv_pypi_types::{Hashes, Requirement, RequirementSource};
 
 use crate::VerbatimParsedUrl;
 
@@ -98,10 +98,7 @@ impl UnresolvedRequirement {
         match self {
             Self::Named(requirement) => requirement.hashes(),
             Self::Unnamed(requirement) => {
-                let ParsedUrl::Archive(ref url) = requirement.url.parsed_url else {
-                    return None;
-                };
-                let fragment = url.url.fragment()?;
+                let fragment = requirement.url.verbatim.fragment()?;
                 Hashes::parse_fragment(fragment).ok()
             }
         }

diff --git a/crates/uv-distribution/src/source/mod.rs b/crates/uv-distribution/src/source/mod.rs
@@ -2377,8 +2377,8 @@ async fn read_egg_info(
                         continue;
                     };
                     if let Some(name) = name {
-                        debug!("Skipping `{file_stem}.egg-info` due to name mismatch (expected: `{name}`)");
                         if file_name.name != *name {
+                            debug!("Skipping `{file_stem}.egg-info` due to name mismatch (expected: `{name}`)");
                             continue;
                         }
                     }

diff --git a/crates/uv-pypi-types/src/parsed_url.rs b/crates/uv-pypi-types/src/parsed_url.rs
@@ -307,15 +307,20 @@ impl ParsedArchiveUrl {
 impl TryFrom<Url> for ParsedArchiveUrl {
     type Error = ParsedUrlError;
 
-    fn try_from(url: Url) -> Result<Self, Self::Error> {
+    fn try_from(mut url: Url) -> Result<Self, Self::Error> {
+        // Extract the `#subdirectory` fragment, if present.
         let subdirectory = get_subdirectory(&url);
+        url.set_fragment(None);
+
+        // Infer the extension from the path.
         let ext = match DistExtension::from_path(url.path()) {
             Ok(ext) => ext,
             Err(..) if looks_like_git_repository(&url) => {
                 return Err(ParsedUrlError::MissingGitPrefix(url.to_string()))
             }
             Err(err) => return Err(ParsedUrlError::MissingExtensionUrl(url.to_string(), err)),
         };
+
         Ok(Self {
             url,
             subdirectory,

diff --git a/crates/uv-resolver/src/lock/mod.rs b/crates/uv-resolver/src/lock/mod.rs
@@ -3752,6 +3752,7 @@ fn normalize_file_location(location: &FileLocation) -> Result<UrlString, ToUrlEr
 /// Convert a [`Url`] into a normalized [`UrlString`].
 fn normalize_url(mut url: Url) -> UrlString {
     url.set_fragment(None);
+    url.set_query(None);
     UrlString::from(url)
 }
 
@@ -3773,15 +3774,20 @@ fn normalize_requirement(
             reference,
             precise,
             subdirectory,
-            url,
+            url: _,
         } => {
             // Redact the credentials.
             redact_credentials(&mut repository);
 
-            // Redact the PEP 508 URL.
-            let mut url = url.to_url();
-            redact_credentials(&mut url);
-            let url = VerbatimUrl::from_url(url);
+            // Remove the fragment and query from the URL; they're already present in the source.
+            repository.set_fragment(None);
+            repository.set_query(None);
+
+            // Reconstruct the PEP 508 URL from the underlying data.
+            let url = Url::from(ParsedGitUrl {
+                url: uv_git::GitUrl::from_reference(repository.clone(), reference.clone()),
+                subdirectory: subdirectory.clone(),
+            });
 
             Ok(Requirement {
                 name: requirement.name,
@@ -3793,7 +3799,7 @@ fn normalize_requirement(
                     reference,
                     precise,
                     subdirectory,
-                    url,
+                    url: VerbatimUrl::from_url(url),
                 },
                 origin: None,
             })
@@ -3871,15 +3877,21 @@ fn normalize_requirement(
             mut location,
             subdirectory,
             ext,
-            url,
+            url: _,
         } => {
             // 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);
+            // Remove the fragment and query from the URL; they're already present in the source.
+            location.set_fragment(None);
+            location.set_query(None);
+
+            // Reconstruct the PEP 508 URL from the underlying data.
+            let url = Url::from(ParsedArchiveUrl {
+                url: location.clone(),
+                subdirectory: subdirectory.clone(),
+                ext,
+            });
 
             Ok(Requirement {
                 name: requirement.name,
@@ -3890,7 +3902,7 @@ fn normalize_requirement(
                     location,
                     subdirectory,
                     ext,
-                    url,
+                    url: VerbatimUrl::from_url(url),
                 },
                 origin: None,
             })

diff --git a/crates/uv/tests/it/lock.rs b/crates/uv/tests/it/lock.rs
@@ -135,6 +135,16 @@ fn lock_wheel_registry() -> Result<()> {
      + sniffio==1.3.1
     "###);
 
+    // Re-install from the lockfile.
+    uv_snapshot!(context.filters(), context.sync().arg("--frozen"), @r###"
+    success: true
+    exit_code: 0
+    ----- stdout -----
+
+    ----- stderr -----
+    Audited 4 packages in [TIME]
+    "###);
+
     Ok(())
 }
 
@@ -220,6 +230,16 @@ fn lock_sdist_registry() -> Result<()> {
      + source-distribution==0.0.1
     "###);
 
+    // Re-install from the lockfile.
+    uv_snapshot!(context.filters(), context.sync().arg("--frozen").env_remove(EnvVars::UV_EXCLUDE_NEWER), @r###"
+    success: true
+    exit_code: 0
+    ----- stdout -----
+
+    ----- stderr -----
+    Audited 2 packages in [TIME]
+    "###);
+
     Ok(())
 }
 
@@ -590,6 +610,16 @@ fn lock_sdist_git_subdirectory() -> Result<()> {
      + project==0.1.0 (from file://[TEMP_DIR]/)
     "###);
 
+    // Re-install from the lockfile.
+    uv_snapshot!(context.filters(), context.sync().arg("--frozen"), @r###"
+    success: true
+    exit_code: 0
+    ----- stdout -----
+
+    ----- stderr -----
+    Audited 2 packages in [TIME]
+    "###);
+
     Ok(())
 }
 
@@ -938,6 +968,16 @@ fn lock_sdist_git_short_rev() -> Result<()> {
      + uv-public-pypackage==0.1.0 (from git+https://github.com/astral-test/uv-public-pypackage@0dacfd662c64cb4ceb16e6cf65a157a8b715b979)
     "###);
 
+    // Re-install from the lockfile.
+    uv_snapshot!(context.filters(), context.sync().arg("--frozen"), @r###"
+    success: true
+    exit_code: 0
+    ----- stdout -----
+
+    ----- stderr -----
+    Audited 2 packages in [TIME]
+    "###);
+
     Ok(())
 }
 
@@ -1085,6 +1125,16 @@ fn lock_wheel_url() -> Result<()> {
      + sniffio==1.3.1
     "###);
 
+    // Re-install from the lockfile.
+    uv_snapshot!(context.filters(), context.sync().arg("--frozen"), @r###"
+    success: true
+    exit_code: 0
+    ----- stdout -----
+
+    ----- stderr -----
+    Audited 4 packages in [TIME]
+    "###);
+
     Ok(())
 }
 
@@ -1219,6 +1269,16 @@ fn lock_sdist_url() -> Result<()> {
      + sniffio==1.3.1
     "###);
 
+    // Re-install from the lockfile.
+    uv_snapshot!(context.filters(), context.sync().arg("--frozen"), @r###"
+    success: true
+    exit_code: 0
+    ----- stdout -----
+
+    ----- stderr -----
+    Audited 4 packages in [TIME]
+    "###);
+
     Ok(())
 }
 
@@ -1350,6 +1410,154 @@ fn lock_sdist_url_subdirectory() -> Result<()> {
      + sniffio==1.3.1
     "###);
 
+    // Re-install from the lockfile.
+    uv_snapshot!(context.filters(), context.sync().arg("--frozen"), @r###"
+    success: true
+    exit_code: 0
+    ----- stdout -----
+
+    ----- stderr -----
+    Audited 5 packages in [TIME]
+    "###);
+
+    Ok(())
+}
+
+/// Lock a requirement from a direct URL to a source distribution, with a subdirectory.
+#[test]
+fn lock_sdist_url_subdirectory_pep508() -> 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 @ https://github.com/user-attachments/files/18216295/subdirectory-test.tar.gz#subdirectory=packages/root"]
+
+        [build-system]
+        requires = ["setuptools>=42"]
+        build-backend = "setuptools.build_meta"
+        "#,
+    )?;
+
+    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 = { 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
+    "###);
+
+    // Re-install from the lockfile.
+    uv_snapshot!(context.filters(), context.sync().arg("--frozen"), @r###"
+    success: true
+    exit_code: 0
+    ----- stdout -----
+
+    ----- stderr -----
+    Audited 5 packages in [TIME]
+    "###);
+
     Ok(())
 }
 
@@ -16768,7 +16976,7 @@ fn lock_strip_fragment() -> Result<()> {
         ]
 
         [package.metadata]
-        requires-dist = [{ name = "iniconfig", url = "https://files.pythonhosted.org/packages/ef/a6/62565a6e1cf69e10f5727360368e451d4b7f58beeac6173dc9db836a5b46/iniconfig-2.0.0-py3-none-any.whl#sha256=b6a85871a79d2e3b22d2d1b94ac2824226a63c6b741c88f7ae975f18b6778374" }]
+        requires-dist = [{ name = "iniconfig", url = "https://files.pythonhosted.org/packages/ef/a6/62565a6e1cf69e10f5727360368e451d4b7f58beeac6173dc9db836a5b46/iniconfig-2.0.0-py3-none-any.whl" }]
         "###
         );
     });