Make git-cl work with OWNERS files in a non .git/hooks/pre-cl-* world

git_cl/git_cl.py

 #!/usr/bin/python
 # git-cl -- a git-command for integrating reviews on Rietveld
 # Copyright (C) 2008 Evan Martin <martine@danga.com>
 
 import errno
 import logging
 import optparse
 import os
 import re
+import StringIO
 import subprocess
 import sys
 import tempfile
 import textwrap
 import upload
 import urlparse
 import urllib2
 
 try:
   import readline
@@ skipping 10 matching lines @@
   sys.path.append(depot_tools_path)
   import breakpad
 except ImportError:
   pass
 
 DEFAULT_SERVER = 'http://codereview.appspot.com'
 POSTUPSTREAM_HOOK_PATTERN = '.git/hooks/post-cl-%s'
 DESCRIPTION_BACKUP_FILE = '~/.git_cl_description_backup'
 
 def DieWithError(message):
-  print >>sys.stderr, message
+  print >> sys.stderr, message
   sys.exit(1)
 
 
 def Popen(cmd, **kwargs):
   """Wrapper for subprocess.Popen() that logs and watch for cygwin issues"""
   logging.info('Popen: ' + ' '.join(cmd))
   try:
     return subprocess.Popen(cmd, **kwargs)
   except OSError, e:
     if e.errno == errno.EAGAIN and sys.platform == 'cygwin':
@@ skipping 423 matching lines @@
 
   SetProperty(settings.GetCCList(), 'CC list', 'cc')
   SetProperty(settings.GetTreeStatusUrl(error_ok=True), 'Tree status URL',
               'tree-status-url')
   SetProperty(settings.GetViewVCUrl(), 'ViewVC URL', 'viewvc-url')
 
   # TODO: configure a default branch to diff against, rather than this
   # svn-based hackery.
 
 
