From ddc290feb4ed2de4740c786af2436cf1f82a3190 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Fri, 20 Dec 2024 19:00:50 -0500 Subject: [PATCH] Add support for subdirectories in direct URLs in `uv.lock` (#10068) ## Summary There were some subtle bugs here and no test coverage. --- .../uv-distribution/src/metadata/lowering.rs | 2 +- crates/uv-pypi-types/src/requirement.rs | 24 +++- crates/uv-resolver/src/lock/mod.rs | 43 +++--- crates/uv/tests/it/lock.rs | 132 +++++++++++++++++- crates/uv/tests/it/sync.rs | 1 + 5 files changed, 177 insertions(+), 25 deletions(-) diff --git a/crates/uv-distribution/src/metadata/lowering.rs b/crates/uv-distribution/src/metadata/lowering.rs index d635a94b42da..adf87ba757ea 100644 --- 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 index 1a1cd5c7d692..cd8d84e11186 100644 --- a/crates/uv-pypi-types/src/requirement.rs +++ b/crates/uv-pypi-types/src/requirement.rs @@ -853,13 +853,23 @@ impl TryFrom 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 index 1d469ddb9ce5..cea818d085e5 100644 --- 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 index 1d42f178f536..5d0983b11984 100644 --- 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 index 70dde1a12fce..ad2453e05033 100644 --- 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" }