git-cl: Run PostUpload hooks via CLI.

Bug: 1042324
Change-Id: Iae90e632e9c1b2c798575ab940d82a145b47204a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2070681
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Josip Sokcevic <sokcevic@google.com>

git_cl.py

@@ -1427,12 +1427,8 @@ class Changelist(object):
 
     self.description = description
 
-  def RunHook(
-      self, committing, may_prompt, verbose, parallel, upstream, description,
-      all_files):
-    """Calls sys.exit() if the hook fails; returns a HookResults otherwise."""
+  def _GetCommonPresubmitArgs(self, verbose, upstream):
     args = [
-        '--commit' if committing else '--upload',
         '--author', self.GetAuthor(),
         '--root', settings.GetRoot(),
         '--upstream', upstream,
@@ -1450,6 +1446,14 @@ class Changelist(object):
     if gerrit_url:
       args.extend(['--gerrit_url', gerrit_url])
 
+    return args
+
+  def RunHook(
+      self, committing, may_prompt, verbose, parallel, upstream, description,
+      all_files):
+    """Calls sys.exit() if the hook fails; returns a HookResults otherwise."""
+    args = self._GetCommonPresubmitArgs(verbose, upstream)
+    args.append('--commit' if committing else '--upload')
     if may_prompt:
       args.append('--may_prompt')
     if parallel:
@@ -1479,6 +1483,17 @@ class Changelist(object):
         json_results = gclient_utils.FileRead(json_output)
         return json.loads(json_results)
 
+  def RunPostUploadHook(self, verbose, upstream, description):
+    args = self._GetCommonPresubmitArgs(verbose, upstream)
+    args.append('--post_upload')
+
+    with gclient_utils.temporary_file() as description_file:
+      gclient_utils.FileWrite(
+          description_file, description.encode('utf-8'), mode='wb')
+      args.extend(['--description_file', description_file])
+      p = subprocess2.Popen(['vpython', PRESUBMIT_SUPPORT] + args)
+      p.wait()
+
   def CMDUpload(self, options, git_diff_args, orig_args):
     """Uploads a change to codereview."""
     custom_cl_base = None
@@ -1505,16 +1520,17 @@ class Changelist(object):
     if not options.bypass_watchlists:
       self.ExtendCC(watchlist.GetWatchersForPaths(files))
 
+    description = change.FullDescriptionText()
+    if options.reviewers or options.tbrs or options.add_owners_to:
+      # Set the reviewer list now so that presubmit checks can access it.
+      change_description = ChangeDescription(description)
+      change_description.update_reviewers(options.reviewers,
+                                          options.tbrs,
+                                          options.add_owners_to,
+                                          change)
+      description = change_description.description
+
     if not options.bypass_hooks:
-      description = change.FullDescriptionText()
-      if options.reviewers or options.tbrs or options.add_owners_to:
-        # Set the reviewer list now so that presubmit checks can access it.
-        change_description = ChangeDescription(description)
-        change_description.update_reviewers(options.reviewers,
-                                            options.tbrs,
-                                            options.add_owners_to,
-                                            change)
-        description = change_description.description
       hook_results = self.RunHook(committing=False,
                                   may_prompt=not options.force,
                                   verbose=options.verbose,
@@ -1531,10 +1547,7 @@ class Changelist(object):
           'last-upload-hash', RunGit(['rev-parse', 'HEAD']).strip())
       # Run post upload hooks, if specified.
       if settings.GetRunPostUploadHook():
-        presubmit_support.DoPostUploadExecuter(
-            change,
-            self.GetGerritObjForPresubmit(),
-            options.verbose)
+        self.RunPostUploadHook(options.verbose, base_branch, description)
 
       # Upload all dependencies if specified.
       if options.dependencies:

presubmit_support.py

@@ -1809,6 +1809,8 @@ def main(argv=None):
                      help='Use commit instead of upload checks.')
   hooks.add_argument('-u', '--upload', action='store_false', dest='commit',
                      help='Use upload instead of commit checks.')
+  hooks.add_argument('--post_upload', action='store_true',
+                     help='Run post-upload commit hooks.')
   parser.add_argument('-r', '--recursive', action='store_true',
                       help='Act recursively.')
   parser.add_argument('-v', '--verbose', action='count', default=0,
@@ -1866,6 +1868,11 @@ def main(argv=None):
   change = _parse_change(parser, options)
 
   try:
+    if options.post_upload:
+      return DoPostUploadExecuter(
+          change,
+          gerrit_obj,
+          options.verbose)
     with canned_check_filter(options.skip_canned):
       return DoPresubmitChecks(
           change,

tests/git_cl_test.py

@@ -2815,7 +2815,6 @@ class ChangelistTest(unittest.TestCase):
     self.assertEqual(expected_results, results)
     subprocess2.Popen.assert_called_once_with([
         'vpython', 'PRESUBMIT_SUPPORT',
-        '--commit',
         '--author', 'author',
         '--root', 'root',
         '--upstream', 'upstream',
@@ -2823,6 +2822,7 @@ class ChangelistTest(unittest.TestCase):
         '--issue', '123456',
         '--patchset', '7',
         '--gerrit_url', 'https://chromium-review.googlesource.com',
+        '--commit',
         '--may_prompt',
         '--parallel',
         '--all_files',
@@ -2857,6 +2857,25 @@ class ChangelistTest(unittest.TestCase):
 
     sys.exit.assert_called_once_with(2)
 
+  def testRunPostUploadHook(self):
+    cl = git_cl.Changelist()
+    cl.RunPostUploadHook(2, 'upstream', 'description')
+
+    subprocess2.Popen.assert_called_once_with([
+        'vpython', 'PRESUBMIT_SUPPORT',
+        '--author', 'author',
+        '--root', 'root',
+        '--upstream', 'upstream',
+        '--verbose', '--verbose',
+        '--issue', '123456',
+        '--patchset', '7',
+        '--gerrit_url', 'https://chromium-review.googlesource.com',
+        '--post_upload',
+        '--description_file', '/tmp/fake-temp1',
+    ])
+    gclient_utils.FileWrite.assert_called_once_with(
+        '/tmp/fake-temp1', b'description', mode='wb')
+
 
 class CMDTestCaseBase(unittest.TestCase):
   _STATUSES = [

tests/presubmit_unittest.py

@@ -936,6 +936,19 @@ def CheckChangeOnCommit(input_api, output_api):
                                                self.fake_root_dir, None, None,
                                                False, output))
 
+  def testMainPostUpload(self):
+    os.path.isfile.side_effect = lambda f: 'PRESUBMIT.py' in f
+    os.listdir.return_value = ['PRESUBMIT.py']
+    gclient_utils.FileRead.return_value = (
+        'def PostUploadHook(gerrit, change, output_api):\n'
+        '  return ()\n')
+    scm.determine_scm.return_value = None
+    presubmit._parse_files.return_value = [('M', 'random_file.txt')]
+    self.assertEqual(
+        0,
+        presubmit.main(
+            ['--root', self.fake_root_dir, 'random_file.txt', '--post_upload']))
+
   @mock.patch(
       'presubmit_support.ListRelevantPresubmitFiles',
       return_value=['PRESUBMIT.py'])