diff --git a/src/cargo/core/compiler/job_queue.rs b/src/cargo/core/compiler/job_queue.rs index c3d76c8b0c7..41c818b9c4f 100644 --- a/src/cargo/core/compiler/job_queue.rs +++ b/src/cargo/core/compiler/job_queue.rs @@ -77,6 +77,7 @@ use crate::core::compiler::future_incompat::{ use crate::core::resolver::ResolveBehavior; use crate::core::{PackageId, Shell, TargetKind}; use crate::util::diagnostic_server::{self, DiagnosticPrinter}; +use crate::util::errors::AlreadyPrintedError; use crate::util::machine_message::{self, Message as _}; use crate::util::CargoResult; use crate::util::{self, internal, profile}; @@ -169,6 +170,41 @@ struct DrainState<'cfg> { per_package_future_incompat_reports: Vec, } +pub struct ErrorsDuringDrain { + pub count: usize, +} + +struct ErrorToHandle { + error: anyhow::Error, + + /// This field is true for "interesting" errors and false for "mundane" + /// errors. If false, we print the above error only if it's the first one + /// encountered so far while draining the job queue. + /// + /// At most places that an error is propagated, we set this to false to + /// avoid scenarios where Cargo might end up spewing tons of redundant error + /// messages. For example if an i/o stream got closed somewhere, we don't + /// care about individually reporting every thread that it broke; just the + /// first is enough. + /// + /// The exception where print_always is true is that we do report every + /// instance of a rustc invocation that failed with diagnostics. This + /// corresponds to errors from Message::Finish. + print_always: bool, +} + +impl From for ErrorToHandle +where + anyhow::Error: From, +{ + fn from(error: E) -> Self { + ErrorToHandle { + error: anyhow::Error::from(error), + print_always: false, + } + } +} + #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] pub struct JobId(pub u32); @@ -612,7 +648,7 @@ impl<'cfg> DrainState<'cfg> { jobserver_helper: &HelperThread, plan: &mut BuildPlan, event: Message, - ) -> CargoResult<()> { + ) -> Result<(), ErrorToHandle> { match event { Message::Run(id, cmd) => { cx.bcx @@ -677,11 +713,14 @@ impl<'cfg> DrainState<'cfg> { debug!("end ({:?}): {:?}", unit, result); match result { Ok(()) => self.finish(id, &unit, artifact, cx)?, - Err(e) => { + Err(error) => { let msg = "The following warnings were emitted during compilation:"; self.emit_warnings(Some(msg), &unit, cx)?; self.back_compat_notice(cx, &unit)?; - return Err(e); + return Err(ErrorToHandle { + error, + print_always: true, + }); } } } @@ -786,14 +825,14 @@ impl<'cfg> DrainState<'cfg> { // After a job has finished we update our internal state if it was // successful and otherwise wait for pending work to finish if it failed // and then immediately return. - let mut error = None; + let mut errors = ErrorsDuringDrain { count: 0 }; // CAUTION! Do not use `?` or break out of the loop early. Every error // must be handled in such a way that the loop is still allowed to // drain event messages. loop { - if error.is_none() { + if errors.count == 0 { if let Err(e) = self.spawn_work_if_possible(cx, jobserver_helper, scope) { - self.handle_error(&mut cx.bcx.config.shell(), &mut error, e); + self.handle_error(&mut cx.bcx.config.shell(), &mut errors, e); } } @@ -804,7 +843,7 @@ impl<'cfg> DrainState<'cfg> { } if let Err(e) = self.grant_rustc_token_requests() { - self.handle_error(&mut cx.bcx.config.shell(), &mut error, e); + self.handle_error(&mut cx.bcx.config.shell(), &mut errors, e); } // And finally, before we block waiting for the next event, drop any @@ -814,7 +853,7 @@ impl<'cfg> DrainState<'cfg> { // to the jobserver itself. for event in self.wait_for_events() { if let Err(event_err) = self.handle_event(cx, jobserver_helper, plan, event) { - self.handle_error(&mut cx.bcx.config.shell(), &mut error, event_err); + self.handle_error(&mut cx.bcx.config.shell(), &mut errors, event_err); } } } @@ -839,30 +878,24 @@ impl<'cfg> DrainState<'cfg> { } let time_elapsed = util::elapsed(cx.bcx.config.creation_time().elapsed()); - if let Err(e) = self.timings.finished(cx, &error) { - if error.is_some() { - crate::display_error(&e, &mut cx.bcx.config.shell()); - } else { - return Some(e); - } + if let Err(e) = self.timings.finished(cx, &errors.to_error()) { + self.handle_error(&mut cx.bcx.config.shell(), &mut errors, e); } if cx.bcx.build_config.emit_json() { let mut shell = cx.bcx.config.shell(); let msg = machine_message::BuildFinished { - success: error.is_none(), + success: errors.count == 0, } .to_json_string(); if let Err(e) = writeln!(shell.out(), "{}", msg) { - if error.is_some() { - crate::display_error(&e.into(), &mut shell); - } else { - return Some(e.into()); - } + self.handle_error(&mut shell, &mut errors, e); } } - if let Some(e) = error { - Some(e) + if let Some(error) = errors.to_error() { + // Any errors up to this point have already been printed via the + // `display_error` inside `handle_error`. + Some(anyhow::Error::new(AlreadyPrintedError::new(error))) } else if self.queue.is_empty() && self.pending_queue.is_empty() { let message = format!( "{} [{}] target(s) in {}", @@ -887,18 +920,18 @@ impl<'cfg> DrainState<'cfg> { fn handle_error( &self, shell: &mut Shell, - err_state: &mut Option, - new_err: anyhow::Error, + err_state: &mut ErrorsDuringDrain, + new_err: impl Into, ) { - if err_state.is_some() { - // Already encountered one error. - log::warn!("{:?}", new_err); - } else if !self.active.is_empty() { - crate::display_error(&new_err, shell); - drop(shell.warn("build failed, waiting for other jobs to finish...")); - *err_state = Some(anyhow::format_err!("build failed")); + let new_err = new_err.into(); + if new_err.print_always || err_state.count == 0 { + crate::display_error(&new_err.error, shell); + if err_state.count == 0 && !self.active.is_empty() { + drop(shell.warn("build failed, waiting for other jobs to finish...")); + } + err_state.count += 1; } else { - *err_state = Some(new_err); + log::warn!("{:?}", new_err.error); } } @@ -1216,3 +1249,13 @@ feature resolver. Try updating to diesel 1.4.8 to fix this error. Ok(()) } } + +impl ErrorsDuringDrain { + fn to_error(&self) -> Option { + match self.count { + 0 => None, + 1 => Some(format_err!("1 job failed")), + n => Some(format_err!("{} jobs failed", n)), + } + } +} diff --git a/src/cargo/lib.rs b/src/cargo/lib.rs index 17fcf8c0fe1..7ce2ce9dd2e 100644 --- a/src/cargo/lib.rs +++ b/src/cargo/lib.rs @@ -13,7 +13,7 @@ use crate::core::Shell; use anyhow::Error; use log::debug; -pub use crate::util::errors::{InternalError, VerboseError}; +pub use crate::util::errors::{AlreadyPrintedError, InternalError, VerboseError}; pub use crate::util::{indented_lines, CargoResult, CliError, CliResult, Config}; pub use crate::version::version; @@ -76,31 +76,27 @@ pub fn display_warning_with_error(warning: &str, err: &Error, shell: &mut Shell) } fn _display_error(err: &Error, shell: &mut Shell, as_err: bool) -> bool { - let verbosity = shell.verbosity(); - let is_verbose = |e: &(dyn std::error::Error + 'static)| -> bool { - verbosity != Verbose && e.downcast_ref::().is_some() - }; - // Generally the top error shouldn't be verbose, but check it anyways. - if is_verbose(err.as_ref()) { - return true; - } - if as_err { - drop(shell.error(&err)); - } else { - drop(writeln!(shell.err(), "{}", err)); - } - for cause in err.chain().skip(1) { - // If we're not in verbose mode then print remaining errors until one + for (i, err) in err.chain().enumerate() { + // If we're not in verbose mode then only print cause chain until one // marked as `VerboseError` appears. - if is_verbose(cause) { + // + // Generally the top error shouldn't be verbose, but check it anyways. + if shell.verbosity() != Verbose && err.is::() { return true; } - drop(writeln!(shell.err(), "\nCaused by:")); - drop(write!( - shell.err(), - "{}", - indented_lines(&cause.to_string()) - )); + if err.is::() { + break; + } + if i == 0 { + if as_err { + drop(shell.error(&err)); + } else { + drop(writeln!(shell.err(), "{}", err)); + } + } else { + drop(writeln!(shell.err(), "\nCaused by:")); + drop(write!(shell.err(), "{}", indented_lines(&err.to_string()))); + } } false } diff --git a/src/cargo/util/errors.rs b/src/cargo/util/errors.rs index 524a5d627c4..3668c92152f 100644 --- a/src/cargo/util/errors.rs +++ b/src/cargo/util/errors.rs @@ -99,6 +99,39 @@ impl fmt::Display for InternalError { } } +// ============================================================================= +// Already printed error + +/// An error that does not need to be printed because it does not add any new +/// information to what has already been printed. +pub struct AlreadyPrintedError { + inner: Error, +} + +impl AlreadyPrintedError { + pub fn new(inner: Error) -> Self { + AlreadyPrintedError { inner } + } +} + +impl std::error::Error for AlreadyPrintedError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + self.inner.source() + } +} + +impl fmt::Debug for AlreadyPrintedError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.inner.fmt(f) + } +} + +impl fmt::Display for AlreadyPrintedError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.inner.fmt(f) + } +} + // ============================================================================= // Manifest error diff --git a/tests/testsuite/build.rs b/tests/testsuite/build.rs index f3b8b16a69b..949cfbfbc8d 100644 --- a/tests/testsuite/build.rs +++ b/tests/testsuite/build.rs @@ -5672,7 +5672,6 @@ fn close_output() { hello stderr! [ERROR] [..] [WARNING] build failed, waiting for other jobs to finish... -[ERROR] [..] ", &stderr, None, diff --git a/tests/testsuite/install.rs b/tests/testsuite/install.rs index 4ba58c9a3d3..99d4c0b163e 100644 --- a/tests/testsuite/install.rs +++ b/tests/testsuite/install.rs @@ -879,11 +879,9 @@ fn compile_failure() { .with_status(101) .with_stderr_contains( "\ +[ERROR] could not compile `foo` due to previous error [ERROR] failed to compile `foo v0.0.1 ([..])`, intermediate artifacts can be \ found at `[..]target` - -Caused by: - could not compile `foo` due to previous error ", ) .run();