Skip to content

Commit

Permalink
Rollup merge of rust-lang#64119 - pietroalbini:validate-toolstate-mai…
Browse files Browse the repository at this point in the history
…ntainers, r=kennytm

ci: ensure all tool maintainers are assignable on issues

GitHub only allows people explicitly listed as collaborators on the repository or who commented on the issue/PR to be assignees, failing to create the issue if non-assignable people are assigned.

This adds an extra check on CI to make sure all the people listed as tool maintainers can be assigned to toolstate issues. The check won't be executed on PR builds due to the lack of a valid token.

r? @kennytm
  • Loading branch information
Centril authored Sep 16, 2019
2 parents be327a8 + ce451b9 commit 63bc6ae
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 19 deletions.
7 changes: 7 additions & 0 deletions src/ci/azure-pipelines/steps/run.yml
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,15 @@ steps:
git clone --depth=1 https://github.com/rust-lang-nursery/rust-toolstate.git
cd rust-toolstate
python2.7 "$BUILD_SOURCESDIRECTORY/src/tools/publish_toolstate.py" "$(git rev-parse HEAD)" "$(git log --format=%s -n1 HEAD)" "" ""
# Only check maintainers if this build is supposed to publish toolstate.
# Builds that are not supposed to publish don't have the access token.
if [ -n "${TOOLSTATE_PUBLISH+is_set}" ]; then
TOOLSTATE_VALIDATE_MAINTAINERS_REPO=rust-lang/rust python2.7 "${BUILD_SOURCESDIRECTORY}/src/tools/publish_toolstate.py"
fi
cd ..
rm -rf rust-toolstate
env:
TOOLSTATE_REPO_ACCESS_TOKEN: $(TOOLSTATE_REPO_ACCESS_TOKEN)
condition: and(succeeded(), not(variables.SKIP_JOB), eq(variables['IMAGE'], 'mingw-check'))
displayName: Verify the publish_toolstate script works

Expand Down
99 changes: 80 additions & 19 deletions src/tools/publish_toolstate.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
## It is set as callback for `src/ci/docker/x86_64-gnu-tools/repo.sh` by the CI scripts
## when a new commit lands on `master` (i.e., after it passed all checks on `auto`).

from __future__ import print_function

import sys
import re
import os
Expand All @@ -20,21 +22,26 @@
import urllib.request as urllib2

# List of people to ping when the status of a tool or a book changed.
# These should be collaborators of the rust-lang/rust repository (with at least
# read privileges on it). CI will fail otherwise.
MAINTAINERS = {
'miri': '@oli-obk @RalfJung @eddyb',
'clippy-driver': '@Manishearth @llogiq @mcarton @oli-obk @phansch @flip1995 @yaahc',
'rls': '@Xanewok',
'rustfmt': '@topecongiro',
'book': '@carols10cents @steveklabnik',
'nomicon': '@frewsxcv @Gankro',
'reference': '@steveklabnik @Havvy @matthewjasper @ehuss',
'rust-by-example': '@steveklabnik @marioidival @projektir',
'embedded-book': (
'@adamgreig @andre-richter @jamesmunns @korken89 '
'@ryankurte @thejpster @therealprof'
),
'edition-guide': '@ehuss @Centril @steveklabnik',
'rustc-guide': '@mark-i-m @spastorino @amanjeev'
'miri': {'oli-obk', 'RalfJung', 'eddyb'},
'clippy-driver': {
'Manishearth', 'llogiq', 'mcarton', 'oli-obk', 'phansch', 'flip1995',
'yaahc',
},
'rls': {'Xanewok'},
'rustfmt': {'topecongiro'},
'book': {'carols10cents', 'steveklabnik'},
'nomicon': {'frewsxcv', 'Gankra'},
'reference': {'steveklabnik', 'Havvy', 'matthewjasper', 'ehuss'},
'rust-by-example': {'steveklabnik', 'marioidival'},
'embedded-book': {
'adamgreig', 'andre-richter', 'jamesmunns', 'korken89',
'ryankurte', 'thejpster', 'therealprof',
},
'edition-guide': {'ehuss', 'Centril', 'steveklabnik'},
'rustc-guide': {'mark-i-m', 'spastorino', 'amanjeev'},
}

