Add support for post-dcommit hooks

git_cl.py

Index: git_cl.py
diff --git a/git_cl.py b/git_cl.py
index ea1238261dc8c1aa2fe6d30485a13264bd1de242..67bbb21a12826c56077b56d05bf5f2aa8b862b1d 100644
--- a/git_cl.py
+++ b/git_cl.py
@@ -29,6 +29,7 @@ except ImportError:
 
 DEFAULT_SERVER = 'http://codereview.appspot.com'
 PREDCOMMIT_HOOK = '.git/hooks/pre-cl-dcommit'
+POSTDCOMMIT_HOOK = '.git/hooks/post-cl-dcommit'
 PREUPLOAD_HOOK = '.git/hooks/pre-cl-upload'
 DESCRIPTION_BACKUP_FILE = '~/.git_cl_description_backup'
 
@@ -971,6 +972,7 @@ def SendUpstream(parser, args, cmd):
   # Stuff our change into the merge branch.
   # We wrap in a try...finally block so if anything goes wrong,
   # we clean up the branches.
+  retcode = -1
   try:
     RunGit(['checkout', '-q', '-b', MERGE_BRANCH, base_branch])
     RunGit(['merge', '--squash', cl.GetBranchRef()])
@@ -986,11 +988,13 @@ def SendUpstream(parser, args, cmd):
       logging.debug(output)
     else:
       # dcommit the merge branch.
-      output = RunGit(['svn', 'dcommit', '--no-rebase'])
+      retcode, output = RunGitWithCode(['svn', 'dcommit', '--no-rebase'])
   finally:
     # And then swap back to the original branch and clean up.
     RunGit(['checkout', '-q', cl.GetBranch()])
     RunGit(['branch', '-D', MERGE_BRANCH])
+    if retcode == 0 and os.path.isfile(POSTDCOMMIT_HOOK):

[Evan Martin, 2010/12/22 22:14:57]

It's common in the git world for hooks like these to exist but to not have +x
permissions (indicating they shouldn't ber run).  I guess the RunHook would fail
below in that case, hopefully silently, and on Windows we can't believe +x
anyway.

[sadrul, 2010/12/22 22:46:29]

Looks like if the pre-dcommit hook is -x, it's not handled properly, i.e. 'cl
presubmit' crashes with a child_exception. Do we want to handle the post-dcommit
hook differently?

+      RunHook(POSTDCOMMIT_HOOK, upstream_branch=base_branch, error_ok=True)
 
   if cl.GetIssue():

[Evan Martin, 2010/12/22 22:14:57]

Should we not run all of the remainder of this code when retcode != 0?

[sadrul, 2010/12/22 22:46:29]

I think so, because I have noticed an issue getting closed when dcommit failed
(once, maybe twice). But I am not familiar enough with this code to be sure.

[Evan Martin, 2010/12/22 23:05:44]

Oh actually, since we don't pass error_ok=True above, maybe we'll never get here
on error.

So maybe you want to move your new code outside of the "finally" block and
remove the retcode test.

[sadrul, 2010/12/22 23:17:24]

I don't think I understand. By 'above', do you mean with RunGitWithCode?

     if cmd == 'dcommit' and 'Committed r' in output: