Merge pull request #46 from Skgland/fix-issue-43

Attempt to fix issue 43

Author: pietroalbini

CHANGELOG.md

@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - allow workspaces by having validate_manifest now use `metadata --no-deps` instead of deprecated `read-manifest`
   therefor no longer failing on workspaces and TomlTweaker no longer removing the workspace table from `Cargo.toml`
 - `Command` now warns when it is not used.
+- remove directory/file error now mentions path
 
 ## [0.10.0] - 2020-08-08
 

src/build.rs

@@ -2,7 +2,6 @@ use crate::cmd::{Command, MountKind, Runnable, SandboxBuilder};
 use crate::prepare::Prepare;
 use crate::{Crate, Toolchain, Workspace};
 use failure::Error;
-use remove_dir_all::remove_dir_all;
 use std::path::PathBuf;
 use std::vec::Vec;
 
@@ -146,7 +145,7 @@ impl BuildDirectory {
     ) -> Result<R, Error> {
         let source_dir = self.source_dir();
         if source_dir.exists() {
-            remove_dir_all(&source_dir)?;
+            crate::utils::remove_dir_all(&source_dir)?;
         }
 
         let mut prepare = Prepare::new(&self.workspace, toolchain, krate, &source_dir, patches);
@@ -159,15 +158,15 @@ impl BuildDirectory {
             sandbox,
         })?;
 
-        remove_dir_all(&source_dir)?;
+        crate::utils::remove_dir_all(&source_dir)?;
         Ok(res)
     }
 
     /// Remove all the contents of the build directory, freeing disk space.
     pub fn purge(&mut self) -> Result<(), Error> {
         let build_dir = self.build_dir();
         if build_dir.exists() {
-            remove_dir_all(build_dir)?;
+            crate::utils::remove_dir_all(&build_dir)?;
         }
         Ok(())
     }

src/crates/cratesio.rs

@@ -3,7 +3,6 @@ use crate::Workspace;
 use failure::Error;
 use flate2::read::GzDecoder;
 use log::info;
-use remove_dir_all::remove_dir_all;
 use std::fs::File;
 use std::io::{BufReader, BufWriter, Read};
 use std::path::{Path, PathBuf};