REPOS = {
Expand All @@ -52,6 +59,50 @@
}


def validate_maintainers(repo, github_token):
'''Ensure all maintainers are assignable on a GitHub repo'''
next_link_re = re.compile(r'<([^>]+)>; rel="next"')

# Load the list of assignable people in the GitHub repo
assignable = []
url = 'https://api.github.com/repos/%s/collaborators?per_page=100' % repo
while url is not None:
response = urllib2.urlopen(urllib2.Request(url, headers={
'Authorization': 'token ' + github_token,
# Properly load nested teams.
'Accept': 'application/vnd.github.hellcat-preview+json',
}))
assignable.extend(user['login'] for user in json.load(response))
# Load the next page if available
url = None
link_header = response.headers.get('Link')
if link_header:
matches = next_link_re.match(link_header)
if matches is not None:
url = matches.group(1)

errors = False
for tool, maintainers in MAINTAINERS.items():
for maintainer in maintainers:
if maintainer not in assignable:
errors = True
print(
"error: %s maintainer @%s is not assignable in the %s repo"
% (tool, maintainer, repo),
)

if errors:
print()
print(" To be assignable, a person needs to be explicitly listed as a")
print(" collaborator in the repository settings. The simple way to")
print(" fix this is to ask someone with 'admin' privileges on the repo")
print(" to add the person or whole team as a collaborator with 'read'")
print(" privileges. Those privileges don't grant any extra permissions")
print(" so it's safe to apply them.")
print()
print("The build will fail due to this.")
exit(1)

def read_current_status(current_commit, path):
'''Reads build status of `current_commit` from content of `history/*.tsv`
'''
Expand All @@ -73,13 +124,12 @@ def maybe_delink(message):
def issue(
tool,
status,
maintainers,
assignees,
relevant_pr_number,
relevant_pr_user,
pr_reviewer,
):
# Open an issue about the toolstate failure.
assignees = [x.strip() for x in maintainers.split('@') if x != '']
if status == 'test-fail':
status_description = 'has failing tests'
else:
Expand All @@ -100,7 +150,7 @@ def issue(
REPOS.get(tool), relevant_pr_user, pr_reviewer
)),
'title': '`{}` no longer builds after {}'.format(tool, relevant_pr_number),
'assignees': assignees,
'assignees': list(assignees),
'labels': ['T-compiler', 'I-nominated'],
})
print("Creating issue:\n{}".format(request))
Expand Down Expand Up @@ -150,18 +200,19 @@ def update_latest(
old = status[os]
new = s.get(tool, old)
status[os] = new
maintainers = ' '.join('@'+name for name in MAINTAINERS[tool])
if new > old: # comparing the strings, but they are ordered appropriately!
# 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))
.format(tool, os, old, new, maintainers)
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))
.format(title, maintainers)
# Most tools only create issues for build failures.
# Other failures can be spurious.
if new == 'build-fail' or (tool == 'miri' and new == 'test-fail'):
Expand Down Expand Up @@ -200,6 +251,16 @@ def update_latest(


if __name__ == '__main__':
repo = os.environ.get('TOOLSTATE_VALIDATE_MAINTAINERS_REPO')
if repo:
github_token = os.environ.get('TOOLSTATE_REPO_ACCESS_TOKEN')
if github_token:
validate_maintainers(repo, github_token)
else:
print('skipping toolstate maintainers validation since no GitHub token is present')
# When validating maintainers don't run the full script.
exit(0)

cur_commit = sys.argv[1]
cur_datetime = datetime.datetime.utcnow().strftime('%Y-%m-%dT%H:%M:%SZ')
cur_commit_msg = sys.argv[2]
Expand Down

0 comments on commit 63bc6ae

Please sign in to comment.