Auto merge of #46393 - kennytm:45861-step-2-3-make-tools-job-not-fail-fast, r=alexcrichton

[auto-toolstate][2+3/8] Move external tools tests into its own job with --no-fail-fast

This PR performs these  things:

1. The `aux` job now performs "cargotest" and "pretty" tests. The clippy/rustfmt/rls/miri tests are moved into its own job.
2. These tests are run with `--no-fail-fast`, so that we can get the maximum number of failures of all tools from a single CI run.
3. The test results are stored into a JSON file, ready to be uploaded in the future.

This is step 2 and 3/8 of automatic management of broken tools #45861.

Author: bors

.travis.yml

@@ -168,7 +168,7 @@ matrix:
       if: branch = auto
     - env: IMAGE=x86_64-gnu-aux
       if: branch = auto
-    - env: IMAGE=x86_64-gnu-cargotest
+    - env: IMAGE=x86_64-gnu-tools
       if: branch = auto
     - env: IMAGE=x86_64-gnu-debug
       if: branch = auto

appveyor.yml

@@ -25,10 +25,10 @@ environment:
     RUST_CHECK_TARGET: check-aux
     RUST_CONFIGURE_ARGS: --build=x86_64-pc-windows-msvc
 
-  # MSVC cargotest
+  # MSVC tools tests
   - MSYS_BITS: 64
-    SCRIPT: python x.py test src/tools/cargotest
-    RUST_CONFIGURE_ARGS: --build=x86_64-pc-windows-msvc
+    SCRIPT: src/ci/docker/x86_64-gnu-tools/checktools.sh x.py toolstates.json
+    RUST_CONFIGURE_ARGS: --build=x86_64-pc-windows-msvc --save-toolstates=toolstates.json
 
   # 32/64-bit MinGW builds.
   #

config.toml.example

@@ -301,6 +301,10 @@
 # As a side-effect also generates MIR for all libraries.
 #test-miri = false
 
+# After building or testing extended tools (e.g. clippy and rustfmt), append the
+# result (broken, compiling, testing) into this JSON file.
+#save-toolstates = "/path/to/toolstates.json"
+
 # =============================================================================
 # Options for specific targets
 #

src/bootstrap/check.rs

@@ -65,19 +65,21 @@ impl fmt::Display for TestKind {
     }
 }
 
-fn try_run_expecting(build: &Build, cmd: &mut Command, expect: BuildExpectation) {
+fn try_run_expecting(build: &Build, cmd: &mut Command, expect: BuildExpectation) -> bool {
     if !build.fail_fast {
         if !build.try_run(cmd, expect) {
             let mut failures = build.delayed_failures.borrow_mut();
             failures.push(format!("{:?}", cmd));
+            return false;
         }
     } else {
         build.run_expecting(cmd, expect);
     }
+    true
 }
 
 fn try_run(build: &Build, cmd: &mut Command) {
-    try_run_expecting(build, cmd, BuildExpectation::None)
+    try_run_expecting(build, cmd, BuildExpectation::None);
 }
 
 fn try_run_quiet(build: &Build, cmd: &mut Command) {
@@ -257,11 +259,13 @@ impl Step for Rls {
 
         builder.add_rustc_lib_path(compiler, &mut cargo);
 
-        try_run_expecting(
+        if try_run_expecting(
             build,
             &mut cargo,
             builder.build.config.toolstate.rls.passes(ToolState::Testing),
-        );
+        ) {
+            build.save_toolstate("rls", ToolState::Testing);
+        }
     }
 }
 
@@ -305,11 +309,13 @@ impl Step for Rustfmt {
 
         builder.add_rustc_lib_path(compiler, &mut cargo);
 
-        try_run_expecting(
+        if try_run_expecting(
             build,
             &mut cargo,
             builder.build.config.toolstate.rustfmt.passes(ToolState::Testing),
-        );
+        ) {
+            build.save_toolstate("rustfmt", ToolState::Testing);
+        }
     }
 }
 
@@ -354,11 +360,13 @@ impl Step for Miri {
 
             builder.add_rustc_lib_path(compiler, &mut cargo);
 
-            try_run_expecting(
+            if try_run_expecting(
                 build,
                 &mut cargo,
                 builder.build.config.toolstate.miri.passes(ToolState::Testing),
-            );
+            ) {
+                build.save_toolstate("miri", ToolState::Testing);
+            }
         } else {
             eprintln!("failed to test miri: could not build");
         }
