From 5a3abffc77f11aa66b75874b446fbbf257758f91 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Tue, 18 Dec 2018 13:33:02 +0100 Subject: [PATCH 1/8] Automatically open an issue when a tool breaks --- src/tools/publish_toolstate.py | 42 +++++++++++++++++++++++++++++++--- 1 file changed, 39 insertions(+), 3 deletions(-) diff --git a/src/tools/publish_toolstate.py b/src/tools/publish_toolstate.py index 4ade87f5d65bd..b863b9dde0d79 100755 --- a/src/tools/publish_toolstate.py +++ b/src/tools/publish_toolstate.py @@ -45,11 +45,41 @@ def read_current_status(current_commit, path): return json.loads(status) return {} +def issue( + title, + tool, + maintainers, + relevant_pr_number, + relevant_pr_user, +): + # Open an issue about the toolstate failure. + gh_url = 'https://api.github.com/repos/rust-lang/rust/issues' + assignees = [x.strip() for x in maintainers.split('@') if x != ''] + assignees.append(relevant_pr_user) + response = urllib2.urlopen(urllib2.Request( + gh_url, + json.dumps({ + 'body': '''\ + @{}: your PR ({}) broke {} + + If you have the time it would be great if you could open a PR against {} that + fixes the fallout from your PR. + '''.format(relevant_pr_user, relevant_pr_number, tool, tool), + 'title': title, + 'assignees': assignees, + }), + { + 'Authorization': 'token ' + github_token, + 'Content-Type': 'application/json', + } + )) + response.read() def update_latest( current_commit, relevant_pr_number, relevant_pr_url, + relevant_pr_user, current_datetime ): '''Updates `_data/latest.json` to match build result of the given commit. @@ -85,8 +115,11 @@ def update_latest( .format(tool, os, old, new, MAINTAINERS.get(tool)) elif new < old: changed = True - message += '💔 {} on {}: {} → {} (cc {}, @rust-lang/infra).\n' \ - .format(tool, os, old, new, MAINTAINERS.get(tool)) + title = '💔 {} on {}: {} → {}' \ + .format(tool, os, old, new) + message += '{} (cc {}, @rust-lang/infra).\n' \ + .format(title, MAINTAINERS.get(tool)) + issue(title, tool, MAINTAINERS.get(tool), relevant_pr_number, relevant_pr_user) if changed: status['commit'] = current_commit @@ -109,13 +142,15 @@ def update_latest( 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('Auto merge of #([0-9]+) - ([^:]+)', cur_commit_msg) if relevant_pr_match: number = relevant_pr_match.group(1) + relevant_pr_user = relevant_pr_match.group(2) relevant_pr_number = 'rust-lang/rust#' + number relevant_pr_url = 'https://github.com/rust-lang/rust/pull/' + number else: number = '-1' + relevant_pr_user = '' relevant_pr_number = '' relevant_pr_url = '' @@ -123,6 +158,7 @@ def update_latest( cur_commit, relevant_pr_number, relevant_pr_url, + relevant_pr_user, cur_datetime ) if not message: From 46a8fcdf3cf0eb64b5ac70b4847f1facbdd12be1 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Tue, 18 Dec 2018 16:30:40 +0100 Subject: [PATCH 2/8] Automatically tag as nominated for T-compiler --- src/tools/publish_toolstate.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/tools/publish_toolstate.py b/src/tools/publish_toolstate.py index b863b9dde0d79..c05cd912991ca 100755 --- a/src/tools/publish_toolstate.py +++ b/src/tools/publish_toolstate.py @@ -67,6 +67,7 @@ def issue( '''.format(relevant_pr_user, relevant_pr_number, tool, tool), 'title': title, 'assignees': assignees, + 'labels': ['T-compiler', 'I-nominated'], }), { 'Authorization': 'token ' + github_token, From c485acb27c248d36c847ef4602770dcaa76b286d Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Wed, 19 Dec 2018 15:54:51 +0100 Subject: [PATCH 3/8] Only open one issue per tool --- src/tools/publish_toolstate.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/tools/publish_toolstate.py b/src/tools/publish_toolstate.py index c05cd912991ca..5cacdb7ef0b57 100755 --- a/src/tools/publish_toolstate.py +++ b/src/tools/publish_toolstate.py @@ -46,11 +46,11 @@ def read_current_status(current_commit, path): return {} def issue( - title, tool, maintainers, relevant_pr_number, relevant_pr_user, + msg, ): # Open an issue about the toolstate failure. gh_url = 'https://api.github.com/repos/rust-lang/rust/issues' @@ -64,8 +64,11 @@ def issue( If you have the time it would be great if you could open a PR against {} that fixes the fallout from your PR. - '''.format(relevant_pr_user, relevant_pr_number, tool, tool), - 'title': title, + + {} + + '''.format(relevant_pr_user, relevant_pr_number, tool, tool, msg), + 'title': '💔 {}'.format(tool), 'assignees': assignees, 'labels': ['T-compiler', 'I-nominated'], }), @@ -105,6 +108,7 @@ def update_latest( for status in latest: tool = status['tool'] changed = False + failures = '' for os, s in current_status.items(): old = status[os] @@ -120,7 +124,11 @@ def update_latest( .format(tool, os, old, new) message += '{} (cc {}, @rust-lang/infra).\n' \ .format(title, MAINTAINERS.get(tool)) - issue(title, tool, MAINTAINERS.get(tool), relevant_pr_number, relevant_pr_user) + failures += title + failures += '\n' + + if failures != '': + issue(tool, MAINTAINERS.get(tool), relevant_pr_number, relevant_pr_user, failures) if changed: status['commit'] = current_commit From ab5fc7fb5b2028323ac30a0f67bf13202071936c Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Wed, 19 Dec 2018 16:37:16 +0100 Subject: [PATCH 4/8] Only emit issues for build failures to supress spurious failures --- src/tools/publish_toolstate.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/tools/publish_toolstate.py b/src/tools/publish_toolstate.py index 5cacdb7ef0b57..095ef0df496e6 100755 --- a/src/tools/publish_toolstate.py +++ b/src/tools/publish_toolstate.py @@ -115,17 +115,21 @@ def update_latest( new = s.get(tool, old) status[os] = new if new > old: + # things got fixed or at least the status quo improved changed = True message += '🎉 {} on {}: {} → {} (cc {}, @rust-lang/infra).\n' \ .format(tool, os, old, new, MAINTAINERS.get(tool)) elif new < old: + # tests or builds are failing and were not failing before changed = True title = '💔 {} on {}: {} → {}' \ .format(tool, os, old, new) message += '{} (cc {}, @rust-lang/infra).\n' \ .format(title, MAINTAINERS.get(tool)) - failures += title - failures += '\n' + # only create issues for build failures. Other failures can be spurious + if new == 'build-fail': + failures += title + failures += '\n' if failures != '': issue(tool, MAINTAINERS.get(tool), relevant_pr_number, relevant_pr_user, failures) @@ -151,6 +155,7 @@ def update_latest( save_message_to_path = sys.argv[3] github_token = sys.argv[4] + # assume that PR authors are also owners of the repo where the branch lives relevant_pr_match = re.search('Auto merge of #([0-9]+) - ([^:]+)', cur_commit_msg) if relevant_pr_match: number = relevant_pr_match.group(1) From a4c317ef980a8c0ef443937c78143535781e791c Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Wed, 19 Dec 2018 17:44:46 +0100 Subject: [PATCH 5/8] Be more cheerful and helpful --- src/tools/publish_toolstate.py | 41 +++++++++++++++++++++++----------- 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/src/tools/publish_toolstate.py b/src/tools/publish_toolstate.py index 095ef0df496e6..fbb3bc9a75154 100755 --- a/src/tools/publish_toolstate.py +++ b/src/tools/publish_toolstate.py @@ -34,6 +34,17 @@ 'rust-by-example': '@steveklabnik @marioidival @projektir', } +REPOS = { + 'miri': 'https://github.com/solson/miri', + 'clippy-driver': 'https://github.com/rust-lang/rust-clippy', + 'rls': 'https://github.com/rust-lang/rls', + 'rustfmt': 'https://github.com/rust-lang/rustfmt', + 'book': 'https://github.com/rust-lang/book', + 'nomicon': 'https://github.com/rust-lang-nursery/nomicon', + 'reference': 'https://github.com/rust-lang-nursery/reference', + 'rust-by-example': 'https://github.com/rust-lang/rust-by-example', +} + def read_current_status(current_commit, path): '''Reads build status of `current_commit` from content of `history/*.tsv` @@ -50,7 +61,7 @@ def issue( maintainers, relevant_pr_number, relevant_pr_user, - msg, + pr_reviewer, ): # Open an issue about the toolstate failure. gh_url = 'https://api.github.com/repos/rust-lang/rust/issues' @@ -60,15 +71,16 @@ def issue( gh_url, json.dumps({ 'body': '''\ - @{}: your PR ({}) broke {} + Hello, this is your friendly neighborhood mergebot. + After merging PR {}, I observed that the tool {} no longer builds. + A follow-up PR to the repository {} is needed to fix the fallout. - If you have the time it would be great if you could open a PR against {} that - fixes the fallout from your PR. + cc @{}, do you think you would have time to do the follow-up work? If so, that would be great! - {} + cc @{}, the PR reviewer, and @rust-lang/compiler -- nominating for prioritization. - '''.format(relevant_pr_user, relevant_pr_number, tool, tool, msg), - 'title': '💔 {}'.format(tool), + '''.format(relevant_pr_number, tool, REPOS[tool], relevant_pr_user, pr_reviewer), + 'title': '`{}` no longer builds after {}'.format(tool, relevant_pr_number), 'assignees': assignees, 'labels': ['T-compiler', 'I-nominated'], }), @@ -84,6 +96,7 @@ def update_latest( relevant_pr_number, relevant_pr_url, relevant_pr_user, + pr_reviewer, current_datetime ): '''Updates `_data/latest.json` to match build result of the given commit. @@ -108,7 +121,7 @@ def update_latest( for status in latest: tool = status['tool'] changed = False - failures = '' + build_failed = False for os, s in current_status.items(): old = status[os] @@ -128,11 +141,10 @@ def update_latest( .format(title, MAINTAINERS.get(tool)) # only create issues for build failures. Other failures can be spurious if new == 'build-fail': - failures += title - failures += '\n' + build_failed = True - if failures != '': - issue(tool, MAINTAINERS.get(tool), relevant_pr_number, relevant_pr_user, failures) + if build_failed: + issue(tool, MAINTAINERS.get(tool), relevant_pr_number, relevant_pr_user, pr_reviewer) if changed: status['commit'] = current_commit @@ -156,23 +168,26 @@ def update_latest( github_token = sys.argv[4] # assume that PR authors are also owners of the repo where the branch lives - relevant_pr_match = re.search('Auto merge of #([0-9]+) - ([^:]+)', cur_commit_msg) + relevant_pr_match = re.search('Auto merge of #([0-9]+) - ([^:]+):[^,]+ r=([^\s]+)', cur_commit_msg) if relevant_pr_match: number = relevant_pr_match.group(1) relevant_pr_user = relevant_pr_match.group(2) relevant_pr_number = 'rust-lang/rust#' + number relevant_pr_url = 'https://github.com/rust-lang/rust/pull/' + number + pr_reviewer = relevant_pr_match.group(3) else: number = '-1' relevant_pr_user = '' relevant_pr_number = '' relevant_pr_url = '' + pr_reviewer = '' message = update_latest( cur_commit, relevant_pr_number, relevant_pr_url, relevant_pr_user, + pr_reviewer, cur_datetime ) if not message: From b454474c84ed96a8af55ed76e68e428cc11e69c6 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Thu, 20 Dec 2018 14:29:42 +0100 Subject: [PATCH 6/8] Tidy --- src/tools/publish_toolstate.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/tools/publish_toolstate.py b/src/tools/publish_toolstate.py index fbb3bc9a75154..3bf40e336d352 100755 --- a/src/tools/publish_toolstate.py +++ b/src/tools/publish_toolstate.py @@ -75,7 +75,8 @@ def issue( After merging PR {}, I observed that the tool {} no longer builds. A follow-up PR to the repository {} is needed to fix the fallout. - cc @{}, do you think you would have time to do the follow-up work? If so, that would be great! + cc @{}, do you think you would have time to do the follow-up work? + If so, that would be great! cc @{}, the PR reviewer, and @rust-lang/compiler -- nominating for prioritization. @@ -144,7 +145,10 @@ def update_latest( build_failed = True if build_failed: - issue(tool, MAINTAINERS.get(tool), relevant_pr_number, relevant_pr_user, pr_reviewer) + issue( + tool, MAINTAINERS.get(tool), + relevant_pr_number, relevant_pr_user, pr_reviewer, + ) if changed: status['commit'] = current_commit @@ -168,7 +172,10 @@ def update_latest( github_token = sys.argv[4] # assume that PR authors are also owners of the repo where the branch lives - relevant_pr_match = re.search('Auto merge of #([0-9]+) - ([^:]+):[^,]+ r=([^\s]+)', cur_commit_msg) + relevant_pr_match = re.search( + 'Auto merge of #([0-9]+) - ([^:]+):[^,]+ r=([^\s]+)', + cur_commit_msg, + ) if relevant_pr_match: number = relevant_pr_match.group(1) relevant_pr_user = relevant_pr_match.group(2) From 488f16a85041e57ce5934df44ed64ffe5303be03 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sun, 23 Dec 2018 13:27:37 +0100 Subject: [PATCH 7/8] Dedent mergebot message --- src/tools/publish_toolstate.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tools/publish_toolstate.py b/src/tools/publish_toolstate.py index 3bf40e336d352..18d447bcdb684 100755 --- a/src/tools/publish_toolstate.py +++ b/src/tools/publish_toolstate.py @@ -70,7 +70,7 @@ def issue( response = urllib2.urlopen(urllib2.Request( gh_url, json.dumps({ - 'body': '''\ + 'body': textwrap.dedent('''\ Hello, this is your friendly neighborhood mergebot. After merging PR {}, I observed that the tool {} no longer builds. A follow-up PR to the repository {} is needed to fix the fallout. @@ -80,7 +80,7 @@ def issue( cc @{}, the PR reviewer, and @rust-lang/compiler -- nominating for prioritization. - '''.format(relevant_pr_number, tool, REPOS[tool], relevant_pr_user, pr_reviewer), + ''').format(relevant_pr_number, tool, REPOS[tool], relevant_pr_user, pr_reviewer), 'title': '`{}` no longer builds after {}'.format(tool, relevant_pr_number), 'assignees': assignees, 'labels': ['T-compiler', 'I-nominated'], From 6ed4401609817e18a0ff781529e35b2a209ff0da Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Tue, 12 Feb 2019 14:48:53 +0100 Subject: [PATCH 8/8] Permit issue posting to have network failures --- src/tools/publish_toolstate.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/tools/publish_toolstate.py b/src/tools/publish_toolstate.py index 18d447bcdb684..6f3f409b8feb6 100755 --- a/src/tools/publish_toolstate.py +++ b/src/tools/publish_toolstate.py @@ -145,10 +145,18 @@ def update_latest( build_failed = True if build_failed: - issue( - tool, MAINTAINERS.get(tool), - relevant_pr_number, relevant_pr_user, pr_reviewer, - ) + try: + issue( + tool, MAINTAINERS.get(tool), + relevant_pr_number, relevant_pr_user, pr_reviewer, + ) + except IOError as (errno, strerror): + # network errors will simply end up not creating an issue, but that's better + # than failing the entire build job + print "I/O error({0}): {1}".format(errno, strerror) + except: + print "Unexpected error:", sys.exc_info()[0] + raise if changed: status['commit'] = current_commit