Skip to content

Commit

Permalink
Enable --all-targets for x.py check unconditionally
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jyn514 committed Aug 13, 2021
1 parent 2fc3c69 commit c6f4eed
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 42 deletions.
2 changes: 1 addition & 1 deletion src/bootstrap/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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[..]),
Expand Down
66 changes: 29 additions & 37 deletions src/bootstrap/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
}
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)]`
Expand Down
10 changes: 6 additions & 4 deletions src/bootstrap/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,6 @@ pub enum Subcommand {
paths: Vec<PathBuf>,
},
Check {
// Whether to run checking over all targets (e.g., unit / integration
// tests).
all_targets: bool,
paths: Vec<PathBuf>,
},
Clippy {
Expand Down Expand Up @@ -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 },
Expand Down

0 comments on commit c6f4eed

Please sign in to comment.