Remove support for generic "hook files" in git-cl, require depot_tools

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 subprocess
 import sys
 import tempfile
 import textwrap
 import upload
 import urlparse
 import urllib2
 
 try:
   import readline
 except ImportError:
   pass
 
 try:
-  # Add the parent directory in case it's a depot_tools checkout.
+  # TODO(dpranke): We wrap this in a try block for a limited form of

[M-A Ruel, 2011/03/11 14:42:52]

My take is to move git_cl up a directory first and them do more severe hacking
on it. But I don't mind, do as you prefer.

+  # backwards-compatibility with older versions of git-cl that weren't
+  # dependent on depot_tools. This version should still work outside of
+  # depot_tools as long as --bypass-hooks is used. We should remove this
+  # once this has baked for a while and things seem safe.
   depot_tools_path = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
   sys.path.append(depot_tools_path)
   import breakpad
 except ImportError:
   pass
 
 DEFAULT_SERVER = 'http://codereview.appspot.com'
-PREDCOMMIT_HOOK = '.git/hooks/pre-cl-dcommit'
-POSTUPSTREAM_HOOK_PATTERN = '.git/hooks/post-cl-%s'
-PREUPLOAD_HOOK = '.git/hooks/pre-cl-upload'
 DESCRIPTION_BACKUP_FILE = '~/.git_cl_description_backup'
 
 def DieWithError(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))
@@ skipping 489 matching lines @@
   SetProperty('tree-status-url', 'STATUS', unset_error_ok=True)
   SetProperty('viewvc-url', 'VIEW_VC', unset_error_ok=True)
 
   if 'PUSH_URL_CONFIG' in keyvals and 'ORIGIN_URL_CONFIG' in keyvals:
     #should be of the form
     #PUSH_URL_CONFIG: url.ssh://gitrw.chromium.org.pushinsteadof
     #ORIGIN_URL_CONFIG: http://src.chromium.org/git
     RunGit(['config', keyvals['PUSH_URL_CONFIG'],
             keyvals['ORIGIN_URL_CONFIG']])
 
-  # Update the hooks if the local hook files aren't present already.
-  if GetProperty('GITCL_PREUPLOAD') and not os.path.isfile(PREUPLOAD_HOOK):
-    DownloadToFile(GetProperty('GITCL_PREUPLOAD'), PREUPLOAD_HOOK)
-  if GetProperty('GITCL_PREDCOMMIT') and not os.path.isfile(PREDCOMMIT_HOOK):
-    DownloadToFile(GetProperty('GITCL_PREDCOMMIT'), PREDCOMMIT_HOOK)
-  return 0
-
 
 @usage('[repo root containing codereview.settings]')
 def CMDconfig(parser, args):
   """edit configuration for this tree"""
 
   (options, args) = parser.parse_args(args)
   if len(args) == 0:
     GetCodereviewSettingsInteractively()
     return 0
 
@@ skipping 101 matching lines @@
     return
 
   fileobj = open(filename)
   text = fileobj.read()
   fileobj.close()
   os.remove(filename)
   stripcomment_re = re.compile(r'^#.*$', re.MULTILINE)
   return stripcomment_re.sub('', text).strip()
 
 
-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)
+def ConvertToInteger(inputval):

[M-A Ruel, 2011/03/11 14:42:52]

(random idea)
def safe_convert(inputval, outputtype):
try:
  return outputtype(inputval):
...

:D
But I don't mind.

+  """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):
+  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.")
+
+  # We use the sha1 of HEAD as a name of this change.
+  name = RunCommand(['git', 'rev-parse', 'HEAD']).strip()
+  files = scm.GIT.CaptureStatus([root], upstream_branch)
+
+  cl = Changelist()
+  issue = ConvertToInteger(cl.GetIssue())
+  patchset = ConvertToInteger(cl.GetPatchset())
+  if issue:
+    description = cl.GetDescription()
+  else:
+    # If the change was never uploaded, use the log messages of all commits
+    # up to the branch point, as git cl upload will prefill the description
+    # with these log messages.
+    description = RunCommand(['git', 'log', '--pretty=format:%s%n%n%b',
+                             '%s...' % (upstream_branch)]).strip()
+  change = presubmit_support.GitChange(name, description, absroot, files,
+                                       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,

[M-A Ruel, 2011/03/11 14:42:52]

It changes from running out of process to in-process. There is known issues I
want to get rid of. :/

But running out of process is a matter of integration inside presubmit_support
than git-cl or gcl. It's just sad to have a temporary regression on that side in
the meantime.

+      verbose=None, output_stream=sys.stdout, input_stream=sys.stdin,
+      default_presubmit=None, may_prompt=None)
 
 
 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
 
   if args:
     base_branch = args[0]
   else:
     # Default to diffing against the "upstream" branch.
     base_branch = Changelist().GetUpstreamBranch()
 
   if options.upload:
     print '*** Presubmit checks for UPLOAD would report: ***'
-    return not RunHook(PREUPLOAD_HOOK, upstream_branch=base_branch,
-        error_ok=True)
+    return RunHook(committing=False, upstream_branch=base_branch)
   else:
     print '*** Presubmit checks for DCOMMIT would report: ***'
-    return not RunHook(PREDCOMMIT_HOOK, upstream_branch=base_branch,
-        error_ok=True)
+    return RunHook(committing=True, upstream_branch=base_branch)
 
 
 @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('-m', dest='message', help='message for patch')
   parser.add_option('-r', '--reviewers',
                     help='reviewer email addresses')
@@ skipping 18 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(PREUPLOAD_HOOK, upstream_branch=base_branch, error_ok=False)
+    RunHook(committing=False, upstream_branch=base_branch)
 
   # --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)
 
@@ skipping 136 matching lines @@
                        '--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.force and not options.bypass_hooks:
-    RunHook(PREDCOMMIT_HOOK, upstream_branch=base_branch, error_ok=False)
+    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 '
@@ skipping 107 matching lines @@
     else:
       return 1
     viewvc_url = settings.GetViewVCUrl()
     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)
-
   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
 to run 'git svn init', see your project's git mirror documentation.
@@ skipping 233 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:]))