Skip to content

Commit 058baec

Browse files
committed
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.
2 parents 22c091c + ce86e86 commit 058baec

File tree

15 files changed

+69
-37
lines changed

15 files changed

+69
-37
lines changed

src/cargo/core/compiler/mod.rs

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

src/cargo/core/compiler/output_depinfo.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
//! be detected via changes to `Cargo.lock`.
2424
2525
use std::collections::{BTreeSet, HashSet};
26-
use std::fs::File;
2726
use std::io::{BufWriter, Write};
2827
use std::path::{Path, PathBuf};
2928

@@ -148,7 +147,7 @@ pub fn output_depinfo(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<()>
148147
}
149148

150149
// Otherwise write it all out
151-
let mut outfile = BufWriter::new(File::create(output_path)?);
150+
let mut outfile = BufWriter::new(paths::create(output_path)?);
152151
write!(outfile, "{}:", target_fn)?;
153152
for dep in &deps {
154153
write!(outfile, " {}", dep)?;

src/cargo/core/compiler/timings.rs

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ use crate::util::cpu::State;
1010
use crate::util::machine_message::{self, Message};
1111
use crate::util::{paths, CargoResult, Config};
1212
use std::collections::HashMap;
13-
use std::fs::File;
1413
use std::io::{BufWriter, Write};
1514
use std::time::{Duration, Instant, SystemTime};
1615

@@ -122,6 +121,17 @@ impl<'cfg> Timings<'cfg> {
122121
.collect();
123122
let start_str = humantime::format_rfc3339_seconds(SystemTime::now()).to_string();
124123
let profile = bcx.build_config.requested_profile.to_string();
124+
let last_cpu_state = if enabled {
125+
match State::current() {
126+
Ok(state) => Some(state),
127+
Err(e) => {
128+
log::info!("failed to get CPU state, CPU tracking disabled: {:?}", e);
129+
None
130+
}
131+
}
132+
} else {
133+
None
134+
};
125135

126136
Timings {
127137
config: bcx.config,
@@ -138,7 +148,7 @@ impl<'cfg> Timings<'cfg> {
138148
unit_times: Vec::new(),
139149
active: HashMap::new(),
140150
concurrency: Vec::new(),
141-
last_cpu_state: if enabled { State::current().ok() } else { None },
151+
last_cpu_state,
142152
last_cpu_recording: Instant::now(),
143153
cpu_usage: Vec::new(),
144154
}
@@ -287,7 +297,10 @@ impl<'cfg> Timings<'cfg> {
287297
}
288298
let current = match State::current() {
289299
Ok(s) => s,
290-
Err(_) => return,
300+
Err(e) => {
301+
log::info!("failed to get CPU state: {:?}", e);
302+
return;
303+
}
291304
};
292305
let pct_idle = current.idle_since(prev);
293306
*prev = current;
@@ -323,7 +336,7 @@ impl<'cfg> Timings<'cfg> {
323336
let duration = d_as_f64(self.start.elapsed());
324337
let timestamp = self.start_str.replace(&['-', ':'][..], "");
325338
let filename = format!("cargo-timing-{}.html", timestamp);
326-
let mut f = BufWriter::new(File::create(&filename)?);
339+
let mut f = BufWriter::new(paths::create(&filename)?);
327340
let roots: Vec<&str> = self
328341
.root_targets
329342
.iter()

src/cargo/ops/cargo_install.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -374,9 +374,7 @@ fn install_one(
374374
if !source_id.is_path() && fs::rename(src, &dst).is_ok() {
375375
continue;
376376
}
377-
fs::copy(src, &dst).chain_err(|| {
378-
format_err!("failed to copy `{}` to `{}`", src.display(), dst.display())
379-
})?;
377+
paths::copy(src, &dst)?;
380378
}
381379

382380
let (to_replace, to_install): (Vec<&str>, Vec<&str>) = binaries

src/cargo/ops/cargo_new.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ use serde::Deserialize;
99
use std::collections::BTreeMap;
1010
use std::env;
1111
use std::fmt;
12-
use std::fs;
1312
use std::io::{BufRead, BufReader, ErrorKind};
1413
use std::path::{Path, PathBuf};
1514
use std::process::Command;
@@ -562,10 +561,10 @@ fn write_ignore_file(
562561
VersionControl::NoVcs => return Ok("".to_string()),
563562
};
564563

565-
let ignore: String = match fs::File::open(&fp_ignore) {
566-
Err(why) => match why.kind() {
567-
ErrorKind::NotFound => list.format_new(vcs),
568-
_ => return Err(anyhow::format_err!("{}", why)),
564+
let ignore: String = match paths::open(&fp_ignore) {
565+
Err(err) => match err.downcast_ref::<std::io::Error>() {
566+
Some(io_err) if io_err.kind() == ErrorKind::NotFound => list.format_new(vcs),
567+
_ => return Err(err),
569568
},
570569
Ok(file) => list.format_existing(BufReader::new(file), vcs),
571570
};

src/cargo/ops/fix.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@
4141
use std::collections::{BTreeSet, HashMap, HashSet};
4242
use std::env;
4343
use std::ffi::OsString;
44-
use std::fs;
4544
use std::path::{Path, PathBuf};
4645
use std::process::{self, Command, ExitStatus};
4746
use std::str;
@@ -55,7 +54,7 @@ use crate::core::Workspace;
5554
use crate::ops::{self, CompileOptions};
5655
use crate::util::diagnostic_server::{Message, RustfixDiagnosticServer};
5756
use crate::util::errors::CargoResult;
58-
use crate::util::{self, Config, ProcessBuilder};
57+
use crate::util::{self, paths, Config, ProcessBuilder};
5958
use crate::util::{existing_vcs_repo, LockServer, LockServerClient};
6059

6160
const FIX_ENV: &str = "__CARGO_FIX_PLZ";
@@ -256,8 +255,7 @@ pub fn fix_maybe_exec_rustc() -> CargoResult<bool> {
256255
if !output.status.success() {
257256
if env::var_os(BROKEN_CODE_ENV).is_none() {
258257
for (path, file) in fixes.files.iter() {
259-
fs::write(path, &file.original_code)
260-
.with_context(|| format!("failed to write file `{}`", path))?;
258+
paths::write(path, &file.original_code)?;
261259
}
262260
}
263261
log_failed_fix(&output.stderr)?;
@@ -517,7 +515,7 @@ fn rustfix_and_fix(
517515
}
518516
}
519517
let new_code = fixed.finish()?;
520-
fs::write(&file, new_code).with_context(|| format!("failed to write file `{}`", file))?;
518+
paths::write(&file, new_code)?;
521519
}
522520

523521
Ok(())

src/cargo/ops/vendor.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -330,8 +330,7 @@ fn cp_sources(
330330

331331
paths::create_dir_all(dst.parent().unwrap())?;
332332

333-
fs::copy(&p, &dst)
334-
.chain_err(|| format!("failed to copy `{}` to `{}`", p.display(), dst.display()))?;
333+
paths::copy(&p, &dst)?;
335334
let cksum = Sha256::new().update_path(dst)?.finish_hex();
336335
cksums.insert(relative.to_str().unwrap().replace("\\", "/"), cksum);
337336
}

src/cargo/sources/git/utils.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ use serde::ser;
1010
use serde::Serialize;
1111
use std::env;
1212
use std::fmt;
13-
use std::fs::File;
1413
use std::path::{Path, PathBuf};
1514
use std::process::Command;
1615
use url::Url;
@@ -363,7 +362,7 @@ impl<'a> GitCheckout<'a> {
363362
info!("reset {} to {}", self.repo.path().display(), self.revision);
364363
let object = self.repo.find_object(self.revision.0, None)?;
365364
reset(&self.repo, &object, config)?;
366-
File::create(ok_file)?;
365+
paths::create(ok_file)?;
367366
Ok(())
368367
}
369368

src/cargo/sources/registry/local.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ impl<'cfg> RegistryData for LocalRegistry<'cfg> {
8585
// crate files here never change in that we're not the one writing them,
8686
// so it's not our responsibility to synchronize access to them.
8787
let path = self.root.join(&crate_file).into_path_unlocked();
88-
let mut crate_file = File::open(&path)?;
88+
let mut crate_file = paths::open(&path)?;
8989

9090
// If we've already got an unpacked version of this crate, then skip the
9191
// checksum below as it is in theory already verified.

src/cargo/sources/registry/mod.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -468,7 +468,8 @@ impl<'cfg> RegistrySource<'cfg> {
468468
.create(true)
469469
.read(true)
470470
.write(true)
471-
.open(&path)?;
471+
.open(&path)
472+
.chain_err(|| format!("failed to open `{}`", path.display()))?;
472473

473474
let gz = GzDecoder::new(tarball);
474475
let mut tar = Archive::new(gz);

0 commit comments

Comments
 (0)