Skip to content

Commit

Permalink
Cache failures in the rustc info cache
Browse files Browse the repository at this point in the history
This commit updates the rustc info cache to cache failures to execute
rustc as well as successes. This fixes a weird issue where if you're
probing for flags the `rustc_info_cache` test fails on channels which
don't have the flag since previously a failure to execute rustc resulted
in never caching the result.
  • Loading branch information
alexcrichton committed Feb 1, 2021
1 parent ed4568e commit cc5e9df
Show file tree
Hide file tree
Showing 10 changed files with 157 additions and 120 deletions.
116 changes: 46 additions & 70 deletions crates/cargo-test-support/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -798,13 +798,17 @@ impl Execs {
match res {
Ok(out) => self.match_output(&out),
Err(e) => {
let err = e.downcast_ref::<ProcessError>();
if let Some(&ProcessError {
output: Some(ref out),
if let Some(ProcessError {
stdout: Some(stdout),
stderr: Some(stderr),
code,
..
}) = err
}) = e.downcast_ref::<ProcessError>()
{
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))
}
Expand All @@ -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<i32>, 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!(
Expand All @@ -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('{'))
Expand All @@ -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('{'))
Expand All @@ -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,
)
}
Expand Down
2 changes: 1 addition & 1 deletion src/bin/cargo/commands/bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}),
Expand Down
3 changes: 1 addition & 2 deletions src/bin/cargo/commands/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,15 +90,14 @@ 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),
};

// 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)
Expand Down
2 changes: 1 addition & 1 deletion src/bin/cargo/commands/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion src/bin/cargo/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ fn execute_external_subcommand(config: &Config, cmd: &str, args: &[&str]) -> Cli
};

if let Some(perr) = err.downcast_ref::<ProcessError>() {
if let Some(code) = perr.exit.as_ref().and_then(|c| c.code()) {
if let Some(code) = perr.code {
return Err(CliError::code(code));
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/core/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ fn rustc(cx: &mut Context<'_, '_>, unit: &Unit, exec: &Arc<dyn Executor>) -> Car
match err
.downcast_ref::<ProcessError>()
.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,
Expand Down
67 changes: 51 additions & 16 deletions src/cargo/util/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<ExitStatus>,
/// 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<i32>,

/// The stdout from the process.
///
/// This can be `None` if the process failed to launch, or the output was
/// not captured.
pub stdout: Option<Vec<u8>>,

/// The stderr from the process.
///
/// This can be `None` if the process failed to launch, or the output was not captured.
pub output: Option<Output>,
/// This can be `None` if the process failed to launch, or the output was
/// not captured.
pub stderr: Option<Vec<u8>>,
}

impl fmt::Display for ProcessError {
Expand All @@ -218,7 +229,7 @@ impl std::error::Error for ProcessError {}
pub struct CargoTestError {
pub test: Test,
pub desc: String,
pub exit: Option<ExitStatus>,
pub code: Option<i32>,
pub causes: Vec<ProcessError>,
}

Expand Down Expand Up @@ -254,7 +265,7 @@ impl CargoTestError {
CargoTestError {
test,
desc,
exit: errors[0].exit,
code: errors[0].code,
causes: errors,
}
}
Expand Down Expand Up @@ -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<i32>,
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);
Expand All @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/util/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down
Loading

0 comments on commit cc5e9df

Please sign in to comment.