+class HookResults(object):
+  """Contains the parsed output of the presubmit hooks."""
+  def __init__(self, output_from_hooks=None):
+    self.reviewers = []
+    self.output = None
+    self._ParseOutputFromHooks(output_from_hooks)
+
+  def _ParseOutputFromHooks(self, output_from_hooks):
+    if not output_from_hooks:
+      return
+    lines = []
+    reviewers = []
+    reviewer_regexp = re.compile('ADD: R=(.+)')
+    for l in output_from_hooks.splitlines():
+      m = reviewer_regexp.match(l)
+      if m:
+        reviewers.extend(m.group(1).split(','))
+      else:
+        lines.append(l)
+    self.output = '\n'.join(lines)
+    self.reviewers = ','.join(reviewers)
+
+
+class ChangeDescription(object):
+  """Contains a parsed form of the change description."""
+  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.
 
   Only looks up to the top of the repository unless an
   'inherit-review-settings-ok' file exists in the root of the repository.
   """
   inherit_ok_file = 'inherit-review-settings-ok'
   cwd = os.getcwd()
   root = os.path.abspath(RunGit(['rev-parse', '--show-cdup']).strip())
   if os.path.isfile(os.path.join(root, inherit_ok_file)):
     root = '/'
   while True:
     if filename in os.listdir(cwd):
       if os.path.isfile(os.path.join(cwd, filename)):
         return open(os.path.join(cwd, filename))
     if cwd == root:
       break
     cwd = os.path.dirname(cwd)
 
 
 def LoadCodereviewSettingsFromFile(fileobj):
   """Parse a codereview.settings file and updates hooks."""
-  def DownloadToFile(url, filename):
-    filename = os.path.join(settings.GetRoot(), filename)
-    contents = urllib2.urlopen(url).read()
-    fileobj = open(filename, 'w')
-    fileobj.write(contents)
-    fileobj.close()
-    os.chmod(filename, 0755)
-    return 0
-
   keyvals = {}
   for line in fileobj.read().splitlines():
     if not line or line.startswith("#"):
       continue
     k, v = line.split(": ", 1)
     keyvals[k] = v
 
-  def GetProperty(name):
-    return keyvals.get(name)
-
   def SetProperty(name, setting, unset_error_ok=False):
     fullname = 'rietveld.' + name
     if setting in keyvals:
       RunGit(['config', fullname, keyvals[setting]])
     else:
       RunGit(['config', '--unset-all', fullname], error_ok=unset_error_ok)
 
   SetProperty('server', 'CODE_REVIEW_SERVER')
   # Only server setting is required. Other settings can be absent.
   # In that case, we ignore errors raised during option deletion attempt.
@@ skipping 130 matching lines @@
 
 
 def ConvertToInteger(inputval):
   """Convert a string to integer, but returns either an int or None."""
   try:
     return int(inputval)
   except (TypeError, ValueError):
     return None
 
 
-def RunHook(committing, upstream_branch):
+def RunHook(committing, upstream_branch, rietveld_server, tbr, may_prompt):
+  """Calls sys.exit() if the hook fails; returns a HookResults otherwise."""
   import presubmit_support
   import scm
   import watchlists
 
   root = RunCommand(['git', 'rev-parse', '--show-cdup']).strip()
   if not root:
     root = "."
   absroot = os.path.abspath(root)
   if not root:
     raise Exception("Could not get root directory.")
@@ skipping 17 matching lines @@
                                        issue, patchset)
 
   # Apply watchlists on upload.
   if not committing:
     watchlist = watchlists.Watchlists(change.RepositoryRoot())
     files = [f.LocalPath() for f in change.AffectedFiles()]
     watchers = watchlist.GetWatchersForPaths(files)
     RunCommand(['git', 'config', '--replace-all',
                'rietveld.extracc', ','.join(watchers)])
 
-  return presubmit_support.DoPresubmitChecks(change, committing,
-      verbose=None, output_stream=sys.stdout, input_stream=sys.stdin,
-      default_presubmit=None, may_prompt=None)
+  output = StringIO.StringIO()
+  res = presubmit_support.DoPresubmitChecks(change, committing,
+      verbose=None, output_stream=output, input_stream=sys.stdin,
+      default_presubmit=None, may_prompt=may_prompt, tbr=tbr,
+      host_url=cl.GetRietveldServer())
+  hook_results = HookResults(output.getvalue())
+  if hook_results.output:
+    print hook_results.output
+
+  # TODO(dpranke): We should propagate the error out instead of calling exit().
+  if not res:
+    sys.exit(1)
+  return hook_results
 
 
 def CMDpresubmit(parser, args):
   """run presubmit tests on the current changelist"""
   parser.add_option('--upload', action='store_true',
                     help='Run upload hook instead of the push/dcommit hook')
   (options, args) = parser.parse_args(args)
 
   # Make sure index is up-to-date before running diff-index.
   RunGit(['update-index', '--refresh', '-q'], error_ok=True)
   if RunGit(['diff-index', 'HEAD']):
     # TODO(maruel): Is this really necessary?
     print 'Cannot presubmit with a dirty tree.  You must commit locally first.'
     return 1
 
+  cl = Changelist()
   if args:
     base_branch = args[0]
   else:
     # Default to diffing against the "upstream" branch.
-    base_branch = Changelist().GetUpstreamBranch()
+    base_branch = cl.GetUpstreamBranch()
 
   if options.upload:
     print '*** Presubmit checks for UPLOAD would report: ***'
-    return RunHook(committing=False, upstream_branch=base_branch)
+    RunHook(committing=False, upstream_branch=base_branch,
+            rietveld_server=cl.GetRietveldServer(), tbr=False,
+            may_prompt=False)
+    return 0
   else:
     print '*** Presubmit checks for DCOMMIT would report: ***'
-    return RunHook(committing=True, upstream_branch=base_branch)
+    RunHook(committing=True, upstream_branch=base_branch,
+            rietveld_server=cl.GetRietveldServer, tbr=False,
+            may_prompt=False)
+    return 0
 
 
 @usage('[args to "git diff"]')
 def CMDupload(parser, args):
   """upload the current changelist to codereview"""
   parser.add_option('--bypass-hooks', action='store_true', dest='bypass_hooks',
                     help='bypass upload presubmit hook')
+  parser.add_option('-f', action='store_true', dest='force',
+                    help="force yes to questions (don't prompt)")
   parser.add_option('-m', dest='message', help='message for patch')
   parser.add_option('-r', '--reviewers',
                     help='reviewer email addresses')
   parser.add_option('--cc',
                     help='cc email addresses')
   parser.add_option('--send-mail', action='store_true',
                     help='send email to reviewer immediately')
   parser.add_option("--emulate_svn_auto_props", action="store_true",
                     dest="emulate_svn_auto_props",
                     help="Emulate Subversion's auto properties feature.")
@@ skipping 11 matching lines @@
 
   cl = Changelist()
   if args:
     base_branch = args[0]
   else:
     # Default to diffing against the "upstream" branch.
     base_branch = cl.GetUpstreamBranch()
     args = [base_branch + "..."]
 
   if not options.bypass_hooks:
-    RunHook(committing=False, upstream_branch=base_branch)
+    hook_results = RunHook(committing=False, upstream_branch=base_branch,
+                           rietveld_server=cl.GetRietveldServer(), tbr=False,
+                           may_prompt=(not options.force))

[M-A Ruel, 2011/03/12 03:02:21]

Can you test it to see what happens when may_prompt=True and a PromptWarning is
generated?

+  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
   # git config key "diff.external" is used).
   env = os.environ.copy()
   if 'GIT_EXTERNAL_DIFF' in env:
     del env['GIT_EXTERNAL_DIFF']
   subprocess.call(['git', 'diff', '--no-ext-diff', '--stat', '-M'] + args,
                   env=env)
 
   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:
     if not options.reviewers:
       DieWithError("Must specify reviewers to send email.")
     upload_args.append('--send_mail')
   if options.from_logs and not options.message:
     print 'Must set message for subject line if using desc_from_logs'
     return 1
 
   change_desc = None
 
   if cl.GetIssue():
     if options.message:
       upload_args.extend(['--message', options.message])
     upload_args.extend(['--issue', cl.GetIssue()])
     print ("This branch is associated with issue %s. "
            "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])
 
   # Include the upstream repo's URL in the change -- this is useful for
   # projects that have their source spread across multiple repos.
   remote_url = None
   if settings.GetIsGitSvn():
     # URL is dependent on the current directory.
     data = RunGit(['svn', 'info'], cwd=settings.GetRoot())
@@ skipping 11 matching lines @@
   try:
     issue, patchset = upload.RealMain(['upload'] + upload_args + args)
   except:
     # If we got an exception after the user typed a description for their
     # change, back up the description before re-raising.
     if change_desc:
       backup_path = os.path.expanduser(DESCRIPTION_BACKUP_FILE)
       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
 
   if not cl.GetIssue():
     cl.SetIssue(issue)
   cl.SetPatchset(patchset)
   return 0
 
 
 def SendUpstream(parser, args, cmd):
@@ skipping 47 matching lines @@
     svn_head = RunGit(['log', '--grep=^git-svn-id:', '-1',
                        '--pretty=format:%H'])
     extra_commits = RunGit(['rev-list', '^' + svn_head, base_branch])
     if extra_commits:
       print ('This branch has %d additional commits not upstreamed yet.'
              % len(extra_commits.splitlines()))
       print ('Upstream "%s" or rebase this branch on top of the upstream trunk '
              'before attempting to %s.' % (base_branch, cmd))
       return 1
 
+  if not options.bypass_hooks:
+    RunHook(committing=True, upstream_branch=base_branch,
+            rietveld_server=cl.GetRietveldServer(), tbr=options.tbr,
+            may_prompt=(not options.force))
+
   if not options.force and not options.bypass_hooks:
-    RunHook(committing=False, upstream_branch=base_branch)
-
     if cmd == 'dcommit':
       # Check the tree status if the tree status URL is set.
       status = GetTreeStatus()
       if 'closed' == status:
         print ('The tree is closed.  Please wait for it to reopen. Use '
                '"git cl dcommit -f" to commit on a closed tree.')
         return 1
       elif 'unknown' == status:
         print ('Unable to determine tree status.  Please verify manually and '
                'use "git cl dcommit -f" to commit on a closed tree.')
@@ skipping 18 matching lines @@
 
     description += "\n\nReview URL: %s" % cl.GetIssueURL()
   else:
     if not description:
       # Submitting TBR.  See if there's already a description in Rietveld, else
       # create a template description. Eitherway, give the user a chance to edit
       # it to fill in the TBR= field.
       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.
 
 """
         description += CreateDescriptionFromLog(args)
 
       description = UserEditedLog(description + '\nTBR=')
 
     if not description:
