add changes to presubmit needed for OWNERS support

git_cl/git_cl.py

Index: git_cl/git_cl.py
diff --git a/git_cl/git_cl.py b/git_cl/git_cl.py
index a89b1e7e329411e53203b680f67e9a052b94807d..9da734e9e21e8ee681ff6a778081f40164beab74 100644
--- a/git_cl/git_cl.py
+++ b/git_cl/git_cl.py
@@ -54,7 +54,8 @@ def Popen(cmd, **kwargs):
 
 
 def RunCommand(cmd, error_ok=False, error_message=None,

[M-A Ruel, 2011/03/08 19:58:32]

I want to merge this code into gclient_utils.CheckCallAndFilter() eventually.
For now it could be sufficient.

[Dirk Pranke, 2011/03/08 22:26:03]

I agree that would be good. I can look at doing that in a separate patch if you
like.

-               redirect_stdout=True, swallow_stderr=False, **kwargs):
+               redirect_stdout=True, swallow_stderr=False,
+               tee_filter=None, **kwargs):
   if redirect_stdout:
     stdout = subprocess.PIPE
   else:
@@ -64,7 +65,18 @@ def RunCommand(cmd, error_ok=False, error_message=None,
   else:
     stderr = None
   proc = Popen(cmd, stdout=stdout, stderr=stderr, **kwargs)
-  output = proc.communicate()[0]
+  if tee_filter:
+    output = ''
+    while True:
+      line = proc.stdout.readline()
+      if line == '' and proc.poll() != None:
+        break
+    if not tee_filter(line):
+      print line
+    output += line + '\n'
+  else:
+    output = proc.communicate()[0]
+
   if not error_ok and proc.returncode != 0:
     DieWithError('Command "%s" failed.\n' % (' '.join(cmd)) +
                  (error_message or output or ''))
@@ -479,6 +491,78 @@ def GetCodereviewSettingsInteractively():
   # svn-based hackery.
 
 
+class HookResults(object):
+  """First stab at a framework for letting hooks alter what happens next.

[M-A Ruel, 2011/03/08 19:58:32]

Can you define what it is instead?

Same for ChangeDescription

[Dirk Pranke, 2011/03/08 22:26:03]

Will do.

+  For now, all they can do is suggest reviewers."""
+  def __init__(self, output_from_hooks=None):
+    self.reviewers = []
+    self.output = None
+    self._ParseOutputFromHooks(output_from_hooks)
+
+  def print_output(self):
+    if self.output:
+      print '\n'.join(self.output)
+
+  def _ParseOutputFromHooks(self, output_from_hooks):
+    if not output_from_hooks:
+      return
+    lines = []
+    reviewer_regexp = re.compile('ADD: R=(.+)')
+    for l in output_from_hooks.split('\n'):

[M-A Ruel, 2011/03/08 19:58:32]

splitlines()

[Dirk Pranke, 2011/03/08 22:26:03]

I wasn't sure if this was a builtin on strings, and the docs I looked at lied to
me. Will change.

+      m = reviewer_regexp.match(l)
+      if m:
+        self.reviewers = m.group(1)

[M-A Ruel, 2011/03/08 19:58:32]

What about multiple R= lines?

[Dirk Pranke, 2011/03/08 22:26:03]

Obviously, this version doesn't support that. I'm not sure how common that is in
practice, but there doesn't seem to be any downside and it's trivial to add, so
I will add it.

+      else:
+        lines.append(l)
+    self.output = '\n'.join(lines)
+
+
+class ChangeDescription(object):
+  def __init__(self, subject, log_desc, reviewers):
+    self.subject = subject
+    self.log_desc = log_desc
+    self.reviewers = reviewers
+    self.description = self.log_desc
+
+  def Update(self):
+    initial_text = """# Enter a description of the change.
+# This will displayed on the codereview site.
+# The first line will also be used as the subject of the review.
+"""
+    initial_text += self.description
+    if 'R=' not in self.description and self.reviewers:
+      initial_text += '\nR=' + self.reviewers
+    if 'BUG=' not in self.description:
+      initial_text += '\nBUG='
+    if 'TEST=' not in self.description:
+      initial_text += '\nTEST='
+    self._ParseDescription(UserEditedLog(initial_text))
+
+  def _ParseDescription(self, description):
+    if not description:
+      self.description = description
+      return
+
+    parsed_lines = []
+    reviewers_regexp = re.compile('\s*R=(.+)')
+    reviewers = ''
+    subject = ''
+    for l in description.splitlines():
+      if not subject:
+        subject = l
+      matched_reviewers = reviewers_regexp.match(l)
+      if matched_reviewers:
+        reviewers = matched_reviewers.group(1)
+      parsed_lines.append(l)
+
+    self.description = '\n'.join(parsed_lines) + '\n'
+    self.subject = subject
+    self.reviewers = reviewers
+
+  def IsEmpty(self):
+    return not self.description
+
+
 def FindCodereviewSettingsFile(filename='codereview.settings'):
   """Finds the given file starting in the cwd and going up.
 
@@ -673,9 +757,12 @@ def RunHook(hook, upstream_branch, error_ok=False):
   """Run a given hook if it exists. By default, we fail on errors."""
   hook = '%s/%s' % (settings.GetRoot(), hook)
   if not os.path.exists(hook):
-    return
-  return RunCommand([hook, upstream_branch], error_ok=error_ok,
-                    redirect_stdout=False)
+    return HookResults()
+
+  output = RunCommand([hook, upstream_branch], error_ok=error_ok,
+                      redirect_stdout=True,
+                      tee_filter=lambda l: l.startswith('ADD:'))
+  return HookResults(output)
 
 
 def CMDpresubmit(parser, args):
@@ -700,11 +787,11 @@ def CMDpresubmit(parser, args):
   if options.upload:
     print '*** Presubmit checks for UPLOAD would report: ***'
     return not RunHook(PREUPLOAD_HOOK, upstream_branch=base_branch,
-        error_ok=True)
+        error_ok=True).output
   else:
     print '*** Presubmit checks for DCOMMIT would report: ***'
     return not RunHook(PREDCOMMIT_HOOK, upstream_branch=base_branch,
-        error_ok=True)
+        error_ok=True).output
 
 
 @usage('[args to "git diff"]')
@@ -743,7 +830,13 @@ def CMDupload(parser, args):
     args = [base_branch + "..."]
 
   if not options.bypass_hooks:
-    RunHook(PREUPLOAD_HOOK, upstream_branch=base_branch, error_ok=False)
+    hook_results = RunHook(PREUPLOAD_HOOK, upstream_branch=base_branch,
+                           error_ok=False)
+  else:
+    hook_results = HookResults()
+
+  if not options.reviewers and hook_results.reviewers:
+    options.reviewers = hook_results.reviewers
 
   # --no-ext-diff is broken in some versions of Git, so try to work around
   # this by overriding the environment (but there is still a problem if the
@@ -756,8 +849,6 @@ def CMDupload(parser, args):
 
   upload_args = ['--assume_yes']  # Don't ask about untracked files.
   upload_args.extend(['--server', cl.GetRietveldServer()])
-  if options.reviewers:
-    upload_args.extend(['--reviewers', options.reviewers])
   if options.emulate_svn_auto_props:
     upload_args.append('--emulate_svn_auto_props')
   if options.send_mail:
@@ -778,28 +869,19 @@ def CMDupload(parser, args):
            "Adding patch to that issue." % cl.GetIssue())
   else:
     log_desc = CreateDescriptionFromLog(args)
-    if options.from_logs:
-      # Uses logs as description and message as subject.
-      subject = options.message
-      change_desc = subject + '\n\n' + log_desc
-    else:
-      initial_text = """# Enter a description of the change.
-# This will displayed on the codereview site.
-# The first line will also be used as the subject of the review.
-"""
-      if 'BUG=' not in log_desc:
-        log_desc += '\nBUG='
-      if 'TEST=' not in log_desc:
-        log_desc += '\nTEST='
-      change_desc = UserEditedLog(initial_text + log_desc)
-      subject = ''
-      if change_desc:
-        subject = change_desc.splitlines()[0]
-    if not change_desc:
+    change_desc = ChangeDescription(options.message, log_desc,
+                                    options.reviewers)
+    if not options.from_logs:
+      change_desc.Update()
+
+    if change_desc.IsEmpty():
       print "Description is empty; aborting."
       return 1
-    upload_args.extend(['--message', subject])
-    upload_args.extend(['--description', change_desc])
+
+    upload_args.extend(['--message', change_desc.subject])
+    upload_args.extend(['--description', change_desc.description])
+    if change_desc.reviewers:
+      upload_args.extend(['--reviewers', change_desc.reviewers])
     cc = ','.join(filter(None, (settings.GetCCList(), options.cc)))
     if cc:
       upload_args.extend(['--cc', cc])
@@ -831,7 +913,7 @@ def CMDupload(parser, args):
       print '\nGot exception while uploading -- saving description to %s\n' \
           % backup_path
       backup_file = open(backup_path, 'w')
-      backup_file.write(change_desc)
+      backup_file.write(change_desc.description)
       backup_file.close()
     raise
 
@@ -940,6 +1022,7 @@ def SendUpstream(parser, args, cmd):
       if cl.GetIssue():
         description = cl.GetDescription()
 
+      # TODO(dpranke): Update to use ChangeDescription object.
       if not description:
         description = """# Enter a description of the change.
 # This will be used as the change log for the commit.