@@ -62,7 +61,7 @@ impl CrateTrait for CratesIOCrate {
     fn purge_from_cache(&self, workspace: &Workspace) -> Result<(), Error> {
         let path = self.cache_path(workspace);
         if path.exists() {
-            std::fs::remove_file(&path)?;
+            crate::utils::remove_file(&path)?;
         }
         Ok(())
     }
@@ -79,7 +78,7 @@ impl CrateTrait for CratesIOCrate {
             dest.display()
         );
         if let Err(err) = unpack_without_first_dir(&mut tar, dest) {
-            let _ = remove_dir_all(dest);
+            let _ = crate::utils::remove_dir_all(dest);
             Err(err
                 .context(format!(
                     "unable to download {} version {}",

src/crates/git.rs

@@ -121,7 +121,7 @@ impl CrateTrait for GitRepo {
     fn purge_from_cache(&self, workspace: &Workspace) -> Result<(), Error> {
         let path = self.cached_path(workspace);
         if path.exists() {
-            remove_dir_all::remove_dir_all(&path)?;
+            crate::utils::remove_dir_all(&path)?;
         }
         Ok(())
     }

src/crates/local.rs

@@ -145,7 +145,7 @@ mod tests {
         println!("{} should cause copy to fail", bad_link.display());
         assert_copy_err_has_filename();
 
-        fs::remove_file(&bad_link)?;
+        crate::utils::remove_file(&bad_link)?;
         // make sure it works without that link
         super::copy_dir(tmp_src.path(), tmp_dest.path())?;
 

src/crates/mod.rs

@@ -5,7 +5,6 @@ mod local;
 use crate::Workspace;
 use failure::Error;
 use log::info;
-use remove_dir_all::remove_dir_all;
 use std::path::Path;
 
 trait CrateTrait: std::fmt::Display {
@@ -69,7 +68,7 @@ impl Crate {
                 "crate source directory {} already exists, cleaning it up",
                 dest.display()
             );
-            remove_dir_all(dest)?;
+            crate::utils::remove_dir_all(dest)?;
         }
         self.as_trait().copy_source_to(workspace, dest)
     }

src/prepare.rs

@@ -2,7 +2,6 @@ use crate::cmd::Command;
 use crate::{build::CratePatch, Crate, Toolchain, Workspace};
 use failure::{Error, Fail, ResultExt};
 use log::info;
-use std::fs::remove_file;
 use std::path::Path;
 use toml::{
     value::{Array, Table},
@@ -73,8 +72,8 @@ impl<'a> Prepare<'a> {
     fn remove_cargo_config(&self) -> Result<(), Error> {
         let path = self.source_dir.join(".cargo").join("config");
         if path.exists() {
-            remove_file(path.clone())?;
-            info!("removed {}", path.as_path().display());
+            crate::utils::remove_file(&path)?;
+            info!("removed {}", path.display());
         }
         Ok(())
     }

src/utils.rs

@@ -50,6 +50,63 @@ fn strip_verbatim_from_prefix(prefix: &PrefixComponent<'_>) -> Option<PathBuf> {
     Some(ret)
 }
 
+pub(crate) fn remove_file(path: &Path) -> std::io::Result<()> {
+    std::fs::remove_file(&path).map_err(|error| crate::utils::improve_remove_error(error, &path))
+}
+
+pub(crate) fn remove_dir_all(path: &Path) -> std::io::Result<()> {
+    remove_dir_all::remove_dir_all(path)
+        .map_err(|error| crate::utils::improve_remove_error(error, path))
+}
+
+#[derive(Debug)]
+struct RemoveError {
+    kind: std::io::ErrorKind,
+    path: PathBuf,
+}
+
+impl std::error::Error for RemoveError {}
+
+impl std::fmt::Display for RemoveError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.write_fmt(format_args!(
+            "failed to remove '{}' : {:?}",
+            self.path.display(),
+            self.kind
+        ))
+    }
+}
+
+fn improve_remove_error(error: std::io::Error, path: &Path) -> std::io::Error {
+    std::io::Error::new(
+        error.kind(),
+        RemoveError {
+            kind: error.kind(),
+            path: path.to_path_buf(),
+        },
+    )
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn custom_remove_error() {
+        let path = "test/path".as_ref();
+
+        let expected = "failed to remove 'test/path' : PermissionDenied";
+        let tested = format!(
+            "{}",
+            improve_remove_error(
+                std::io::Error::from(std::io::ErrorKind::PermissionDenied),
+                path
+            )
+        );
+        assert_eq!(expected, tested);
+    }
+}
+
 pub(crate) fn normalize_path(path: &Path) -> PathBuf {
     let mut p = std::fs::canonicalize(path).unwrap_or_else(|_| path.to_path_buf());
 

src/workspace.rs

@@ -4,7 +4,6 @@ use crate::inside_docker::CurrentContainer;
 use crate::Toolchain;
 use failure::{Error, ResultExt};
 use log::info;
-use remove_dir_all::remove_dir_all;
 use std::path::{Path, PathBuf};
 use std::sync::Arc;
 use std::time::Duration;
@@ -213,7 +212,7 @@ impl Workspace {
     pub fn purge_all_build_dirs(&self) -> Result<(), Error> {
         let dir = self.builds_dir();
         if dir.exists() {
-            remove_dir_all(&dir)?;
+            crate::utils::remove_dir_all(&dir)?;
         }
         Ok(())
     }
@@ -236,7 +235,7 @@ impl Workspace {
 
         for path in &paths {
             if path.exists() {
-                remove_dir_all(&path)?;
+                crate::utils::remove_dir_all(&path)?;
             }
         }