From 33cb3497aa682042b53753f498b9fa93c3bcfac4 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 22 Dec 2024 09:07:04 -0500 Subject: [PATCH] Strip fragment when storing direct URL (#10093) ## Summary Closes https://github.com/astral-sh/uv/issues/10088#issuecomment-2558280467. --- .../src/specified_requirement.rs | 7 +- crates/uv-distribution/src/source/mod.rs | 2 +- crates/uv-pypi-types/src/parsed_url.rs | 7 +- crates/uv-resolver/src/lock/mod.rs | 36 ++- crates/uv/tests/it/lock.rs | 210 +++++++++++++++++- 5 files changed, 242 insertions(+), 20 deletions(-) diff --git a/crates/uv-distribution-types/src/specified_requirement.rs b/crates/uv-distribution-types/src/specified_requirement.rs index 97a096ea9..62744020b 100644 --- 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 index 227b2f2fc..2beae3ca4 100644 --- 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 index 4f4088de2..c5d8b5aed 100644 --- a/crates/uv-pypi-types/src/parsed_url.rs +++ b/crates/uv-pypi-types/src/parsed_url.rs @@ -307,8 +307,12 @@ impl ParsedArchiveUrl { impl TryFrom for ParsedArchiveUrl { type Error = ParsedUrlError; - fn try_from(url: Url) -> Result { + fn try_from(mut url: Url) -> Result { + // 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) => { @@ -316,6 +320,7 @@ impl TryFrom for ParsedArchiveUrl { } 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 index 04a29f98b..91aefe78d 100644 --- 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 { 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 index 2d8220444..f9add3e8c 100644 --- 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" }] "### ); });