From c6f4eed45c940ed214cc377505aea9bb7d9f8df2 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Fri, 13 Aug 2021 11:03:17 -0500 Subject: [PATCH] Enable `--all-targets` for `x.py check` unconditionally Now that Cargo deduplicates diagnostics from different targets, this doesn't flood the console with duplicate errors. Note that this doesn't add `--all-targets` in `Builder::cargo` directly because `impl Step for Std` actually wants to omit `--all-targets` the first time while it's still building libtest. When passed `--all-targets`, this warns that the option isn't needed, but still continues to compile. --- src/bootstrap/builder.rs | 2 +- src/bootstrap/check.rs | 66 ++++++++++++++++++---------------------- src/bootstrap/flags.rs | 10 +++--- 3 files changed, 36 insertions(+), 42 deletions(-) diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs index 3904c718a2544..e2f348261112e 100644 --- a/src/bootstrap/builder.rs +++ b/src/bootstrap/builder.rs @@ -578,7 +578,7 @@ impl<'a> Builder<'a> { pub fn new(build: &Build) -> Builder<'_> { let (kind, paths) = match build.config.cmd { Subcommand::Build { ref paths } => (Kind::Build, &paths[..]), - Subcommand::Check { ref paths, all_targets: _ } => (Kind::Check, &paths[..]), + Subcommand::Check { ref paths } => (Kind::Check, &paths[..]), Subcommand::Clippy { ref paths, .. } => (Kind::Clippy, &paths[..]), Subcommand::Fix { ref paths } => (Kind::Fix, &paths[..]), Subcommand::Doc { ref paths, .. } => (Kind::Doc, &paths[..]), diff --git a/src/bootstrap/check.rs b/src/bootstrap/check.rs index bc106746e57e0..4eb335979b983 100644 --- a/src/bootstrap/check.rs +++ b/src/bootstrap/check.rs @@ -113,38 +113,35 @@ impl Step for Std { // since we initialize with an empty sysroot. // // Currently only the "libtest" tree of crates does this. + let mut cargo = builder.cargo( + compiler, + Mode::Std, + SourceType::InTree, + target, + cargo_subcommand(builder.kind), + ); + cargo.arg("--all-targets"); + std_cargo(builder, target, compiler.stage, &mut cargo); - if let Subcommand::Check { all_targets: true, .. } = builder.config.cmd { - let mut cargo = builder.cargo( - compiler, - Mode::Std, - SourceType::InTree, - target, - cargo_subcommand(builder.kind), - ); - std_cargo(builder, target, compiler.stage, &mut cargo); - cargo.arg("--all-targets"); - - // Explicitly pass -p for all dependencies krates -- this will force cargo - // to also check the tests/benches/examples for these crates, rather - // than just the leaf crate. - for krate in builder.in_tree_crates("test", Some(target)) { - cargo.arg("-p").arg(krate.name); - } - - builder.info(&format!( - "Checking stage{} std test/bench/example targets ({} -> {})", - builder.top_stage, &compiler.host, target - )); - run_cargo( - builder, - cargo, - args(builder), - &libstd_test_stamp(builder, compiler, target), - vec![], - true, - ); + // Explicitly pass -p for all dependencies krates -- this will force cargo + // to also check the tests/benches/examples for these crates, rather + // than just the leaf crate. + for krate in builder.in_tree_crates("test", Some(target)) { + cargo.arg("-p").arg(krate.name); } + + builder.info(&format!( + "Checking stage{} std test/bench/example targets ({} -> {})", + builder.top_stage, &compiler.host, target + )); + run_cargo( + builder, + cargo, + args(builder), + &libstd_test_stamp(builder, compiler, target), + vec![], + true, + ); } } @@ -195,9 +192,7 @@ impl Step for Rustc { cargo_subcommand(builder.kind), ); rustc_cargo(builder, &mut cargo, target); - if let Subcommand::Check { all_targets: true, .. } = builder.config.cmd { - cargo.arg("--all-targets"); - } + cargo.arg("--all-targets"); // Explicitly pass -p for all compiler krates -- this will force cargo // to also check the tests/benches/examples for these crates, rather @@ -318,10 +313,7 @@ macro_rules! tool_check_step { $source_type, &[], ); - - if let Subcommand::Check { all_targets: true, .. } = builder.config.cmd { - cargo.arg("--all-targets"); - } + cargo.arg("--all-targets"); // Enable internal lints for clippy and rustdoc // NOTE: this doesn't enable lints for any other tools unless they explicitly add `#![warn(rustc::internal)]` diff --git a/src/bootstrap/flags.rs b/src/bootstrap/flags.rs index 7b74a909c286e..80c33fa4d7c97 100644 --- a/src/bootstrap/flags.rs +++ b/src/bootstrap/flags.rs @@ -78,9 +78,6 @@ pub enum Subcommand { paths: Vec, }, Check { - // Whether to run checking over all targets (e.g., unit / integration - // tests). - all_targets: bool, paths: Vec, }, Clippy { @@ -553,7 +550,12 @@ Arguments: let cmd = match subcommand.as_str() { "build" | "b" => Subcommand::Build { paths }, "check" | "c" => { - Subcommand::Check { paths, all_targets: matches.opt_present("all-targets") } + if matches.opt_present("all-targets") { + eprintln!( + "Warning: --all-targets is now on by default and does not need to be passed explicitly." + ); + } + Subcommand::Check { paths } } "clippy" => Subcommand::Clippy { paths, fix: matches.opt_present("fix") }, "fix" => Subcommand::Fix { paths },