Show error paths in install-wheel-rs (#514)

Ensure that we consistently show a path for all io errors in
install-wheel-rs either (preferred) through `fs_err`, or as fallback by
a custom error type. For zip reading errors, we rely on the caller to
add the name and/or location of the wheel.

crates/install-wheel-rs/src/lib.rs

@@ -1,6 +1,7 @@
 //! Takes a wheel and installs it into a venv..
 
 use std::io;
+use std::path::PathBuf;
 use std::str::FromStr;
 
 use distribution_filename::WheelFilename;
@@ -29,10 +30,19 @@ mod script;
 mod uninstall;
 mod wheel;
 
+/// Note: The caller is responsible for adding the path of the wheel we're installing.
 #[derive(Error, Debug)]
 pub enum Error {
     #[error(transparent)]
-    IO(#[from] io::Error),
+    Io(#[from] io::Error),
+    /// Custom error type to add a path to error reading a file from a zip
+    #[error("Failed to reflink {from} to {to}")]
+    Reflink {
+        from: PathBuf,
+        to: PathBuf,
+        #[source]
+        err: io::Error,
+    },
     /// Tags/metadata didn't match platform
     #[error("The wheel is incompatible with the current platform {os} {arch}")]
     IncompatibleWheel { os: Os, arch: Arch },
@@ -45,7 +55,8 @@ pub enum Error {
     /// Doesn't follow file name schema
     #[error(transparent)]
     InvalidWheelFileName(#[from] distribution_filename::WheelFilenameError),
-    #[error("Failed to read the wheel file {0}")]
+    /// The caller must add the name of the zip file (See note on type).
+    #[error("Failed to read {0} from zip file")]
     Zip(String, #[source] ZipError),
     #[error("Failed to run python subcommand")]
     PythonSubcommand(#[source] io::Error),
@@ -71,15 +82,6 @@ pub enum Error {
     MultipleDistInfo(String),
 }
 
-impl Error {
-    pub(crate) fn from_zip_error(file: String, value: ZipError) -> Self {
-        match value {
-            ZipError::Io(io_error) => Self::IO(io_error),
-            _ => Self::Zip(file, value),
-        }
-    }
-}
-
 /// The metadata name may be uppercase, while the wheel and dist info names are lowercase, or
 /// the metadata name and the dist info name are lowercase, while the wheel name is uppercase.
 /// Either way, we just search the wheel for the name.

crates/install-wheel-rs/src/linker.rs

@@ -1,7 +1,6 @@
 //! Like `wheel.rs`, but for installing wheels that have already been unzipped, rather than
 //! reading from a zip file.
 
-use std::io::Read;
 use std::path::Path;
 
 use configparser::ini::Ini;
@@ -61,7 +60,7 @@ pub fn install_wheel(
     let wheel_file_path = wheel
         .as_ref()
         .join(format!("{dist_info_prefix}.dist-info/WHEEL"));
-    let wheel_text = std::fs::read_to_string(wheel_file_path)?;
+    let wheel_text = fs::read_to_string(wheel_file_path)?;
     parse_wheel_version(&wheel_text)?;
 
     // > 1.c If Root-Is-Purelib == ‘true’, unpack archive into purelib (site-packages).
@@ -136,7 +135,7 @@ pub fn install_wheel(
 /// <https://github.com/PyO3/python-pkginfo-rs>
 fn find_dist_info(path: impl AsRef<Path>) -> Result<String, Error> {
     // Iterate over `path` to find the `.dist-info` directory. It should be at the top-level.
-    let Some(dist_info) = std::fs::read_dir(path)?.find_map(|entry| {
+    let Some(dist_info) = fs::read_dir(path.as_ref())?.find_map(|entry| {
         let entry = entry.ok()?;
         let path = entry.path();
         if path.is_dir() {
@@ -172,9 +171,7 @@ fn read_metadata(
         .as_ref()
         .join(format!("{dist_info_prefix}.dist-info/METADATA"));
 
-    // Read into a buffer.
+    let content = fs::read(&metadata_file)?;
-    let mut content = Vec::new();
-    File::open(&metadata_file)?.read_to_end(&mut content)?;
 
     // HACK: trick mailparse to parse as UTF-8 instead of ASCII
     let mut mail = b"Content-Type: text/plain; charset=utf-8\n".to_vec();
@@ -228,7 +225,7 @@ fn parse_scripts(
         .join(format!("{dist_info_prefix}.dist-info/entry_points.txt"));
 
     // Read the entry points mapping. If the file doesn't exist, we just return an empty mapping.
-    let Ok(ini) = std::fs::read_to_string(entry_points_path) else {
+    let Ok(ini) = fs::read_to_string(entry_points_path) else {
         return Ok((Vec::new(), Vec::new()));
     };
 
@@ -301,7 +298,7 @@ fn clone_wheel_files(
     // On macOS, directly can be recursively copied with a single `clonefile` call.
     // So we only need to iterate over the top-level of the directory, and copy each file or
     // subdirectory.
-    for entry in std::fs::read_dir(&wheel)? {
+    for entry in fs::read_dir(wheel.as_ref())? {
         let entry = entry?;
         let from = entry.path();
         let to = site_packages
@@ -314,7 +311,7 @@ fn clone_wheel_files(
             .ok();
 
         // Copy the file.
-        reflink_copy::reflink(&from, &to)?;
+        reflink_copy::reflink(&from, &to).map_err(|err| Error::Reflink { from, to, err })?;
 
         count += 1;
     }

crates/install-wheel-rs/src/wheel.rs

@@ -95,7 +95,7 @@ fn parse_scripts<R: Read + Seek>(
                 .map_err(|err| Error::InvalidWheel(format!("entry_points.txt is invalid: {err}")))?
         }
         Err(ZipError::FileNotFound) => return Ok((Vec::new(), Vec::new())),
-        Err(err) => return Err(Error::from_zip_error(entry_points_path, err)),
+        Err(err) => return Err(Error::Zip(entry_points_path, err)),
     };
 
     // TODO: handle extras
@@ -163,9 +163,10 @@ fn unpack_wheel_files<R: Read + Seek>(
     let mut created_dirs = FxHashSet::default();
     // https://github.com/zip-rs/zip/blob/7edf2489d5cff8b80f02ee6fc5febf3efd0a9442/examples/extract.rs
     for i in 0..archive.len() {
-        let mut file = archive
+        let mut file = archive.by_index(i).map_err(|err| {
-            .by_index(i)
+            let file1 = format!("(index {i})");
-            .map_err(|err| Error::from_zip_error(format!("(index {i})"), err))?;
+            Error::Zip(file1, err)
+        })?;
         // enclosed_name takes care of evil zip paths
         let relative = match file.enclosed_name() {
             Some(path) => path.to_owned(),
@@ -583,7 +584,7 @@ pub fn relative_to(path: &Path, base: &Path) -> Result<PathBuf, Error> {
                 .map(|stripped| (stripped, ancestor))
         })
         .ok_or_else(|| {
-            Error::IO(io::Error::new(
+            Error::Io(io::Error::new(
                 io::ErrorKind::Other,
                 format!(
                     "trivial strip case should have worked: {} vs {}",
@@ -926,8 +927,10 @@ pub fn install_wheel(
 
     debug!(name = name.as_ref(), "Opening zip");
     // No BufReader: https://github.com/zip-rs/zip/issues/381
-    let mut archive =
+    let mut archive = ZipArchive::new(reader).map_err(|err| {
-        ZipArchive::new(reader).map_err(|err| Error::from_zip_error("(index)".to_string(), err))?;
+        let file = "(index)".to_string();
+        Error::Zip(file, err)
+    })?;
 
     debug!(name = name.as_ref(), "Getting wheel metadata");
     let dist_info_prefix = find_dist_info(filename, archive.file_names().map(|name| (name, name)))?
@@ -937,11 +940,10 @@ pub fn install_wheel(
     // TODO: Check that name and version match
 
     let record_path = format!("{dist_info_prefix}.dist-info/RECORD");
-    let mut record = read_record_file(
+    let mut record = read_record_file(&mut archive.by_name(&record_path).map_err(|err| {
-        &mut archive
+        let file = record_path.clone();
-            .by_name(&record_path)
+        Error::Zip(file, err)
-            .map_err(|err| Error::from_zip_error(record_path.clone(), err))?,
+    })?)?;
-    )?;
 
     // We're going step by step though
     // https://packaging.python.org/en/latest/specifications/binary-distribution-format/#installing-a-wheel-distribution-1-0-py32-none-any-whl
@@ -951,7 +953,7 @@ pub fn install_wheel(
     let mut wheel_text = String::new();
     archive
         .by_name(&wheel_file_path)
-        .map_err(|err| Error::from_zip_error(wheel_file_path, err))?
+        .map_err(|err| Error::Zip(wheel_file_path, err))?
         .read_to_string(&mut wheel_text)?;
     parse_wheel_version(&wheel_text)?;
     // > 1.c If Root-Is-Purelib == ‘true’, unpack archive into purelib (site-packages).
@@ -1044,7 +1046,10 @@ fn read_metadata(
     let metadata_file = format!("{dist_info_prefix}.dist-info/METADATA");
     archive
         .by_name(&metadata_file)
-        .map_err(|err| Error::from_zip_error(metadata_file.to_string(), err))?
+        .map_err(|err| {
+            let file = metadata_file.to_string();
+            Error::Zip(file, err)
+        })?
         .read_to_end(&mut content)?;
     // HACK: trick mailparse to parse as UTF-8 instead of ASCII
     let mut mail = b"Content-Type: text/plain; charset=utf-8\n".to_vec();