From f2961638c8b9c4494b962236aabfa9daa531f029 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Fri, 2 Oct 2020 19:51:36 -0400 Subject: [PATCH 1/3] Place all-targets checking behind a flag This matches Cargo behavior and avoids the (somewhat expensive) double checking, as well as the unfortunate duplicate error messages (#76822, rust-lang/cargo#5128). --- src/bootstrap/CHANGELOG.md | 2 +- src/bootstrap/builder.rs | 2 +- src/bootstrap/check.rs | 69 +++++++++++++++++++++----------------- src/bootstrap/flags.rs | 10 +++++- 4 files changed, 50 insertions(+), 33 deletions(-) diff --git a/src/bootstrap/CHANGELOG.md b/src/bootstrap/CHANGELOG.md index fe426c4cec7..c9c1f90277a 100644 --- a/src/bootstrap/CHANGELOG.md +++ b/src/bootstrap/CHANGELOG.md @@ -6,7 +6,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## [Non-breaking changes since the last major version] -None. +- x.py check needs opt-in to check tests (--all-targets) [#77473](https://github.com/rust-lang/rust/pull/77473) ## [Version 2] - 2020-09-25 diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs index 4beeb9c87c4..6856cd338cf 100644 --- a/src/bootstrap/builder.rs +++ b/src/bootstrap/builder.rs @@ -532,7 +532,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 } => (Kind::Check, &paths[..]), + Subcommand::Check { ref paths, all_targets: _ } => (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 ead0bd0413b..371631154f7 100644 --- a/src/bootstrap/check.rs +++ b/src/bootstrap/check.rs @@ -1,9 +1,12 @@ //! Implementation of compiling the compiler and standard library, in "check"-based modes. -use crate::builder::{Builder, Kind, RunConfig, ShouldRun, Step}; use crate::compile::{add_to_sysroot, run_cargo, rustc_cargo, std_cargo}; use crate::config::TargetSelection; use crate::tool::{prepare_tool_cargo, SourceType}; +use crate::{ + builder::{Builder, Kind, RunConfig, ShouldRun, Step}, + Subcommand, +}; use crate::{Compiler, Mode}; use std::path::PathBuf; @@ -74,35 +77,37 @@ impl Step for Std { // // Currently only the "libtest" tree of crates does this. - 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"); + 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") { - cargo.arg("-p").arg(krate.name); + // 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") { + cargo.arg("-p").arg(krate.name); + } + + builder.info(&format!( + "Checking std test/bench/example targets ({} -> {})", + &compiler.host, target + )); + run_cargo( + builder, + cargo, + args(builder.kind), + &libstd_test_stamp(builder, compiler, target), + vec![], + true, + ); } - - builder.info(&format!( - "Checking std test/bench/example targets ({} -> {})", - &compiler.host, target - )); - run_cargo( - builder, - cargo, - args(builder.kind), - &libstd_test_stamp(builder, compiler, target), - vec![], - true, - ); } } @@ -143,7 +148,9 @@ impl Step for Rustc { cargo_subcommand(builder.kind), ); rustc_cargo(builder, &mut cargo, target); - cargo.arg("--all-targets"); + if let Subcommand::Check { all_targets: true, .. } = builder.config.cmd { + 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 @@ -205,7 +212,9 @@ macro_rules! tool_check_step { &[], ); - cargo.arg("--all-targets"); + if let Subcommand::Check { all_targets: true, .. } = builder.config.cmd { + cargo.arg("--all-targets"); + } builder.info(&format!( "Checking {} artifacts ({} -> {})", diff --git a/src/bootstrap/flags.rs b/src/bootstrap/flags.rs index 795244e7cd4..c1a9d4fcd23 100644 --- a/src/bootstrap/flags.rs +++ b/src/bootstrap/flags.rs @@ -47,6 +47,9 @@ pub enum Subcommand { paths: Vec, }, Check { + // Whether to run checking over all targets (e.g., unit / integration + // tests). + all_targets: bool, paths: Vec, }, Clippy { @@ -250,6 +253,9 @@ To learn more about a subcommand, run `./x.py -h`", `//rustfix_missing_coverage.txt`", ); } + "check" => { + opts.optflag("", "all-targets", "Check all targets"); + } "bench" => { opts.optmulti("", "test-args", "extra arguments", "ARGS"); } @@ -484,7 +490,9 @@ Arguments: let cmd = match subcommand.as_str() { "build" | "b" => Subcommand::Build { paths }, - "check" | "c" => Subcommand::Check { paths }, + "check" | "c" => { + Subcommand::Check { paths, all_targets: matches.opt_present("all-targets") } + } "clippy" => Subcommand::Clippy { paths }, "fix" => Subcommand::Fix { paths }, "test" | "t" => Subcommand::Test { From bcab97c12e7522c09de74f7f58a3f8e81ca2943f Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Fri, 2 Oct 2020 19:53:05 -0400 Subject: [PATCH 2/3] Check all Cargo targets on CI --- src/ci/docker/host-x86_64/mingw-check/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ci/docker/host-x86_64/mingw-check/Dockerfile b/src/ci/docker/host-x86_64/mingw-check/Dockerfile index 8fc9a009dce..b2aa5844e47 100644 --- a/src/ci/docker/host-x86_64/mingw-check/Dockerfile +++ b/src/ci/docker/host-x86_64/mingw-check/Dockerfile @@ -24,7 +24,7 @@ COPY host-x86_64/mingw-check/validate-toolstate.sh /scripts/ ENV RUN_CHECK_WITH_PARALLEL_QUERIES 1 ENV SCRIPT python3 ../x.py --stage 2 test src/tools/expand-yaml-anchors && \ - python3 ../x.py check --target=i686-pc-windows-gnu --host=i686-pc-windows-gnu && \ + python3 ../x.py check --target=i686-pc-windows-gnu --host=i686-pc-windows-gnu --all-targets && \ python3 ../x.py build --stage 0 src/tools/build-manifest && \ python3 ../x.py test --stage 0 src/tools/compiletest && \ python3 ../x.py test --stage 2 src/tools/tidy && \ From eaa0186662b0a494b2536f75f9e2811b6aa8366b Mon Sep 17 00:00:00 2001 From: ecstatic-morse Date: Sat, 3 Oct 2020 09:29:50 -0700 Subject: [PATCH 3/3] Add quotes around command in CHANGELOG --- src/bootstrap/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bootstrap/CHANGELOG.md b/src/bootstrap/CHANGELOG.md index c9c1f90277a..d8c704f451b 100644 --- a/src/bootstrap/CHANGELOG.md +++ b/src/bootstrap/CHANGELOG.md @@ -6,7 +6,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## [Non-breaking changes since the last major version] -- x.py check needs opt-in to check tests (--all-targets) [#77473](https://github.com/rust-lang/rust/pull/77473) +- `x.py check` needs opt-in to check tests (--all-targets) [#77473](https://github.com/rust-lang/rust/pull/77473) ## [Version 2] - 2020-09-25