From 1124df9bc5956e2d54688a34775f513fa2af5441 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 19 May 2024 22:01:57 -0400 Subject: [PATCH] Remove subdirectory from direct wheel URL type (#3667) ## Summary Closes #3665. --- crates/distribution-types/src/lib.rs | 30 ++++++++++++--------- crates/distribution-types/src/parsed_url.rs | 14 +++++----- crates/distribution-types/src/resolution.rs | 2 +- crates/uv-client/tests/remote_metadata.rs | 1 - crates/uv-dev/src/wheel_metadata.rs | 11 ++------ crates/uv-installer/src/plan.rs | 1 - crates/uv-resolver/src/lock.rs | 11 ++------ 7 files changed, 31 insertions(+), 39 deletions(-) diff --git a/crates/distribution-types/src/lib.rs b/crates/distribution-types/src/lib.rs index c50d9a56b..020451d12 100644 --- a/crates/distribution-types/src/lib.rs +++ b/crates/distribution-types/src/lib.rs @@ -202,9 +202,7 @@ pub struct DirectUrlBuiltDist { pub filename: WheelFilename, /// The URL without the subdirectory fragment. pub location: Url, - /// The subdirectory fragment, if any. - pub subdirectory: Option, - /// The URL with the subdirectory fragment. + /// The URL as it was provided by the user. pub url: VerbatimUrl, } @@ -212,8 +210,10 @@ pub struct DirectUrlBuiltDist { #[derive(Debug, Clone)] pub struct PathBuiltDist { pub filename: WheelFilename, - pub url: VerbatimUrl, + /// The path to the wheel. pub path: PathBuf, + /// The URL as it was provided by the user. + pub url: VerbatimUrl, } /// A source distribution that exists in a registry, like `PyPI`. @@ -240,8 +240,9 @@ pub struct DirectUrlSourceDist { pub name: PackageName, /// The URL without the subdirectory fragment. pub location: Url, + /// The subdirectory within the archive in which the source distribution is located. pub subdirectory: Option, - /// The URL with the subdirectory fragment. + /// The URL as it was provided by the user, including the subdirectory fragment. pub url: VerbatimUrl, } @@ -251,8 +252,9 @@ pub struct GitSourceDist { pub name: PackageName, /// The URL without the revision and subdirectory fragment. pub git: Box, + /// The subdirectory within the Git repository in which the source distribution is located. pub subdirectory: Option, - /// The URL with the revision and subdirectory fragment. + /// The URL as it was provided by the user, including the revision and subdirectory fragment. pub url: VerbatimUrl, } @@ -260,17 +262,22 @@ pub struct GitSourceDist { #[derive(Debug, Clone)] pub struct PathSourceDist { pub name: PackageName, - pub url: VerbatimUrl, + /// The path to the archive. pub path: PathBuf, + /// The URL as it was provided by the user. + pub url: VerbatimUrl, } /// A source distribution that exists in a local directory. #[derive(Debug, Clone)] pub struct DirectorySourceDist { pub name: PackageName, - pub url: VerbatimUrl, + /// The path to the directory. pub path: PathBuf, + /// Whether the package should be installed in editable mode. pub editable: bool, + /// The URL as it was provided by the user. + pub url: VerbatimUrl, } impl Dist { @@ -299,7 +306,6 @@ impl Dist { Ok(Self::Built(BuiltDist::DirectUrl(DirectUrlBuiltDist { filename, location, - subdirectory, url, }))) } else { @@ -332,9 +338,9 @@ impl Dist { if path.is_dir() { Ok(Self::Source(SourceDist::Directory(DirectorySourceDist { name, - url, path, editable, + url, }))) } else if path .extension() @@ -356,8 +362,8 @@ impl Dist { Ok(Self::Built(BuiltDist::Path(PathBuiltDist { filename, - url, path, + url, }))) } else { if editable { @@ -366,8 +372,8 @@ impl Dist { Ok(Self::Source(SourceDist::Path(PathSourceDist { name, - url, path, + url, }))) } } diff --git a/crates/distribution-types/src/parsed_url.rs b/crates/distribution-types/src/parsed_url.rs index c48835263..d1c39d74a 100644 --- a/crates/distribution-types/src/parsed_url.rs +++ b/crates/distribution-types/src/parsed_url.rs @@ -33,7 +33,8 @@ pub struct VerbatimParsedUrl { /// * The path to a file or directory (`file://`) /// * A Git repository (`git+https://` or `git+ssh://`), optionally with a subdirectory and/or /// string to checkout. -/// * A remote archive (`https://`), optional with a subdirectory (source dist only) +/// * A remote archive (`https://`), optional with a subdirectory (source dist only). +/// /// A URL in a requirement `foo @ ` must be one of the above. #[derive(Debug, Clone, Eq, PartialEq, Hash)] pub enum ParsedUrl { @@ -41,7 +42,8 @@ pub enum ParsedUrl { Path(ParsedPathUrl), /// The direct URL is path to a Git repository. Git(ParsedGitUrl), - /// The direct URL is a URL to an archive. + /// The direct URL is a URL to a source archive (e.g., a `.tar.gz` file) or built archive + /// (i.e., a `.whl` file). Archive(ParsedArchiveUrl), } @@ -88,12 +90,12 @@ impl TryFrom for ParsedGitUrl { } } -/// An archive URL. +/// A URL to a source or built archive. /// /// Examples: -/// * wheel: `https://download.pytorch.org/whl/torch-2.0.1-cp39-cp39-manylinux2014_aarch64.whl#sha256=423e0ae257b756bb45a4b49072046772d1ad0c592265c5080070e0767da4e490` -/// * source dist, correctly named: `https://files.pythonhosted.org/packages/62/06/d5604a70d160f6a6ca5fd2ba25597c24abd5c5ca5f437263d177ac242308/tqdm-4.66.1.tar.gz` -/// * source dist, only extension recognizable: `https://github.com/foo-labs/foo/archive/master.zip#egg=pkg&subdirectory=packages/bar` +/// * A built distribution: `https://files.pythonhosted.org/packages/62/06/d5604a70d160f6a6ca5fd2ba25597c24abd5c5ca5f437263d177ac242308/tqdm-4.66.1-py2.py3-none-any.whl` +/// * A source distribution with a valid name: `https://files.pythonhosted.org/packages/62/06/d5604a70d160f6a6ca5fd2ba25597c24abd5c5ca5f437263d177ac242308/tqdm-4.66.1.tar.gz` +/// * A source dist with a recognizable extension but invalid name: `https://github.com/foo-labs/foo/archive/master.zip#egg=pkg&subdirectory=packages/bar` #[derive(Debug, Clone, Eq, PartialEq, Hash)] pub struct ParsedArchiveUrl { pub url: Url, diff --git a/crates/distribution-types/src/resolution.rs b/crates/distribution-types/src/resolution.rs index 65f28ec65..52cdc2c8e 100644 --- a/crates/distribution-types/src/resolution.rs +++ b/crates/distribution-types/src/resolution.rs @@ -88,7 +88,7 @@ impl From<&ResolvedDist> for Requirement { RequirementSource::Url { url: wheel.url.clone(), location, - subdirectory: wheel.subdirectory.clone(), + subdirectory: None, } } Dist::Built(BuiltDist::Path(wheel)) => RequirementSource::Path { diff --git a/crates/uv-client/tests/remote_metadata.rs b/crates/uv-client/tests/remote_metadata.rs index daac77d09..d0a15cf2e 100644 --- a/crates/uv-client/tests/remote_metadata.rs +++ b/crates/uv-client/tests/remote_metadata.rs @@ -22,7 +22,6 @@ async fn remote_metadata_with_and_without_cache() -> Result<()> { let dist = BuiltDist::DirectUrl(DirectUrlBuiltDist { filename, location: Url::parse(url).unwrap(), - subdirectory: None, url: VerbatimUrl::from_str(url).unwrap(), }); let metadata = client.wheel_metadata(&dist).await.unwrap(); diff --git a/crates/uv-dev/src/wheel_metadata.rs b/crates/uv-dev/src/wheel_metadata.rs index beb915488..fd4a768b4 100644 --- a/crates/uv-dev/src/wheel_metadata.rs +++ b/crates/uv-dev/src/wheel_metadata.rs @@ -5,7 +5,7 @@ use anyhow::{bail, Result}; use clap::Parser; use distribution_filename::WheelFilename; -use distribution_types::{BuiltDist, DirectUrlBuiltDist, ParsedUrl}; +use distribution_types::{BuiltDist, DirectUrlBuiltDist, ParsedUrl, RemoteSource}; use pep508_rs::VerbatimUrl; use uv_cache::{Cache, CacheArgs}; use uv_client::RegistryClientBuilder; @@ -21,13 +21,7 @@ pub(crate) async fn wheel_metadata(args: WheelMetadataArgs) -> Result<()> { let cache = Cache::try_from(args.cache_args)?.init()?; let client = RegistryClientBuilder::new(cache).build(); - let filename = WheelFilename::from_str( - args.url - .path() - .rsplit_once('/') - .unwrap_or(("", args.url.path())) - .1, - )?; + let filename = WheelFilename::from_str(&args.url.filename()?)?; let ParsedUrl::Archive(archive) = ParsedUrl::try_from(args.url.to_url())? else { bail!("Only HTTPS is supported"); @@ -37,7 +31,6 @@ pub(crate) async fn wheel_metadata(args: WheelMetadataArgs) -> Result<()> { .wheel_metadata(&BuiltDist::DirectUrl(DirectUrlBuiltDist { filename, location: archive.url, - subdirectory: archive.subdirectory, url: args.url, })) .await?; diff --git a/crates/uv-installer/src/plan.rs b/crates/uv-installer/src/plan.rs index 67c7c2b98..02aa0274d 100644 --- a/crates/uv-installer/src/plan.rs +++ b/crates/uv-installer/src/plan.rs @@ -247,7 +247,6 @@ impl<'a> Planner<'a> { let wheel = DirectUrlBuiltDist { filename, location: location.clone(), - subdirectory: subdirectory.clone(), url: url.clone(), }; diff --git a/crates/uv-resolver/src/lock.rs b/crates/uv-resolver/src/lock.rs index fe39500b2..5972c5e44 100644 --- a/crates/uv-resolver/src/lock.rs +++ b/crates/uv-resolver/src/lock.rs @@ -261,12 +261,11 @@ impl Distribution { let filename: WheelFilename = self.wheels[best_wheel_index].filename.clone(); let url = Url::from(ParsedArchiveUrl { url: self.id.source.url.clone(), - subdirectory: direct.subdirectory.as_ref().map(PathBuf::from), + subdirectory: None, }); let direct_dist = DirectUrlBuiltDist { filename, location: self.id.source.url.clone(), - subdirectory: direct.subdirectory.as_ref().map(PathBuf::from), url: VerbatimUrl::from_url(url), }; let built_dist = BuiltDist::DirectUrl(direct_dist); @@ -483,13 +482,7 @@ impl Source { fn from_direct_built_dist(direct_dist: &DirectUrlBuiltDist) -> Source { Source { - kind: SourceKind::Direct(DirectSource { - subdirectory: direct_dist - .subdirectory - .as_deref() - .and_then(Path::to_str) - .map(ToString::to_string), - }), + kind: SourceKind::Direct(DirectSource { subdirectory: None }), url: direct_dist.url.to_url(), } }