Update branch repohooks to TOT of cros/master

Update to commit 1562fb843cee08739cb9dfd2b37b4ad34479d213 in cros/master.

BUG=None
TEST=Uploaded this change

Change-Id: I7373fe0a308de0462683c5df1a31f4fd922f16f5
Reviewed-on: http://gerrit.chromium.org/gerrit/693
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()