diff --git a/crates/puffin-distribution/src/distribution_database.rs b/crates/puffin-distribution/src/distribution_database.rs index 83f719287..1ba253c8f 100644 --- a/crates/puffin-distribution/src/distribution_database.rs +++ b/crates/puffin-distribution/src/distribution_database.rs @@ -4,8 +4,6 @@ use std::path::Path; use std::sync::Arc; use futures::{FutureExt, TryStreamExt}; -use thiserror::Error; -use tokio::task::JoinError; use tokio_util::compat::FuturesAsyncReadCompatExt; use tracing::{info_span, instrument, Instrument}; use url::Url; @@ -25,29 +23,7 @@ use pypi_types::Metadata21; use crate::download::{BuiltWheel, UnzippedWheel}; use crate::locks::Locks; use crate::reporter::Facade; -use crate::{DiskWheel, LocalWheel, Reporter, SourceDistCachedBuilder, SourceDistError}; - -#[derive(Debug, Error)] -pub enum DistributionDatabaseError { - #[error("Failed to parse URL: {0}")] - Url(String, #[source] url::ParseError), - #[error(transparent)] - Client(#[from] puffin_client::Error), - #[error(transparent)] - Request(#[from] reqwest::Error), - #[error(transparent)] - Io(#[from] io::Error), - #[error(transparent)] - SourceBuild(#[from] SourceDistError), - #[error("Git operation failed")] - Git(#[source] anyhow::Error), - #[error("The task executor is broken, did some other task panic?")] - Join(#[from] JoinError), - #[error("Building source distributions is disabled")] - NoBuild, - #[error("Using pre-built wheels is disabled")] - NoBinary, -} +use crate::{DiskWheel, Error, LocalWheel, Reporter, SourceDistCachedBuilder}; /// A cached high-level interface to convert distributions (a requirement resolved to a location) /// to a wheel or wheel metadata. @@ -103,10 +79,7 @@ impl<'a, Context: BuildContext + Send + Sync> DistributionDatabase<'a, Context> /// If `no_remote_wheel` is set, the wheel will be built from a source distribution /// even if compatible pre-built wheels are available. #[instrument(skip(self))] - pub async fn get_or_build_wheel( - &self, - dist: Dist, - ) -> Result { + pub async fn get_or_build_wheel(&self, dist: Dist) -> Result { let no_binary = match self.build_context.no_binary() { NoBinary::None => false, NoBinary::All => true, @@ -115,15 +88,16 @@ impl<'a, Context: BuildContext + Send + Sync> DistributionDatabase<'a, Context> match &dist { Dist::Built(BuiltDist::Registry(wheel)) => { if no_binary { - return Err(DistributionDatabaseError::NoBinary); + return Err(Error::NoBinary); } let url = match &wheel.file.url { FileLocation::RelativeUrl(base, url) => base .join_relative(url) - .map_err(|err| DistributionDatabaseError::Url(url.clone(), err))?, - FileLocation::AbsoluteUrl(url) => Url::parse(url) - .map_err(|err| DistributionDatabaseError::Url(url.clone(), err))?, + .map_err(|err| Error::Url(url.clone(), err))?, + FileLocation::AbsoluteUrl(url) => { + Url::parse(url).map_err(|err| Error::Url(url.clone(), err))? + } FileLocation::Path(path) => { let url = Url::from_file_path(path).expect("path is absolute"); let cache_entry = self.cache.entry( @@ -136,9 +110,10 @@ impl<'a, Context: BuildContext + Send + Sync> DistributionDatabase<'a, Context> // return it. match cache_entry.path().canonicalize() { Ok(archive) => { - if let (Some(cache_metadata), Some(path_metadata)) = - (metadata_if_exists(&archive)?, metadata_if_exists(path)?) - { + if let (Some(cache_metadata), Some(path_metadata)) = ( + metadata_if_exists(&archive).map_err(Error::CacheRead)?, + metadata_if_exists(path).map_err(Error::CacheRead)?, + ) { let cache_modified = Timestamp::from_metadata(&cache_metadata); let path_modified = Timestamp::from_metadata(&path_metadata); if cache_modified >= path_modified { @@ -151,7 +126,7 @@ impl<'a, Context: BuildContext + Send + Sync> DistributionDatabase<'a, Context> } } Err(err) if err.kind() == io::ErrorKind::NotFound => {} - Err(err) => return Err(err.into()), + Err(err) => return Err(Error::CacheRead(err)), } // Otherwise, unzip the file. @@ -180,20 +155,26 @@ impl<'a, Context: BuildContext + Send + Sync> DistributionDatabase<'a, Context> .into_async_read(); // Download and unzip the wheel to a temporary directory. - let temp_dir = tempfile::tempdir_in(self.cache.root())?; + let temp_dir = + tempfile::tempdir_in(self.cache.root()).map_err(Error::CacheWrite)?; unzip_no_seek(reader.compat(), temp_dir.path()).await?; // Persist the temporary directory to the directory store. - Ok(self + let archive = self .cache - .persist(temp_dir.into_path(), wheel_entry.path())?) + .persist(temp_dir.into_path(), wheel_entry.path()) + .map_err(Error::CacheRead)?; + Ok(archive) } .instrument(info_span!("download", wheel = %wheel)) }; let req = self.client.cached_client().uncached().get(url).build()?; - let cache_control = - CacheControl::from(self.cache.freshness(&http_entry, Some(wheel.name()))?); + let cache_control = CacheControl::from( + self.cache + .freshness(&http_entry, Some(wheel.name())) + .map_err(Error::CacheRead)?, + ); let archive = self .client .cached_client() @@ -201,7 +182,7 @@ impl<'a, Context: BuildContext + Send + Sync> DistributionDatabase<'a, Context> .await .map_err(|err| match err { CachedClientError::Callback(err) => err, - CachedClientError::Client(err) => SourceDistError::Client(err), + CachedClientError::Client(err) => Error::Client(err), })?; Ok(LocalWheel::Unzipped(UnzippedWheel { @@ -213,7 +194,7 @@ impl<'a, Context: BuildContext + Send + Sync> DistributionDatabase<'a, Context> Dist::Built(BuiltDist::DirectUrl(wheel)) => { if no_binary { - return Err(DistributionDatabaseError::NoBinary); + return Err(Error::NoBinary); } // Create an entry for the wheel itself alongside its HTTP cache. @@ -232,13 +213,16 @@ impl<'a, Context: BuildContext + Send + Sync> DistributionDatabase<'a, Context> .into_async_read(); // Download and unzip the wheel to a temporary directory. - let temp_dir = tempfile::tempdir_in(self.cache.root())?; + let temp_dir = + tempfile::tempdir_in(self.cache.root()).map_err(Error::CacheWrite)?; unzip_no_seek(reader.compat(), temp_dir.path()).await?; // Persist the temporary directory to the directory store. - Ok(self + let archive = self .cache - .persist(temp_dir.into_path(), wheel_entry.path())?) + .persist(temp_dir.into_path(), wheel_entry.path()) + .map_err(Error::CacheRead)?; + Ok(archive) } .instrument(info_span!("download", wheel = %wheel)) }; @@ -249,8 +233,11 @@ impl<'a, Context: BuildContext + Send + Sync> DistributionDatabase<'a, Context> .uncached() .get(wheel.url.raw().clone()) .build()?; - let cache_control = - CacheControl::from(self.cache.freshness(&http_entry, Some(wheel.name()))?); + let cache_control = CacheControl::from( + self.cache + .freshness(&http_entry, Some(wheel.name())) + .map_err(Error::CacheRead)?, + ); let archive = self .client .cached_client() @@ -258,7 +245,7 @@ impl<'a, Context: BuildContext + Send + Sync> DistributionDatabase<'a, Context> .await .map_err(|err| match err { CachedClientError::Callback(err) => err, - CachedClientError::Client(err) => SourceDistError::Client(err), + CachedClientError::Client(err) => Error::Client(err), })?; Ok(LocalWheel::Unzipped(UnzippedWheel { @@ -270,7 +257,7 @@ impl<'a, Context: BuildContext + Send + Sync> DistributionDatabase<'a, Context> Dist::Built(BuiltDist::Path(wheel)) => { if no_binary { - return Err(DistributionDatabaseError::NoBinary); + return Err(Error::NoBinary); } let cache_entry = self.cache.entry( @@ -284,8 +271,8 @@ impl<'a, Context: BuildContext + Send + Sync> DistributionDatabase<'a, Context> match cache_entry.path().canonicalize() { Ok(archive) => { if let (Some(cache_metadata), Some(path_metadata)) = ( - metadata_if_exists(&archive)?, - metadata_if_exists(&wheel.path)?, + metadata_if_exists(&archive).map_err(Error::CacheRead)?, + metadata_if_exists(&wheel.path).map_err(Error::CacheRead)?, ) { let cache_modified = Timestamp::from_metadata(&cache_metadata); let path_modified = Timestamp::from_metadata(&path_metadata); @@ -299,7 +286,7 @@ impl<'a, Context: BuildContext + Send + Sync> DistributionDatabase<'a, Context> } } Err(err) if err.kind() == io::ErrorKind::NotFound => {} - Err(err) => return Err(err.into()), + Err(err) => return Err(Error::CacheRead(err)), } Ok(LocalWheel::Disk(DiskWheel { @@ -332,7 +319,7 @@ impl<'a, Context: BuildContext + Send + Sync> DistributionDatabase<'a, Context> filename: built_wheel.filename, })) } - Err(err) => return Err(err.into()), + Err(err) => return Err(Error::CacheRead(err)), } } } @@ -348,7 +335,7 @@ impl<'a, Context: BuildContext + Send + Sync> DistributionDatabase<'a, Context> pub async fn get_or_build_wheel_metadata( &self, dist: &Dist, - ) -> Result<(Metadata21, Option), DistributionDatabaseError> { + ) -> Result<(Metadata21, Option), Error> { match dist { Dist::Built(built_dist) => { Ok((self.client.wheel_metadata(built_dist).boxed().await?, None)) @@ -356,7 +343,7 @@ impl<'a, Context: BuildContext + Send + Sync> DistributionDatabase<'a, Context> Dist::Source(source_dist) => { // Optimization: Skip source dist download when we must not build them anyway. if self.build_context.no_build() { - return Err(DistributionDatabaseError::NoBuild); + return Err(Error::NoBuild); } let lock = self.locks.acquire(dist).await; @@ -385,7 +372,7 @@ impl<'a, Context: BuildContext + Send + Sync> DistributionDatabase<'a, Context> &self, editable: &LocalEditable, editable_wheel_dir: &Path, - ) -> Result<(LocalWheel, Metadata21), DistributionDatabaseError> { + ) -> Result<(LocalWheel, Metadata21), Error> { let (dist, disk_filename, filename, metadata) = self .builder .build_editable(editable, editable_wheel_dir) @@ -408,14 +395,14 @@ impl<'a, Context: BuildContext + Send + Sync> DistributionDatabase<'a, Context> /// This method takes into account various normalizations that are independent from the Git /// layer. For example: removing `#subdirectory=pkg_dir`-like fragments, and removing `git+` /// prefix kinds. - async fn precise(&self, dist: &SourceDist) -> Result, DistributionDatabaseError> { + async fn precise(&self, dist: &SourceDist) -> Result, Error> { let SourceDist::Git(source_dist) = dist else { return Ok(None); }; let git_dir = self.build_context.cache().bucket(CacheBucket::Git); - let DirectGitUrl { url, subdirectory } = DirectGitUrl::try_from(source_dist.url.raw()) - .map_err(DistributionDatabaseError::Git)?; + let DirectGitUrl { url, subdirectory } = + DirectGitUrl::try_from(source_dist.url.raw()).map_err(Error::Git)?; // If the commit already contains a complete SHA, short-circuit. if url.precise().is_some() { @@ -431,7 +418,7 @@ impl<'a, Context: BuildContext + Send + Sync> DistributionDatabase<'a, Context> }; let precise = tokio::task::spawn_blocking(move || source.fetch()) .await? - .map_err(DistributionDatabaseError::Git)?; + .map_err(Error::Git)?; let url = precise.into_git(); // Re-encode as a URL. diff --git a/crates/puffin-distribution/src/error.rs b/crates/puffin-distribution/src/error.rs index f77e16e84..2f3eb579a 100644 --- a/crates/puffin-distribution/src/error.rs +++ b/crates/puffin-distribution/src/error.rs @@ -1,9 +1,60 @@ -#[derive(thiserror::Error, Debug)] -pub(crate) enum Error { +use tokio::task::JoinError; +use zip::result::ZipError; + +use distribution_filename::WheelFilenameError; +use puffin_normalize::PackageName; + +#[derive(Debug, thiserror::Error)] +pub enum Error { + #[error("Building source distributions is disabled")] + NoBuild, + #[error("Using pre-built wheels is disabled")] + NoBinary, + + // Network error + #[error("Failed to parse URL: `{0}`")] + Url(String, #[source] url::ParseError), + #[error("Git operation failed")] + Git(#[source] anyhow::Error), #[error(transparent)] - IO(#[from] std::io::Error), + Request(#[from] reqwest::Error), #[error(transparent)] - PypiTypes(#[from] pypi_types::Error), - #[error(transparent)] - Zip(#[from] zip::result::ZipError), + Client(#[from] puffin_client::Error), + + // Cache writing error + #[error("Failed to read from the distribution cache")] + CacheRead(#[source] std::io::Error), + #[error("Failed to write to the distribution cache")] + CacheWrite(#[source] std::io::Error), + #[error("Failed to deserialize cache entry")] + CacheDecode(#[from] rmp_serde::decode::Error), + #[error("Failed to serialize cache entry")] + CacheEncode(#[from] rmp_serde::encode::Error), + + // Build error + #[error("Failed to build: {0}")] + Build(String, #[source] anyhow::Error), + #[error("Failed to build editable: {0}")] + BuildEditable(String, #[source] anyhow::Error), + #[error("Built wheel has an invalid filename")] + WheelFilename(#[from] WheelFilenameError), + #[error("Package metadata name `{metadata}` does not match given name `{given}`")] + NameMismatch { + given: PackageName, + metadata: PackageName, + }, + #[error("Failed to parse metadata from built wheel")] + Metadata(#[from] pypi_types::Error), + #[error("Failed to read `dist-info` metadata from built wheel")] + DistInfo(#[from] install_wheel_rs::Error), + #[error("Failed to read zip archive from built wheel")] + Zip(#[from] ZipError), + #[error("Source distribution directory contains neither readable pyproject.toml nor setup.py")] + DirWithoutEntrypoint, + #[error("Failed to extract source distribution: {0}")] + Extract(#[from] puffin_extract::Error), + + /// Should not occur; only seen when another task panicked. + #[error("The task executor is broken, did some other task panic?")] + Join(#[from] JoinError), } diff --git a/crates/puffin-distribution/src/index/built_wheel_index.rs b/crates/puffin-distribution/src/index/built_wheel_index.rs index 3e8ddf184..b402f6134 100644 --- a/crates/puffin-distribution/src/index/built_wheel_index.rs +++ b/crates/puffin-distribution/src/index/built_wheel_index.rs @@ -6,7 +6,7 @@ use puffin_normalize::PackageName; use crate::index::cached_wheel::CachedWheel; use crate::source::{read_http_manifest, read_timestamp_manifest, MANIFEST}; -use crate::SourceDistError; +use crate::Error; /// A local index of built distributions for a specific source distribution. pub struct BuiltWheelIndex; @@ -20,7 +20,7 @@ impl BuiltWheelIndex { source_dist: &DirectUrlSourceDist, cache: &Cache, tags: &Tags, - ) -> Result, SourceDistError> { + ) -> Result, Error> { // For direct URLs, cache directly under the hash of the URL itself. let cache_shard = cache.shard( CacheBucket::BuiltWheels, @@ -47,7 +47,7 @@ impl BuiltWheelIndex { source_dist: &PathSourceDist, cache: &Cache, tags: &Tags, - ) -> Result, SourceDistError> { + ) -> Result, Error> { let cache_shard = cache.shard( CacheBucket::BuiltWheels, WheelCache::Path(&source_dist.url).remote_wheel_dir(source_dist.name().as_ref()), @@ -56,7 +56,7 @@ impl BuiltWheelIndex { // Determine the last-modified time of the source distribution. let Some(modified) = ArchiveTimestamp::from_path(&source_dist.path).expect("archived") else { - return Err(SourceDistError::DirWithoutEntrypoint); + return Err(Error::DirWithoutEntrypoint); }; // Read the manifest from the cache. There's no need to enforce freshness, since we diff --git a/crates/puffin-distribution/src/lib.rs b/crates/puffin-distribution/src/lib.rs index 134b9d19b..94283796d 100644 --- a/crates/puffin-distribution/src/lib.rs +++ b/crates/puffin-distribution/src/lib.rs @@ -1,8 +1,9 @@ -pub use distribution_database::{DistributionDatabase, DistributionDatabaseError}; +pub use distribution_database::DistributionDatabase; pub use download::{BuiltWheel, DiskWheel, LocalWheel}; +pub use error::Error; pub use index::{BuiltWheelIndex, RegistryWheelIndex}; pub use reporter::Reporter; -pub use source::{SourceDistCachedBuilder, SourceDistError}; +pub use source::SourceDistCachedBuilder; pub use unzip::Unzip; mod distribution_database; diff --git a/crates/puffin-distribution/src/source/error.rs b/crates/puffin-distribution/src/source/error.rs deleted file mode 100644 index 2f2c4e814..000000000 --- a/crates/puffin-distribution/src/source/error.rs +++ /dev/null @@ -1,59 +0,0 @@ -use thiserror::Error; -use tokio::task::JoinError; -use zip::result::ZipError; - -use distribution_filename::WheelFilenameError; - -use puffin_normalize::PackageName; - -/// The caller is responsible for adding the source dist information to the error chain -#[derive(Debug, Error)] -pub enum SourceDistError { - #[error("Building source distributions is disabled")] - NoBuild, - - // Network error - #[error("Failed to parse URL: `{0}`")] - UrlParse(String, #[source] url::ParseError), - #[error("Git operation failed")] - Git(#[source] anyhow::Error), - #[error(transparent)] - Request(#[from] reqwest::Error), - #[error(transparent)] - Client(#[from] puffin_client::Error), - - // Cache writing error - #[error("Failed to write to source distribution cache")] - Io(#[from] std::io::Error), - #[error("Cache deserialization failed")] - Decode(#[from] rmp_serde::decode::Error), - #[error("Cache serialization failed")] - Encode(#[from] rmp_serde::encode::Error), - - // Build error - #[error("Failed to build: {0}")] - Build(String, #[source] anyhow::Error), - #[error("Failed to build editable: {0}")] - BuildEditable(String, #[source] anyhow::Error), - #[error("Built wheel has an invalid filename")] - WheelFilename(#[from] WheelFilenameError), - #[error("Package metadata name `{metadata}` does not match given name `{given}`")] - NameMismatch { - given: PackageName, - metadata: PackageName, - }, - #[error("Failed to parse metadata from built wheel")] - Metadata(#[from] pypi_types::Error), - #[error("Failed to read `dist-info` metadata from built wheel")] - DistInfo(#[from] install_wheel_rs::Error), - #[error("Failed to read zip archive from built wheel")] - Zip(#[from] ZipError), - #[error("Source distribution directory contains neither readable pyproject.toml nor setup.py")] - DirWithoutEntrypoint, - #[error("Failed to extract source distribution: {0}")] - Extract(#[from] puffin_extract::Error), - - /// Should not occur; only seen when another task panicked. - #[error("The task executor is broken, did some other task panic?")] - Join(#[from] JoinError), -} diff --git a/crates/puffin-distribution/src/source/mod.rs b/crates/puffin-distribution/src/source/mod.rs index 02a78f4d9..e86e7c8bf 100644 --- a/crates/puffin-distribution/src/source/mod.rs +++ b/crates/puffin-distribution/src/source/mod.rs @@ -31,14 +31,13 @@ use puffin_git::{Fetch, GitSource}; use puffin_traits::{BuildContext, BuildKind, SourceBuildTrait}; use pypi_types::Metadata21; +use crate::error::Error; use crate::reporter::Facade; use crate::source::built_wheel_metadata::BuiltWheelMetadata; -pub use crate::source::error::SourceDistError; use crate::source::manifest::Manifest; use crate::Reporter; mod built_wheel_metadata; -mod error; mod manifest; /// Fetch and build a source distribution from a remote source, or from a local cache. @@ -79,7 +78,7 @@ impl<'a, T: BuildContext> SourceDistCachedBuilder<'a, T> { pub async fn download_and_build( &self, source_dist: &SourceDist, - ) -> Result { + ) -> Result { let built_wheel_metadata = match &source_dist { SourceDist::DirectUrl(direct_url_source_dist) => { let filename = direct_url_source_dist @@ -108,9 +107,10 @@ impl<'a, T: BuildContext> SourceDistCachedBuilder<'a, T> { let url = match ®istry_source_dist.file.url { FileLocation::RelativeUrl(base, url) => base .join_relative(url) - .map_err(|err| SourceDistError::UrlParse(url.clone(), err))?, - FileLocation::AbsoluteUrl(url) => Url::parse(url) - .map_err(|err| SourceDistError::UrlParse(url.clone(), err))?, + .map_err(|err| Error::Url(url.clone(), err))?, + FileLocation::AbsoluteUrl(url) => { + Url::parse(url).map_err(|err| Error::Url(url.clone(), err))? + } FileLocation::Path(path) => { let path_source_dist = PathSourceDist { name: registry_source_dist.filename.name.clone(), @@ -155,7 +155,7 @@ impl<'a, T: BuildContext> SourceDistCachedBuilder<'a, T> { pub async fn download_and_build_metadata( &self, source_dist: &SourceDist, - ) -> Result { + ) -> Result { let metadata = match &source_dist { SourceDist::DirectUrl(direct_url_source_dist) => { let filename = direct_url_source_dist @@ -184,9 +184,10 @@ impl<'a, T: BuildContext> SourceDistCachedBuilder<'a, T> { let url = match ®istry_source_dist.file.url { FileLocation::RelativeUrl(base, url) => base .join_relative(url) - .map_err(|err| SourceDistError::UrlParse(url.clone(), err))?, - FileLocation::AbsoluteUrl(url) => Url::parse(url) - .map_err(|err| SourceDistError::UrlParse(url.clone(), err))?, + .map_err(|err| Error::Url(url.clone(), err))?, + FileLocation::AbsoluteUrl(url) => { + Url::parse(url).map_err(|err| Error::Url(url.clone(), err))? + } FileLocation::Path(path) => { let path_source_dist = PathSourceDist { name: registry_source_dist.filename.name.clone(), @@ -246,12 +247,13 @@ impl<'a, T: BuildContext> SourceDistCachedBuilder<'a, T> { url: &'data Url, cache_shard: &CacheShard, subdirectory: Option<&'data Path>, - ) -> Result { + ) -> Result { let cache_entry = cache_shard.entry(MANIFEST); let cache_control = CacheControl::from( self.build_context .cache() - .freshness(&cache_entry, Some(source_dist.name()))?, + .freshness(&cache_entry, Some(source_dist.name())) + .map_err(Error::CacheRead)?, ); let download = |response| { @@ -277,7 +279,7 @@ impl<'a, T: BuildContext> SourceDistCachedBuilder<'a, T> { .await .map_err(|err| match err { CachedClientError::Callback(err) => err, - CachedClientError::Client(err) => SourceDistError::Client(err), + CachedClientError::Client(err) => Error::Client(err), })?; // From here on, scope all operations to the current build. Within the manifest shard, @@ -315,7 +317,9 @@ impl<'a, T: BuildContext> SourceDistCachedBuilder<'a, T> { // Store the metadata. let metadata_entry = cache_shard.entry(METADATA); - write_atomic(metadata_entry.path(), rmp_serde::to_vec(&metadata)?).await?; + write_atomic(metadata_entry.path(), rmp_serde::to_vec(&metadata)?) + .await + .map_err(Error::CacheWrite)?; Ok(BuiltWheelMetadata { path: cache_shard.join(&disk_filename), @@ -336,12 +340,13 @@ impl<'a, T: BuildContext> SourceDistCachedBuilder<'a, T> { url: &'data Url, cache_shard: &CacheShard, subdirectory: Option<&'data Path>, - ) -> Result { + ) -> Result { let cache_entry = cache_shard.entry(MANIFEST); let cache_control = CacheControl::from( self.build_context .cache() - .freshness(&cache_entry, Some(source_dist.name()))?, + .freshness(&cache_entry, Some(source_dist.name())) + .map_err(Error::CacheRead)?, ); let download = |response| { @@ -367,7 +372,7 @@ impl<'a, T: BuildContext> SourceDistCachedBuilder<'a, T> { .await .map_err(|err| match err { CachedClientError::Callback(err) => err, - CachedClientError::Client(err) => SourceDistError::Client(err), + CachedClientError::Client(err) => Error::Client(err), })?; // From here on, scope all operations to the current build. Within the manifest shard, @@ -394,8 +399,12 @@ impl<'a, T: BuildContext> SourceDistCachedBuilder<'a, T> { { // Store the metadata. let cache_entry = cache_shard.entry(METADATA); - fs::create_dir_all(cache_entry.dir()).await?; - write_atomic(cache_entry.path(), rmp_serde::to_vec(&metadata)?).await?; + fs::create_dir_all(cache_entry.dir()) + .await + .map_err(Error::CacheWrite)?; + write_atomic(cache_entry.path(), rmp_serde::to_vec(&metadata)?) + .await + .map_err(Error::CacheWrite)?; return Ok(metadata); } @@ -417,7 +426,9 @@ impl<'a, T: BuildContext> SourceDistCachedBuilder<'a, T> { // Store the metadata. let cache_entry = cache_shard.entry(METADATA); - write_atomic(cache_entry.path(), rmp_serde::to_vec(&metadata)?).await?; + write_atomic(cache_entry.path(), rmp_serde::to_vec(&metadata)?) + .await + .map_err(Error::CacheWrite)?; if let Some(task) = task { if let Some(reporter) = self.reporter.as_ref() { @@ -433,7 +444,7 @@ impl<'a, T: BuildContext> SourceDistCachedBuilder<'a, T> { &self, source_dist: &SourceDist, path_source_dist: &PathSourceDist, - ) -> Result { + ) -> Result { let cache_shard = self.build_context.cache().shard( CacheBucket::BuiltWheels, WheelCache::Path(&path_source_dist.url) @@ -441,8 +452,10 @@ impl<'a, T: BuildContext> SourceDistCachedBuilder<'a, T> { ); // Determine the last-modified time of the source distribution. - let Some(modified) = ArchiveTimestamp::from_path(&path_source_dist.path)? else { - return Err(SourceDistError::DirWithoutEntrypoint); + let Some(modified) = + ArchiveTimestamp::from_path(&path_source_dist.path).map_err(Error::CacheRead)? + else { + return Err(Error::DirWithoutEntrypoint); }; // Read the existing metadata from the cache. @@ -450,7 +463,8 @@ impl<'a, T: BuildContext> SourceDistCachedBuilder<'a, T> { let manifest_freshness = self .build_context .cache() - .freshness(&manifest_entry, Some(source_dist.name()))?; + .freshness(&manifest_entry, Some(source_dist.name())) + .map_err(Error::CacheRead)?; let manifest = refresh_timestamp_manifest(&manifest_entry, manifest_freshness, modified).await?; @@ -483,7 +497,9 @@ impl<'a, T: BuildContext> SourceDistCachedBuilder<'a, T> { // Store the metadata. let cache_entry = cache_shard.entry(METADATA); - write_atomic(cache_entry.path(), rmp_serde::to_vec(&metadata)?).await?; + write_atomic(cache_entry.path(), rmp_serde::to_vec(&metadata)?) + .await + .map_err(Error::CacheWrite)?; Ok(BuiltWheelMetadata { path: cache_shard.join(&disk_filename), @@ -500,7 +516,7 @@ impl<'a, T: BuildContext> SourceDistCachedBuilder<'a, T> { &self, source_dist: &SourceDist, path_source_dist: &PathSourceDist, - ) -> Result { + ) -> Result { let cache_shard = self.build_context.cache().shard( CacheBucket::BuiltWheels, WheelCache::Path(&path_source_dist.url) @@ -508,8 +524,10 @@ impl<'a, T: BuildContext> SourceDistCachedBuilder<'a, T> { ); // Determine the last-modified time of the source distribution. - let Some(modified) = ArchiveTimestamp::from_path(&path_source_dist.path)? else { - return Err(SourceDistError::DirWithoutEntrypoint); + let Some(modified) = + ArchiveTimestamp::from_path(&path_source_dist.path).map_err(Error::CacheRead)? + else { + return Err(Error::DirWithoutEntrypoint); }; // Read the existing metadata from the cache, to clear stale entries. @@ -517,7 +535,8 @@ impl<'a, T: BuildContext> SourceDistCachedBuilder<'a, T> { let manifest_freshness = self .build_context .cache() - .freshness(&manifest_entry, Some(source_dist.name()))?; + .freshness(&manifest_entry, Some(source_dist.name())) + .map_err(Error::CacheRead)?; let manifest = refresh_timestamp_manifest(&manifest_entry, manifest_freshness, modified).await?; @@ -549,8 +568,12 @@ impl<'a, T: BuildContext> SourceDistCachedBuilder<'a, T> { { // Store the metadata. let cache_entry = cache_shard.entry(METADATA); - fs::create_dir_all(cache_entry.dir()).await?; - write_atomic(cache_entry.path(), rmp_serde::to_vec(&metadata)?).await?; + fs::create_dir_all(cache_entry.dir()) + .await + .map_err(Error::CacheWrite)?; + write_atomic(cache_entry.path(), rmp_serde::to_vec(&metadata)?) + .await + .map_err(Error::CacheWrite)?; return Ok(metadata); } @@ -573,7 +596,9 @@ impl<'a, T: BuildContext> SourceDistCachedBuilder<'a, T> { // Store the metadata. let cache_entry = cache_shard.entry(METADATA); - write_atomic(cache_entry.path(), rmp_serde::to_vec(&metadata)?).await?; + write_atomic(cache_entry.path(), rmp_serde::to_vec(&metadata)?) + .await + .map_err(Error::CacheWrite)?; Ok(metadata) } @@ -583,7 +608,7 @@ impl<'a, T: BuildContext> SourceDistCachedBuilder<'a, T> { &self, source_dist: &SourceDist, git_source_dist: &GitSourceDist, - ) -> Result { + ) -> Result { let (fetch, subdirectory) = self.download_source_dist_git(&git_source_dist.url).await?; let git_sha = fetch.git().precise().expect("Exact commit after checkout"); @@ -620,7 +645,9 @@ impl<'a, T: BuildContext> SourceDistCachedBuilder<'a, T> { // Store the metadata. let cache_entry = cache_shard.entry(METADATA); - write_atomic(cache_entry.path(), rmp_serde::to_vec(&metadata)?).await?; + write_atomic(cache_entry.path(), rmp_serde::to_vec(&metadata)?) + .await + .map_err(Error::CacheWrite)?; Ok(BuiltWheelMetadata { path: cache_shard.join(&disk_filename), @@ -637,7 +664,7 @@ impl<'a, T: BuildContext> SourceDistCachedBuilder<'a, T> { &self, source_dist: &SourceDist, git_source_dist: &GitSourceDist, - ) -> Result { + ) -> Result { let (fetch, subdirectory) = self.download_source_dist_git(&git_source_dist.url).await?; let git_sha = fetch.git().precise().expect("Exact commit after checkout"); @@ -669,8 +696,12 @@ impl<'a, T: BuildContext> SourceDistCachedBuilder<'a, T> { { // Store the metadata. let cache_entry = cache_shard.entry(METADATA); - fs::create_dir_all(cache_entry.dir()).await?; - write_atomic(cache_entry.path(), rmp_serde::to_vec(&metadata)?).await?; + fs::create_dir_all(cache_entry.dir()) + .await + .map_err(Error::CacheWrite)?; + write_atomic(cache_entry.path(), rmp_serde::to_vec(&metadata)?) + .await + .map_err(Error::CacheWrite)?; return Ok(metadata); } @@ -698,7 +729,9 @@ impl<'a, T: BuildContext> SourceDistCachedBuilder<'a, T> { // Store the metadata. let cache_entry = cache_shard.entry(METADATA); - write_atomic(cache_entry.path(), rmp_serde::to_vec(&metadata)?).await?; + write_atomic(cache_entry.path(), rmp_serde::to_vec(&metadata)?) + .await + .map_err(Error::CacheWrite)?; Ok(metadata) } @@ -710,7 +743,7 @@ impl<'a, T: BuildContext> SourceDistCachedBuilder<'a, T> { source_dist: &SourceDist, filename: &str, cache_entry: &'data CacheEntry, - ) -> Result<&'data Path, SourceDistError> { + ) -> Result<&'data Path, Error> { let cache_path = cache_entry.path(); if cache_path.is_dir() { debug!("Distribution is already cached: {source_dist}"); @@ -735,7 +768,8 @@ impl<'a, T: BuildContext> SourceDistCachedBuilder<'a, T> { // Persist the unzipped distribution to the cache. self.build_context .cache() - .persist(source_dist_dir, cache_path)?; + .persist(source_dist_dir, cache_path) + .map_err(Error::CacheWrite)?; Ok(cache_path) } @@ -770,24 +804,23 @@ impl<'a, T: BuildContext> SourceDistCachedBuilder<'a, T> { } /// Download a source distribution from a Git repository. - async fn download_source_dist_git( - &self, - url: &Url, - ) -> Result<(Fetch, Option), SourceDistError> { + async fn download_source_dist_git(&self, url: &Url) -> Result<(Fetch, Option), Error> { debug!("Fetching source distribution from Git: {url}"); let git_dir = self.build_context.cache().bucket(CacheBucket::Git); // Avoid races between different processes, too. let lock_dir = git_dir.join("locks"); - fs::create_dir_all(&lock_dir).await?; + fs::create_dir_all(&lock_dir) + .await + .map_err(Error::CacheWrite)?; let canonical_url = cache_key::CanonicalUrl::new(url); let _lock = LockedFile::acquire( lock_dir.join(cache_key::digest(&canonical_url)), &canonical_url, - )?; + ) + .map_err(Error::CacheWrite)?; - let DirectGitUrl { url, subdirectory } = - DirectGitUrl::try_from(url).map_err(SourceDistError::Git)?; + let DirectGitUrl { url, subdirectory } = DirectGitUrl::try_from(url).map_err(Error::Git)?; let source = if let Some(reporter) = &self.reporter { GitSource::new(url, git_dir).with_reporter(Facade::from(reporter.clone())) @@ -796,7 +829,7 @@ impl<'a, T: BuildContext> SourceDistCachedBuilder<'a, T> { }; let fetch = tokio::task::spawn_blocking(move || source.fetch()) .await? - .map_err(SourceDistError::Git)?; + .map_err(Error::Git)?; Ok((fetch, subdirectory)) } @@ -810,15 +843,17 @@ impl<'a, T: BuildContext> SourceDistCachedBuilder<'a, T> { source_dist: &Path, subdirectory: Option<&Path>, cache_shard: &CacheShard, - ) -> Result<(String, WheelFilename, Metadata21), SourceDistError> { + ) -> Result<(String, WheelFilename, Metadata21), Error> { debug!("Building: {dist}"); if self.build_context.no_build() { - return Err(SourceDistError::NoBuild); + return Err(Error::NoBuild); } // Build the wheel. - fs::create_dir_all(&cache_shard).await?; + fs::create_dir_all(&cache_shard) + .await + .map_err(Error::CacheWrite)?; let disk_filename = self .build_context .setup_build( @@ -828,10 +863,10 @@ impl<'a, T: BuildContext> SourceDistCachedBuilder<'a, T> { BuildKind::Wheel, ) .await - .map_err(|err| SourceDistError::Build(dist.to_string(), err))? + .map_err(|err| Error::Build(dist.to_string(), err))? .wheel(cache_shard) .await - .map_err(|err| SourceDistError::Build(dist.to_string(), err))?; + .map_err(|err| Error::Build(dist.to_string(), err))?; // Read the metadata from the wheel. let filename = WheelFilename::from_str(&disk_filename)?; @@ -839,7 +874,7 @@ impl<'a, T: BuildContext> SourceDistCachedBuilder<'a, T> { // Validate the metadata. if &metadata.name != dist.name() { - return Err(SourceDistError::NameMismatch { + return Err(Error::NameMismatch { metadata: metadata.name, given: dist.name().clone(), }); @@ -856,7 +891,7 @@ impl<'a, T: BuildContext> SourceDistCachedBuilder<'a, T> { dist: &SourceDist, source_dist: &Path, subdirectory: Option<&Path>, - ) -> Result, SourceDistError> { + ) -> Result, Error> { debug!("Preparing metadata for: {dist}"); // Setup the builder. @@ -869,24 +904,27 @@ impl<'a, T: BuildContext> SourceDistCachedBuilder<'a, T> { BuildKind::Wheel, ) .await - .map_err(|err| SourceDistError::Build(dist.to_string(), err))?; + .map_err(|err| Error::Build(dist.to_string(), err))?; // Build the metadata. let dist_info = builder .metadata() .await - .map_err(|err| SourceDistError::Build(dist.to_string(), err))?; + .map_err(|err| Error::Build(dist.to_string(), err))?; let Some(dist_info) = dist_info else { return Ok(None); }; // Read the metadata from disk. debug!("Prepared metadata for: {dist}"); - let metadata = Metadata21::parse(&fs::read(dist_info.join("METADATA")).await?)?; + let content = fs::read(dist_info.join("METADATA")) + .await + .map_err(Error::CacheRead)?; + let metadata = Metadata21::parse(&content)?; // Validate the metadata. if &metadata.name != dist.name() { - return Err(SourceDistError::NameMismatch { + return Err(Error::NameMismatch { metadata: metadata.name, given: dist.name().clone(), }); @@ -900,7 +938,7 @@ impl<'a, T: BuildContext> SourceDistCachedBuilder<'a, T> { &self, editable: &LocalEditable, editable_wheel_dir: &Path, - ) -> Result<(Dist, String, WheelFilename, Metadata21), SourceDistError> { + ) -> Result<(Dist, String, WheelFilename, Metadata21), Error> { debug!("Building (editable) {editable}"); let disk_filename = self .build_context @@ -911,10 +949,10 @@ impl<'a, T: BuildContext> SourceDistCachedBuilder<'a, T> { BuildKind::Editable, ) .await - .map_err(|err| SourceDistError::BuildEditable(editable.to_string(), err))? + .map_err(|err| Error::BuildEditable(editable.to_string(), err))? .wheel(editable_wheel_dir) .await - .map_err(|err| SourceDistError::BuildEditable(editable.to_string(), err))?; + .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 { @@ -931,15 +969,13 @@ impl<'a, T: BuildContext> SourceDistCachedBuilder<'a, T> { } /// Read an existing HTTP-cached [`Manifest`], if it exists. -pub(crate) fn read_http_manifest( - cache_entry: &CacheEntry, -) -> Result, SourceDistError> { +pub(crate) fn read_http_manifest(cache_entry: &CacheEntry) -> Result, Error> { match std::fs::read(cache_entry.path()) { Ok(cached) => Ok(Some( rmp_serde::from_slice::>(&cached)?.data, )), Err(err) if err.kind() == std::io::ErrorKind::NotFound => Ok(None), - Err(err) => Err(err.into()), + Err(err) => Err(Error::CacheRead(err)), } } @@ -949,7 +985,7 @@ pub(crate) fn read_http_manifest( pub(crate) fn read_timestamp_manifest( cache_entry: &CacheEntry, modified: ArchiveTimestamp, -) -> Result, SourceDistError> { +) -> Result, Error> { // If the cache entry is up-to-date, return it. match std::fs::read(cache_entry.path()) { Ok(cached) => { @@ -959,7 +995,7 @@ pub(crate) fn read_timestamp_manifest( } } Err(err) if err.kind() == std::io::ErrorKind::NotFound => {} - Err(err) => return Err(err.into()), + Err(err) => return Err(Error::CacheRead(err)), } Ok(None) } @@ -971,7 +1007,7 @@ pub(crate) async fn refresh_timestamp_manifest( cache_entry: &CacheEntry, freshness: Freshness, modified: ArchiveTimestamp, -) -> Result { +) -> Result { // If we know the exact modification time, we don't need to force a revalidate. if matches!(modified, ArchiveTimestamp::Exact(_)) || freshness.is_fresh() { if let Some(manifest) = read_timestamp_manifest(cache_entry, modified)? { @@ -981,7 +1017,9 @@ pub(crate) async fn refresh_timestamp_manifest( // Otherwise, create a new manifest. let manifest = Manifest::new(); - fs::create_dir_all(&cache_entry.dir()).await?; + fs::create_dir_all(&cache_entry.dir()) + .await + .map_err(Error::CacheWrite)?; write_atomic( cache_entry.path(), rmp_serde::to_vec(&CachedByTimestamp { @@ -989,18 +1027,19 @@ pub(crate) async fn refresh_timestamp_manifest( data: manifest.clone(), })?, ) - .await?; + .await + .map_err(Error::CacheWrite)?; Ok(manifest) } /// Read an existing cached [`Metadata21`], if it exists. pub(crate) async fn read_cached_metadata( cache_entry: &CacheEntry, -) -> Result, SourceDistError> { +) -> Result, Error> { match fs::read(&cache_entry.path()).await { Ok(cached) => Ok(Some(rmp_serde::from_slice::(&cached)?)), Err(err) if err.kind() == std::io::ErrorKind::NotFound => Ok(None), - Err(err) => Err(err.into()), + Err(err) => Err(Error::CacheRead(err)), } } @@ -1008,8 +1047,8 @@ pub(crate) async fn read_cached_metadata( fn read_wheel_metadata( filename: &WheelFilename, wheel: impl Into, -) -> Result { - let file = fs_err::File::open(wheel)?; +) -> Result { + let file = fs_err::File::open(wheel).map_err(Error::CacheRead)?; let reader = std::io::BufReader::new(file); let mut archive = ZipArchive::new(reader)?; let dist_info = read_dist_info(filename, &mut archive)?; diff --git a/crates/puffin-installer/src/downloader.rs b/crates/puffin-installer/src/downloader.rs index 25defa9d2..55090ea0d 100644 --- a/crates/puffin-installer/src/downloader.rs +++ b/crates/puffin-installer/src/downloader.rs @@ -11,7 +11,7 @@ use distribution_types::{CachedDist, Dist, Identifier, LocalEditable, RemoteSour use platform_tags::Tags; use puffin_cache::Cache; use puffin_client::RegistryClient; -use puffin_distribution::{DistributionDatabase, DistributionDatabaseError, LocalWheel, Unzip}; +use puffin_distribution::{DistributionDatabase, LocalWheel, Unzip}; use puffin_traits::{BuildContext, InFlight}; use crate::editable::BuiltEditable; @@ -21,12 +21,12 @@ pub enum Error { #[error("Failed to unzip wheel: {0}")] Unzip(Dist, #[source] puffin_extract::Error), #[error("Failed to fetch wheel: {0}")] - Fetch(Dist, #[source] DistributionDatabaseError), + Fetch(Dist, #[source] puffin_distribution::Error), /// Should not occur; only seen when another task panicked. #[error("The task executor is broken, did some other task panic?")] Join(#[from] JoinError), #[error(transparent)] - Editable(#[from] DistributionDatabaseError), + Editable(#[from] puffin_distribution::Error), #[error("Unzip failed in another thread: {0}")] Thread(String), #[error(transparent)] diff --git a/crates/puffin-resolver/src/error.rs b/crates/puffin-resolver/src/error.rs index 5ea909286..c84a3181d 100644 --- a/crates/puffin-resolver/src/error.rs +++ b/crates/puffin-resolver/src/error.rs @@ -5,14 +5,12 @@ use std::fmt::Formatter; use indexmap::IndexMap; use pubgrub::range::Range; use pubgrub::report::{DefaultStringReporter, DerivationTree, Reporter}; -use thiserror::Error; use url::Url; use distribution_types::{BuiltDist, PathBuiltDist, PathSourceDist, SourceDist}; use once_map::OnceMap; use pep440_rs::Version; use pep508_rs::Requirement; -use puffin_distribution::DistributionDatabaseError; use puffin_normalize::PackageName; use crate::candidate_selector::CandidateSelector; @@ -20,7 +18,7 @@ use crate::pubgrub::{PubGrubPackage, PubGrubPython, PubGrubReportFormatter}; use crate::python_requirement::PythonRequirement; use crate::version_map::VersionMap; -#[derive(Error, Debug)] +#[derive(Debug, thiserror::Error)] pub enum ResolveError { #[error("Failed to find a version of {0} that satisfies the requirement")] NotFound(Requirement), @@ -62,16 +60,16 @@ pub enum ResolveError { DistributionType(#[from] distribution_types::Error), #[error("Failed to download: {0}")] - Fetch(Box, #[source] DistributionDatabaseError), + Fetch(Box, #[source] puffin_distribution::Error), #[error("Failed to download and build: {0}")] - FetchAndBuild(Box, #[source] DistributionDatabaseError), + FetchAndBuild(Box, #[source] puffin_distribution::Error), #[error("Failed to read: {0}")] - Read(Box, #[source] DistributionDatabaseError), + Read(Box, #[source] puffin_distribution::Error), #[error("Failed to build: {0}")] - Build(Box, #[source] DistributionDatabaseError), + Build(Box, #[source] puffin_distribution::Error), #[error(transparent)] NoSolution(#[from] NoSolutionError), diff --git a/crates/puffin-resolver/src/resolver/provider.rs b/crates/puffin-resolver/src/resolver/provider.rs index 128a9b8d0..b8f7e26a8 100644 --- a/crates/puffin-resolver/src/resolver/provider.rs +++ b/crates/puffin-resolver/src/resolver/provider.rs @@ -8,7 +8,7 @@ use url::Url; use distribution_types::Dist; use platform_tags::Tags; use puffin_client::{FlatIndex, RegistryClient}; -use puffin_distribution::{DistributionDatabase, DistributionDatabaseError}; +use puffin_distribution::DistributionDatabase; use puffin_normalize::PackageName; use puffin_traits::{BuildContext, NoBinary}; use pypi_types::Metadata21; @@ -18,7 +18,7 @@ use crate::version_map::VersionMap; use crate::yanks::AllowedYanks; type VersionMapResponse = Result; -type WheelMetadataResponse = Result<(Metadata21, Option), DistributionDatabaseError>; +type WheelMetadataResponse = Result<(Metadata21, Option), puffin_distribution::Error>; pub trait ResolverProvider: Send + Sync { /// Get the version map for a package.