@@ -411,11 +419,13 @@ impl Step for Clippy {
 
             builder.add_rustc_lib_path(compiler, &mut cargo);
 
-            try_run_expecting(
+            if try_run_expecting(
                 build,
                 &mut cargo,
                 builder.build.config.toolstate.clippy.passes(ToolState::Testing),
-            );
+            ) {
+                build.save_toolstate("clippy-driver", ToolState::Testing);
+            }
         } else {
             eprintln!("failed to test clippy: could not build");
         }

src/bootstrap/config.rs

@@ -112,6 +112,8 @@ pub struct Config {
     pub channel: String,
     pub quiet_tests: bool,
     pub test_miri: bool,
+    pub save_toolstates: Option<PathBuf>,
+
     // Fallback musl-root for all targets
     pub musl_root: Option<PathBuf>,
     pub prefix: Option<PathBuf>,
@@ -279,6 +281,7 @@ struct Rust {
     dist_src: Option<bool>,
     quiet_tests: Option<bool>,
     test_miri: Option<bool>,
+    save_toolstates: Option<String>,
 }
 
 /// TOML representation of how each build target is configured.
@@ -473,6 +476,7 @@ impl Config {
             set(&mut config.test_miri, rust.test_miri);
             config.rustc_default_linker = rust.default_linker.clone();
             config.musl_root = rust.musl_root.clone().map(PathBuf::from);
+            config.save_toolstates = rust.save_toolstates.clone().map(PathBuf::from);
 
             match rust.codegen_units {
                 Some(0) => config.rust_codegen_units = Some(num_cpus::get() as u32),

src/bootstrap/configure.py

@@ -77,6 +77,7 @@ def v(*args):
 o("debuginfo-lines", "rust.debuginfo-lines", "build with line number debugger metadata")
 o("debuginfo-only-std", "rust.debuginfo-only-std", "build only libstd with debugging information")
 o("debug-jemalloc", "rust.debug-jemalloc", "build jemalloc with --enable-debug --enable-fill")
+v("save-toolstates", "rust.save-toolstates", "save build and test status of external tools into this file")
 
 v("prefix", "install.prefix", "set installation prefix")
 v("localstatedir", "install.localstatedir", "local state directory")

src/bootstrap/lib.rs

@@ -190,6 +190,7 @@ mod job {
 pub use config::Config;
 use flags::Subcommand;
 use cache::{Interned, INTERNER};
+use toolstate::ToolState;
 
 /// A structure representing a Rust compiler.
 ///
@@ -874,6 +875,30 @@ impl Build {
         }
     }
 
+    /// Updates the actual toolstate of a tool.
+    ///
+    /// The toolstates are saved to the file specified by the key
+    /// `rust.save-toolstates` in `config.toml`. If unspecified, nothing will be
+    /// done. The file is updated immediately after this function completes.
+    pub fn save_toolstate(&self, tool: &str, state: ToolState) {
+        use std::io::{Seek, SeekFrom};
+
+        if let Some(ref path) = self.config.save_toolstates {
+            let mut file = t!(fs::OpenOptions::new()
+                .create(true)
+                .read(true)
+                .write(true)
+                .open(path));
+
+            let mut current_toolstates: HashMap<Box<str>, ToolState> =
+                serde_json::from_reader(&mut file).unwrap_or_default();
+            current_toolstates.insert(tool.into(), state);
+            t!(file.seek(SeekFrom::Start(0)));
+            t!(file.set_len(0));
+            t!(serde_json::to_writer(file, &current_toolstates));
+        }
+    }
+
     /// Get a list of crates from a root crate.
     ///
     /// Returns Vec<(crate, path to crate, is_root_crate)>

src/bootstrap/mk/Makefile.in

@@ -53,9 +53,7 @@ check:
 check-aux:
 	$(Q)$(BOOTSTRAP) test \
 		src/tools/cargo \
-		src/tools/rls \
-		src/tools/rustfmt \
-		src/tools/miri \
+		src/tools/cargotest \
 		src/test/pretty \
 		src/test/run-pass/pretty \
 		src/test/run-fail/pretty \

src/bootstrap/tool.rs

@@ -115,7 +115,19 @@ impl Step for ToolBuild {
         println!("Building stage{} tool {} ({})", compiler.stage, tool, target);
 
         let mut cargo = prepare_tool_cargo(builder, compiler, target, "build", path);
-        if !build.try_run(&mut cargo, expectation) {
+        let is_expected = build.try_run(&mut cargo, expectation);
+        // If the expectation is "Failing", `try_run` returning true actually
+        // means a build-failure is successfully observed, i.e. the tool is
+        // broken. Thus the XOR here.
+        // Sorry for the complicated logic, but we can remove this expectation
+        // logic after #45861 is fully fixed.
+        build.save_toolstate(tool, if is_expected ^ (expectation == BuildExpectation::Failing) {
+            ToolState::Compiling
+        } else {
+            ToolState::Broken
+        });
+
+        if !is_expected {
             if expectation == BuildExpectation::None {
                 exit(1);
             } else {

src/bootstrap/toolstate.rs

@@ -10,7 +10,7 @@
 
 use build_helper::BuildExpectation;
 
-#[derive(Copy, Clone, Debug, Deserialize, PartialEq, Eq)]
+#[derive(Copy, Clone, Debug, Deserialize, Serialize, PartialEq, Eq)]
 /// Whether a tool can be compiled, tested or neither
 pub enum ToolState {
     /// The tool compiles successfully, but the test suite fails

src/ci/docker/x86_64-gnu-aux/Dockerfile

@@ -12,7 +12,11 @@ RUN apt-get update && apt-get install -y --no-install-recommends \
   libssl-dev \
   sudo \
   xz-utils \
-  pkg-config
+  pkg-config \
+  libgl1-mesa-dev \
+  llvm-dev \
+  libfreetype6-dev \
+  libexpat1-dev
 
 COPY scripts/sccache.sh /scripts/
 RUN sh /scripts/sccache.sh

src/ci/docker/x86_64-gnu-cargotest/Dockerfile renamed to src/ci/docker/x86_64-gnu-tools/Dockerfile

@@ -12,14 +12,12 @@ RUN apt-get update && apt-get install -y --no-install-recommends \
   libssl-dev \
   sudo \
   xz-utils \
-  pkg-config \
-  libgl1-mesa-dev \
-  llvm-dev \
-  libfreetype6-dev \
-  libexpat1-dev
+  pkg-config
 
 COPY scripts/sccache.sh /scripts/
 RUN sh /scripts/sccache.sh
 
-ENV RUST_CONFIGURE_ARGS --build=x86_64-unknown-linux-gnu
-ENV SCRIPT python2.7 ../x.py test src/tools/cargotest
+COPY x86_64-gnu-tools/checktools.sh /tmp/
+
+ENV RUST_CONFIGURE_ARGS --build=x86_64-unknown-linux-gnu --save-toolstates=/tmp/toolstates.json
+ENV SCRIPT /tmp/checktools.sh ../x.py /tmp/toolstates.json

src/ci/docker/x86_64-gnu-tools/checktools.sh

@@ -0,0 +1,39 @@
+#!/bin/sh
+
+# Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+# file at the top-level directory of this distribution and at
+# http://rust-lang.org/COPYRIGHT.
+#
+# Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+# http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+# <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+# option. This file may not be copied, modified, or distributed
+# except according to those terms.
+
+set -eu
+
+X_PY="$1"
+TOOLSTATE_FILE="$2"
+
+touch "$TOOLSTATE_FILE"
+
+set +e
+python2.7 "$X_PY" test --no-fail-fast \
+    src/tools/rls \
+    src/tools/rustfmt \
+    src/tools/miri \
+    src/tools/clippy
+TEST_RESULT=$?
+set -e
+
+# FIXME: Upload this file to the repository.
+cat "$TOOLSTATE_FILE"
+
+# FIXME: After we can properly inform dev-tool maintainers about failure,
+#        comment out the `exit 0` below.
+if [ "$RUST_RELEASE_CHANNEL" = nightly ]; then
+    # exit 0
+    true
+fi
+
+exit $TEST_RESULT

src/ci/run.sh

@@ -42,8 +42,9 @@ fi
 #
 # FIXME: need a scheme for changing this `nightly` value to `beta` and `stable`
 #        either automatically or manually.
+export RUST_RELEASE_CHANNEL=nightly
 if [ "$DEPLOY$DEPLOY_ALT" != "" ]; then
-  RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --release-channel=nightly"
+  RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --release-channel=$RUST_RELEASE_CHANNEL"
   RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --enable-llvm-static-stdcpp"
 
   if [ "$NO_LLVM_ASSERTIONS" = "1" ]; then