From 992f54ec1789428c069f3398d2739f8b4c6672f6 Mon Sep 17 00:00:00 2001 From: Aria Desires Date: Wed, 2 Apr 2025 11:57:00 -0400 Subject: [PATCH] improve archive error messages (#12627) --- .../src/distribution_database.rs | 28 +++++++++++++------ crates/uv-distribution/src/error.rs | 4 +-- crates/uv-distribution/src/source/mod.rs | 22 ++++++++++++--- crates/uv-extract/src/error.rs | 2 +- crates/uv-metadata/src/lib.rs | 2 +- crates/uv/tests/it/pip_install.rs | 4 +-- 6 files changed, 43 insertions(+), 19 deletions(-) diff --git a/crates/uv-distribution/src/distribution_database.rs b/crates/uv-distribution/src/distribution_database.rs index 44ac2dc51..ab11e16c2 100644 --- a/crates/uv-distribution/src/distribution_database.rs +++ b/crates/uv-distribution/src/distribution_database.rs @@ -225,7 +225,7 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> { filename: wheel.filename.clone(), cache: CacheInfo::default(), }), - Err(Error::Extract(err)) => { + Err(Error::Extract(name, err)) => { if err.is_http_streaming_unsupported() { warn!( "Streaming unsupported for {dist}; downloading wheel to disk ({err})" @@ -233,7 +233,7 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> { } else if err.is_http_streaming_failed() { warn!("Streaming failed for {dist}; downloading wheel to disk ({err})"); } else { - return Err(Error::Extract(err)); + return Err(Error::Extract(name, err)); } // If the request failed because streaming is unsupported, download the @@ -570,10 +570,14 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> { match progress { Some((reporter, progress)) => { let mut reader = ProgressReader::new(&mut hasher, progress, &**reporter); - uv_extract::stream::unzip(&mut reader, temp_dir.path()).await?; + uv_extract::stream::unzip(&mut reader, temp_dir.path()) + .await + .map_err(|err| Error::Extract(filename.to_string(), err))?; } None => { - uv_extract::stream::unzip(&mut hasher, temp_dir.path()).await?; + uv_extract::stream::unzip(&mut hasher, temp_dir.path()) + .await + .map_err(|err| Error::Extract(filename.to_string(), err))?; } } @@ -734,7 +738,8 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> { Ok(()) } }) - .await??; + .await? + .map_err(|err| Error::Extract(filename.to_string(), err))?; HashDigests::empty() } else { @@ -742,7 +747,9 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> { let algorithms = hashes.algorithms(); let mut hashers = algorithms.into_iter().map(Hasher::from).collect::>(); let mut hasher = uv_extract::hash::HashReader::new(file, &mut hashers); - uv_extract::stream::unzip(&mut hasher, temp_dir.path()).await?; + uv_extract::stream::unzip(&mut hasher, temp_dir.path()) + .await + .map_err(|err| Error::Extract(filename.to_string(), err))?; // If necessary, exhaust the reader to compute the hash. hasher.finish().await.map_err(Error::HashExhaustion)?; @@ -902,7 +909,9 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> { let mut hasher = uv_extract::hash::HashReader::new(file, &mut hashers); // Unzip the wheel to a temporary directory. - uv_extract::stream::unzip(&mut hasher, temp_dir.path()).await?; + uv_extract::stream::unzip(&mut hasher, temp_dir.path()) + .await + .map_err(|err| Error::Extract(filename.to_string(), err))?; // Exhaust the reader to compute the hash. hasher.finish().await.map_err(Error::HashExhaustion)?; @@ -949,8 +958,9 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> { move || -> Result { // Unzip the wheel into a temporary directory. let temp_dir = tempfile::tempdir_in(root).map_err(Error::CacheWrite)?; - let reader = fs_err::File::open(path).map_err(Error::CacheWrite)?; - uv_extract::unzip(reader, temp_dir.path())?; + let reader = fs_err::File::open(&path).map_err(Error::CacheWrite)?; + uv_extract::unzip(reader, temp_dir.path()) + .map_err(|err| Error::Extract(path.to_string_lossy().into_owned(), err))?; Ok(temp_dir) } }) diff --git a/crates/uv-distribution/src/error.rs b/crates/uv-distribution/src/error.rs index ff2ab28d8..ac6518145 100644 --- a/crates/uv-distribution/src/error.rs +++ b/crates/uv-distribution/src/error.rs @@ -84,8 +84,8 @@ pub enum Error { ReadInstalled(Box, #[source] InstalledDistError), #[error("Failed to read zip archive from built wheel")] Zip(#[from] ZipError), - #[error("Failed to extract archive")] - Extract(#[from] uv_extract::Error), + #[error("Failed to extract archive: {0}")] + Extract(String, #[source] uv_extract::Error), #[error("The source distribution is missing a `PKG-INFO` file")] MissingPkgInfo, #[error("The source distribution `{}` has no subdirectory `{}`", _0, _1.display())] diff --git a/crates/uv-distribution/src/source/mod.rs b/crates/uv-distribution/src/source/mod.rs index 9781aa54b..9051f7ace 100644 --- a/crates/uv-distribution/src/source/mod.rs +++ b/crates/uv-distribution/src/source/mod.rs @@ -2089,7 +2089,9 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> { // Download and unzip the source distribution into a temporary directory. let span = info_span!("download_source_dist", source_dist = %source); - uv_extract::stream::archive(&mut hasher, ext, temp_dir.path()).await?; + uv_extract::stream::archive(&mut hasher, ext, temp_dir.path()) + .await + .map_err(|err| Error::Extract(source.to_string(), err))?; drop(span); // If necessary, exhaust the reader to compute the hash. @@ -2103,7 +2105,12 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> { let extracted = match uv_extract::strip_component(temp_dir.path()) { Ok(top_level) => top_level, Err(uv_extract::Error::NonSingularArchive(_)) => temp_dir.into_path(), - Err(err) => return Err(err.into()), + Err(err) => { + return Err(Error::Extract( + temp_dir.path().to_string_lossy().into_owned(), + err, + )) + } }; // Persist it to the cache. @@ -2151,7 +2158,9 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> { let mut hasher = uv_extract::hash::HashReader::new(reader, &mut hashers); // Unzip the archive into a temporary directory. - uv_extract::stream::archive(&mut hasher, ext, &temp_dir.path()).await?; + uv_extract::stream::archive(&mut hasher, ext, &temp_dir.path()) + .await + .map_err(|err| Error::Extract(temp_dir.path().to_string_lossy().into_owned(), err))?; // If necessary, exhaust the reader to compute the hash. if !algorithms.is_empty() { @@ -2164,7 +2173,12 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> { let extracted = match uv_extract::strip_component(temp_dir.path()) { Ok(top_level) => top_level, Err(uv_extract::Error::NonSingularArchive(_)) => temp_dir.path().to_path_buf(), - Err(err) => return Err(err.into()), + Err(err) => { + return Err(Error::Extract( + temp_dir.path().to_string_lossy().into_owned(), + err, + )) + } }; // Persist it to the cache. diff --git a/crates/uv-extract/src/error.rs b/crates/uv-extract/src/error.rs index 6027bc773..09191bb0a 100644 --- a/crates/uv-extract/src/error.rs +++ b/crates/uv-extract/src/error.rs @@ -14,7 +14,7 @@ pub enum Error { NonSingularArchive(Vec), #[error("The top-level of the archive must only contain a list directory, but it's empty")] EmptyArchive, - #[error("Bad CRC (got {computed:08x}, expected {expected:08x}): {}", path.display())] + #[error("Bad CRC (got {computed:08x}, expected {expected:08x}) for file: {}", path.display())] BadCrc32 { path: PathBuf, computed: u32, diff --git a/crates/uv-metadata/src/lib.rs b/crates/uv-metadata/src/lib.rs index 418589fd8..5d9fd7079 100644 --- a/crates/uv-metadata/src/lib.rs +++ b/crates/uv-metadata/src/lib.rs @@ -33,7 +33,7 @@ pub enum Error { InvalidName(#[from] InvalidNameError), #[error("The metadata at {0} is invalid")] InvalidMetadata(String, Box), - #[error("Bad CRC (got {computed:08x}, expected {expected:08x}): {path}")] + #[error("Bad CRC (got {computed:08x}, expected {expected:08x}) for file: {path}")] BadCrc32 { path: String, computed: u32, diff --git a/crates/uv/tests/it/pip_install.rs b/crates/uv/tests/it/pip_install.rs index 9e5a1d5e8..12e2a4cc0 100644 --- a/crates/uv/tests/it/pip_install.rs +++ b/crates/uv/tests/it/pip_install.rs @@ -8907,8 +8907,8 @@ fn bad_crc32() -> Result<()> { ----- stderr ----- Resolved 7 packages in [TIME] × Failed to download `osqp @ https://files.pythonhosted.org/packages/00/04/5959347582ab970e9b922f27585d34f7c794ed01125dac26fb4e7dd80205/osqp-1.0.2-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.whl` - ├─▶ Failed to extract archive - ╰─▶ Bad CRC (got ca5f1131, expected d5c95dfa): osqp/ext_builtin.cpython-311-x86_64-linux-gnu.so + ├─▶ Failed to extract archive: osqp-1.0.2-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.whl + ╰─▶ Bad CRC (got ca5f1131, expected d5c95dfa) for file: osqp/ext_builtin.cpython-311-x86_64-linux-gnu.so " );