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

Auto-toolstate management follow-up.

Tracking comment: https://github.com/rust-lang/rust/issues/45861#issuecomment-367302777

* 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.

.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")
+        run.never()
-    }
-
-    fn make_run(run: RunConfig) {
-        run.builder.ensure(Docs {
-            compiler: run.builder.compiler(run.builder.top_stage, run.host),
-        });
     }
 
     /// 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,11 +17,18 @@ 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 \
@@ -29,27 +36,38 @@ python2.7 "$X_PY" test --no-fail-fast \
 set -e
 
 cat "$TOOLSTATE_FILE"
+echo
 
-# If this PR is intended to update one of these tools, do not let the build pass
+verify_status() {
-# when they do not test-pass.
+    echo "Verifying status of $1..."
-for TOOL in rls rustfmt clippy; do
+    if echo "$CHANGED_FILES" | grep -q "^M[[:blank:]]$2$"; then
-    echo "Verifying status of $TOOL..."
+        echo "This PR updated '$2', verifying if status is 'test-pass'..."
-    if echo "$CHANGED_FILES" | grep -q "^M[[:blank:]]src/tools/$TOOL$"; then
+        if grep -vq '"'"$1"'":"test-pass"' "$TOOLSTATE_FILE"; then
-        echo "This PR updated 'src/tools/$TOOL', verifying if status is 'test-pass'..."
-        if grep -vq '"'"$TOOL"'[^"]*":"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 "If you do NOT intend to update '$1', please ensure you did not accidentally"
-            echo "change the submodule at 'src/tools/$TOOL'. You may ask your reviewer for the"
+            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

src/tools/publish_toolstate.py

@@ -17,6 +17,11 @@ import json
 import copy
 import datetime
 import collections
+import textwrap
+try:
+    import urllib2
+except ImportError:
+    import urllib.request as urllib2
 
 # List of people to ping when the status of a tool changed.
 MAINTAINERS = {
@@ -24,6 +29,10 @@ MAINTAINERS = {
     'clippy-driver': '@Manishearth @llogiq @mcarton @oli-obk',
     'rls': '@nrc',
     'rustfmt': '@nrc',
+    'book': '@carols10cents @steveklabnik',
+    'nomicon': '@frewsxcv @Gankro',
+    'reference': '@steveklabnik @Havvy @matthewjasper @alercah',
+    'rust-by-example': '@steveklabnik @marioidival @projektir',
 }
 
 
@@ -38,7 +47,12 @@ def read_current_status(current_commit, path):
     return {}
 
 
-def update_latest(current_commit, relevant_pr_number, current_datetime):
+def update_latest(
+    current_commit,
+    relevant_pr_number,
+    relevant_pr_url,
+    current_datetime
+):
     '''Updates `_data/latest.json` to match build result of the given commit.
     '''
     with open('_data/latest.json', 'rb+') as f:
@@ -50,8 +64,13 @@ def update_latest(current_commit, relevant_pr_number, current_datetime):
         }
 
         slug = 'rust-lang/rust'
-        message = '📣 Toolstate changed by {}!\n\nTested on commit {}@{}.\n\n' \
+        message = textwrap.dedent('''\
-            .format(relevant_pr_number, slug, current_commit)
+            📣 Toolstate changed by {}!
+
+            Tested on commit {}@{}.
+            Direct link to PR: <{}>
+
+        ''').format(relevant_pr_number, slug, current_commit, relevant_pr_url)
         anything_changed = False
         for status in latest:
             tool = status['tool']
@@ -68,7 +87,7 @@ def update_latest(current_commit, relevant_pr_number, current_datetime):
                 elif new < old:
                     changed = True
                     message += '💔 {} on {}: {} → {} (cc {}).\n' \
-                        .format(tool, os, old, new, MAINTAINERS[tool])
+                        .format(tool, os, old, new, MAINTAINERS.get(tool))
 
             if changed:
                 status['commit'] = current_commit
@@ -89,17 +108,41 @@ if __name__ == '__main__':
     cur_datetime = datetime.datetime.utcnow().strftime('%Y-%m-%dT%H:%M:%SZ')
     cur_commit_msg = sys.argv[2]
     save_message_to_path = sys.argv[3]
+    github_token = sys.argv[4]
 
-    relevant_pr_match = re.search('#[0-9]+', cur_commit_msg)
+    relevant_pr_match = re.search('#([0-9]+)', cur_commit_msg)
     if relevant_pr_match:
-        relevant_pr_number = 'rust-lang/rust' + relevant_pr_match.group(0)
+        number = relevant_pr_match.group(1)
+        relevant_pr_number = 'rust-lang/rust#' + number
+        relevant_pr_url = 'https://github.com/rust-lang/rust/pull/' + number
     else:
+        number = '-1'
         relevant_pr_number = '<unknown PR>'
+        relevant_pr_url = '<unknown>'
+
+    message = update_latest(
+        cur_commit,
+        relevant_pr_number,
+        relevant_pr_url,
+        cur_datetime
+    )
+    if not message:
+        print('<Nothing changed>')
+        sys.exit(0)
 
-    message = update_latest(cur_commit, relevant_pr_number, cur_datetime)
-    if message:
     print(message)
     with open(save_message_to_path, 'w') as f:
         f.write(message)
-    else:
+
-        print('<Nothing changed>')
+    # Write the toolstate comment on the PR as well.
+    gh_url = 'https://api.github.com/repos/rust-lang/rust/issues/{}/comments' \
+        .format(number)
+    response = urllib2.urlopen(urllib2.Request(
+        gh_url,
+        json.dumps({'body': message}),
+        {
+            'Authorization': 'token ' + github_token,
+            'Content-Type': 'application/json',
+        }
+    ))
+    response.read()