Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automatically open an issue when a tool breaks #56951

Merged
merged 8 commits into from
Feb 13, 2019
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 83 additions & 3 deletions src/tools/publish_toolstate.py
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand All @@ -45,11 +56,48 @@ def read_current_status(current_commit, path):
return json.loads(status)
return {}

def issue(
tool,
maintainers,
relevant_pr_number,
relevant_pr_user,
pr_reviewer,
):
# 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(
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
gh_url,
json.dumps({
'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.

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_number, tool, REPOS[tool], relevant_pr_user, pr_reviewer),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we have a new enough version of Python, it might be more maintainable to use named parameters here, instead of positional. I don't have to touch this code, so it's no skin off my back!

'title': '`{}` no longer builds after {}'.format(tool, relevant_pr_number),
'assignees': assignees,
'labels': ['T-compiler', 'I-nominated'],
}),
{
'Authorization': 'token ' + github_token,
'Content-Type': 'application/json',
}
))
response.read()

def update_latest(
current_commit,
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.
Expand All @@ -74,19 +122,41 @@ def update_latest(
for status in latest:
tool = status['tool']
changed = False
build_failed = False

for os, s in current_status.items():
old = status[os]
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
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))
# only create issues for build failures. Other failures can be spurious
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So far, at least for Miri, every single case of "test-fail" was a real failure.

Copy link
Contributor

@Centril Centril Feb 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might want to special-case miri in a follow-up PR?

if new == 'build-fail':
build_failed = True

if build_failed:
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
Expand All @@ -109,20 +179,30 @@ 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)
# 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,
)
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 = '<unknown user>'
relevant_pr_number = '<unknown PR>'
relevant_pr_url = '<unknown>'
pr_reviewer = '<unknown reviewer>'

message = update_latest(
cur_commit,
relevant_pr_number,
relevant_pr_url,
relevant_pr_user,
pr_reviewer,
cur_datetime
)
if not message:
Expand Down