Auto merge of #48405 - kennytm:autotoolstate-follow-up, r=Mark-Simulacrum

Auto-toolstate management follow-up.

Tracking comment: #45861 (comment)

* Fixed rust-lang-nursery/rust-toolstate#1, a proper link to the PR will be included.
* Fixed rust-lang-nursery/rust-toolstate#2, a comment will be posted to the PR if the toolstate changed
* Toolstate regression will be rejected at the last week of the 6-week cycle (currently entirely date-based).
* Implemented https://internals.rust-lang.org/t/the-current-submodule-setup-is-not-tenable/6593, moved doc tests of Nomicon, Reference, Rust-by-Example and The Book to the "tools" job and thus allowed to fail like other external tools.

Author: bors

.travis.yml

@@ -188,7 +188,7 @@ matrix:
       script:
         MESSAGE_FILE=$(mktemp -t msg.XXXXXX);
         . src/ci/docker/x86_64-gnu-tools/repo.sh;
-        commit_toolstate_change "$MESSAGE_FILE" "$TRAVIS_BUILD_DIR/src/tools/publish_toolstate.py" "$(git rev-parse HEAD)" "$(git log --format=%s -n1 HEAD)" "$MESSAGE_FILE"
+        commit_toolstate_change "$MESSAGE_FILE" "$TRAVIS_BUILD_DIR/src/tools/publish_toolstate.py" "$(git rev-parse HEAD)" "$(git log --format=%s -n1 HEAD)" "$MESSAGE_FILE" "$TOOLSTATE_REPO_ACCESS_TOKEN";
 
 env:
   global:

src/bootstrap/builder.rs

@@ -231,7 +231,7 @@ pub struct ShouldRun<'a> {
     paths: BTreeSet<PathSet>,
 
     // If this is a default rule, this is an additional constraint placed on
-    // it's run. Generally something like compiler docs being enabled.
+    // its run. Generally something like compiler docs being enabled.
     is_really_default: bool,
 }
 
