diff --git a/cli/bep/BUILD.bazel b/cli/bep/BUILD.bazel index ffbf9277b7..13cacb8127 100644 --- a/cli/bep/BUILD.bazel +++ b/cli/bep/BUILD.bazel @@ -12,6 +12,7 @@ exports_files([ rust_library( name = "dx_bep", srcs = [ + "src/cache_errors.rs", "src/failures.rs", "src/ledger.rs", "src/lib.rs", diff --git a/cli/bep/src/cache_errors.rs b/cli/bep/src/cache_errors.rs new file mode 100644 index 0000000000..6029f62ac6 --- /dev/null +++ b/cli/bep/src/cache_errors.rs @@ -0,0 +1,115 @@ +use std::io::BufRead; +use std::path::Path; + +use serde_json::Value; + +const TEST_OUTPUT_START: &str = "==================== Test output for "; +const SIGNATURES: &[&str] = &["remote cache", "does not exist remotely", "cachenotfound"]; + +/// Reports whether Bazel's own output in the stream at `path` holds a remote cache error line. +/// +/// Test output blocks are skipped. A missing file is an `io::ErrorKind::NotFound` error. +pub fn stream_remote_cache_error(path: &Path) -> std::io::Result { + let reader = std::io::BufReader::new(std::fs::File::open(path)?); + Ok(has_remote_cache_error(reader)) +} + +/// Reports whether the progress output of a build event stream holds a remote cache error line. +pub fn has_remote_cache_error(reader: impl BufRead) -> bool { + let mut text = String::new(); + for line in reader.lines() { + let Ok(line) = line else { break }; + if !line.contains("\"progress\"") { + continue; + } + let Ok(event) = serde_json::from_str::(&line) else { + continue; + }; + if let Some(chunk) = event.pointer("/progress/stderr").and_then(Value::as_str) { + text.push_str(chunk); + } + } + bazel_lines(&text).any(is_cache_error) +} + +fn bazel_lines(text: &str) -> impl Iterator { + let mut in_test_output = false; + text.lines().filter(move |line| { + if line.starts_with(TEST_OUTPUT_START) { + in_test_output = true; + return false; + } + if in_test_output { + if line.len() >= 40 && line.bytes().all(|byte| byte == b'=') { + in_test_output = false; + } + return false; + } + true + }) +} + +fn is_cache_error(line: &str) -> bool { + let Some(rest) = line.trim_start().strip_prefix("ERROR:") else { + return false; + }; + let rest = rest.to_ascii_lowercase(); + SIGNATURES.iter().any(|signature| rest.contains(signature)) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn stream(stderr: &str) -> String { + let event = serde_json::json!({"id": {"progress": {}}, "progress": {"stderr": stderr}}); + format!("{event}\n") + } + + fn found(stderr: &str) -> bool { + has_remote_cache_error(stream(stderr).as_bytes()) + } + + #[test] + fn error_lines_naming_the_remote_cache_count() { + assert!(found("ERROR: Remote Cache: connection refused\n")); + assert!(found( + "INFO: x\nERROR: /w/BUILD:1:1: Compiling x failed: Remote Cache: gone\n" + )); + assert!(found( + "ERROR: Failed to fetch file with hash 'ab' because it does not exist remotely.\n" + )); + } + + #[test] + fn other_lines_do_not() { + assert!(!found("")); + assert!(!found("INFO: Remote Cache hit rate 90%\n")); + assert!(!found("ERROR: /w/BUILD:1:1: Compiling x failed\n")); + assert!(!has_remote_cache_error(&b"not json\n{\"id\":{}}\n"[..])); + } + + #[test] + fn test_output_is_skipped() { + let eq = "=".repeat(80); + let noisy = format!( + "{TEST_OUTPUT_START}//a:t\nERROR: Remote Cache: from the test\n{eq}\nINFO: done\n" + ); + assert!(!found(&noisy)); + let after = format!("{noisy}ERROR: Remote Cache: real\n"); + assert!(found(&after)); + } + + #[test] + fn chunks_join_across_events() { + let mut text = stream("ERROR: Remote "); + text.push_str(&stream("Cache: down\n")); + assert!(has_remote_cache_error(text.as_bytes())); + } + + #[test] + fn missing_file_is_not_found() { + let error = stream_remote_cache_error(Path::new("/nonexistent/bep.json")).unwrap_err(); + assert_eq!(error.kind(), std::io::ErrorKind::NotFound); + } +} diff --git a/cli/bep/src/lib.rs b/cli/bep/src/lib.rs index ef7450e6e6..baa7b3073a 100644 --- a/cli/bep/src/lib.rs +++ b/cli/bep/src/lib.rs @@ -8,12 +8,14 @@ ) )] +pub mod cache_errors; pub mod failures; pub mod ledger; pub mod metrics; pub mod outputs; pub mod test_outputs; +pub use cache_errors::{has_remote_cache_error, stream_remote_cache_error}; pub use failures::{ BuildFailure, BuildFailures, FailedTargetKind, FailureStage, StderrRef, collect_build_failures, }; diff --git a/cli/cli/src/bazel_restart.rs b/cli/cli/src/bazel_restart.rs index 51297b4ead..dcf5cc1c8e 100644 --- a/cli/cli/src/bazel_restart.rs +++ b/cli/cli/src/bazel_restart.rs @@ -5,8 +5,11 @@ //! "Server terminated abruptly"). The client exits 37. The command never ran, //! so running it again is safe. A crash after the command started is never //! repeated. +//! +//! A `build`, `test` or `coverage` call that fails with a remote cache error +//! line in Bazel's own output runs once more with the remote cache off. -use std::cell::RefCell; +use std::cell::{Cell, RefCell}; use std::io::{self, Write}; use std::path::{Path, PathBuf}; use std::time::Duration; @@ -61,6 +64,34 @@ pub fn restartable(code: Option, evidence: Evidence<'_>) -> bool { } } +/// The Bazel options that turn the remote cache off. +pub const CACHE_OFF_FLAGS: [&str; 2] = ["--remote_cache=", "--noremote_accept_cached"]; + +const CACHE_COMMANDS: [&str; 3] = ["build", "test", "coverage"]; + +/// `argv` with the remote cache turned off, or `None` when it is not a `build`, `test` or `coverage` call. +pub fn cache_off_argv(argv: &[String]) -> Option> { + if !is_bazel(argv) { + return None; + } + let command = argv + .iter() + .skip(1) + .position(|arg| !arg.starts_with('-'))? + + 1; + if !CACHE_COMMANDS.contains(&argv[command].as_str()) { + return None; + } + let end = argv[command..] + .iter() + .position(|arg| arg == "--") + .map_or(argv.len(), |at| command + at); + let mut off = argv[..end].to_vec(); + off.extend(CACHE_OFF_FLAGS.map(String::from)); + off.extend_from_slice(&argv[end..]); + Some(off) +} + fn is_bazel(argv: &[String]) -> bool { argv.first() .is_some_and(|arg0| arg0 == dx_process::launcher_argv0()) @@ -115,6 +146,8 @@ impl Policy { pub struct RestartingRunner { inner: R, policy: Policy, + cache_retry: bool, + cache_retries: Cell, } impl RestartingRunner { @@ -122,12 +155,40 @@ impl RestartingRunner { RestartingRunner { inner, policy: Policy::real(), + cache_retry: false, + cache_retries: Cell::new(0), } } + /// Turns the retry without the remote cache on or off; it starts off. + #[must_use] + pub fn with_cache_retry(mut self, on: bool) -> Self { + self.cache_retry = on; + self + } + + /// How many calls ran again without the remote cache. + pub fn cache_retries(&self) -> u32 { + self.cache_retries.get() + } + #[cfg(test)] fn with_policy(inner: R, policy: Policy) -> Self { - RestartingRunner { inner, policy } + RestartingRunner { + inner, + policy, + cache_retry: true, + cache_retries: Cell::new(0), + } + } + + fn cache_failed(&self, argv: &[String], cwd: &Path, status: &ChildStatus) -> bool { + self.cache_retry + && status.code.is_some_and(|code| code != 0) + && !(self.policy.stop)() + && bep_path_of(argv).is_some_and(|path| { + dx_bep::stream_remote_cache_error(&cwd.join(path)).unwrap_or(false) + }) } fn evidence(argv: &[String], cwd: &Path) -> Evidence<'static> { @@ -148,30 +209,47 @@ impl RestartingRunner { argv: &[String], cwd: &Path, withdraw: &dyn Fn() -> bool, - call: &dyn Fn() -> io::Result>, + call: &dyn Fn(&[String]) -> io::Result>, ) -> io::Result> { let mut restarts = 0; + let mut current = argv.to_vec(); + let mut cache_off = false; loop { - let Some(status) = call()? else { + let Some(status) = call(¤t)? else { return Ok(None); }; - if restarts >= MAX_RESTARTS - || !is_bazel(argv) - || status.code != Some(RESTART_EXIT) - || (self.policy.stop)() - || !restartable(status.code, Self::evidence(argv, cwd)) - { + let restart = restarts < MAX_RESTARTS + && is_bazel(argv) + && status.code == Some(RESTART_EXIT) + && !(self.policy.stop)() + && restartable(status.code, Self::evidence(argv, cwd)); + let off = if restart || cache_off || !self.cache_failed(argv, cwd, &status) { + None + } else { + cache_off_argv(argv) + }; + if !restart && off.is_none() { return Ok(Some(status)); } if withdraw() { return Ok(None); } - restarts += 1; if let Some(path) = bep_path_of(argv) { let _ = std::fs::remove_file(cwd.join(path)); } - if !self.policy.pause(RESTART_EXIT, restarts) { - return Ok(Some(status)); + if let Some(off) = off { + self.cache_retries.set(self.cache_retries.get() + 1); + let _ = writeln!( + self.policy.notice.borrow_mut(), + "dx: notice: bazel reported a remote cache error; running once more without the remote cache" + ); + current = off; + cache_off = true; + } else { + restarts += 1; + if !self.policy.pause(RESTART_EXIT, restarts) { + return Ok(Some(status)); + } } if withdraw() { return Ok(None); @@ -182,7 +260,7 @@ impl RestartingRunner { impl Runner for RestartingRunner { fn run(&self, argv: &[String], cwd: &Path, env: &[(&str, &str)]) -> io::Result { - self.attempts(argv, cwd, &|| false, &|| { + self.attempts(argv, cwd, &|| false, &|argv| { self.inner.run(argv, cwd, env).map(Some) }) .map(|status| status.unwrap_or_else(|| ChildStatus::exited(RESTART_EXIT))) @@ -204,7 +282,7 @@ impl Runner for RestartingRunner { env: &[(&str, &str)], withdraw: &dyn Fn() -> bool, ) -> io::Result> { - self.attempts(argv, cwd, withdraw, &|| { + self.attempts(argv, cwd, withdraw, &|argv| { self.inner.run_withdrawable(argv, cwd, env, withdraw) }) } @@ -220,7 +298,7 @@ impl Runner for RestartingRunner { // Restarted clients may start a new Bazel server that would inherit the // handles, so only the first attempt carries them. let first = std::cell::Cell::new(true); - self.attempts(argv, cwd, withdraw.unwrap_or(&|| false), &|| { + self.attempts(argv, cwd, withdraw.unwrap_or(&|| false), &|argv| { let carried: &[&std::fs::File] = if first.replace(false) { held } else { &[] }; self.inner.run_app(argv, cwd, env, withdraw, carried) }) @@ -573,6 +651,119 @@ mod tests { RestartingQueryRunner::with_policy(fake, policy(s.stop.clone(), s.out.clone())) } + fn strs(words: &[&str]) -> Vec { + words.iter().map(ToString::to_string).collect() + } + + #[test] + fn cache_off_argv_inserts_before_the_end_of_options() { + let launcher = dx_process::launcher_argv0(); + let with = |tail: &[&str]| { + let mut argv = vec![launcher.to_owned()]; + argv.extend(tail.iter().map(ToString::to_string)); + argv + }; + assert_eq!( + cache_off_argv(&with(&["--output_base=/x", "test", "--k", "--", "//a:b"])), + Some(with(&[ + "--output_base=/x", + "test", + "--k", + "--remote_cache=", + "--noremote_accept_cached", + "--", + "//a:b" + ])) + ); + assert!(cache_off_argv(&with(&["coverage"])).is_some()); + assert!(cache_off_argv(&with(&["query", "//..."])).is_none()); + assert!(cache_off_argv(&with(&["run", "//a:b"])).is_none()); + assert!(cache_off_argv(&strs(&["git", "build"])).is_none()); + assert!(cache_off_argv(&with(&[])).is_none()); + } + + struct Cache { + calls: RefCell>>, + codes: RefCell>, + stderr: &'static str, + bep: PathBuf, + } + + impl Runner for Cache { + fn run(&self, argv: &[String], _: &Path, _: &[(&str, &str)]) -> io::Result { + let fresh = self.calls.borrow().is_empty(); + self.calls.borrow_mut().push(argv.to_vec()); + assert!(fresh || !self.bep.exists(), "stale stream"); + let text = if fresh { self.stderr } else { "" }; + let event = serde_json::json!({"id": {"progress": {}}, "progress": {"stderr": text}}); + std::fs::write(&self.bep, format!("{event}\n"))?; + let code = self.codes.borrow_mut().pop_front().unwrap_or(0); + Ok(ChildStatus::exited(code)) + } + } + + fn cache_runner(s: &Setup, codes: &[i32], stderr: &'static str) -> RestartingRunner { + let fake = Cache { + calls: RefCell::default(), + codes: RefCell::new(codes.iter().copied().collect()), + stderr, + bep: s.dir.path().join("bep.json"), + }; + RestartingRunner::with_policy(fake, policy(s.stop.clone(), s.out.clone())) + } + + fn bazel_build() -> Vec { + argv(dx_process::launcher_argv0(), true) + } + + #[test] + fn a_remote_cache_error_runs_once_more_without_the_cache() { + let s = setup(); + let r = cache_runner(&s, &[1, 0], "ERROR: Remote Cache: connection refused\n"); + let status = r.run(&bazel_build(), s.dir.path(), &[]).unwrap(); + assert_eq!(status.code, Some(0)); + let calls = r.inner.calls.borrow(); + assert_eq!(calls.len(), 2); + assert_eq!(calls[0], bazel_build()); + assert!(calls[1].ends_with(&strs(&CACHE_OFF_FLAGS))); + assert_eq!(r.cache_retries(), 1); + let text = String::from_utf8(s.out.borrow().clone()).unwrap(); + assert!(text.contains("without the remote cache"), "{text}"); + } + + #[test] + fn the_cache_retry_happens_once() { + let s = setup(); + let r = cache_runner(&s, &[1, 1, 0], "ERROR: Remote Cache: down\n"); + let status = r.run(&bazel_build(), s.dir.path(), &[]).unwrap(); + assert_eq!(status.code, Some(1)); + assert_eq!(r.inner.calls.borrow().len(), 2); + } + + #[test] + fn no_cache_retry_without_a_cache_error_a_failure_or_the_opt_in() { + let s = setup(); + let r = cache_runner(&s, &[1, 0], "ERROR: Compiling x failed\n"); + assert_eq!( + r.run(&bazel_build(), s.dir.path(), &[]).unwrap().code, + Some(1) + ); + assert_eq!(r.inner.calls.borrow().len(), 1); + let r = cache_runner(&s, &[0], "ERROR: Remote Cache: down\n"); + assert_eq!( + r.run(&bazel_build(), s.dir.path(), &[]).unwrap().code, + Some(0) + ); + assert_eq!(r.inner.calls.borrow().len(), 1); + let r = cache_runner(&s, &[1, 0], "ERROR: Remote Cache: down\n").with_cache_retry(false); + assert_eq!( + r.run(&bazel_build(), s.dir.path(), &[]).unwrap().code, + Some(1) + ); + assert_eq!(r.inner.calls.borrow().len(), 1); + assert_eq!(r.cache_retries(), 0); + } + #[test] fn query_restarts_only_on_known_stderr() { let s = setup(); diff --git a/cli/cli/src/exec.rs b/cli/cli/src/exec.rs index 0ed9fc2f90..7cf4ebb6e8 100644 --- a/cli/cli/src/exec.rs +++ b/cli/cli/src/exec.rs @@ -73,7 +73,7 @@ mod worktree; use dx_plan::args::{Command, Invocation}; pub use audit::AUDIT_TODAY_ENV; -pub use common::{ActionTally, Env, print_actions}; +pub use common::{ActionTally, Env, print_actions, print_cache_retries}; pub use failure::{cancel, install_panic_events, install_panic_hint, set_usage_command, slot_stop}; pub(crate) use managed_env::{ENV_ARTIFACTS_DIR, ENV_VALUES_FILE}; pub use update_version::COMMIT_JSON_ENV as EDIT_COMMIT_JSON_ENV; diff --git a/cli/cli/src/exec/common.rs b/cli/cli/src/exec/common.rs index fe7dcaf782..e4c6792ca1 100644 --- a/cli/cli/src/exec/common.rs +++ b/cli/cli/src/exec/common.rs @@ -411,6 +411,16 @@ pub fn print_actions(output: OutputMode, tally: &ActionTally, err: &mut dyn Writ } } +/// Prints on stderr in visible text mode that `retries` calls ran again without the remote cache. +pub fn print_cache_retries(output: OutputMode, retries: u32, err: &mut dyn Write) { + if retries > 0 && dx_output::dx_text_visible(&output) { + let _ = writeln!( + err, + "dx: {retries} Bazel call(s) ran again without the remote cache after a remote cache error" + ); + } +} + fn actions_line(counts: &ActionCounts) -> String { let cached = counts.cached(); format!( diff --git a/cli/cli/src/lib.rs b/cli/cli/src/lib.rs index c56198661c..b203ff0a69 100644 --- a/cli/cli/src/lib.rs +++ b/cli/cli/src/lib.rs @@ -18,5 +18,5 @@ pub mod startup; #[cfg(test)] mod test_support; -pub use exec::{ActionTally, Env, execute, print_actions}; +pub use exec::{ActionTally, Env, execute, print_actions, print_cache_retries}; // LCOV_EXCL_STOP - reason: end re-export only, issue: 1055, policy: docs/cli/commands/build-test-coverage.md diff --git a/cli/cli/src/main.rs b/cli/cli/src/main.rs index a1fc132a81..ca265e638f 100644 --- a/cli/cli/src/main.rs +++ b/cli/cli/src/main.rs @@ -13,7 +13,7 @@ use std::path::Path; use dx_cli::bazel_restart::{RestartingQueryRunner, RestartingRunner}; use dx_cli::exec::common::flush_out; -use dx_cli::{ActionTally, Env, execute, print_actions}; +use dx_cli::{ActionTally, Env, execute, print_actions, print_cache_retries}; use dx_output::OutputMode; use dx_plan::args::{is_discovery_exempt, json_output_intent, parse_with_ci}; use dx_plan::plan::{create_run_temp_dir, remove_on_stop}; @@ -375,8 +375,15 @@ fn run() -> i32 { } dx_plan::plan::remove_stale_run_dirs(&temp_base); let reaped = dx_process::groups::reap_orphans(); - let runner = - RestartingRunner::new(dx_process::BinaryRunner::new(inherit_stdout, die_by_signal)); + let cache_retry = matches!( + invocation.command, + dx_plan::args::Command::Build + | dx_plan::args::Command::Test + | dx_plan::args::Command::Coverage + | dx_plan::args::Command::Check + ) && !invocation.no_retry; + let runner = RestartingRunner::new(dx_process::BinaryRunner::new(inherit_stdout, die_by_signal)) + .with_cache_retry(cache_retry); let query_runner = RestartingQueryRunner::new(BinaryQueryRunner { runner: dx_process::BinaryRunner::new(inherit_stdout, die_by_signal), }); @@ -456,6 +463,7 @@ fn run() -> i32 { ) }); print_actions(invocation.output, &actions, &mut err); + print_cache_retries(invocation.output, runner.cache_retries(), &mut err); let temp_display = temp_dir.path().display().to_string(); if let Err(error) = temp_dir.close() { let _ = writeln!( diff --git a/cli/cli/tests/fixtures/help_goldens/build_help.golden b/cli/cli/tests/fixtures/help_goldens/build_help.golden index 8ac0bab72a..5a29cb052d 100644 --- a/cli/cli/tests/fixtures/help_goldens/build_help.golden +++ b/cli/cli/tests/fixtures/help_goldens/build_help.golden @@ -1,6 +1,6 @@ run Bazel build over resolved targets -Usage: dx build [--here] [--affected [--since ]] [--debug|--release] [--timings] [scope ...] [-- bazel-options ...] +Usage: dx build [--here] [--affected [--since ]] [--debug|--release] [--timings] [--no-retry] [scope ...] [-- bazel-options ...] Arguments: [TARGETS]... @@ -47,6 +47,9 @@ Options: --timings Print where build time went + --no-retry + Do not rerun without the remote cache after a remote cache error + --report Write a report file diff --git a/cli/plan/src/args/command.rs b/cli/plan/src/args/command.rs index ca994d3d3a..d2288ca166 100644 --- a/cli/plan/src/args/command.rs +++ b/cli/plan/src/args/command.rs @@ -193,7 +193,7 @@ pub static COMMANDS: [CommandMeta; 25] = [ name: "build", scope_policy: "default-//...", describe: "run Bazel build over resolved targets", - usage: "Usage: dx build [--here] [--affected [--since ]] [--debug|--release] [--timings] [scope ...] [-- bazel-options ...]", + usage: "Usage: dx build [--here] [--affected [--since ]] [--debug|--release] [--timings] [--no-retry] [scope ...] [-- bazel-options ...]", scopes: "Scopes: explicit Bazel labels/patterns (//..., //pkg:target), or workspace-relative files/dirs resolved via Bazel query. With no scope, this command runs //...; pass --affected to run only what your changes reach. Pass --here for the current directory tree instead (//path/...; //... at the root); --here cannot be combined with explicit scopes. --affected selects targets changed since the default branch merge-base (or the ref given with --since) plus their reverse dependencies; it cannot be combined with scopes or --here.", is_managed: false, is_adoption: false, @@ -207,7 +207,7 @@ pub static COMMANDS: [CommandMeta; 25] = [ name: "test", scope_policy: "default-//...", describe: "run Bazel test over resolved targets", - usage: "Usage: dx test [--here] [--affected [--since ]] [--debug|--release] [--timings] [scope ...] [-- bazel-options ...]", + usage: "Usage: dx test [--here] [--affected [--since ]] [--debug|--release] [--timings] [--no-retry] [scope ...] [-- bazel-options ...]", scopes: "Scopes: explicit Bazel labels/patterns (//..., //pkg:target), or workspace-relative files/dirs resolved via Bazel query. With no scope, this command runs //...; pass --affected to run only what your changes reach. Pass --here for the current directory tree instead (//path/...; //... at the root); --here cannot be combined with explicit scopes. --affected selects targets changed since the default branch merge-base (or the ref given with --since) plus their reverse dependencies; it cannot be combined with scopes or --here.", is_managed: false, is_adoption: false, @@ -221,7 +221,7 @@ pub static COMMANDS: [CommandMeta; 25] = [ name: "coverage", scope_policy: "default-//...", describe: "collect LCOV coverage with optional threshold", - usage: "Usage: dx coverage [--here] [--affected [--since ]] [--timings] [--min-coverage ] [--min-changed-line-coverage ] [scope ...] [-- bazel-options ...]", + usage: "Usage: dx coverage [--here] [--affected [--since ]] [--timings] [--no-retry] [--min-coverage ] [--min-changed-line-coverage ] [scope ...] [-- bazel-options ...]", scopes: "Scopes: explicit Bazel labels/patterns (//..., //pkg:target), or workspace-relative files/dirs resolved via Bazel query. With no scope, this command runs //...; pass --affected to run only what your changes reach. Pass --here for the current directory tree instead (//path/...; //... at the root); --here cannot be combined with explicit scopes. --affected selects targets changed since the default branch merge-base (or the ref given with --since) plus their reverse dependencies; it cannot be combined with scopes or --here.", is_managed: false, is_adoption: false, @@ -277,7 +277,7 @@ pub static COMMANDS: [CommandMeta; 25] = [ name: "check", scope_policy: "default-//...", describe: "run generate+format+lint+typecheck in order (check by default; --apply writes fixes, then verifies with a read-only check)", - usage: "Usage: dx check [--apply] [--here] [--affected [--since ]] [scope ...] [-- bazel-options ...]", + usage: "Usage: dx check [--apply] [--no-retry] [--here] [--affected [--since ]] [scope ...] [-- bazel-options ...]", scopes: "Scopes: explicit Bazel labels/patterns (//..., //pkg:target), or workspace-relative files/dirs resolved via Bazel query. With no scope, this command runs //...; pass --affected to run only what your changes reach. Pass --here for the current directory tree instead (//path/...; //... at the root); --here cannot be combined with explicit scopes. --affected selects targets changed since the default branch merge-base (or the ref given with --since) plus their reverse dependencies; it cannot be combined with scopes or --here.", is_managed: false, is_adoption: false, diff --git a/cli/plan/src/args/grammar.rs b/cli/plan/src/args/grammar.rs index 6bcebee3f7..cdf3ff2ebc 100644 --- a/cli/plan/src/args/grammar.rs +++ b/cli/plan/src/args/grammar.rs @@ -162,6 +162,7 @@ pub(crate) struct Flags { pub(crate) capabilities: bool, pub(crate) files: bool, pub(crate) timings: bool, + pub(crate) no_retry: bool, pub(crate) budget: Option, pub(crate) jobs: Option, pub(crate) max_regression: Option, @@ -790,6 +791,14 @@ composite_group! { } } +flag_group! { + /// Do not rerun without the remote cache after a remote cache error. + NoRetryFlag, { + #[arg(long = "no-retry", overrides_with = "no_retry")] + no_retry: bool, + } +} + flag_group! { /// Print where build time went. TimingsFlag, { @@ -1053,6 +1062,7 @@ command_args! { output: OutputFlag, own: BuildModeArgs, timings: TimingsFlag, + no_retry: NoRetryFlag, report: ReportFlag, here: HereFlag, affected: AffectedFlag, @@ -1063,6 +1073,7 @@ command_args! { output: OutputFlag, own: BuildModeArgs, timings: TimingsFlag, + no_retry: NoRetryFlag, report: ReportFlag, here: HereFlag, affected: AffectedFlag, @@ -1075,6 +1086,7 @@ command_args! { min_coverage: MinCoverageFlag, min_changed_line_coverage: MinChangedLineCoverageFlag, timings: TimingsFlag, + no_retry: NoRetryFlag, report: ReportFlag, here: HereFlag, affected: AffectedFlag, @@ -1105,6 +1117,7 @@ command_args! { apply: ApplyFlag, unsafe_fixes: UnsafeFlag, fail_on: FailOnFlag, + no_retry: NoRetryFlag, report: ReportFlag, here: HereFlag, affected: AffectedFlag, diff --git a/cli/plan/src/args/invocation.rs b/cli/plan/src/args/invocation.rs index 26497a8537..a083d6ac95 100644 --- a/cli/plan/src/args/invocation.rs +++ b/cli/plan/src/args/invocation.rs @@ -324,6 +324,8 @@ pub struct Invocation { pub files: bool, /// `build`/`test`/`coverage --timings`: print where build time went. pub timings: bool, + /// `--no-retry`: do not rerun once without the remote cache after a remote cache error. + pub no_retry: bool, /// `dx fuzz --budget`: the budget per target; 60s when absent. pub budget: Option, /// `dx fuzz --jobs`: jobs each fuzz runner may use. @@ -740,6 +742,7 @@ mod tests { capabilities: false, files: false, timings: false, + no_retry: false, budget: None, jobs: None, bench_regression: None, diff --git a/cli/plan/src/args/parser.rs b/cli/plan/src/args/parser.rs index fc5a47e94d..63ffbc3f8f 100644 --- a/cli/plan/src/args/parser.rs +++ b/cli/plan/src/args/parser.rs @@ -367,6 +367,7 @@ fn parse_inner>( capabilities, files, timings, + no_retry, budget, jobs, max_regression, @@ -741,6 +742,7 @@ fn parse_inner>( capabilities, files, timings, + no_retry, budget, jobs, bench_regression: max_regression, diff --git a/cli/plan/src/args/parser_commands.rs b/cli/plan/src/args/parser_commands.rs index 28f1c0c879..39e4f8e081 100644 --- a/cli/plan/src/args/parser_commands.rs +++ b/cli/plan/src/args/parser_commands.rs @@ -1114,6 +1114,19 @@ fn timings_parses_on_build_test_coverage_and_run() { assert!(parse(&strings(&["lint", "--timings"])).is_err()); } +#[test] +fn no_retry_parses_on_build_test_coverage_and_check() { + for command in ["build", "test", "coverage", "check"] { + assert!( + parse(&strings(&[command, "--no-retry"])) + .expect(command) + .no_retry + ); + assert!(!parse(&strings(&[command])).expect(command).no_retry); + } + assert!(parse(&strings(&["lint", "--no-retry"])).is_err()); +} + #[test] fn explain_is_not_a_command_or_a_flag() { for words in [ diff --git a/docs/cli/commands/build-test-coverage.md b/docs/cli/commands/build-test-coverage.md index 48a184c304..3c7b93083f 100644 --- a/docs/cli/commands/build-test-coverage.md +++ b/docs/cli/commands/build-test-coverage.md @@ -14,19 +14,19 @@ dx deploy --apply //app:deploy ## `dx build` ```text -dx build [--here] [--affected] [--debug|--release] [--timings] [--report failures=] [scope...] [-- bazel-options...] +dx build [--here] [--affected] [--debug|--release] [--timings] [--no-retry] [--report failures=] [scope...] [-- bazel-options...] ``` ## `dx test` ```text -dx test [--here] [--affected] [--debug|--release] [--timings] [--report junit|failures|results=] [scope...] [-- bazel-options...] +dx test [--here] [--affected] [--debug|--release] [--timings] [--no-retry] [--report junit|failures|results=] [scope...] [-- bazel-options...] ``` ## `dx coverage` ```text -dx coverage [--here] [--affected] [--timings] [--min-coverage ] [--min-changed-line-coverage ] [--report lcov|sources|failures|results=] [scope...] [-- bazel-options...] +dx coverage [--here] [--affected] [--timings] [--no-retry] [--min-coverage ] [--min-changed-line-coverage ] [--report lcov|sources|failures|results=] [scope...] [-- bazel-options...] ``` ## Flags @@ -38,6 +38,7 @@ dx coverage [--here] [--affected] [--timings] [--min-coverage ] [--min- | `--debug` | Compile with the `dx_build_debug` build mode (`build`, `test`, `run`, `deploy`). | | `--release` | Compile with the `dx_build_release` build mode. No flag uses `dx_build_dev`. | | `--timings` | (`build`, `test`, `coverage`, `run`; `run` reports on its build step.) Pass `--profile` to Bazel and print where build time went: wall time, critical path with its top five actions (with Bazel mnemonics when the trace has them), cache hit rate (disk, remote, local), bytes downloaded, Bazel peak heap and CPU time per action mnemonic (from the build event stream, absent for `run`), the ten slowest actions and repository fetch time. The trace is `.dx/local/timings/.json.gz` (the last five are kept) and the report `.dx/local/timings/report.json`; with `--output=json` the report is the message of a `timings_report` notice. | +| `--no-retry` | (`build`, `test`, `coverage`, `check`.) Do not run Bazel again without the remote cache after a remote cache error. | | `--min-coverage ` | `coverage`: fail below this whole percent (0-100). | | `--min-changed-line-coverage ` | `coverage`: fail below this whole percent (0-100) on lines changed since the merge-base. | | `--report =` | Write a report. Repeatable. `-` is stdout. | @@ -48,6 +49,8 @@ dx coverage [--here] [--affected] [--timings] [--min-coverage ] [--min- When stdout is not a terminal, dx passes Bazel the quiet flags `--noshow_progress`, `--noshow_loading_progress`, `--noshow_timestamps`, `--curses=no`, `--ui_event_filters=-info,-debug` and `--test_summary=none`. `-v` keeps full Bazel output. +When a Bazel `build`, `test` or `coverage` call fails and its own output (not test output) has an `ERROR:` line about the remote cache, dx runs it once more with `--remote_cache= --noremote_accept_cached`. dx prints a notice when it does, and the summary on stderr says how many calls ran again. `--no-retry` turns this off. + ## Scopes No scope means `//...`. Pass labels, patterns, files, or directories. A file runs its owning targets. For `test` and `coverage`, a file runs the tests that depend on its owners. A `manual` test runs only when it owns the file. diff --git a/docs/cli/commands/check.md b/docs/cli/commands/check.md index e3620ef4c6..030177349d 100644 --- a/docs/cli/commands/check.md +++ b/docs/cli/commands/check.md @@ -9,7 +9,7 @@ dx check --output json --report sarif=out.sarif //pkg/... ## `dx check` ```text -dx check [--here] [--affected] [--apply] [--diff] [--unsafe] [--include-edits] [--max-findings n] [--fail-on info|warning|error] [--report sarif=] [scope...] [-- bazel-options...] +dx check [--here] [--affected] [--no-retry] [--apply] [--diff] [--unsafe] [--include-edits] [--max-findings n] [--fail-on info|warning|error] [--report sarif=] [scope...] [-- bazel-options...] ``` Runs `generate`, `format`, `lint`, and `typecheck` in order. It only reports. `--apply` writes validated fixes, then re-checks read-only. @@ -17,6 +17,7 @@ Runs `generate`, `format`, `lint`, and `typecheck` in order. It only reports. `- | Flag | What it does | | --- | --- | | `--apply` | Write fixes, then verify with a read-only check. | +| `--no-retry` | Do not run Bazel again without the remote cache after a remote cache error. | | `--here` | Use the current directory tree. | | `--affected` | Use the targets your git changes reach, then run the affected tests as a last phase. | | `--diff` | Print the edits as a patch. |