Update branch repohooks to TOT of cros/master
Update to commit 1562fb843cee08739cb9dfd2b37b4ad34479d213 in cros/master.
BUG=None
TEST=Uploaded this change
Change-Id: Ifaac56a70b8e586c1f8ca0da6f56f63b3325af63
Reviewed-on: http://gerrit.chromium.org/gerrit/704
Reviewed-by: Ryan Cui <rcui@chromium.org>
Tested-by: Ryan Cui <rcui@chromium.org>
diff --git a/errors.py b/errors.py
new file mode 100644
index 0000000..198d0e2
--- /dev/null
+++ b/errors.py
@@ -0,0 +1,102 @@
+# Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+import re
+import sys
+
+
+class VerifyException(Exception):
+ pass
+
+
+class HookFailure(object):
+ """Contains an error message and a list of error details."""
+ def __init__(self, msg, items=None):
+ self.msg = msg
+ self.items = items
+
+
+_INDENT = ' ' * 4
+_PROJECT_INFO = 'Errors in PROJECT *%s*!'
+
+def _PrintWithIndent(msg, indent_level):
+ """Print a block of text with a specified indent level to stderr.
+
+ Args:
+ msg: A string to print (may contain newlines).
+ indent_level: The number of indents to prefix each line with. Each indent
+ is four characters wide.
+ """
+ regex = re.compile(r'^', re.M)
+ msg = regex.sub(_INDENT * indent_level, msg)
+ print >> sys.stderr, msg
+
+
+def _FormatCommitDesc(desc):
+ """Returns the properly prefixed commit description."""
+ regex = re.compile(r'^', re.M)
+ return regex.sub('>', desc)
+
+
+def _FormatHookFailure(hook_failure):
+ """Returns the properly formatted VerifyException as a string."""
+ item_prefix = '\n%s* ' % _INDENT
+ formatted_items = ''
+ if hook_failure.items:
+ formatted_items = item_prefix + item_prefix.join(hook_failure.items)
+ return '* ' + hook_failure.msg + formatted_items
+
+
+def PrintErrorForProject(project, error):
+ """Prints the project and its error.
+
+ Args:
+ project: project name
+ error: An instance of the HookFailure class
+ """
+ _PrintWithIndent(_PROJECT_INFO % project, 0)
+ _PrintWithIndent(_FormatHookFailure(error), 1)
+ print >> sys.stderr, ''
+
+
+def PrintErrorsForCommit(project, commit, commit_desc, error_list):
+ """Prints the hook error to stderr with project and commit context
+
+ A sample error output for a project would be:
+ ----------------------------------------------------------------------------
+ Errors in PROJECT *chromiumos/repohooks*!
+ COMMIT 10041758:
+ Description:
+ >staged
+ >
+ >TEST=some
+ >Change-Id: I2c4f545a20a659541c02be16aa9dc440c876a604
+ >
+ Errors:
+ * Changelist description needs BUG field (after first line)
+ * Found line ending with white space in:
+ * src/repohooks/pre-upload.py, line 307
+ * Found lines longer than 80 characters (first 5 shown):
+ * src/repohooks/pre-upload.py, line 335, 85 chars
+ ----------------------------------------------------------------------------
+
+ Args:
+ project: project name
+ commit: the commit hash the errors belong to
+ commit_desc: a string containing the commit message
+ error_list: a list of HookFailure instances
+ """
+ _PrintWithIndent(_PROJECT_INFO % project, 0)
+
+ formatted_desc = _FormatCommitDesc(commit_desc)
+ _PrintWithIndent('COMMIT %s:' % commit[:8], 1)
+ _PrintWithIndent('Description:', 2)
+ _PrintWithIndent(formatted_desc, 3)
+ _PrintWithIndent('Errors:', 2)
+
+ for error in error_list:
+ _PrintWithIndent(_FormatHookFailure(error), 3)
+
+ print >> sys.stderr, ''
+
diff --git a/pre-upload.py b/pre-upload.py
index a8c1288..4b4799c 100644
--- a/pre-upload.py
+++ b/pre-upload.py
@@ -2,12 +2,15 @@
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
+import json
import os
import re
+import sys
import subprocess
+from errors import (VerifyException, HookFailure, PrintErrorForProject,
+ PrintErrorsForCommit)
-# General Helpers
COMMON_INCLUDED_PATHS = [
# C++ and friends
@@ -22,19 +25,32 @@
r".*\.java$", r".*\.mk$", r".*\.am$",
]
+
COMMON_EXCLUDED_PATHS = [
# avoid doing source file checks for kernel
r"/src/third_party/kernel/",
r"/src/third_party/kernel-next/",
+ r"/src/third_party/ktop/",
+ r"/src/third_party/punybench/",
r".*\bexperimental[\\\/].*",
r".*\b[A-Z0-9_]{2,}$",
r".*[\\\/]debian[\\\/]rules$",
]
+
+# General Helpers
+
+
+def _run_command(cmd):
+ """Executes the passed in command and returns raw stdout output."""
+ return subprocess.Popen(cmd, stdout=subprocess.PIPE).communicate()[0]
+
+
def _get_hooks_dir():
"""Returns the absolute path to the repohooks directory."""
cmd = ['repo', 'forall', 'chromiumos/repohooks', '-c', 'pwd']
- return subprocess.Popen(cmd, stdout=subprocess.PIPE).communicate()[0].strip()
+ return _run_command(cmd).strip()
+
def _match_regex_list(subject, expressions):
"""Try to match a list of regular expressions to a string.
@@ -51,6 +67,7 @@
return True
return False
+
def _filter_files(files, include_list, exclude_list=[]):
"""Filter out files based on the conditions passed in.
@@ -74,32 +91,38 @@
filtered.append(f)
return filtered
-def _report_error(msg, items=None):
- """Raises an exception with the passed in error message.
-
- If extra error detail is passed in, it will be appended to the error message.
-
- Args:
- msg: Error message header.
- items: A list of lines that follow the header that give extra error
- information.
- """
- if items:
- msg += '\n' + '\n'.join(items)
- raise Exception(msg)
-
# Git Helpers
+
+def _get_upstream_branch():
+ """Returns the upstream tracking branch of the current branch.
+
+ Raises:
+ Error if there is no tracking branch
+ """
+ current_branch = _run_command(['git', 'symbolic-ref', 'HEAD']).strip()
+ current_branch = current_branch.replace('refs/heads/', '')
+ if not current_branch:
+ raise VerifyException('Need to be on a tracking branch')
+
+ cfg_option = 'branch.' + current_branch + '.%s'
+ full_upstream = _run_command(['git', 'config', cfg_option % 'merge']).strip()
+ remote = _run_command(['git', 'config', cfg_option % 'remote']).strip()
+ if not remote or not full_upstream:
+ raise VerifyException('Need to be on a tracking branch')
+
+ return full_upstream.replace('heads', 'remotes/' + remote)
+
+
def _get_diff(commit):
"""Returns the diff for this commit."""
- cmd = ['git', 'show', commit]
- return subprocess.Popen(cmd, stdout=subprocess.PIPE).communicate()[0]
+ return _run_command(['git', 'show', commit])
+
def _get_file_diff(file, commit):
"""Returns a list of (linenum, lines) tuples that the commit touched."""
- cmd = ['git', 'show', '-p', '--no-ext-diff', commit, file]
- output = subprocess.Popen(cmd, stdout=subprocess.PIPE).communicate()[0]
+ output = _run_command(['git', 'show', '-p', '--no-ext-diff', commit, file])
new_lines = []
line_num = 0
@@ -114,10 +137,10 @@
line_num += 1
return new_lines
+
def _get_affected_files(commit):
"""Returns list of absolute filepaths that were modified/added."""
- cmd = ['git', 'diff', '--name-status', commit + '^!']
- output = subprocess.Popen(cmd, stdout=subprocess.PIPE).communicate()[0]
+ output = _run_command(['git', 'diff', '--name-status', commit + '^!'])
files = []
for statusline in output.splitlines():
m = re.match('^(\w)+\t(.+)$', statusline.rstrip())
@@ -127,21 +150,21 @@
files.append(os.path.join(pwd, m.group(2)))
return files
+
def _get_commits():
"""Returns a list of commits for this review."""
- cmd = ['git', 'log', 'm/master..', '--format=%H']
- commits = subprocess.Popen(cmd, stdout=subprocess.PIPE).communicate()[0]
- return commits.split()
+ cmd = ['git', 'log', '%s..' % _get_upstream_branch(), '--format=%H']
+ return _run_command(cmd).split()
+
def _get_commit_desc(commit):
"""Returns the full commit message of a commit."""
- cmd = ['git', 'log', '--format=%B', commit + '^!']
- description = subprocess.Popen(cmd, stdout=subprocess.PIPE).communicate()[0]
- return description.splitlines()
+ return _run_command(['git', 'log', '--format=%s%n%n%b', commit + '^!'])
# Common Hooks
+
def _check_no_long_lines(project, commit):
"""Checks that there aren't any lines longer than maxlen characters in any of
the text files to be submitted.
@@ -171,7 +194,8 @@
if errors:
msg = 'Found lines longer than %s characters (first 5 shown):' % MAX_LEN
- _report_error(msg, errors)
+ return HookFailure(msg, errors)
+
def _check_no_stray_whitespace(project, commit):
"""Checks that there is no stray whitespace at source lines end."""
@@ -185,11 +209,13 @@
if line.rstrip() != line:
errors.append('%s, line %s' % (afile, line_num))
if errors:
- _report_error('Found line ending with white space in:', errors)
+ return HookFailure('Found line ending with white space in:', errors)
+
def _check_no_tabs(project, commit):
"""Checks there are no unexpanded tabs."""
TAB_OK_PATHS = [
+ r"/src/platform/u-boot-config/",
r"/src/third_party/u-boot/",
r"/src/third_party/u-boot-next/",
r".*\.ebuild$",
@@ -207,34 +233,34 @@
if '\t' in line:
errors.append('%s, line %s' % (afile, line_num))
if errors:
- _report_error('Found a tab character in:', errors)
+ return HookFailure('Found a tab character in:', errors)
+
def _check_change_has_test_field(project, commit):
"""Check for a non-empty 'TEST=' field in the commit message."""
- TEST_RE = r'^\s*TEST\s*=\s*\S+.*$'
+ TEST_RE = r'\n\s*TEST\s*=[^\n]*\S+'
- found_field = False
- for line in _get_commit_desc(commit):
+ if not re.search(TEST_RE, _get_commit_desc(commit)):
+ msg = 'Changelist description needs TEST field (after first line)'
+ return HookFailure(msg)
- if re.match(TEST_RE, line):
- found_field = True
- break
-
- if not found_field:
- _report_error('Changelist description needs TEST field')
def _check_change_has_bug_field(project, commit):
"""Check for a non-empty 'BUG=' field in the commit message."""
- BUG_RE = r'^\s*BUG\s*=\s*\S+.*$'
+ BUG_RE = r'\n\s*BUG\s*=[^\n]*\S+'
- found_field = False
- for line in _get_commit_desc(commit):
- if re.match(BUG_RE, line):
- found_field = True
- break
+ if not re.search(BUG_RE, _get_commit_desc(commit)):
+ msg = 'Changelist description needs BUG field (after first line)'
+ return HookFailure(msg)
- if not found_field:
- _report_error('Changelist description needs BUG field')
+
+def _check_change_has_proper_changeid(project, commit):
+ """Verify that Change-ID is present in last paragraph of commit message."""
+ desc = _get_commit_desc(commit)
+ loc = desc.rfind('\nChange-Id:')
+ if loc == -1 or re.search('\n\s*\n\s*\S+', desc[loc:]):
+ return HookFailure('Change-Id must be in last paragraph of description.')
+
def _check_license(project, commit):
"""Verifies the license header."""
@@ -259,13 +285,14 @@
if not license_re.search(contents):
bad_files.append(f)
if bad_files:
- _report_error('License must match:\n%s\n' % license_re.pattern +
- 'Found a bad license header in these files:',
- bad_files)
+ return HookFailure('License must match:\n%s\n' % license_re.pattern +
+ 'Found a bad license header in these files:',
+ bad_files)
# Project-specific hooks
+
def _run_checkpatch(project, commit):
"""Runs checkpatch.pl on the given project"""
hooks_dir = _get_hooks_dir()
@@ -273,30 +300,50 @@
p = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE)
output = p.communicate(_get_diff(commit))[0]
if p.returncode:
- _report_error('checkpatch.pl errors/warnings\n\n' + output)
+ return HookFailure('checkpatch.pl errors/warnings\n\n' + output)
+
+
+def _run_json_check(project, commit):
+ """Checks that all JSON files are syntactically valid."""
+ for f in _filter_files(_get_affected_files(commit), [r'.*\.json']):
+ try:
+ json.load(open(f))
+ except Exception, e:
+ return HookFailure('Invalid JSON in %s: %s' % (f, e))
# Base
-COMMON_HOOKS = [_check_no_long_lines,
- _check_no_stray_whitespace,
- _check_no_tabs,
+
+COMMON_HOOKS = [_check_change_has_bug_field,
_check_change_has_test_field,
- _check_change_has_bug_field,
- _check_license]
+ _check_change_has_proper_changeid,
+ _check_no_stray_whitespace,
+ _check_no_long_lines,
+ _check_license,
+ _check_no_tabs]
+
def _setup_project_hooks():
"""Returns a dictionay of callbacks: dict[project] = [callback1, callback2]"""
return {
"chromiumos/third_party/kernel": [_run_checkpatch],
"chromiumos/third_party/kernel-next": [_run_checkpatch],
+ "chromeos/autotest-tools": [_run_json_check],
}
+
def _run_project_hooks(project, hooks):
- """For each project run its project specific hook from the hooks dictionary"""
- cmd = ['repo', 'forall', project, '-c', 'pwd']
- proj_dir = subprocess.Popen(cmd, stdout=subprocess.PIPE).communicate()[0]
- proj_dir = proj_dir.strip()
+ """For each project run its project specific hook from the hooks dictionary.
+
+ Args:
+ project: name of project to run hooks for.
+ hooks: a dictionary of hooks indexed by project name
+
+ Returns:
+ Boolean value of whether any errors were ecountered while running the hooks.
+ """
+ proj_dir = _run_command(['repo', 'forall', project, '-c', 'pwd']).strip()
pwd = os.getcwd()
# hooks assume they are run from the root of the project
os.chdir(proj_dir)
@@ -305,17 +352,48 @@
if project in hooks:
project_specific_hooks = hooks[project]
- for commit in _get_commits():
+ try:
+ commit_list = _get_commits()
+ except VerifyException as e:
+ PrintErrorForProject(project, HookFailure(str(e)))
+ os.chdir(pwd)
+ return True
+
+ error_found = False
+ for commit in commit_list:
+ error_list = []
for hook in COMMON_HOOKS + project_specific_hooks:
- hook(project, commit)
+ hook_error = hook(project, commit)
+ if hook_error:
+ error_list.append(hook_error)
+ error_found = True
+ if error_list:
+ PrintErrorsForCommit(project, commit, _get_commit_desc(commit),
+ error_list)
+
os.chdir(pwd)
+ return error_found
+
# Main
+
def main(project_list, **kwargs):
hooks = _setup_project_hooks()
+
+ found_error = False
for project in project_list:
- _run_project_hooks(project, hooks)
+ if _run_project_hooks(project, hooks):
+ found_error = True
+
+ if (found_error):
+ msg = ('Preupload failed due to errors in project(s). HINTS:\n'
+ '- To upload only current project, run \'repo upload .\'\n'
+ '- Errors may also be due to old upload hooks. Please run '
+ '\'repo sync chromiumos/repohooks\' to update.')
+ print >> sys.stderr, msg
+ sys.exit(1)
+
if __name__ == '__main__':
main()