@@ -326,7 +326,9 @@ impl<'a> Builder<'a> {
                 test::RunPassPretty, test::RunFailPretty, test::RunPassValgrindPretty,
                 test::RunPassFullDepsPretty, test::RunFailFullDepsPretty, test::RunMake,
                 test::Crate, test::CrateLibrustc, test::Rustdoc, test::Linkcheck, test::Cargotest,
-                test::Cargo, test::Rls, test::Docs, test::ErrorIndex, test::Distcheck,
+                test::Cargo, test::Rls, test::ErrorIndex, test::Distcheck,
+                test::Nomicon, test::Reference, test::RustdocBook, test::RustByExample,
+                test::TheBook, test::UnstableBook,
                 test::Rustfmt, test::Miri, test::Clippy, test::RustdocJS, test::RustdocTheme),
             Kind::Bench => describe!(test::Crate, test::CrateLibrustc),
             Kind::Doc => describe!(doc::UnstableBook, doc::UnstableBookGen, doc::TheBook,

src/bootstrap/compile.rs

@@ -629,6 +629,8 @@ impl Step for CodegenBackend {
             .arg(build.src.join("src/librustc_trans/Cargo.toml"));
         rustc_cargo_env(build, &mut cargo);
 
+        let _folder = build.fold_output(|| format!("stage{}-rustc_trans", compiler.stage));
+
         match &*self.backend {
             "llvm" | "emscripten" => {
                 // Build LLVM for our target. This will implicitly build the
@@ -642,7 +644,6 @@ impl Step for CodegenBackend {
                     features.push_str(" emscripten");
                 }
 
-                let _folder = build.fold_output(|| format!("stage{}-rustc_trans", compiler.stage));
                 println!("Building stage{} codegen artifacts ({} -> {}, {})",
                          compiler.stage, &compiler.host, target, self.backend);
 

src/bootstrap/test.rs

@@ -78,15 +78,17 @@ fn try_run(build: &Build, cmd: &mut Command) -> bool {
     true
 }
 
-fn try_run_quiet(build: &Build, cmd: &mut Command) {
+fn try_run_quiet(build: &Build, cmd: &mut Command) -> bool {
     if !build.fail_fast {
         if !build.try_run_quiet(cmd) {
             let mut failures = build.delayed_failures.borrow_mut();
             failures.push(format!("{:?}", cmd));
+            return false;
         }
     } else {
         build.run_quiet(cmd);
     }
+    true
 }
 
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
@@ -994,23 +996,19 @@ impl Step for Compiletest {
 }
 
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
-pub struct Docs {
+struct DocTest {
     compiler: Compiler,
+    path: &'static str,
+    name: &'static str,
+    is_ext_doc: bool,
 }
 
-impl Step for Docs {
+impl Step for DocTest {
     type Output = ();
-    const DEFAULT: bool = true;
     const ONLY_HOSTS: bool = true;
 
     fn should_run(run: ShouldRun) -> ShouldRun {
-        run.path("src/doc")
-    }
-
-    fn make_run(run: RunConfig) {
-        run.builder.ensure(Docs {
-            compiler: run.builder.compiler(run.builder.top_stage, run.host),
-        });
+        run.never()
     }
 
     /// Run `rustdoc --test` for all documentation in `src/doc`.
@@ -1026,9 +1024,9 @@ impl Step for Docs {
 
         // Do a breadth-first traversal of the `src/doc` directory and just run
         // tests for all files that end in `*.md`
-        let mut stack = vec![build.src.join("src/doc")];
+        let mut stack = vec![build.src.join(self.path)];
         let _time = util::timeit();
-        let _folder = build.fold_output(|| "test_docs");
+        let _folder = build.fold_output(|| format!("test_{}", self.name));
 
         while let Some(p) = stack.pop() {
             if p.is_dir() {
@@ -1046,11 +1044,64 @@ impl Step for Docs {
                 continue;
             }
 
-            markdown_test(builder, compiler, &p);
+            let test_result = markdown_test(builder, compiler, &p);
+            if self.is_ext_doc {
+                let toolstate = if test_result {
+                    ToolState::TestPass
+                } else {
+                    ToolState::TestFail
+                };
+                build.save_toolstate(self.name, toolstate);
+            }
         }
     }
 }
 
+macro_rules! test_book {
+    ($($name:ident, $path:expr, $book_name:expr, default=$default:expr;)+) => {
+        $(
+            #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
+            pub struct $name {
+                compiler: Compiler,
+            }
+
+            impl Step for $name {
+                type Output = ();
+                const DEFAULT: bool = $default;
+                const ONLY_HOSTS: bool = true;
+
+                fn should_run(run: ShouldRun) -> ShouldRun {
+                    run.path($path)
+                }
+
+                fn make_run(run: RunConfig) {
+                    run.builder.ensure($name {
+                        compiler: run.builder.compiler(run.builder.top_stage, run.host),
+                    });
+                }
+
+                fn run(self, builder: &Builder) {
+                    builder.ensure(DocTest {
+                        compiler: self.compiler,
+                        path: $path,
+                        name: $book_name,
+                        is_ext_doc: !$default,
+                    });
+                }
+            }
+        )+
+    }
+}
+
+test_book!(
+    Nomicon, "src/doc/nomicon", "nomicon", default=false;
+    Reference, "src/doc/reference", "reference", default=false;
+    RustdocBook, "src/doc/rustdoc", "rustdoc", default=true;
+    RustByExample, "src/doc/rust-by-example", "rust-by-example", default=false;
+    TheBook, "src/doc/book", "book", default=false;
+    UnstableBook, "src/doc/unstable-book", "unstable-book", default=true;
+);
+
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
 pub struct ErrorIndex {
     compiler: Compiler,
@@ -1101,13 +1152,13 @@ impl Step for ErrorIndex {
     }
 }
 
-fn markdown_test(builder: &Builder, compiler: Compiler, markdown: &Path) {
+fn markdown_test(builder: &Builder, compiler: Compiler, markdown: &Path) -> bool {
     let build = builder.build;
     let mut file = t!(File::open(markdown));
     let mut contents = String::new();
     t!(file.read_to_string(&mut contents));
     if !contents.contains("```") {
-        return;
+        return true;
     }
 
     println!("doc tests for: {}", markdown.display());
@@ -1121,9 +1172,9 @@ fn markdown_test(builder: &Builder, compiler: Compiler, markdown: &Path) {
     cmd.arg("--test-args").arg(test_args);
 
     if build.config.quiet_tests {
-        try_run_quiet(build, &mut cmd);
+        try_run_quiet(build, &mut cmd)
     } else {
-        try_run(build, &mut cmd);
+        try_run(build, &mut cmd)
     }
 }
 

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

@@ -17,6 +17,7 @@ RUN apt-get update && apt-get install -y --no-install-recommends \
 COPY scripts/sccache.sh /scripts/
 RUN sh /scripts/sccache.sh
 
+COPY x86_64-gnu-tools/checkregression.py /tmp/
 COPY x86_64-gnu-tools/checktools.sh /tmp/
 COPY x86_64-gnu-tools/repo.sh /tmp/
 

src/ci/docker/x86_64-gnu-tools/checkregression.py

@@ -0,0 +1,40 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
+# Copyright 2018 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.
+
+import sys
+import json
+
+if __name__ == '__main__':
+    os_name = sys.argv[1]
+    toolstate_file = sys.argv[2]
+    current_state = sys.argv[3]
+
+    with open(toolstate_file, 'r') as f:
+        toolstate = json.load(f)
+    with open(current_state, 'r') as f:
+        current = json.load(f)
+
+    regressed = False
+    for cur in current:
+        tool = cur['tool']
+        state = cur[os_name]
+        new_state = toolstate.get(tool, '')
+        if new_state < state:
+            print(
+                'Error: The state of "{}" has regressed from "{}" to "{}"'
+                .format(tool, state, new_state)
+            )
+            regressed = True
+
+    if regressed:
+        sys.exit(1)

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

@@ -17,39 +17,57 @@ TOOLSTATE_FILE="$(realpath $2)"
 OS="$3"
 COMMIT="$(git rev-parse HEAD)"
 CHANGED_FILES="$(git diff --name-status HEAD HEAD^)"
+SIX_WEEK_CYCLE="$(( ($(date +%s) / 604800 - 3) % 6 ))"
+# ^ 1970 Jan 1st is a Thursday, and our release dates are also on Thursdays,
+#   thus we could divide by 604800 (7 days in seconds) directly.
 
 touch "$TOOLSTATE_FILE"
 
 set +e
 python2.7 "$X_PY" test --no-fail-fast \
+    src/doc/book \
+    src/doc/nomicon \
+    src/doc/reference \
+    src/doc/rust-by-example \
     src/tools/rls \
     src/tools/rustfmt \
     src/tools/miri \
     src/tools/clippy
 set -e
 
 cat "$TOOLSTATE_FILE"
+echo
 
-# If this PR is intended to update one of these tools, do not let the build pass
-# when they do not test-pass.
-for TOOL in rls rustfmt clippy; do
-    echo "Verifying status of $TOOL..."
-    if echo "$CHANGED_FILES" | grep -q "^M[[:blank:]]src/tools/$TOOL$"; then
-        echo "This PR updated 'src/tools/$TOOL', verifying if status is 'test-pass'..."
-        if grep -vq '"'"$TOOL"'[^"]*":"test-pass"' "$TOOLSTATE_FILE"; then
+verify_status() {
+    echo "Verifying status of $1..."
+    if echo "$CHANGED_FILES" | grep -q "^M[[:blank:]]$2$"; then
+        echo "This PR updated '$2', verifying if status is 'test-pass'..."
+        if grep -vq '"'"$1"'":"test-pass"' "$TOOLSTATE_FILE"; then
             echo
-            echo "⚠️ We detected that this PR updated '$TOOL', but its tests failed."
+            echo "⚠️ We detected that this PR updated '$1', but its tests failed."
             echo
-            echo "If you do intend to update '$TOOL', please check the error messages above and"
+            echo "If you do intend to update '$1', please check the error messages above and"
             echo "commit another update."
             echo
-            echo "If you do NOT intend to update '$TOOL', please ensure you did not accidentally"
-            echo "change the submodule at 'src/tools/$TOOL'. You may ask your reviewer for the"
+            echo "If you do NOT intend to update '$1', please ensure you did not accidentally"
+            echo "change the submodule at '$2'. You may ask your reviewer for the"
             echo "proper steps."
             exit 3
         fi
     fi
-done
+}
+
+# If this PR is intended to update one of these tools, do not let the build pass
+# when they do not test-pass.
+
+verify_status book src/doc/book
+verify_status nomicon src/doc/nomicon
+verify_status reference src/doc/reference
+verify_status rust-by-example src/doc/rust-by-example
+verify_status rls src/tool/rls
+verify_status rustfmt src/tool/rustfmt
+verify_status clippy-driver src/tool/clippy
+#verify_status miri src/tool/miri
 
 if [ "$RUST_RELEASE_CHANNEL" = nightly -a -n "${TOOLSTATE_REPO_ACCESS_TOKEN+is_set}" ]; then
     . "$(dirname $0)/repo.sh"
@@ -59,6 +77,11 @@ if [ "$RUST_RELEASE_CHANNEL" = nightly -a -n "${TOOLSTATE_REPO_ACCESS_TOKEN+is_s
         sed -i "1 a\\
 $COMMIT\t$(cat "$TOOLSTATE_FILE")
 " "history/$OS.tsv"
+    # if we are at the last week in the 6-week release cycle, reject any kind of regression.
+    if [ $SIX_WEEK_CYCLE -eq 5 ]; then
+        python2.7 "$(dirname $0)/checkregression.py" \
+            "$OS" "$TOOLSTATE_FILE" "rust-toolstate/_data/latest.json"
+    fi
     rm -f "$MESSAGE_FILE"
     exit 0
 fi