From 1acd3789bb70c39f23c8fb13f0db50402ebb146a Mon Sep 17 00:00:00 2001 From: kennytm Date: Wed, 21 Feb 2018 22:25:12 +0800 Subject: [PATCH 1/6] Provides direct link to the PR when toolstate is changed. Fix rust-lang-nursery/rust-toolstate#1. --- src/tools/publish_toolstate.py | 33 ++++++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/src/tools/publish_toolstate.py b/src/tools/publish_toolstate.py index b90947e5a434a..e2dbdb301e286 100755 --- a/src/tools/publish_toolstate.py +++ b/src/tools/publish_toolstate.py @@ -17,6 +17,7 @@ import copy import datetime import collections +import textwrap # List of people to ping when the status of a tool changed. MAINTAINERS = { @@ -38,7 +39,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 +56,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' \ - .format(relevant_pr_number, slug, current_commit) + message = textwrap.dedent('''\ + 📣 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'] @@ -90,13 +101,21 @@ def update_latest(current_commit, relevant_pr_number, current_datetime): cur_commit_msg = sys.argv[2] save_message_to_path = sys.argv[3] - 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: relevant_pr_number = '' - - message = update_latest(cur_commit, relevant_pr_number, cur_datetime) + relevant_pr_url = '' + + message = update_latest( + cur_commit, + relevant_pr_number, + relevant_pr_url, + cur_datetime + ) if message: print(message) with open(save_message_to_path, 'w') as f: From e18105079e4d0b925897fbb786cebb9539662e13 Mon Sep 17 00:00:00 2001 From: kennytm Date: Wed, 21 Feb 2018 22:58:06 +0800 Subject: [PATCH 2/6] Submit a comment to the PR in additional to pushing a commit. Fix rust-lang-nursery/rust-toolstate#2. --- .travis.yml | 2 +- src/tools/publish_toolstate.py | 30 +++++++++++++++++++++++++----- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/.travis.yml b/.travis.yml index 280da05699506..0d8641e45ed15 100644 --- a/.travis.yml +++ b/.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: diff --git a/src/tools/publish_toolstate.py b/src/tools/publish_toolstate.py index e2dbdb301e286..40abe81c449ef 100755 --- a/src/tools/publish_toolstate.py +++ b/src/tools/publish_toolstate.py @@ -18,6 +18,10 @@ 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 = { @@ -100,6 +104,7 @@ def update_latest( 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) if relevant_pr_match: @@ -107,6 +112,7 @@ def update_latest( relevant_pr_number = 'rust-lang/rust#' + number relevant_pr_url = 'https://github.com/rust-lang/rust/pull/' + number else: + number = '-1' relevant_pr_number = '' relevant_pr_url = '' @@ -116,9 +122,23 @@ def update_latest( relevant_pr_url, cur_datetime ) - if message: - print(message) - with open(save_message_to_path, 'w') as f: - f.write(message) - else: + if not message: print('') + sys.exit(0) + + print(message) + with open(save_message_to_path, 'w') as f: + f.write(message) + + # 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() From f6e4751ebeaac327db7fe444ed7991f70e8fd929 Mon Sep 17 00:00:00 2001 From: kennytm Date: Thu, 22 Feb 2018 00:37:14 +0800 Subject: [PATCH 3/6] Disallow toolstate regression at the last week of the 6-week cycle. --- src/ci/docker/x86_64-gnu-tools/Dockerfile | 1 + .../x86_64-gnu-tools/checkregression.py | 40 +++++++++++++++++++ src/ci/docker/x86_64-gnu-tools/checktools.sh | 8 ++++ 3 files changed, 49 insertions(+) create mode 100755 src/ci/docker/x86_64-gnu-tools/checkregression.py diff --git a/src/ci/docker/x86_64-gnu-tools/Dockerfile b/src/ci/docker/x86_64-gnu-tools/Dockerfile index 8975d419d2055..bab9145cbcb9c 100644 --- a/src/ci/docker/x86_64-gnu-tools/Dockerfile +++ b/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/ diff --git a/src/ci/docker/x86_64-gnu-tools/checkregression.py b/src/ci/docker/x86_64-gnu-tools/checkregression.py new file mode 100755 index 0000000000000..df791d12645fd --- /dev/null +++ b/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 or the MIT license +# , 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) diff --git a/src/ci/docker/x86_64-gnu-tools/checktools.sh b/src/ci/docker/x86_64-gnu-tools/checktools.sh index 61bb5a84d21ac..a7d0c6a1e6a7f 100755 --- a/src/ci/docker/x86_64-gnu-tools/checktools.sh +++ b/src/ci/docker/x86_64-gnu-tools/checktools.sh @@ -17,6 +17,9 @@ 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" @@ -59,6 +62,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 From 51238c77e676f399a5b47d922e0da6af4837aff5 Mon Sep 17 00:00:00 2001 From: kennytm Date: Thu, 22 Feb 2018 02:05:32 +0800 Subject: [PATCH 4/6] CI: Fixed the incorrect folder region when building codegen dylib. --- src/bootstrap/compile.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/bootstrap/compile.rs b/src/bootstrap/compile.rs index 2dcc0e0e7cd9f..84dd8f099f3df 100644 --- a/src/bootstrap/compile.rs +++ b/src/bootstrap/compile.rs @@ -630,6 +630,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 @@ -643,7 +645,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); From 0d300d4b9d83b5243217aea98551c1f1676567d4 Mon Sep 17 00:00:00 2001 From: kennytm Date: Thu, 22 Feb 2018 03:13:34 +0800 Subject: [PATCH 5/6] Split test::Docs into one Step for each book. The *.md at the root directory in src/doc are no longer tested, but this should be fine since all files there are deprecated. --- src/bootstrap/builder.rs | 6 ++-- src/bootstrap/test.rs | 65 ++++++++++++++++++++++++++++++++-------- 2 files changed, 57 insertions(+), 14 deletions(-) diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs index 66a1c97246200..49c7fc4770563 100644 --- a/src/bootstrap/builder.rs +++ b/src/bootstrap/builder.rs @@ -231,7 +231,7 @@ pub struct ShouldRun<'a> { paths: BTreeSet, // 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, diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs index bd8c36a296c09..03b7c18c55162 100644 --- a/src/bootstrap/test.rs +++ b/src/bootstrap/test.rs @@ -994,23 +994,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 +1022,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() { @@ -1051,6 +1047,51 @@ impl Step for Docs { } } +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, From a9f940e320bccc86dc7ab8b6179e918d3c05454d Mon Sep 17 00:00:00 2001 From: kennytm Date: Thu, 22 Feb 2018 03:25:23 +0800 Subject: [PATCH 6/6] Run the external doc tests in tools job. --- src/bootstrap/test.rs | 22 ++++++++--- src/ci/docker/x86_64-gnu-tools/checktools.sh | 39 ++++++++++++++------ src/tools/publish_toolstate.py | 6 ++- 3 files changed, 48 insertions(+), 19 deletions(-) diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs index 03b7c18c55162..b27ddfdbc5e58 100644 --- a/src/bootstrap/test.rs +++ b/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)] @@ -1042,7 +1044,15 @@ impl Step for DocTest { 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); + } } } } @@ -1142,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()); @@ -1162,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) } } diff --git a/src/ci/docker/x86_64-gnu-tools/checktools.sh b/src/ci/docker/x86_64-gnu-tools/checktools.sh index a7d0c6a1e6a7f..da89aa9423b2d 100755 --- a/src/ci/docker/x86_64-gnu-tools/checktools.sh +++ b/src/ci/docker/x86_64-gnu-tools/checktools.sh @@ -25,6 +25,10 @@ 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 \ @@ -32,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 -# 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" diff --git a/src/tools/publish_toolstate.py b/src/tools/publish_toolstate.py index 40abe81c449ef..8e23519f57ebc 100755 --- a/src/tools/publish_toolstate.py +++ b/src/tools/publish_toolstate.py @@ -29,6 +29,10 @@ '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', } @@ -83,7 +87,7 @@ def update_latest( 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