Auto merge of #8232 - ehuss:fs-error-context, r=alexcrichton

Add context to some fs errors.

This adds some extra context to most fs operations that indicates some more detail (particularly the path).  It can be frustrating when cargo says something generic like "Access is denied." without printing a path or information about what it is doing.

Addresses #8211, where it adds extra context to the message.

src/cargo/core/compiler/mod.rs

@@ -1144,7 +1144,7 @@ fn on_stderr_line(
         // Check if caching is enabled.
         if let Some((path, cell)) = &mut options.cache_cell {
             // Cache the output, which will be replayed later when Fresh.
-            let f = cell.try_borrow_mut_with(|| File::create(path))?;
+            let f = cell.try_borrow_mut_with(|| paths::create(path))?;
             debug_assert!(!line.contains('\n'));
             f.write_all(line.as_bytes())?;
             f.write_all(&[b'\n'])?;
@@ -1332,7 +1332,7 @@ fn replay_output_cache(
         // We sometimes have gigabytes of output from the compiler, so avoid
         // loading it all into memory at once, as that can cause OOM where
         // otherwise there would be none.
-        let file = fs::File::open(&path)?;
+        let file = paths::open(&path)?;
         let mut reader = std::io::BufReader::new(file);
         let mut line = String::new();
         loop {

src/cargo/core/compiler/output_depinfo.rs

@@ -23,7 +23,6 @@
 //! be detected via changes to `Cargo.lock`.
 
 use std::collections::{BTreeSet, HashSet};
-use std::fs::File;
 use std::io::{BufWriter, Write};
 use std::path::{Path, PathBuf};
 
@@ -148,7 +147,7 @@ pub fn output_depinfo(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<()>
                 }
 
                 // Otherwise write it all out
-                let mut outfile = BufWriter::new(File::create(output_path)?);
+                let mut outfile = BufWriter::new(paths::create(output_path)?);
                 write!(outfile, "{}:", target_fn)?;
                 for dep in &deps {
                     write!(outfile, " {}", dep)?;

src/cargo/core/compiler/timings.rs

@@ -10,7 +10,6 @@ use crate::util::cpu::State;
 use crate::util::machine_message::{self, Message};
 use crate::util::{paths, CargoResult, Config};
 use std::collections::HashMap;
-use std::fs::File;
 use std::io::{BufWriter, Write};
 use std::time::{Duration, Instant, SystemTime};
 
@@ -122,6 +121,17 @@ impl<'cfg> Timings<'cfg> {
             .collect();
         let start_str = humantime::format_rfc3339_seconds(SystemTime::now()).to_string();
         let profile = bcx.build_config.requested_profile.to_string();
+        let last_cpu_state = if enabled {
+            match State::current() {
+                Ok(state) => Some(state),
+                Err(e) => {
+                    log::info!("failed to get CPU state, CPU tracking disabled: {:?}", e);
+                    None
+                }
+            }
+        } else {
+            None
+        };
 
         Timings {
             config: bcx.config,
@@ -138,7 +148,7 @@ impl<'cfg> Timings<'cfg> {
             unit_times: Vec::new(),
             active: HashMap::new(),
             concurrency: Vec::new(),
-            last_cpu_state: if enabled { State::current().ok() } else { None },
+            last_cpu_state,
             last_cpu_recording: Instant::now(),
             cpu_usage: Vec::new(),
         }
@@ -287,7 +297,10 @@ impl<'cfg> Timings<'cfg> {
         }
         let current = match State::current() {
             Ok(s) => s,
-            Err(_) => return,
+            Err(e) => {
+                log::info!("failed to get CPU state: {:?}", e);
+                return;
+            }
         };
         let pct_idle = current.idle_since(prev);
         *prev = current;
@@ -323,7 +336,7 @@ impl<'cfg> Timings<'cfg> {
         let duration = d_as_f64(self.start.elapsed());
         let timestamp = self.start_str.replace(&['-', ':'][..], "");
         let filename = format!("cargo-timing-{}.html", timestamp);
-        let mut f = BufWriter::new(File::create(&filename)?);
+        let mut f = BufWriter::new(paths::create(&filename)?);
         let roots: Vec<&str> = self
             .root_targets
             .iter()

src/cargo/ops/cargo_install.rs

@@ -374,9 +374,7 @@ fn install_one(
         if !source_id.is_path() && fs::rename(src, &dst).is_ok() {
             continue;
         }
-        fs::copy(src, &dst).chain_err(|| {
-            format_err!("failed to copy `{}` to `{}`", src.display(), dst.display())
-        })?;
+        paths::copy(src, &dst)?;
     }
 
     let (to_replace, to_install): (Vec<&str>, Vec<&str>) = binaries

src/cargo/ops/cargo_new.rs

@@ -9,7 +9,6 @@ use serde::Deserialize;
 use std::collections::BTreeMap;
 use std::env;
 use std::fmt;
-use std::fs;
 use std::io::{BufRead, BufReader, ErrorKind};
 use std::path::{Path, PathBuf};
 use std::process::Command;
@@ -562,10 +561,10 @@ fn write_ignore_file(
         VersionControl::NoVcs => return Ok("".to_string()),
     };
 
-    let ignore: String = match fs::File::open(&fp_ignore) {
-        Err(why) => match why.kind() {
-            ErrorKind::NotFound => list.format_new(vcs),
-            _ => return Err(anyhow::format_err!("{}", why)),
+    let ignore: String = match paths::open(&fp_ignore) {
+        Err(err) => match err.downcast_ref::<std::io::Error>() {
+            Some(io_err) if io_err.kind() == ErrorKind::NotFound => list.format_new(vcs),
+            _ => return Err(err),
         },
         Ok(file) => list.format_existing(BufReader::new(file), vcs),
     };

src/cargo/ops/fix.rs

@@ -41,7 +41,6 @@
 use std::collections::{BTreeSet, HashMap, HashSet};
 use std::env;
 use std::ffi::OsString;
-use std::fs;
 use std::path::{Path, PathBuf};
 use std::process::{self, Command, ExitStatus};
 use std::str;
@@ -55,7 +54,7 @@ use crate::core::Workspace;
 use crate::ops::{self, CompileOptions};
 use crate::util::diagnostic_server::{Message, RustfixDiagnosticServer};
 use crate::util::errors::CargoResult;
-use crate::util::{self, Config, ProcessBuilder};
+use crate::util::{self, paths, Config, ProcessBuilder};
 use crate::util::{existing_vcs_repo, LockServer, LockServerClient};
 
 const FIX_ENV: &str = "__CARGO_FIX_PLZ";
@@ -256,8 +255,7 @@ pub fn fix_maybe_exec_rustc() -> CargoResult<bool> {
         if !output.status.success() {
             if env::var_os(BROKEN_CODE_ENV).is_none() {
                 for (path, file) in fixes.files.iter() {
-                    fs::write(path, &file.original_code)
-                        .with_context(|| format!("failed to write file `{}`", path))?;
+                    paths::write(path, &file.original_code)?;
                 }
             }
             log_failed_fix(&output.stderr)?;
@@ -517,7 +515,7 @@ fn rustfix_and_fix(
             }
         }
         let new_code = fixed.finish()?;
-        fs::write(&file, new_code).with_context(|| format!("failed to write file `{}`", file))?;
+        paths::write(&file, new_code)?;
     }
 
     Ok(())

src/cargo/ops/vendor.rs

@@ -330,8 +330,7 @@ fn cp_sources(
 
         paths::create_dir_all(dst.parent().unwrap())?;
 
-        fs::copy(&p, &dst)
-            .chain_err(|| format!("failed to copy `{}` to `{}`", p.display(), dst.display()))?;
+        paths::copy(&p, &dst)?;
         let cksum = Sha256::new().update_path(dst)?.finish_hex();
         cksums.insert(relative.to_str().unwrap().replace("\\", "/"), cksum);
     }

src/cargo/sources/git/utils.rs

@@ -10,7 +10,6 @@ use serde::ser;
 use serde::Serialize;
 use std::env;
 use std::fmt;
-use std::fs::File;
 use std::path::{Path, PathBuf};
 use std::process::Command;
 use url::Url;
@@ -363,7 +362,7 @@ impl<'a> GitCheckout<'a> {
         info!("reset {} to {}", self.repo.path().display(), self.revision);
         let object = self.repo.find_object(self.revision.0, None)?;
         reset(&self.repo, &object, config)?;
-        File::create(ok_file)?;
+        paths::create(ok_file)?;
         Ok(())
     }
 

src/cargo/sources/registry/local.rs

@@ -85,7 +85,7 @@ impl<'cfg> RegistryData for LocalRegistry<'cfg> {
         // crate files here never change in that we're not the one writing them,
         // so it's not our responsibility to synchronize access to them.
         let path = self.root.join(&crate_file).into_path_unlocked();
-        let mut crate_file = File::open(&path)?;
+        let mut crate_file = paths::open(&path)?;
 
         // If we've already got an unpacked version of this crate, then skip the
         // checksum below as it is in theory already verified.

src/cargo/sources/registry/mod.rs

@@ -468,7 +468,8 @@ impl<'cfg> RegistrySource<'cfg> {
             .create(true)
             .read(true)
             .write(true)
-            .open(&path)?;
+            .open(&path)
+            .chain_err(|| format!("failed to open `{}`", path.display()))?;
 
         let gz = GzDecoder::new(tarball);
         let mut tar = Archive::new(gz);

src/cargo/sources/registry/remote.rs

@@ -225,7 +225,7 @@ impl<'cfg> RegistryData for RemoteRegistry<'cfg> {
 
         // Create a dummy file to record the mtime for when we updated the
         // index.
-        File::create(&path.join(LAST_UPDATED_FILE))?;
+        paths::create(&path.join(LAST_UPDATED_FILE))?;
 
         Ok(())
     }
@@ -283,7 +283,8 @@ impl<'cfg> RegistryData for RemoteRegistry<'cfg> {
             .create(true)
             .read(true)
             .write(true)
-            .open(&path)?;
+            .open(&path)
+            .chain_err(|| format!("failed to open `{}`", path.display()))?;
         let meta = dst.metadata()?;
         if meta.len() > 0 {
             return Ok(dst);

src/cargo/util/config/mod.rs

@@ -1622,9 +1622,11 @@ pub fn save_credentials(cfg: &Config, token: String, registry: Option<String>) -
 
     let contents = toml.to_string();
     file.seek(SeekFrom::Start(0))?;
-    file.write_all(contents.as_bytes())?;
+    file.write_all(contents.as_bytes())
+        .chain_err(|| format!("failed to write to `{}`", file.path().display()))?;
     file.file().set_len(contents.len() as u64)?;
-    set_permissions(file.file(), 0o600)?;
+    set_permissions(file.file(), 0o600)
+        .chain_err(|| format!("failed to set permissions of `{}`", file.path().display()))?;
 
     return Ok(());
 

src/cargo/util/flock.rs

@@ -148,8 +148,7 @@ impl Filesystem {
     /// Handles errors where other Cargo processes are also attempting to
     /// concurrently create this directory.
     pub fn create_dir(&self) -> CargoResult<()> {
-        paths::create_dir_all(&self.root)?;
-        Ok(())
+        paths::create_dir_all(&self.root)
     }
 
     /// Returns an adaptor that can be used to print the path of this

src/cargo/util/paths.rs

@@ -1,6 +1,6 @@
 use std::env;
 use std::ffi::{OsStr, OsString};
-use std::fs::{self, OpenOptions};
+use std::fs::{self, File, OpenOptions};
 use std::io;
 use std::io::prelude::*;
 use std::iter;
@@ -162,6 +162,18 @@ pub fn append(path: &Path, contents: &[u8]) -> CargoResult<()> {
     Ok(())
 }
 
+/// Creates a new file.
+pub fn create<P: AsRef<Path>>(path: P) -> CargoResult<File> {
+    let path = path.as_ref();
+    File::create(path).chain_err(|| format!("failed to create file `{}`", path.display()))
+}
+
+/// Opens an existing file.
+pub fn open<P: AsRef<Path>>(path: P) -> CargoResult<File> {
+    let path = path.as_ref();
+    File::open(path).chain_err(|| format!("failed to open file `{}`", path.display()))
+}
+
 pub fn mtime(path: &Path) -> CargoResult<FileTime> {
     let meta = fs::metadata(path).chain_err(|| format!("failed to stat `{}`", path.display()))?;
     Ok(FileTime::from_last_modification_time(&meta))
@@ -265,7 +277,11 @@ pub fn remove_dir_all<P: AsRef<Path>>(p: P) -> CargoResult<()> {
 }
 
 fn _remove_dir_all(p: &Path) -> CargoResult<()> {
-    if p.symlink_metadata()?.file_type().is_symlink() {
+    if p.symlink_metadata()
+        .chain_err(|| format!("could not get metadata for `{}` to remove", p.display()))?
+        .file_type()
+        .is_symlink()
+    {
         return remove_file(p);
     }
     let entries = p
@@ -394,6 +410,14 @@ fn _link_or_copy(src: &Path, dst: &Path) -> CargoResult<()> {
     Ok(())
 }
 
+/// Copies a file from one location to another.
+pub fn copy<P: AsRef<Path>, Q: AsRef<Path>>(from: P, to: Q) -> CargoResult<u64> {
+    let from = from.as_ref();
+    let to = to.as_ref();
+    fs::copy(from, to)
+        .chain_err(|| format!("failed to copy `{}` to `{}`", from.display(), to.display()))
+}
+
 /// Changes the filesystem mtime (and atime if possible) for the given file.
 ///
 /// This intentionally does not return an error, as this is sometimes not

src/cargo/util/sha256.rs

@@ -1,4 +1,4 @@
-use crate::util::{CargoResult, CargoResultExt};
+use crate::util::{paths, CargoResult, CargoResultExt};
 use crypto_hash::{Algorithm, Hasher};
 use std::fs::File;
 use std::io::{self, Read, Write};
@@ -30,7 +30,7 @@ impl Sha256 {
 
     pub fn update_path<P: AsRef<Path>>(&mut self, path: P) -> CargoResult<&mut Sha256> {
         let path = path.as_ref();
-        let file = File::open(path)?;
+        let file = paths::open(path)?;
         self.update_file(&file)
             .chain_err(|| format!("failed to read `{}`", path.display()))?;
         Ok(self)