@@ skipping 71 matching lines @@
     if viewvc_url and revision:
       cl.description += ('\n\nCommitted: ' + viewvc_url + revision)
     print ('Closing issue '
            '(you may be prompted for your codereview password)...')
     cl.CloseIssue()
     cl.SetIssue(0)
 
   if retcode == 0:
     hook = POSTUPSTREAM_HOOK_PATTERN % cmd
     if os.path.isfile(hook):
-      RunHook(hook, upstream_branch=base_branch, error_ok=True)
+      RunCommand([hook, base_branch], error_ok=True)
 
   return 0
 
 
 @usage('[upstream branch to apply against]')
 def CMDdcommit(parser, args):
   """commit the current changelist via git-svn"""
   if not settings.GetIsGitSvn():
     message = """This doesn't appear to be an SVN repository.
 If your project has a git mirror with an upstream SVN master, you probably need
@@ skipping 115 matching lines @@
   """Fetches the tree status and returns either 'open', 'closed',
   'unknown' or 'unset'."""
   url = settings.GetTreeStatusUrl(error_ok=True)
   if url:
     status = urllib2.urlopen(url).read().lower()
     if status.find('closed') != -1 or status == '0':
       return 'closed'
     elif status.find('open') != -1 or status == '1':
       return 'open'
     return 'unknown'
+  return 'unset'
 
-  return 'unset'
 
 def GetTreeStatusReason():
   """Fetches the tree status from a json url and returns the message
   with the reason for the tree to be opened or closed."""
   # Don't import it at file level since simplejson is not installed by default
   # on python 2.5 and it is only used for git-cl tree which isn't often used,
   # forcing everyone to install simplejson isn't efficient.
   try:
     import simplejson as json
   except ImportError:
     try:
       import json
       # Some versions of python2.5 have an incomplete json module. Check to make
       # sure loads exists.
       json.loads
     except (ImportError, AttributeError):
       print >> sys.stderr, 'Please install simplejson'
       sys.exit(1)
 
   url = settings.GetTreeStatusUrl()
   json_url = urlparse.urljoin(url, '/current?format=json')
   connection = urllib2.urlopen(json_url)
   status = json.loads(connection.read())
   connection.close()
   return status['message']
 
+
 def CMDtree(parser, args):
   """show the status of the tree"""
   (options, args) = parser.parse_args(args)
   status = GetTreeStatus()
   if 'unset' == status:
     print 'You must configure your tree status URL by running "git cl config".'
     return 2
 
   print "The tree is %s" % status
   print
@@ skipping 71 matching lines @@
             ('AppEngine is misbehaving and returned HTTP %d, again. Keep faith '
              'and retry or visit go/isgaeup.\n%s') % (e.code, str(e)))
 
   # Not a known command. Default to help.
   GenUsage(parser, 'help')
   return CMDhelp(parser, argv)
 
 
 if __name__ == '__main__':
   sys.exit(main(sys.argv[1:]))