diff --git a/crates/cargo-test-support/src/lib.rs b/crates/cargo-test-support/src/lib.rs index f12f935fa78..fbaa9dd0d9b 100644 --- a/crates/cargo-test-support/src/lib.rs +++ b/crates/cargo-test-support/src/lib.rs @@ -798,13 +798,17 @@ impl Execs { match res { Ok(out) => self.match_output(&out), Err(e) => { - let err = e.downcast_ref::(); - if let Some(&ProcessError { - output: Some(ref out), + if let Some(ProcessError { + stdout: Some(stdout), + stderr: Some(stderr), + code, .. - }) = err + }) = e.downcast_ref::() { - return self.match_output(out); + return self + .match_status(*code, stdout, stderr) + .and(self.match_stdout(stdout, stderr)) + .and(self.match_stderr(stdout, stderr)); } Err(format!("could not exec process {}: {:?}", process, e)) } @@ -813,119 +817,91 @@ impl Execs { fn match_output(&self, actual: &Output) -> MatchResult { self.verify_checks_output(actual); - self.match_status(actual) - .and(self.match_stdout(actual)) - .and(self.match_stderr(actual)) + self.match_status(actual.status.code(), &actual.stdout, &actual.stderr) + .and(self.match_stdout(&actual.stdout, &actual.stderr)) + .and(self.match_stderr(&actual.stdout, &actual.stderr)) } - fn match_status(&self, actual: &Output) -> MatchResult { + fn match_status(&self, code: Option, stdout: &[u8], stderr: &[u8]) -> MatchResult { match self.expect_exit_code { None => Ok(()), - Some(code) if actual.status.code() == Some(code) => Ok(()), + Some(expected) if code == Some(expected) => Ok(()), Some(_) => Err(format!( - "exited with {}\n--- stdout\n{}\n--- stderr\n{}", - actual.status, - String::from_utf8_lossy(&actual.stdout), - String::from_utf8_lossy(&actual.stderr) + "exited with {:?}\n--- stdout\n{}\n--- stderr\n{}", + code, + String::from_utf8_lossy(&stdout), + String::from_utf8_lossy(&stderr) )), } } - fn match_stdout(&self, actual: &Output) -> MatchResult { + fn match_stdout(&self, stdout: &[u8], stderr: &[u8]) -> MatchResult { self.match_std( self.expect_stdout.as_ref(), - &actual.stdout, + stdout, "stdout", - &actual.stderr, + stderr, MatchKind::Exact, )?; for expect in self.expect_stdout_contains.iter() { - self.match_std( - Some(expect), - &actual.stdout, - "stdout", - &actual.stderr, - MatchKind::Partial, - )?; + self.match_std(Some(expect), stdout, "stdout", stderr, MatchKind::Partial)?; } for expect in self.expect_stderr_contains.iter() { - self.match_std( - Some(expect), - &actual.stderr, - "stderr", - &actual.stdout, - MatchKind::Partial, - )?; + self.match_std(Some(expect), stderr, "stderr", stdout, MatchKind::Partial)?; } for &(ref expect, number) in self.expect_stdout_contains_n.iter() { self.match_std( Some(expect), - &actual.stdout, + stdout, "stdout", - &actual.stderr, + stderr, MatchKind::PartialN(number), )?; } for expect in self.expect_stdout_not_contains.iter() { self.match_std( Some(expect), - &actual.stdout, + stdout, "stdout", - &actual.stderr, + stderr, MatchKind::NotPresent, )?; } for expect in self.expect_stderr_not_contains.iter() { self.match_std( Some(expect), - &actual.stderr, + stderr, "stderr", - &actual.stdout, + stdout, MatchKind::NotPresent, )?; } for expect in self.expect_stderr_unordered.iter() { - self.match_std( - Some(expect), - &actual.stderr, - "stderr", - &actual.stdout, - MatchKind::Unordered, - )?; + self.match_std(Some(expect), stderr, "stderr", stdout, MatchKind::Unordered)?; } for expect in self.expect_neither_contains.iter() { self.match_std( Some(expect), - &actual.stdout, + stdout, "stdout", - &actual.stdout, + stdout, MatchKind::NotPresent, )?; self.match_std( Some(expect), - &actual.stderr, + stderr, "stderr", - &actual.stderr, + stderr, MatchKind::NotPresent, )?; } for expect in self.expect_either_contains.iter() { - let match_std = self.match_std( - Some(expect), - &actual.stdout, - "stdout", - &actual.stdout, - MatchKind::Partial, - ); - let match_err = self.match_std( - Some(expect), - &actual.stderr, - "stderr", - &actual.stderr, - MatchKind::Partial, - ); + let match_std = + self.match_std(Some(expect), stdout, "stdout", stdout, MatchKind::Partial); + let match_err = + self.match_std(Some(expect), stderr, "stderr", stderr, MatchKind::Partial); if let (Err(_), Err(_)) = (match_std, match_err) { return Err(format!( @@ -938,12 +914,12 @@ impl Execs { } for (with, without) in self.expect_stderr_with_without.iter() { - self.match_with_without(&actual.stderr, with, without)?; + self.match_with_without(stderr, with, without)?; } if let Some(ref objects) = self.expect_json { - let stdout = str::from_utf8(&actual.stdout) - .map_err(|_| "stdout was not utf8 encoded".to_owned())?; + let stdout = + str::from_utf8(stdout).map_err(|_| "stdout was not utf8 encoded".to_owned())?; let lines = stdout .lines() .filter(|line| line.starts_with('{')) @@ -962,8 +938,8 @@ impl Execs { } if !self.expect_json_contains_unordered.is_empty() { - let stdout = str::from_utf8(&actual.stdout) - .map_err(|_| "stdout was not utf8 encoded".to_owned())?; + let stdout = + str::from_utf8(stdout).map_err(|_| "stdout was not utf8 encoded".to_owned())?; let mut lines = stdout .lines() .filter(|line| line.starts_with('{')) @@ -990,12 +966,12 @@ impl Execs { Ok(()) } - fn match_stderr(&self, actual: &Output) -> MatchResult { + fn match_stderr(&self, stdout: &[u8], stderr: &[u8]) -> MatchResult { self.match_std( self.expect_stderr.as_ref(), - &actual.stderr, + stderr, "stderr", - &actual.stdout, + stdout, MatchKind::Exact, ) } diff --git a/src/bin/cargo/commands/bench.rs b/src/bin/cargo/commands/bench.rs index 160ccd9abe8..79580862e1f 100644 --- a/src/bin/cargo/commands/bench.rs +++ b/src/bin/cargo/commands/bench.rs @@ -74,7 +74,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult { let err = ops::run_benches(&ws, &ops, &bench_args)?; match err { None => Ok(()), - Some(err) => Err(match err.exit.as_ref().and_then(|e| e.code()) { + Some(err) => Err(match err.code { Some(i) => CliError::new(anyhow::format_err!("bench failed"), i), None => CliError::new(err.into(), 101), }), diff --git a/src/bin/cargo/commands/run.rs b/src/bin/cargo/commands/run.rs index ed17e952b31..c8b711364d0 100644 --- a/src/bin/cargo/commands/run.rs +++ b/src/bin/cargo/commands/run.rs @@ -90,7 +90,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult { // If we never actually spawned the process then that sounds pretty // bad and we always want to forward that up. - let exit = match proc_err.exit { + let exit_code = match proc_err.code { Some(exit) => exit, None => return CliError::new(err, 101), }; @@ -98,7 +98,6 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult { // If `-q` was passed then we suppress extra error information about // a failed process, we assume the process itself printed out enough // information about why it failed so we don't do so as well - let exit_code = exit.code().unwrap_or(101); let is_quiet = config.shell().verbosity() == Verbosity::Quiet; if is_quiet { CliError::code(exit_code) diff --git a/src/bin/cargo/commands/test.rs b/src/bin/cargo/commands/test.rs index 04d4dce34fb..7e87afc8dcb 100644 --- a/src/bin/cargo/commands/test.rs +++ b/src/bin/cargo/commands/test.rs @@ -125,7 +125,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult { None => Ok(()), Some(err) => { let context = anyhow::format_err!("{}", err.hint(&ws, &ops.compile_opts)); - let e = match err.exit.as_ref().and_then(|e| e.code()) { + let e = match err.code { // Don't show "process didn't exit successfully" for simple errors. Some(i) if errors::is_simple_exit_code(i) => CliError::new(context, i), Some(i) => CliError::new(Error::from(err).context(context), i), diff --git a/src/bin/cargo/main.rs b/src/bin/cargo/main.rs index 64a465108a1..85b45a360ea 100644 --- a/src/bin/cargo/main.rs +++ b/src/bin/cargo/main.rs @@ -171,7 +171,7 @@ fn execute_external_subcommand(config: &Config, cmd: &str, args: &[&str]) -> Cli }; if let Some(perr) = err.downcast_ref::() { - if let Some(code) = perr.exit.as_ref().and_then(|c| c.code()) { + if let Some(code) = perr.code { return Err(CliError::code(code)); } } diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index d2cd60b2c79..23c3f384f05 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -297,7 +297,7 @@ fn rustc(cx: &mut Context<'_, '_>, unit: &Unit, exec: &Arc) -> Car match err .downcast_ref::() .as_ref() - .and_then(|perr| perr.exit.and_then(|e| e.code())) + .and_then(|perr| perr.code) { Some(n) if errors::is_simple_exit_code(n) => VerboseError::new(err).into(), _ => err, diff --git a/src/cargo/util/errors.rs b/src/cargo/util/errors.rs index acc301a5587..cf4ceb4bfdb 100644 --- a/src/cargo/util/errors.rs +++ b/src/cargo/util/errors.rs @@ -192,14 +192,25 @@ impl<'a> ::std::iter::FusedIterator for ManifestCauses<'a> {} pub struct ProcessError { /// A detailed description to show to the user why the process failed. pub desc: String, + /// The exit status of the process. /// - /// This can be `None` if the process failed to launch (like process not found). - pub exit: Option, - /// The output from the process. + /// This can be `None` if the process failed to launch (like process not + /// found) or if the exit status wasn't a code but was instead something + /// like termination via a signal. + pub code: Option, + + /// The stdout from the process. + /// + /// This can be `None` if the process failed to launch, or the output was + /// not captured. + pub stdout: Option>, + + /// The stderr from the process. /// - /// This can be `None` if the process failed to launch, or the output was not captured. - pub output: Option, + /// This can be `None` if the process failed to launch, or the output was + /// not captured. + pub stderr: Option>, } impl fmt::Display for ProcessError { @@ -218,7 +229,7 @@ impl std::error::Error for ProcessError {} pub struct CargoTestError { pub test: Test, pub desc: String, - pub exit: Option, + pub code: Option, pub causes: Vec, } @@ -254,7 +265,7 @@ impl CargoTestError { CargoTestError { test, desc, - exit: errors[0].exit, + code: errors[0].code, causes: errors, } } @@ -359,20 +370,39 @@ pub fn process_error( output: Option<&Output>, ) -> ProcessError { let exit = match status { - Some(s) => status_to_string(s), + Some(s) => exit_status_to_string(s), None => "never executed".to_string(), }; - let mut desc = format!("{} ({})", &msg, exit); - if let Some(out) = output { - match str::from_utf8(&out.stdout) { + process_error_raw( + msg, + status.and_then(|s| s.code()), + &exit, + output.map(|s| s.stdout.as_slice()), + output.map(|s| s.stderr.as_slice()), + ) +} + +pub fn process_error_raw( + msg: &str, + code: Option, + status: &str, + stdout: Option<&[u8]>, + stderr: Option<&[u8]>, +) -> ProcessError { + let mut desc = format!("{} ({})", msg, status); + + if let Some(out) = stdout { + match str::from_utf8(out) { Ok(s) if !s.trim().is_empty() => { desc.push_str("\n--- stdout\n"); desc.push_str(s); } Ok(..) | Err(..) => {} } - match str::from_utf8(&out.stderr) { + } + if let Some(out) = stderr { + match str::from_utf8(out) { Ok(s) if !s.trim().is_empty() => { desc.push_str("\n--- stderr\n"); desc.push_str(s); @@ -381,11 +411,16 @@ pub fn process_error( } } - return ProcessError { + ProcessError { desc, - exit: status, - output: output.cloned(), - }; + code, + stdout: stdout.map(|s| s.to_vec()), + stderr: stderr.map(|s| s.to_vec()), + } +} + +pub fn exit_status_to_string(status: ExitStatus) -> String { + return status_to_string(status); #[cfg(unix)] fn status_to_string(status: ExitStatus) -> String { diff --git a/src/cargo/util/mod.rs b/src/cargo/util/mod.rs index b9ca5797fa2..f0408d2a920 100644 --- a/src/cargo/util/mod.rs +++ b/src/cargo/util/mod.rs @@ -4,7 +4,7 @@ pub use self::canonical_url::CanonicalUrl; pub use self::config::{homedir, Config, ConfigValue}; pub use self::dependency_queue::DependencyQueue; pub use self::diagnostic_server::RustfixDiagnosticServer; -pub use self::errors::{internal, process_error}; +pub use self::errors::{exit_status_to_string, internal, process_error, process_error_raw}; pub use self::errors::{CargoResult, CargoResultExt, CliResult, Test}; pub use self::errors::{CargoTestError, CliError, ProcessError}; pub use self::flock::{FileLock, Filesystem}; diff --git a/src/cargo/util/rustc.rs b/src/cargo/util/rustc.rs index aa713818563..a0fc2df6c94 100644 --- a/src/cargo/util/rustc.rs +++ b/src/cargo/util/rustc.rs @@ -1,4 +1,4 @@ -use std::collections::hash_map::{Entry, HashMap}; +use std::collections::hash_map::HashMap; use std::env; use std::hash::{Hash, Hasher}; use std::path::{Path, PathBuf}; @@ -123,10 +123,19 @@ struct Cache { #[derive(Serialize, Deserialize, Debug, Default)] struct CacheData { rustc_fingerprint: u64, - outputs: HashMap, + outputs: HashMap, successes: HashMap, } +#[derive(Serialize, Deserialize, Debug)] +struct Output { + success: bool, + status: String, + code: Option, + stdout: String, + stderr: String, +} + impl Cache { fn load(rustc: &Path, rustup_rustc: &Path, cache_location: Option) -> Cache { match (cache_location, rustc_fingerprint(rustc, rustup_rustc)) { @@ -180,26 +189,49 @@ impl Cache { fn cached_output(&mut self, cmd: &ProcessBuilder) -> CargoResult<(String, String)> { let key = process_fingerprint(cmd); - match self.data.outputs.entry(key) { - Entry::Occupied(entry) => { - debug!("rustc info cache hit"); - Ok(entry.get().clone()) - } - Entry::Vacant(entry) => { - debug!("rustc info cache miss"); - debug!("running {}", cmd); - let output = cmd.exec_with_output()?; - let stdout = String::from_utf8(output.stdout) - .map_err(|e| anyhow::anyhow!("{}: {:?}", e, e.as_bytes())) - .chain_err(|| anyhow::anyhow!("`{}` didn't return utf8 output", cmd))?; - let stderr = String::from_utf8(output.stderr) - .map_err(|e| anyhow::anyhow!("{}: {:?}", e, e.as_bytes())) - .chain_err(|| anyhow::anyhow!("`{}` didn't return utf8 output", cmd))?; - let output = (stdout, stderr); - entry.insert(output.clone()); - self.dirty = true; - Ok(output) - } + if self.data.outputs.contains_key(&key) { + debug!("rustc info cache hit"); + } else { + debug!("rustc info cache miss"); + debug!("running {}", cmd); + let output = cmd + .build_command() + .output() + .chain_err(|| format!("could not execute process {} (never executed)", cmd))?; + let stdout = String::from_utf8(output.stdout) + .map_err(|e| anyhow::anyhow!("{}: {:?}", e, e.as_bytes())) + .chain_err(|| anyhow::anyhow!("`{}` didn't return utf8 output", cmd))?; + let stderr = String::from_utf8(output.stderr) + .map_err(|e| anyhow::anyhow!("{}: {:?}", e, e.as_bytes())) + .chain_err(|| anyhow::anyhow!("`{}` didn't return utf8 output", cmd))?; + self.data.outputs.insert( + key, + Output { + success: output.status.success(), + status: if output.status.success() { + String::new() + } else { + util::exit_status_to_string(output.status) + }, + code: output.status.code(), + stdout, + stderr, + }, + ); + self.dirty = true; + } + let output = &self.data.outputs[&key]; + if output.success { + Ok((output.stdout.clone(), output.stderr.clone())) + } else { + Err(util::process_error_raw( + &format!("process didn't exit successfully: {}", cmd), + output.code, + &output.status, + Some(output.stdout.as_ref()), + Some(output.stderr.as_ref()), + ) + .into()) } } } diff --git a/tests/testsuite/rustc_info_cache.rs b/tests/testsuite/rustc_info_cache.rs index 26a06072199..24e3e075c66 100644 --- a/tests/testsuite/rustc_info_cache.rs +++ b/tests/testsuite/rustc_info_cache.rs @@ -6,11 +6,6 @@ use std::env; #[cargo_test] fn rustc_info_cache() { - // Currently detects a flag only supported on nightly. All other channels - // always have a miss. - if !cargo_test_support::is_nightly() { - return; - } let p = project() .file("src/main.rs", r#"fn main() { println!("hello"); }"#) .build();