Repository: mesos Updated Branches: refs/heads/master 0a6b820d1 -> 9ea22dc74
Added support for github to apply-reviews.py. Review: https://reviews.apache.org/r/39410 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/c3e7ee71 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/c3e7ee71 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/c3e7ee71 Branch: refs/heads/master Commit: c3e7ee7185c41c2177bfd048f745e6d7dc9b89e0 Parents: 0a6b820 Author: Artem Harutyunyan <[email protected]> Authored: Sat Dec 12 10:50:32 2015 -0800 Committer: Joris Van Remoortere <[email protected]> Committed: Sat Dec 12 10:53:39 2015 -0800 ---------------------------------------------------------------------- support/apply-reviews.py | 298 ++++++++++++++++++++++++++++++------------ 1 file changed, 213 insertions(+), 85 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/c3e7ee71/support/apply-reviews.py ---------------------------------------------------------------------- diff --git a/support/apply-reviews.py b/support/apply-reviews.py index 6ea229f..7ef447b 100755 --- a/support/apply-reviews.py +++ b/support/apply-reviews.py @@ -2,39 +2,57 @@ import argparse import atexit import json +import linecache +import os import re import subprocess import sys import urllib2 -REVIEW_REQUEST_BASE_URL = 'https://reviews.apache.org/r' -REVIEW_REQUEST_API_URL = 'https://reviews.apache.org/api/review-requests' -USER_URL = 'https://reviews.apache.org/api/users' +REVIEWBOARD_REVIEW_URL = 'https://reviews.apache.org/r' +REVIEWBOARD_API_URL =\ + 'https://reviews.apache.org/api/review-requests' +REVIEWBOARD_USER_URL = 'https://reviews.apache.org/api/users' + + +GITHUB_URL = 'https://api.github.com/repos/apache/mesos/pulls' +GITHUB_PATCH_URL =\ + 'https://patch-diff.githubusercontent.com/raw/apache/mesos/pull' def review_api_url(review_id): """Returns a Review Board API URL given a review ID.""" # Reviewboard REST API expects '/' at the end of the URL. - return '{api}/{review}/'.format(api=REVIEW_REQUEST_API_URL, review=review_id) + return '{base}/{review}/'.format(base=REVIEWBOARD_API_URL, review=review_id) def review_url(review_id): """Returns a Review Board UI URL given a review ID.""" - return '{base}/{review}'.format(base=REVIEW_REQUEST_BASE_URL, review=review_id) + return '{base}/{review}/'.format(base=REVIEWBOARD_REVIEW_URL, review=review_id) + +def pull_request_url(pull_request_number): + """Returns a GitHub pull request URL given a PR number.""" + return '{base}/{pr}'.format(base=GITHUB_URL, pr=pull_request_number) -def user_url(username): + +def reviewboard_user_url(username): """Returns a Review Board URL for a user given a username.""" # Reviewboard REST API expects '/' at the end of the URL. - return '{base}/{user}/'.format(base=USER_URL, user=username) + return '{base}/{user}/'.format(base=REVIEWBOARD_USER_URL, user=username) -def patch_url(review_id): - """Returns a Review Board URL for a patch given a review ID.""" - # Reviewboard REST API expects '/' at the end of the URL. - return '{base}/{review}/diff/raw/'.format(base=REVIEW_REQUEST_BASE_URL, - review=review_id) +def patch_url(): + """Returns a Review Board or a GitHub URL for a patch.""" + if options['review_id']: + # Reviewboard REST API expects '/' at the end of the URL. + return '{base}/{review}/diff/raw/'.format(base=REVIEWBOARD_REVIEW_URL, + review=options['review_id']) + elif options['github']: + return '{base}/{patch}.patch'.format(base=GITHUB_PATCH_URL, + patch=options['github']) + return None def url_to_json(url): @@ -45,7 +63,7 @@ def url_to_json(url): def extract_review_id(url): """Extracts review ID from Review Board URL.""" - review_id = re.search(REVIEW_REQUEST_API_URL + '/(\d+)/', url) + review_id = re.search(REVIEWBOARD_API_URL + '/(\d+)/', url) if review_id: return review_id.group(1) @@ -60,19 +78,19 @@ def review_chain(review_id): return [] # Verify that the review has exactly one parent. - depends = json_obj.get('review_request').get('depends_on') - - if len(depends) > 1: - sys.stderr.write('Error: Review {review} has more than one ' - 'parent\n'.format(review=review_id)) + parent = json_obj.get('review_request').get('depends_on') + if len(parent) > 1: + sys.stderr.write( + 'Error: Review {review} has more than one parent'\ + .format(review=review_id)) sys.exit(1) - elif len(depends) == 0: + elif len(parent) == 0: return [(review_id, json_obj.get('review_request').get('summary'))] else: # The review has exactly one parent. - review = (review_id, json_obj.get('review_request').get('summary')) - review_list = review_chain(extract_review_id(depends[0].get('href'))) + review_list = review_chain(extract_review_id(parent[0].get('href'))) + review = (review_id, json_obj.get('review_request').get('summary')) if review not in review_list: return review_list + [review] else: @@ -81,10 +99,10 @@ def review_chain(review_id): sys.exit(1) -def shell(command): - """Runs a command in a shell, unless the dry-run option is present (in which +def shell(command, dry_run): + """Runs a command in a shell, unless the dry-run option is set (in which case it just prints the command.""" - if options['dry_run']: + if dry_run: print command return @@ -93,37 +111,63 @@ def shell(command): sys.exit(error_code) -def remove_patch(review_id): - """Removes the patch file.""" - command = 'rm -f %s.patch' % review_id - shell(command); - +def remove_patch(file_name=None): + """Removes the file. In case the file name is not provided it reads the + file name from global options dictionary.""" + if file_name == None: + cmd = 'rm -f {_file}.patch'.format(_file=patch_id()) + else: + cmd = 'rm -f {_file}'.format(_file=file_name) -def apply_review(review_id): - """Applies a review with a given ID locally.""" - atexit.register(lambda: remove_patch(review_id)) - fetch_patch(review_id) - apply_patch(review_id) + # In case of github we always need to fetch the patch to extract username and + # email, so to ensure that it always gets cleaned up we ignore the dry_run + # option by always setting the second parameter to False. + if options['github']: + shell(cmd, False) + else: + shell(cmd, options['dry_run']) - review = review_data(review_id) - commit_patch(review) - if (not options['no_amend']): - amend() - remove_patch(review_id) +def apply_review(): + """Applies a review with a given ID locally.""" + # Make sure we don't leave the patch behind in case of failure. + atexit.register(lambda: remove_patch('{_file}.patch'\ + .format(_file=patch_id()))) + + fetch_patch() + apply_patch() + commit_patch() + remove_patch() + + +def fetch_patch(): + """Fetches a patch from Review Board or GitHub.""" + cmd = ' '.join(['wget', + '--no-check-certificate', + '--no-verbose', + '-O ' + '{review_id}.patch', + '{url}'])\ + .format(review_id=patch_id(), url=patch_url()) + + # In case of github we always need to fetch the patch to extract username + # and email, so we ignore the dry_run option by setting the second parameter + # to False. + if options['github']: + shell(cmd, False) + else: + shell(cmd, options['dry_run']) -def fetch_patch(review_id): - """Fetches patch from the Review Board.""" - command = 'wget --no-check-certificate --no-verbose -O {review_id}.patch '\ - '{url}'.format(review_id=review_id , url=patch_url(review_id)) - shell(command) +def patch_id(): + return (options['review_id'] or options['github']) -def apply_patch(review_id): +def apply_patch(): """Applies patch locally.""" - command = 'git apply --index {review_id}.patch'.format(review_id=review_id) - shell(command) + cmd = 'git apply --index {review_id}.patch'\ + .format(review_id=patch_id()) + shell(cmd, options['dry_run']) def quote(string): @@ -131,61 +175,109 @@ def quote(string): return string.replace("'", "'\\''") -def commit_patch(review): - """Commit patch locally.""" - command = 'git commit --author \'{author} <{email}>\' -am \'{message}\''\ - .format(author=quote(review['author']), - email=quote(review['email']), - message=quote(review['message'])) - shell(command) +def commit_patch(): + """Commits patch locally.""" + data = patch_data() + + # Check whether we need to amend the commit message. + if options['no_amend']: + amend = '' + else: + amend = '-e' + + cmd = 'git commit --author \'{author}\' {_amend} -am \'{message}\''\ + .format(author=quote(data['author']), + _amend=amend, + message=quote(data['message'])) + shell(cmd, options['dry_run']) + + +def patch_data(): + """Populates and returns a dictionary with data necessary for committing the + patch (such as the message, the author, etc.).""" + if options['review_id']: + return reviewboard_data() + elif options['github']: + return github_data() + else: + return None + + +def get_author(patch): + """Reads the author name and email from the .patch file""" + author = linecache.getline(patch, 2) + return author.replace('From: ', '').rstrip() + + +def github_data(): + pull_request_number = options['github'] + pull_request = url_to_json(pull_request_url(pull_request_number)) + title = pull_request.get('title') + description = pull_request.get('body') + url = '{url}/{pr}'.format(url=GITHUB_URL, pr=pull_request_number) + author = get_author('{pr}.patch'.format(pr=pull_request_number)) + message = '\n\n'.join(['{summary}', + '{description}', + 'This closes: {pr}'])\ + .format(summary=title, + description=description, + pr=pull_request_number) + + review_data = { + "summary": title, + "description": description, + "url": url, + "author": author, + "message": message + } -def review_data(review_id): + return review_data + + +def reviewboard_data(): """Fetches review data and populates internal data structure.""" + review_id = options['review_id'] + # Populate review object. review = url_to_json(review_api_url(review_id)).get('review_request') url = review_url(review_id) - # Populate and escape commit message. - message = '{summary}\n\n{description}\n\nReview: {_url}'\ - .format(summary=review.get('summary'), - description=review.get('description'), - _url=url) - # Populate user object. - user = url_to_json(user_url(review.get('links').get('submitter')\ - .get('title'))).get('user') + user = url_to_json(reviewboard_user_url( + review.get('links').get('submitter').get('title'))).get('user') + + author = '{author} <{email}>'.format(author=user.get('fullname'), + email=user.get('email')) + message = '\n\n'.join(['{summary}', + '{description}', + 'Review: {review_url}'])\ + .format(summary=review.get('summary'), + description=review.get('description'), + review_url=url) review_data = { - 'summary': review.get('summary'), - 'description': review.get('description'), - 'url': url, - 'author': user.get('fullname'), - 'email': user.get('email'), - 'message': message + "summary": review.get('summary'), + "description": review.get('description'), + "url": url, + "author": author, + "message": message } return review_data -def amend(): - """Amends commit.""" - command = 'git commit --amend' - shell(command) - # A global dictionary for holding execution options. options = {} -if __name__ == "__main__": - # Parse command line. - parser = argparse.ArgumentParser(description='Recursively apply Review ' - 'Board reviews.') - parser.add_argument('-r', - '--review-id', - metavar='REVIEW_ID', - help='Numeric review ID that needs to be applied.', - required=True) + +def parse_options(): + """Parses command line options and populates the dictionary.""" + parser = argparse.ArgumentParser( + description = 'Recursively apply Review Board reviews' + ' and GitHub pull requests.') + parser.add_argument('-d', '--dry-run', action='store_true', @@ -194,13 +286,25 @@ if __name__ == "__main__": action='store_true', help='Do not amend commit message.') + # Add -g and -r and make them mutually exclusive. + group = parser.add_mutually_exclusive_group(required=True) + group.add_argument('-g', '--github', + metavar='PULL_REQUEST', + help='Pull request number') + group.add_argument('-r', '--review-id', + metavar='REVIEW_ID', + help='Numeric Review ID') + args = parser.parse_args() - # Populate the global options dictionary. options['review_id'] = args.review_id options['dry_run'] = args.dry_run options['no_amend'] = args.no_amend + options['github'] = args.github + +def reviewboard(): + """Applies a chain of reviewboard patches.""" # Retrieve the list of reviews to apply. reviews = review_chain(options['review_id']) @@ -208,4 +312,28 @@ if __name__ == "__main__": for review_id, summary in reviews: if review_id not in applied: applied.add(review_id) - apply_review(review_id) + options['review_id'] = review_id + apply_review() + + +def github(): + """Applies a patch from github.""" + apply_review() + + +# A global dictionary for holding command line options. See parse_options() +# function for details. +# +# TODO(hartem): Consider getting rid of global options variable. Either use +# explicit arguments or classes. +options = {} + + +if __name__ == "__main__": + # Parse command line options and populate the 'options' dictionary. + parse_options() + + if options['review_id']: + reviewboard() + else: + github()
