From 42c3bfa351cc2cc3d0325da20f6f5d97f2fcf7be Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Mon, 13 May 2024 10:03:14 -0400 Subject: [PATCH] Make `Directory` its own distribution kind (#3519) ## Summary I think this is overall good change because it explicitly encodes (in the type system) something that was previously implicit. I'm not a huge fan of the names here, open to input. It covers some of https://github.com/astral-sh/uv/issues/3506 but I don't think it _closes_ it. --- crates/distribution-types/src/buildable.rs | 26 +++- crates/distribution-types/src/cached.rs | 7 + crates/distribution-types/src/lib.rs | 87 ++++++++++-- crates/distribution-types/src/resolution.rs | 5 + crates/uv-cache/src/lib.rs | 78 ++++++----- .../src/index/built_wheel_index.rs | 38 +++++- crates/uv-distribution/src/source/mod.rs | 127 +++++++++--------- crates/uv-installer/src/plan.rs | 27 +++- crates/uv-requirements/src/source_tree.rs | 5 +- crates/uv-requirements/src/unnamed.rs | 20 ++- crates/uv-resolver/src/error.rs | 9 +- crates/uv-resolver/src/lock.rs | 40 ++++-- crates/uv-resolver/src/resolver/mod.rs | 14 +- ...r__lock__tests__hash_optional_missing.snap | 96 +++++++++++++ ...r__lock__tests__hash_required_missing.snap | 16 --- crates/uv/tests/pip_install.rs | 2 +- 16 files changed, 434 insertions(+), 163 deletions(-) create mode 100644 crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__hash_optional_missing.snap delete mode 100644 crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__hash_required_missing.snap diff --git a/crates/distribution-types/src/buildable.rs b/crates/distribution-types/src/buildable.rs index 42e7fe789..b82d40186 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 cc53ebdb8..3f4884def 100644 --- a/crates/distribution-types/src/cached.rs +++ b/crates/distribution-types/src/cached.rs @@ -85,6 +85,13 @@ impl CachedDist { editable: false, }), Dist::Source(SourceDist::Path(dist)) => Self::Url(CachedDirectUrlDist { + filename, + url: dist.url, + hashes, + path, + editable: false, + }), + Dist::Source(SourceDist::Directory(dist)) => Self::Url(CachedDirectUrlDist { filename, url: dist.url, hashes, diff --git a/crates/distribution-types/src/lib.rs b/crates/distribution-types/src/lib.rs index 0d8489187..ac3d9a2f1 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,12 +204,20 @@ 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, pub url: VerbatimUrl, pub path: PathBuf, +} + +/// 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, } @@ -281,7 +290,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")) { @@ -305,11 +322,14 @@ impl Dist { path, }))) } else { + if editable { + return Err(Error::EditableFile(url)); + } + Ok(Self::Source(SourceDist::Path(PathSourceDist { name, url, path, - editable, }))) } } @@ -382,7 +402,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 +474,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 +482,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, } } @@ -487,7 +507,7 @@ impl SourceDist { /// Return true if the distribution is editable. pub fn is_editable(&self) -> bool { match self { - Self::Path(PathSourceDist { editable, .. }) => *editable, + Self::Directory(DirectorySourceDist { editable, .. }) => *editable, _ => false, } } @@ -496,6 +516,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 +564,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 +577,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 +643,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 +656,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 +795,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 +812,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 +822,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 +981,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 +1008,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 +1018,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 +1097,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 +1122,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 b8b9ba770..0e9606329 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-cache/src/lib.rs b/crates/uv-cache/src/lib.rs index 3805d6cf8..97006cc16 100644 --- a/crates/uv-cache/src/lib.rs +++ b/crates/uv-cache/src/lib.rs @@ -770,41 +770,7 @@ impl ArchiveTimestamp { if metadata.is_file() { Ok(Some(Self::Exact(Timestamp::from_metadata(&metadata)))) } else { - // Compute the modification timestamp for the `pyproject.toml`, `setup.py`, and - // `setup.cfg` files, if they exist. - let pyproject_toml = path - .as_ref() - .join("pyproject.toml") - .metadata() - .ok() - .filter(std::fs::Metadata::is_file) - .as_ref() - .map(Timestamp::from_metadata); - - let setup_py = path - .as_ref() - .join("setup.py") - .metadata() - .ok() - .filter(std::fs::Metadata::is_file) - .as_ref() - .map(Timestamp::from_metadata); - - let setup_cfg = path - .as_ref() - .join("setup.cfg") - .metadata() - .ok() - .filter(std::fs::Metadata::is_file) - .as_ref() - .map(Timestamp::from_metadata); - - // Take the most recent timestamp of the three files. - let Some(timestamp) = max(pyproject_toml, max(setup_py, setup_cfg)) else { - return Ok(None); - }; - - Ok(Some(Self::Approximate(timestamp))) + Self::from_source_tree(path) } } @@ -814,6 +780,48 @@ impl ArchiveTimestamp { Ok(Self::Exact(Timestamp::from_metadata(&metadata))) } + /// Return the modification timestamp for a source tree, i.e., a directory. + /// + /// If the source tree doesn't contain an entrypoint (i.e., no `pyproject.toml`, `setup.py`, or + /// `setup.cfg`), returns `None`. + pub fn from_source_tree(path: impl AsRef) -> Result, io::Error> { + // Compute the modification timestamp for the `pyproject.toml`, `setup.py`, and + // `setup.cfg` files, if they exist. + let pyproject_toml = path + .as_ref() + .join("pyproject.toml") + .metadata() + .ok() + .filter(std::fs::Metadata::is_file) + .as_ref() + .map(Timestamp::from_metadata); + + let setup_py = path + .as_ref() + .join("setup.py") + .metadata() + .ok() + .filter(std::fs::Metadata::is_file) + .as_ref() + .map(Timestamp::from_metadata); + + let setup_cfg = path + .as_ref() + .join("setup.cfg") + .metadata() + .ok() + .filter(std::fs::Metadata::is_file) + .as_ref() + .map(Timestamp::from_metadata); + + // Take the most recent timestamp of the three files. + let Some(timestamp) = max(pyproject_toml, max(setup_py, setup_cfg)) else { + return Ok(None); + }; + + Ok(Some(Self::Approximate(timestamp))) + } + /// Return the modification timestamp for an archive. pub fn timestamp(&self) -> Timestamp { match self { diff --git a/crates/uv-distribution/src/index/built_wheel_index.rs b/crates/uv-distribution/src/index/built_wheel_index.rs index e77aec547..343399381 100644 --- a/crates/uv-distribution/src/index/built_wheel_index.rs +++ b/crates/uv-distribution/src/index/built_wheel_index.rs @@ -1,5 +1,5 @@ use distribution_types::{ - git_reference, DirectUrlSourceDist, GitSourceDist, Hashed, PathSourceDist, + git_reference, DirectUrlSourceDist, DirectorySourceDist, GitSourceDist, Hashed, PathSourceDist, }; use platform_tags::Tags; use uv_cache::{ArchiveTimestamp, Cache, CacheBucket, CacheShard, WheelCache}; @@ -67,9 +67,43 @@ impl<'a> BuiltWheelIndex<'a> { return Ok(None); }; + // Determine the last-modified time of the source distribution. + let modified = ArchiveTimestamp::from_file(&source_dist.path).map_err(Error::CacheRead)?; + + // If the distribution is stale, omit it from the index. + if !pointer.is_up_to_date(modified) { + return Ok(None); + } + + // Enforce hash-checking by omitting any wheels that don't satisfy the required hashes. + let revision = pointer.into_revision(); + if !revision.satisfies(self.hasher.get(source_dist)) { + return Ok(None); + } + + Ok(self.find(&cache_shard.shard(revision.id()))) + } + + /// Return the most compatible [`CachedWheel`] for a given source distribution built from a + /// local directory (source tree). + pub fn directory( + &self, + source_dist: &DirectorySourceDist, + ) -> Result, Error> { + let cache_shard = self.cache.shard( + CacheBucket::BuiltWheels, + WheelCache::Path(&source_dist.url).root(), + ); + + // Read the revision from the cache. + let Some(pointer) = LocalRevisionPointer::read_from(cache_shard.entry(LOCAL_REVISION))? + else { + return Ok(None); + }; + // Determine the last-modified time of the source distribution. let Some(modified) = - ArchiveTimestamp::from_path(&source_dist.path).map_err(Error::CacheRead)? + ArchiveTimestamp::from_source_tree(&source_dist.path).map_err(Error::CacheRead)? else { return Err(Error::DirWithoutEntrypoint); }; diff --git a/crates/uv-distribution/src/source/mod.rs b/crates/uv-distribution/src/source/mod.rs index 48eabd9b8..d2951fab7 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, DirectorySourceDist, DirectorySourceUrl, Dist, FileLocation, GitSourceUrl, + HashPolicy, Hashed, LocalEditable, ParsedArchiveUrl, 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,12 +965,12 @@ 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. let Some(modified) = - ArchiveTimestamp::from_path(&resource.path).map_err(Error::CacheRead)? + ArchiveTimestamp::from_source_tree(&resource.path).map_err(Error::CacheRead)? else { return Err(Error::DirWithoutEntrypoint); }; @@ -1432,8 +1430,9 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> { .await .map_err(|err| Error::BuildEditable(editable.to_string(), err))?; let filename = WheelFilename::from_str(&disk_filename)?; + // We finally have the name of the package and can construct the dist. - let dist = Dist::Source(SourceDist::Path(PathSourceDist { + let dist = Dist::Source(SourceDist::Directory(DirectorySourceDist { name: filename.name.clone(), url: editable.url().clone(), path: editable.path.clone(), diff --git a/crates/uv-installer/src/plan.rs b/crates/uv-installer/src/plan.rs index 80befcfd1..b4d13c94e 100644 --- a/crates/uv-installer/src/plan.rs +++ b/crates/uv-installer/src/plan.rs @@ -9,9 +9,10 @@ use tracing::{debug, warn}; use distribution_filename::WheelFilename; use distribution_types::{ - CachedDirectUrlDist, CachedDist, DirectUrlBuiltDist, DirectUrlSourceDist, Error, GitSourceDist, - Hashed, IndexLocations, InstalledDist, InstalledMetadata, InstalledVersion, Name, - PathBuiltDist, PathSourceDist, RemoteSource, Requirement, RequirementSource, Verbatim, + CachedDirectUrlDist, CachedDist, DirectUrlBuiltDist, DirectUrlSourceDist, DirectorySourceDist, + Error, GitSourceDist, Hashed, IndexLocations, InstalledDist, InstalledMetadata, + InstalledVersion, Name, PathBuiltDist, PathSourceDist, RemoteSource, Requirement, + RequirementSource, Verbatim, }; use platform_tags::Tags; use uv_cache::{ArchiveTimestamp, Cache, CacheBucket, WheelCache}; @@ -328,7 +329,23 @@ impl<'a> Planner<'a> { }; // Check if we have a wheel or a source distribution. - if path + if path.is_dir() { + let sdist = DirectorySourceDist { + name: requirement.name.clone(), + url: url.clone(), + path, + editable: false, + }; + + // Find the most-compatible wheel from the cache, since we don't know + // the filename in advance. + if let Some(wheel) = built_index.directory(&sdist)? { + let cached_dist = wheel.into_url_dist(url.clone()); + debug!("Path source requirement already cached: {cached_dist}"); + cached.push(CachedDist::Url(cached_dist)); + continue; + } + } else if path .extension() .is_some_and(|ext| ext.eq_ignore_ascii_case("whl")) { @@ -395,8 +412,8 @@ impl<'a> Planner<'a> { name: requirement.name.clone(), url: url.clone(), path, - editable: false, }; + // Find the most-compatible wheel from the cache, since we don't know // the filename in advance. if let Some(wheel) = built_index.path(&sdist)? { diff --git a/crates/uv-requirements/src/source_tree.rs b/crates/uv-requirements/src/source_tree.rs index 706bac3cf..0cbf5a9d7 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-requirements/src/unnamed.rs b/crates/uv-requirements/src/unnamed.rs index 7aa264d86..f535a89de 100644 --- a/crates/uv-requirements/src/unnamed.rs +++ b/crates/uv-requirements/src/unnamed.rs @@ -10,8 +10,9 @@ use tracing::debug; use distribution_filename::{SourceDistFilename, WheelFilename}; use distribution_types::{ - BuildableSource, DirectSourceUrl, GitSourceUrl, PathSourceUrl, RemoteSource, Requirement, - SourceUrl, UnresolvedRequirement, UnresolvedRequirementSpecification, VersionId, + BuildableSource, DirectSourceUrl, DirectorySourceUrl, GitSourceUrl, PathSourceUrl, + RemoteSource, Requirement, SourceUrl, UnresolvedRequirement, + UnresolvedRequirementSpecification, VersionId, }; use pep508_rs::{Scheme, UnnamedRequirement, VersionOrUrl}; use pypi_types::Metadata10; @@ -222,12 +223,17 @@ impl<'a, Context: BuildContext> NamedRequirementsResolver<'a, Context> { } } } - } - SourceUrl::Path(PathSourceUrl { - url: &requirement.url, - path: Cow::Owned(path), - }) + SourceUrl::Directory(DirectorySourceUrl { + url: &requirement.url, + path: Cow::Owned(path), + }) + } else { + SourceUrl::Path(PathSourceUrl { + url: &requirement.url, + path: Cow::Owned(path), + }) + } } Some(Scheme::Http | Scheme::Https) => SourceUrl::Direct(DirectSourceUrl { url: &requirement.url, diff --git a/crates/uv-resolver/src/error.rs b/crates/uv-resolver/src/error.rs index daf4a57c4..8031f944c 100644 --- a/crates/uv-resolver/src/error.rs +++ b/crates/uv-resolver/src/error.rs @@ -9,10 +9,7 @@ use pubgrub::range::Range; use pubgrub::report::{DefaultStringReporter, DerivationTree, External, Reporter}; use rustc_hash::FxHashMap; -use distribution_types::{ - BuiltDist, IndexLocations, InstalledDist, ParsedUrlError, PathBuiltDist, PathSourceDist, - SourceDist, -}; +use distribution_types::{BuiltDist, IndexLocations, InstalledDist, ParsedUrlError, SourceDist}; use once_map::OnceMap; use pep440_rs::Version; use pep508_rs::Requirement; @@ -75,14 +72,14 @@ pub enum ResolveError { FetchAndBuild(Box, #[source] uv_distribution::Error), #[error("Failed to read `{0}`")] - Read(Box, #[source] uv_distribution::Error), + Read(Box, #[source] uv_distribution::Error), // TODO(zanieb): Use `thiserror` in `InstalledDist` so we can avoid chaining `anyhow` #[error("Failed to read metadata from installed package `{0}`")] ReadInstalled(Box, #[source] anyhow::Error), #[error("Failed to build `{0}`")] - Build(Box, #[source] uv_distribution::Error), + Build(Box, #[source] uv_distribution::Error), #[error(transparent)] NoSolution(#[from] NoSolutionError), diff --git a/crates/uv-resolver/src/lock.rs b/crates/uv-resolver/src/lock.rs index bf81dc161..f67b092a9 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: @@ -1148,7 +1166,7 @@ url = "https://files.pythonhosted.org/packages/14/fd/2f20c40b45e4fb4324834aea24b } #[test] - fn hash_required_missing() { + fn hash_optional_missing() { let data = r#" version = 1 diff --git a/crates/uv-resolver/src/resolver/mod.rs b/crates/uv-resolver/src/resolver/mod.rs index ad4a33807..9514b1769 100644 --- a/crates/uv-resolver/src/resolver/mod.rs +++ b/crates/uv-resolver/src/resolver/mod.rs @@ -1224,10 +1224,13 @@ impl<'a, Provider: ResolverProvider, InstalledPackages: InstalledPackagesProvide .boxed_local() .await .map_err(|err| match dist.clone() { - Dist::Built(BuiltDist::Path(built_dist)) => { + Dist::Built(built_dist @ BuiltDist::Path(_)) => { ResolveError::Read(Box::new(built_dist), err) } - Dist::Source(SourceDist::Path(source_dist)) => { + Dist::Source(source_dist @ SourceDist::Path(_)) => { + ResolveError::Build(Box::new(source_dist), err) + } + Dist::Source(source_dist @ SourceDist::Directory(_)) => { ResolveError::Build(Box::new(source_dist), err) } Dist::Built(built_dist) => ResolveError::Fetch(Box::new(built_dist), err), @@ -1311,10 +1314,13 @@ impl<'a, Provider: ResolverProvider, InstalledPackages: InstalledPackagesProvide .boxed_local() .await .map_err(|err| match dist.clone() { - Dist::Built(BuiltDist::Path(built_dist)) => { + Dist::Built(built_dist @ BuiltDist::Path(_)) => { ResolveError::Read(Box::new(built_dist), err) } - Dist::Source(SourceDist::Path(source_dist)) => { + Dist::Source(source_dist @ SourceDist::Path(_)) => { + ResolveError::Build(Box::new(source_dist), err) + } + Dist::Source(source_dist @ SourceDist::Directory(_)) => { ResolveError::Build(Box::new(source_dist), err) } Dist::Built(built_dist) => { diff --git a/crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__hash_optional_missing.snap b/crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__hash_optional_missing.snap new file mode 100644 index 000000000..c8479f99d --- /dev/null +++ b/crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__hash_optional_missing.snap @@ -0,0 +1,96 @@ +--- +source: crates/uv-resolver/src/lock.rs +expression: result +--- +Ok( + Lock { + version: 1, + distributions: [ + Distribution { + id: DistributionId { + name: PackageName( + "anyio", + ), + version: "4.3.0", + source: Source { + kind: Path, + url: Url { + scheme: "file", + cannot_be_a_base: false, + username: "", + password: None, + host: None, + port: None, + path: "/foo/bar", + query: None, + fragment: None, + }, + }, + }, + marker: None, + sourcedist: None, + wheels: [ + Wheel { + url: Url { + scheme: "file", + cannot_be_a_base: false, + username: "", + password: None, + host: None, + port: None, + path: "/foo/bar/anyio-4.3.0-py3-none-any.whl", + query: None, + fragment: None, + }, + hash: Some( + Hash( + HashDigest { + algorithm: Sha256, + digest: "048e05d0f6caeed70d731f3db756d35dcc1f35747c8c403364a8332c630441b8", + }, + ), + ), + filename: WheelFilename { + name: PackageName( + "anyio", + ), + version: "4.3.0", + python_tag: [ + "py3", + ], + abi_tag: [ + "none", + ], + platform_tag: [ + "any", + ], + }, + }, + ], + dependencies: [], + }, + ], + by_id: { + DistributionId { + name: PackageName( + "anyio", + ), + version: "4.3.0", + source: Source { + kind: Path, + url: Url { + scheme: "file", + cannot_be_a_base: false, + username: "", + password: None, + host: None, + port: None, + path: "/foo/bar", + query: None, + fragment: None, + }, + }, + }: 0, + }, + }, +) diff --git a/crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__hash_required_missing.snap b/crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__hash_required_missing.snap deleted file mode 100644 index 5947d2ea2..000000000 --- a/crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__hash_required_missing.snap +++ /dev/null @@ -1,16 +0,0 @@ ---- -source: crates/uv-resolver/src/lock.rs -expression: result ---- -Err( - Error { - inner: Error { - inner: TomlError { - message: "since the distribution `anyio 4.3.0 path+file:///foo/bar` comes from a path dependency, a hash was not expected but one was found for wheel", - raw: None, - keys: [], - span: None, - }, - }, - }, -) diff --git a/crates/uv/tests/pip_install.rs b/crates/uv/tests/pip_install.rs index 2299cb147..d395d3013 100644 --- a/crates/uv/tests/pip_install.rs +++ b/crates/uv/tests/pip_install.rs @@ -2609,7 +2609,7 @@ requires-python = ">=3.8" "### ); - // Modify the editable package. + // Modify the package. pyproject_toml.write_str( r#"[project] name = "example"