diff --git a/tools/repo_pull/gerrit.py b/tools/repo_pull/gerrit.py index 3bead35ed..185edd0e6 100755 --- a/tools/repo_pull/gerrit.py +++ b/tools/repo_pull/gerrit.py @@ -16,6 +16,8 @@ # limitations under the License. # +"""Gerrit Restful API client library.""" + from __future__ import print_function import argparse @@ -32,8 +34,10 @@ except ImportError: HTTPBasicAuthHandler, Request, build_opener) # PY2 try: + # pylint: disable=ungrouped-imports from urllib.parse import urlencode, urlparse # PY3 except ImportError: + # pylint: disable=ungrouped-imports from urllib import urlencode # PY2 from urlparse import urlparse # PY2 @@ -41,7 +45,7 @@ except ImportError: def load_auth_credentials_from_file(cookie_file): """Load credentials from an opened .gitcookies file.""" credentials = {} - for lineno, line in enumerate(cookie_file, start=1): + for line in cookie_file: if line.startswith('#HttpOnly_'): line = line[len('#HttpOnly_'):] @@ -192,6 +196,7 @@ def _parse_args(): def main(): + """Main function""" args = _parse_args() # Query change lists diff --git a/tools/repo_pull/repo_patch.py b/tools/repo_pull/repo_patch.py index 5905c2a07..393870609 100755 --- a/tools/repo_pull/repo_patch.py +++ b/tools/repo_pull/repo_patch.py @@ -21,11 +21,11 @@ Gerrit.""" from __future__ import print_function -from gerrit import create_url_opener_from_args, query_change_lists, get_patch - import argparse import os +from gerrit import create_url_opener_from_args, query_change_lists, get_patch + def _parse_args(): """Parse command line options.""" @@ -45,6 +45,7 @@ def _parse_args(): def main(): + """Main function""" args = _parse_args() # Query change lists @@ -57,8 +58,8 @@ def main(): num_changes_width = len(str(num_changes)) for i, change in enumerate(change_lists, start=1): print('{:>{}}/{} | {} {}'.format( - i, num_changes_width, num_changes, change['_number'], - change['subject'])) + i, num_changes_width, num_changes, change['_number'], + change['subject'])) patch_file = get_patch(url_opener, args.gerrit, change['id']) with open('{}.patch'.format(change['_number']), 'wb') as output_file: diff --git a/tools/repo_pull/repo_pull.py b/tools/repo_pull/repo_pull.py index 2cde02955..54e70ddd8 100755 --- a/tools/repo_pull/repo_pull.py +++ b/tools/repo_pull/repo_pull.py @@ -20,8 +20,6 @@ from __future__ import print_function -from gerrit import create_url_opener_from_args, query_change_lists - import argparse import collections import itertools @@ -32,7 +30,10 @@ import re import sys import xml.dom.minidom +from gerrit import create_url_opener_from_args, query_change_lists + try: + # pylint: disable=redefined-builtin from __builtin__ import raw_input as input # PY2 except ImportError: pass @@ -44,9 +45,9 @@ except ImportError: # it doesn't have to be quoted. _SHELL_SIMPLE_PATTERN = re.compile('^[a-zA-Z90-9_./-]+$') - def _sh_quote(s): + def _sh_quote(txt): """Quote a string if it contains special characters.""" - return s if _SHELL_SIMPLE_PATTERN.match(s) else json.dumps(s) + return txt if _SHELL_SIMPLE_PATTERN.match(txt) else json.dumps(txt) try: from subprocess import PIPE, run # PY3.5 @@ -54,6 +55,9 @@ except ImportError: from subprocess import CalledProcessError, PIPE, Popen class CompletedProcess(object): + """Process execution result returned by subprocess.run().""" + # pylint: disable=too-few-public-methods + def __init__(self, args, returncode, stdout, stderr): self.args = args self.returncode = returncode @@ -61,18 +65,20 @@ except ImportError: self.stderr = stderr def run(*args, **kwargs): + """Run a command with subprocess.Popen() and redirect input/output.""" + check = kwargs.pop('check', False) try: - input = kwargs.pop('input') + stdin = kwargs.pop('input') assert 'stdin' not in kwargs kwargs['stdin'] = PIPE except KeyError: - input = None + stdin = None proc = Popen(*args, **kwargs) try: - stdout, stderr = proc.communicate(input) + stdout, stderr = proc.communicate(stdin) except: proc.kill() proc.wait() @@ -83,18 +89,22 @@ except ImportError: raise CalledProcessError(returncode, args, stdout) return CompletedProcess(args, returncode, stdout, stderr) + if bytes is str: def write_bytes(data, file): # PY2 """Write bytes to a file.""" + # pylint: disable=redefined-builtin file.write(data) else: def write_bytes(data, file): # PY3 """Write bytes to a file.""" + # pylint: disable=redefined-builtin file.buffer.write(data) def _confirm(question, default, file=sys.stderr): """Prompt a yes/no question and convert the answer to a boolean value.""" + # pylint: disable=redefined-builtin answers = {'': default, 'y': True, 'yes': True, 'n': False, 'no': False} suffix = '[Y/n] ' if default else ' [y/N] ' while True: @@ -107,10 +117,13 @@ def _confirm(question, default, file=sys.stderr): class ChangeList(object): """A ChangeList to be checked out.""" + # pylint: disable=too-few-public-methods,too-many-instance-attributes def __init__(self, project, project_dir, fetch, commit_sha1, commit, change_list): """Initialize a ChangeList instance.""" + # pylint: disable=too-many-arguments + self.project = project self.project_dir = project_dir self.number = change_list['_number'] @@ -154,7 +167,7 @@ def find_manifest_xml(dir_path): raise ValueError('.repo dir not found') -def build_project_name_to_directory_dict(manifest_path): +def build_project_name_dir_dict(manifest_path): """Build the mapping from Gerrit project name to source tree project directory path.""" project_dirs = {} @@ -177,10 +190,14 @@ def group_and_sort_change_lists(change_lists, project_dirs): # Build a dict that map projects to dicts that map commits to changes. projects = collections.defaultdict(dict) for change_list in change_lists: + commit_sha1 = None for commit_sha1, value in change_list['revisions'].items(): fetch = value['fetch'] commit = value['commit'] + if not commit_sha1: + raise ValueError('bad revision') + project = change_list['project'] project_dir = project_dirs[project] @@ -197,17 +214,17 @@ def group_and_sort_change_lists(change_lists, project_dirs): visited_changes = set() sorted_changes = [] - def post_order_traverse(change): + def _post_order_traverse(change): visited_changes.add(change) for parent in change.parents: parent_change = changes.get(parent['commit']) if parent_change and parent_change not in visited_changes: - post_order_traverse(parent_change) + _post_order_traverse(parent_change) sorted_changes.append(change) for change in sorted(changes.values(), key=lambda x: x.number): if change not in visited_changes: - post_order_traverse(change) + _post_order_traverse(change) return sorted_changes @@ -280,7 +297,7 @@ def _main_bash(args): branch_name = _get_local_branch_name_from_args(args) manifest_path = _get_manifest_xml_from_args(args) - project_dirs = build_project_name_to_directory_dict(manifest_path) + project_dirs = build_project_name_dir_dict(manifest_path) change_lists = _get_change_lists_from_args(args) change_list_groups = group_and_sort_change_lists(change_lists, project_dirs) @@ -314,13 +331,32 @@ def _do_pull_change_lists_for_project(task): return None +def _print_pull_failures(failures, file=sys.stderr): + """Print pull failures and tracebacks.""" + # pylint: disable=redefined-builtin + + separator = '=' * 78 + separator_sub = '-' * 78 + + print(separator, file=file) + for failed_change, skipped_changes, cmd, errors in failures: + print('PROJECT:', failed_change.project, file=file) + print('FAILED COMMIT:', failed_change.commit_sha1, file=file) + for change in skipped_changes: + print('PENDING COMMIT:', change.commit_sha1, file=file) + print(separator_sub, file=sys.stderr) + print('FAILED COMMAND:', _sh_quote_command(cmd), file=file) + write_bytes(errors, file=sys.stderr) + print(separator, file=sys.stderr) + + def _main_pull(args): """Pull the change lists.""" branch_name = _get_local_branch_name_from_args(args) manifest_path = _get_manifest_xml_from_args(args) - project_dirs = build_project_name_to_directory_dict(manifest_path) + project_dirs = build_project_name_dir_dict(manifest_path) # Collect change lists change_lists = _get_change_lists_from_args(args) @@ -343,21 +379,9 @@ def _main_pull(args): zip(change_list_groups, itertools.repeat(task_opts))) # Print failures and tracebacks - failed_results = [result for result in results if result] - if failed_results: - _SEPARATOR = '=' * 78 - _SEPARATOR_SUB = '-' * 78 - - print(_SEPARATOR, file=sys.stderr) - for failed_change, skipped_changes, cmd, errors in failed_results: - print('PROJECT:', failed_change.project, file=sys.stderr) - print('FAILED COMMIT:', failed_change.commit_sha1, file=sys.stderr) - for change in skipped_changes: - print('PENDING COMMIT:', change.commit_sha1, file=sys.stderr) - print(_SEPARATOR_SUB, file=sys.stderr) - print('FAILED COMMAND:', _sh_quote_command(cmd), file=sys.stderr) - write_bytes(errors, file=sys.stderr) - print(_SEPARATOR, file=sys.stderr) + failures = [result for result in results if result] + if failures: + _print_pull_failures(failures) sys.exit(1) @@ -422,6 +446,7 @@ def _get_local_branch_name_from_args(args): def main(): + """Main function""" args = _parse_args() if args.command == 'json': _main_json(args) diff --git a/tools/repo_pull/repo_review.py b/tools/repo_pull/repo_review.py index e12606d04..5da1227ea 100755 --- a/tools/repo_pull/repo_review.py +++ b/tools/repo_pull/repo_review.py @@ -20,17 +20,17 @@ from __future__ import print_function -from gerrit import create_url_opener_from_args, query_change_lists, set_review +import argparse +import json +import os +import sys try: from urllib.error import HTTPError # PY3 except ImportError: from urllib2 import HTTPError # PY2 -import argparse -import json -import os -import sys +from gerrit import create_url_opener_from_args, query_change_lists, set_review def _get_labels_from_args(args): @@ -48,6 +48,7 @@ def _get_labels_from_args(args): return labels +# pylint: disable=redefined-builtin def _print_change_lists(change_lists, file=sys.stdout): """Print matching change lists for each projects.""" change_lists = sorted( @@ -134,8 +135,8 @@ def main(): try: res_code, res_json = set_review( url_opener, args.gerrit, change['id'], labels, args.message) - except HTTPError as e: - res_code = e.code + except HTTPError as error: + res_code = error.code res_json = None if res_code != 200: