From f20d3b57e962f507c9d506f41a02bd2267b85e78 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Fri, 10 May 2024 17:13:33 -0400 Subject: [PATCH] Make Directory its own distribution kind --- crates/distribution-types/src/buildable.rs | 26 ++++- crates/distribution-types/src/cached.rs | 7 ++ crates/distribution-types/src/lib.rs | 81 ++++++++++++- crates/distribution-types/src/resolution.rs | 5 + crates/uv-distribution/src/source/mod.rs | 122 ++++++++++---------- crates/uv-requirements/src/source_tree.rs | 5 +- crates/uv-resolver/src/lock.rs | 38 ++++-- 7 files changed, 202 insertions(+), 82 deletions(-) diff --git a/crates/distribution-types/src/buildable.rs b/crates/distribution-types/src/buildable.rs index 42e7fe789e1..b82d401867e 100644 --- a/crates/distribution-types/src/buildable.rs +++ b/crates/distribution-types/src/buildable.rs @@ -6,7 +6,7 @@ use url::Url; use uv_normalize::PackageName; -use crate::{GitSourceDist, Name, PathSourceDist, SourceDist}; +use crate::{DirectorySourceDist, GitSourceDist, Name, PathSourceDist, SourceDist}; /// A reference to a source that can be built into a built distribution. /// @@ -62,6 +62,7 @@ pub enum SourceUrl<'a> { Direct(DirectSourceUrl<'a>), Git(GitSourceUrl<'a>), Path(PathSourceUrl<'a>), + Directory(DirectorySourceUrl<'a>), } impl<'a> SourceUrl<'a> { @@ -71,6 +72,7 @@ impl<'a> SourceUrl<'a> { Self::Direct(dist) => dist.url, Self::Git(dist) => dist.url, Self::Path(dist) => dist.url, + Self::Directory(dist) => dist.url, } } } @@ -81,6 +83,7 @@ impl std::fmt::Display for SourceUrl<'_> { Self::Direct(url) => write!(f, "{url}"), Self::Git(url) => write!(f, "{url}"), Self::Path(url) => write!(f, "{url}"), + Self::Directory(url) => write!(f, "{url}"), } } } @@ -133,3 +136,24 @@ impl<'a> From<&'a PathSourceDist> for PathSourceUrl<'a> { } } } + +#[derive(Debug, Clone)] +pub struct DirectorySourceUrl<'a> { + pub url: &'a Url, + pub path: Cow<'a, Path>, +} + +impl std::fmt::Display for DirectorySourceUrl<'_> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{url}", url = self.url) + } +} + +impl<'a> From<&'a DirectorySourceDist> for DirectorySourceUrl<'a> { + fn from(dist: &'a DirectorySourceDist) -> Self { + Self { + url: &dist.url, + path: Cow::Borrowed(&dist.path), + } + } +} diff --git a/crates/distribution-types/src/cached.rs b/crates/distribution-types/src/cached.rs index cc53ebdb872..432b380ff3d 100644 --- a/crates/distribution-types/src/cached.rs +++ b/crates/distribution-types/src/cached.rs @@ -91,6 +91,13 @@ impl CachedDist { path, editable: dist.editable, }), + Dist::Source(SourceDist::Directory(dist)) => Self::Url(CachedDirectUrlDist { + filename, + url: dist.url, + hashes, + path, + editable: dist.editable, + }), } } diff --git a/crates/distribution-types/src/lib.rs b/crates/distribution-types/src/lib.rs index ad6d75a5dff..5606dd3e43c 100644 --- a/crates/distribution-types/src/lib.rs +++ b/crates/distribution-types/src/lib.rs @@ -152,6 +152,7 @@ pub enum SourceDist { DirectUrl(DirectUrlSourceDist), Git(GitSourceDist), Path(PathSourceDist), + Directory(DirectorySourceDist), } /// A built distribution (wheel) that exists in a registry, like `PyPI`. @@ -203,7 +204,7 @@ pub struct GitSourceDist { pub url: VerbatimUrl, } -/// A source distribution that exists in a local directory. +/// A source distribution that exists in a local archive (e.g., a `.tar.gz` file). #[derive(Debug, Clone)] pub struct PathSourceDist { pub name: PackageName, @@ -212,6 +213,15 @@ pub struct PathSourceDist { pub editable: bool, } +/// A source distribution that exists in a local directory. +#[derive(Debug, Clone)] +pub struct DirectorySourceDist { + pub name: PackageName, + pub url: VerbatimUrl, + pub path: PathBuf, + pub editable: bool, +} + impl Dist { /// Create a [`Dist`] for a registry-based distribution. pub fn from_registry(filename: DistFilename, file: File, index: IndexUrl) -> Self { @@ -281,7 +291,15 @@ impl Dist { Err(err) => return Err(err.into()), }; - if path + // Determine whether the path represents an archive or a directory. + if path.is_dir() { + Ok(Self::Source(SourceDist::Directory(DirectorySourceDist { + name, + url, + path, + editable, + }))) + } else if path .extension() .is_some_and(|ext| ext.eq_ignore_ascii_case("whl")) { @@ -382,7 +400,7 @@ impl Dist { /// Create a [`Dist`] for a local editable distribution. pub fn from_editable(name: PackageName, editable: LocalEditable) -> Result { let LocalEditable { url, path, .. } = editable; - Ok(Self::Source(SourceDist::Path(PathSourceDist { + Ok(Self::Source(SourceDist::Directory(DirectorySourceDist { name, url, path, @@ -454,7 +472,7 @@ impl SourceDist { pub fn index(&self) -> Option<&IndexUrl> { match self { Self::Registry(registry) => Some(®istry.index), - Self::DirectUrl(_) | Self::Git(_) | Self::Path(_) => None, + Self::DirectUrl(_) | Self::Git(_) | Self::Path(_) | Self::Directory(_) => None, } } @@ -462,14 +480,14 @@ impl SourceDist { pub fn file(&self) -> Option<&File> { match self { Self::Registry(registry) => Some(®istry.file), - Self::DirectUrl(_) | Self::Git(_) | Self::Path(_) => None, + Self::DirectUrl(_) | Self::Git(_) | Self::Path(_) | Self::Directory(_) => None, } } pub fn version(&self) -> Option<&Version> { match self { Self::Registry(source_dist) => Some(&source_dist.filename.version), - Self::DirectUrl(_) | Self::Git(_) | Self::Path(_) => None, + Self::DirectUrl(_) | Self::Git(_) | Self::Path(_) | Self::Directory(_) => None, } } @@ -496,6 +514,7 @@ impl SourceDist { pub fn as_path(&self) -> Option<&Path> { match self { Self::Path(dist) => Some(&dist.path), + Self::Directory(dist) => Some(&dist.path), _ => None, } } @@ -543,6 +562,12 @@ impl Name for PathSourceDist { } } +impl Name for DirectorySourceDist { + fn name(&self) -> &PackageName { + &self.name + } +} + impl Name for SourceDist { fn name(&self) -> &PackageName { match self { @@ -550,6 +575,7 @@ impl Name for SourceDist { Self::DirectUrl(dist) => dist.name(), Self::Git(dist) => dist.name(), Self::Path(dist) => dist.name(), + Self::Directory(dist) => dist.name(), } } } @@ -615,6 +641,12 @@ impl DistributionMetadata for PathSourceDist { } } +impl DistributionMetadata for DirectorySourceDist { + fn version_or_url(&self) -> VersionOrUrlRef { + VersionOrUrlRef::Url(&self.url) + } +} + impl DistributionMetadata for SourceDist { fn version_or_url(&self) -> VersionOrUrlRef { match self { @@ -622,6 +654,7 @@ impl DistributionMetadata for SourceDist { Self::DirectUrl(dist) => dist.version_or_url(), Self::Git(dist) => dist.version_or_url(), Self::Path(dist) => dist.version_or_url(), + Self::Directory(dist) => dist.version_or_url(), } } } @@ -760,6 +793,16 @@ impl RemoteSource for PathSourceDist { } } +impl RemoteSource for DirectorySourceDist { + fn filename(&self) -> Result, Error> { + self.url.filename() + } + + fn size(&self) -> Option { + self.url.size() + } +} + impl RemoteSource for SourceDist { fn filename(&self) -> Result, Error> { match self { @@ -767,6 +810,7 @@ impl RemoteSource for SourceDist { Self::DirectUrl(dist) => dist.filename(), Self::Git(dist) => dist.filename(), Self::Path(dist) => dist.filename(), + Self::Directory(dist) => dist.filename(), } } @@ -776,6 +820,7 @@ impl RemoteSource for SourceDist { Self::DirectUrl(dist) => dist.size(), Self::Git(dist) => dist.size(), Self::Path(dist) => dist.size(), + Self::Directory(dist) => dist.size(), } } } @@ -934,6 +979,16 @@ impl Identifier for PathSourceDist { } } +impl Identifier for DirectorySourceDist { + fn distribution_id(&self) -> DistributionId { + self.url.distribution_id() + } + + fn resource_id(&self) -> ResourceId { + self.url.resource_id() + } +} + impl Identifier for GitSourceDist { fn distribution_id(&self) -> DistributionId { self.url.distribution_id() @@ -951,6 +1006,7 @@ impl Identifier for SourceDist { Self::DirectUrl(dist) => dist.distribution_id(), Self::Git(dist) => dist.distribution_id(), Self::Path(dist) => dist.distribution_id(), + Self::Directory(dist) => dist.distribution_id(), } } @@ -960,6 +1016,7 @@ impl Identifier for SourceDist { Self::DirectUrl(dist) => dist.resource_id(), Self::Git(dist) => dist.resource_id(), Self::Path(dist) => dist.resource_id(), + Self::Directory(dist) => dist.resource_id(), } } } @@ -1038,12 +1095,23 @@ impl Identifier for PathSourceUrl<'_> { } } +impl Identifier for DirectorySourceUrl<'_> { + fn distribution_id(&self) -> DistributionId { + self.url.distribution_id() + } + + fn resource_id(&self) -> ResourceId { + self.url.resource_id() + } +} + impl Identifier for SourceUrl<'_> { fn distribution_id(&self) -> DistributionId { match self { Self::Direct(url) => url.distribution_id(), Self::Git(url) => url.distribution_id(), Self::Path(url) => url.distribution_id(), + Self::Directory(url) => url.distribution_id(), } } @@ -1052,6 +1120,7 @@ impl Identifier for SourceUrl<'_> { Self::Direct(url) => url.resource_id(), Self::Git(url) => url.resource_id(), Self::Path(url) => url.resource_id(), + Self::Directory(url) => url.resource_id(), } } } diff --git a/crates/distribution-types/src/resolution.rs b/crates/distribution-types/src/resolution.rs index b8b9ba77067..0e9606329c0 100644 --- a/crates/distribution-types/src/resolution.rs +++ b/crates/distribution-types/src/resolution.rs @@ -126,6 +126,11 @@ impl From<&ResolvedDist> for Requirement { url: sdist.url.clone(), editable: None, }, + Dist::Source(SourceDist::Directory(sdist)) => RequirementSource::Path { + path: sdist.path.clone(), + url: sdist.url.clone(), + editable: None, + }, }, ResolvedDist::Installed(dist) => RequirementSource::Registry { specifier: pep440_rs::VersionSpecifiers::from( diff --git a/crates/uv-distribution/src/source/mod.rs b/crates/uv-distribution/src/source/mod.rs index 48eabd9b8cb..9d12385b817 100644 --- a/crates/uv-distribution/src/source/mod.rs +++ b/crates/uv-distribution/src/source/mod.rs @@ -16,8 +16,9 @@ use zip::ZipArchive; use distribution_filename::WheelFilename; use distribution_types::{ - BuildableSource, Dist, FileLocation, GitSourceUrl, HashPolicy, Hashed, LocalEditable, - ParsedArchiveUrl, PathSourceDist, PathSourceUrl, RemoteSource, SourceDist, SourceUrl, + BuildableSource, DirectorySourceUrl, Dist, FileLocation, GitSourceUrl, HashPolicy, Hashed, + LocalEditable, ParsedArchiveUrl, PathSourceDist, PathSourceUrl, RemoteSource, SourceDist, + SourceUrl, }; use install_wheel_rs::metadata::read_archive_metadata; use platform_tags::Tags; @@ -163,26 +164,25 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> { .boxed_local() .await? } - BuildableSource::Dist(SourceDist::Path(dist)) => { - if dist.path.is_dir() { - self.source_tree(source, &PathSourceUrl::from(dist), tags, hashes) - .boxed_local() - .await? - } else { - let cache_shard = self - .build_context - .cache() - .shard(CacheBucket::BuiltWheels, WheelCache::Path(&dist.url).root()); - self.archive( - source, - &PathSourceUrl::from(dist), - &cache_shard, - tags, - hashes, - ) + BuildableSource::Dist(SourceDist::Directory(dist)) => { + self.source_tree(source, &DirectorySourceUrl::from(dist), tags, hashes) .boxed_local() .await? - } + } + BuildableSource::Dist(SourceDist::Path(dist)) => { + let cache_shard = self + .build_context + .cache() + .shard(CacheBucket::BuiltWheels, WheelCache::Path(&dist.url).root()); + self.archive( + source, + &PathSourceUrl::from(dist), + &cache_shard, + tags, + hashes, + ) + .boxed_local() + .await? } BuildableSource::Url(SourceUrl::Direct(resource)) => { let filename = resource @@ -216,20 +216,19 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> { .boxed_local() .await? } + BuildableSource::Url(SourceUrl::Directory(resource)) => { + self.source_tree(source, resource, tags, hashes) + .boxed_local() + .await? + } BuildableSource::Url(SourceUrl::Path(resource)) => { - if resource.path.is_dir() { - self.source_tree(source, resource, tags, hashes) - .boxed_local() - .await? - } else { - let cache_shard = self.build_context.cache().shard( - CacheBucket::BuiltWheels, - WheelCache::Path(resource.url).root(), - ); - self.archive(source, resource, &cache_shard, tags, hashes) - .boxed_local() - .await? - } + let cache_shard = self.build_context.cache().shard( + CacheBucket::BuiltWheels, + WheelCache::Path(resource.url).root(), + ); + self.archive(source, resource, &cache_shard, tags, hashes) + .boxed_local() + .await? } }; @@ -319,20 +318,19 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> { .boxed_local() .await? } + BuildableSource::Dist(SourceDist::Directory(dist)) => { + self.source_tree_metadata(source, &DirectorySourceUrl::from(dist), hashes) + .boxed_local() + .await? + } BuildableSource::Dist(SourceDist::Path(dist)) => { - if dist.path.is_dir() { - self.source_tree_metadata(source, &PathSourceUrl::from(dist), hashes) - .boxed_local() - .await? - } else { - let cache_shard = self - .build_context - .cache() - .shard(CacheBucket::BuiltWheels, WheelCache::Path(&dist.url).root()); - self.archive_metadata(source, &PathSourceUrl::from(dist), &cache_shard, hashes) - .boxed_local() - .await? - } + let cache_shard = self + .build_context + .cache() + .shard(CacheBucket::BuiltWheels, WheelCache::Path(&dist.url).root()); + self.archive_metadata(source, &PathSourceUrl::from(dist), &cache_shard, hashes) + .boxed_local() + .await? } BuildableSource::Url(SourceUrl::Direct(resource)) => { let filename = resource @@ -365,20 +363,20 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> { .boxed_local() .await? } + BuildableSource::Url(SourceUrl::Directory(resource)) => { + self.source_tree_metadata(source, resource, hashes) + .boxed_local() + .await? + } + BuildableSource::Url(SourceUrl::Path(resource)) => { - if resource.path.is_dir() { - self.source_tree_metadata(source, resource, hashes) - .boxed_local() - .await? - } else { - let cache_shard = self.build_context.cache().shard( - CacheBucket::BuiltWheels, - WheelCache::Path(resource.url).root(), - ); - self.archive_metadata(source, resource, &cache_shard, hashes) - .boxed_local() - .await? - } + let cache_shard = self.build_context.cache().shard( + CacheBucket::BuiltWheels, + WheelCache::Path(resource.url).root(), + ); + self.archive_metadata(source, resource, &cache_shard, hashes) + .boxed_local() + .await? } }; @@ -826,7 +824,7 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> { async fn source_tree( &self, source: &BuildableSource<'_>, - resource: &PathSourceUrl<'_>, + resource: &DirectorySourceUrl<'_>, tags: &Tags, hashes: HashPolicy<'_>, ) -> Result { @@ -891,7 +889,7 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> { async fn source_tree_metadata( &self, source: &BuildableSource<'_>, - resource: &PathSourceUrl<'_>, + resource: &DirectorySourceUrl<'_>, hashes: HashPolicy<'_>, ) -> Result { // Before running the build, check that the hashes match. @@ -967,7 +965,7 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> { async fn source_tree_revision( &self, source: &BuildableSource<'_>, - resource: &PathSourceUrl<'_>, + resource: &DirectorySourceUrl<'_>, cache_shard: &CacheShard, ) -> Result { // Determine the last-modified time of the source distribution. diff --git a/crates/uv-requirements/src/source_tree.rs b/crates/uv-requirements/src/source_tree.rs index 706bac3cfc4..0cbf5a9d754 100644 --- a/crates/uv-requirements/src/source_tree.rs +++ b/crates/uv-requirements/src/source_tree.rs @@ -7,10 +7,9 @@ use futures::TryStreamExt; use url::Url; use distribution_types::{ - BuildableSource, HashPolicy, PathSourceUrl, Requirement, SourceUrl, VersionId, + BuildableSource, DirectorySourceUrl, HashPolicy, Requirement, SourceUrl, VersionId, }; use pep508_rs::RequirementOrigin; - use uv_distribution::{DistributionDatabase, Reporter}; use uv_fs::Simplified; use uv_resolver::{InMemoryIndex, MetadataResponse}; @@ -97,7 +96,7 @@ impl<'a, Context: BuildContext> SourceTreeResolver<'a, Context> { let Ok(url) = Url::from_directory_path(source_tree) else { return Err(anyhow::anyhow!("Failed to convert path to URL")); }; - let source = SourceUrl::Path(PathSourceUrl { + let source = SourceUrl::Directory(DirectorySourceUrl { url: &url, path: Cow::Borrowed(source_tree), }); diff --git a/crates/uv-resolver/src/lock.rs b/crates/uv-resolver/src/lock.rs index bf81dc1613e..f958fdca40f 100644 --- a/crates/uv-resolver/src/lock.rs +++ b/crates/uv-resolver/src/lock.rs @@ -6,9 +6,10 @@ use std::collections::VecDeque; use distribution_filename::WheelFilename; use distribution_types::{ - BuiltDist, DirectUrlBuiltDist, DirectUrlSourceDist, Dist, DistributionMetadata, FileLocation, - GitSourceDist, IndexUrl, Name, PathBuiltDist, PathSourceDist, RegistryBuiltDist, - RegistrySourceDist, Resolution, ResolvedDist, ToUrlError, VersionOrUrlRef, + BuiltDist, DirectUrlBuiltDist, DirectUrlSourceDist, DirectorySourceDist, Dist, + DistributionMetadata, FileLocation, GitSourceDist, IndexUrl, Name, PathBuiltDist, + PathSourceDist, RegistryBuiltDist, RegistrySourceDist, Resolution, ResolvedDist, ToUrlError, + VersionOrUrlRef, }; use pep440_rs::Version; use pep508_rs::{MarkerEnvironment, VerbatimUrl}; @@ -360,6 +361,9 @@ impl Source { distribution_types::SourceDist::Path(ref path_dist) => { Source::from_path_source_dist(path_dist) } + distribution_types::SourceDist::Directory(ref directory) => { + Source::from_directory_source_dist(directory) + } } } @@ -399,6 +403,13 @@ impl Source { } } + fn from_directory_source_dist(directory_dist: &DirectorySourceDist) -> Source { + Source { + kind: SourceKind::Directory, + url: directory_dist.url.to_url(), + } + } + fn from_index_url(index_url: &IndexUrl) -> Source { match *index_url { IndexUrl::Pypi(ref verbatim_url) => Source { @@ -497,6 +508,7 @@ pub(crate) enum SourceKind { Git(GitSource), Direct, Path, + Directory, } impl SourceKind { @@ -506,6 +518,7 @@ impl SourceKind { SourceKind::Git(_) => "git", SourceKind::Direct => "direct", SourceKind::Path => "path", + SourceKind::Directory => "directory", } } @@ -515,13 +528,8 @@ impl SourceKind { /// _not_ be present. fn requires_hash(&self) -> bool { match *self { - SourceKind::Registry | SourceKind::Direct => true, - // TODO: A `Path` dependency, if it points to a specific source - // distribution or wheel, should have a hash. But if it points to a - // directory, then it should not have a hash. - // - // See: https://github.com/astral-sh/uv/issues/3506 - SourceKind::Git(_) | SourceKind::Path => false, + SourceKind::Registry | SourceKind::Direct | SourceKind::Path => true, + SourceKind::Git(_) | SourceKind::Directory => false, } } } @@ -620,6 +628,9 @@ impl SourceDist { distribution_types::SourceDist::Path(ref path_dist) => { Ok(SourceDist::from_path_dist(path_dist)) } + distribution_types::SourceDist::Directory(ref directory_dist) => { + Ok(SourceDist::from_directory_dist(directory_dist)) + } } } @@ -659,6 +670,13 @@ impl SourceDist { hash: None, } } + + fn from_directory_dist(directory_dist: &DirectorySourceDist) -> SourceDist { + SourceDist { + url: directory_dist.url.to_url(), + hash: None, + } + } } /// Inspired by: