Revert "git-cl: Use bb to schedule try jobs."

This reverts commit 5b6ae8bc74.

Reason for revert: Suspected breakage of wpt-importer (https://ci.chromium.org/p/infra/builders/cron/wpt-importer)

Original change's description:
> git-cl: Use bb to schedule try jobs.
>
> Bug: 976104
> Change-Id: I3423667f1ed9edfc5fa17842932de7704951fc62
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1801799
> Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
> Reviewed-by: Andrii Shyshkalov <tandrii@google.com>

TBR=nodir@chromium.org,tandrii@google.com,ehmaldonado@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 976104
Change-Id: I885c1e71b34928c402d3375f820b28f3d6535c54
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1806148
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@google.com>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>

git_cl.py

@@ -416,80 +416,82 @@ def _trigger_try_jobs(auth_config, changelist, buckets, options, patchset):
     buckets: A nested dict mapping bucket names to builders to tests.
     options: Command-line options.
   """
-  _, ret_code = subprocess2.communicate(
+  assert changelist.GetIssue(), 'CL must be uploaded first'
-      ['bb', 'auth-info'], stdout=subprocess2.VOID, stderr=subprocess2.VOID)
+  codereview_url = changelist.GetCodereviewServer()
-  if ret_code:
+  assert codereview_url, 'CL must be uploaded first'
-    DieWithError('"bb auth-login" must be executed before scheduling try jobs.')
+  patchset = patchset or changelist.GetMostRecentPatchset()
-
+  assert patchset, 'CL must be uploaded first'
-  gerrit_change = changelist.GetGerritChange()
+
-  shared_properties = {
+  codereview_host = urlparse.urlparse(codereview_url).hostname
-      'category': options.category,
+  # Cache the buildbucket credentials under the codereview host key, so that
-  }
+  # users can use different credentials for different buckets.
+  authenticator = auth.get_authenticator_for_host(codereview_host, auth_config)
+  http = authenticator.authorize(httplib2.Http())
+  http.force_exception_to_status_code = True
+
+  buildbucket_put_url = (
+      'https://{hostname}/_ah/api/buildbucket/v1/builds/batch'.format(
+          hostname=options.buildbucket_host))
+  buildset = 'patch/gerrit/{hostname}/{issue}/{patch}'.format(
+      hostname=codereview_host,
+      issue=changelist.GetIssue(),
+      patch=patchset)
+
+  shared_parameters_properties = changelist.GetTryJobProperties(patchset)
+  shared_parameters_properties['category'] = options.category
   if options.clobber:
-    shared_properties['clobber'] = True
+    shared_parameters_properties['clobber'] = True
   extra_properties = _get_properties_from_options(options)
   if extra_properties:
-    shared_properties.update(extra_properties)
+    shared_parameters_properties.update(extra_properties)
 
-  batch_request = {'requests': []}
+  batch_req_body = {'builds': []}
   print_text = []
   print_text.append('Tried jobs on:')
   for bucket, builders_and_tests in sorted(buckets.iteritems()):
     print_text.append('Bucket: %s' % bucket)
-    project, bucket = bucket.split('.', 1)
     for builder, tests in sorted(builders_and_tests.iteritems()):
       print_text.append('  %s: %s' % (builder, tests))
-      properties = shared_properties.copy()
+      parameters = {
+          'builder_name': builder,
+          'changes': [{
+              'author': {'email': changelist.GetIssueOwner()},
+              'revision': options.revision,
+          }],
+          'properties': shared_parameters_properties.copy(),
+      }
       if 'presubmit' in builder.lower():
-        properties['dry_run'] = 'true'
+        parameters['properties']['dry_run'] = 'true'
       if tests:
-        properties['testfilter'] = tests
+        parameters['properties']['testfilter'] = tests
 
       tags = [
-          {
+          'builder:%s' % builder,
-              'key': 'builder',
+          'buildset:%s' % buildset,
-              'value': builder,
+          'user_agent:git_cl_try',
-          },
-          {
-              'key': 'user_agent',
-              'value': 'git_cl_try',
-          },
       ]
 
-      batch_request['requests'].append(
+      batch_req_body['builds'].append(
           {
-              'scheduleBuild': {
+              'bucket': bucket,
-                  'builder': {
+              'parameters_json': json.dumps(parameters),
-                      'project': options.project or project,
+              'client_operation_id': str(uuid.uuid4()),
-                      'bucket': bucket,
+              'tags': tags,
-                      'builder': builder,
-                  },
-                  'properties': properties,
-                  'requestId': str(uuid.uuid4()),
-                  'gerritChanges': [gerrit_change],
-                  'tags': tags,
-              }
           }
       )
 
-  (stdout, _), ret_code = subprocess2.communicate(
+  _buildbucket_retry(
-      ['bb', 'batch', '-host', options.buildbucket_host],
+      'triggering tryjobs',
-      stdin=json.dumps(batch_request, sort_keys=True),
+      http,
-      stdout=subprocess2.PIPE)
+      buildbucket_put_url,
-  if ret_code:
+      'PUT',
-    print_text.append('Failed to schedule builds for some bots:')
+      body=json.dumps(batch_req_body),
-    if stdout:
+      headers={'Content-Type': 'application/json'}
-      responses = json.loads(stdout)
+  )
-    print_text.extend(
-        '  ' + response['error']['message']
-        for response in responses['responses'] if 'error' in response)
-
   print_text.append('To see results here, run:        git cl try-results')
   print_text.append('To see results in browser, run:  git cl web')
   print('\n'.join(print_text))
 
-  return ret_code
-
 
 def fetch_try_jobs(auth_config, changelist, buildbucket_host,
                    patchset=None):
@@ -2688,27 +2690,26 @@ class Changelist(object):
     if data['status'] in ('ABANDONED', 'MERGED'):
       return 'CL %s is closed' % self.GetIssue()
 
-  def GetGerritChange(self, patchset=None):
+  def GetTryJobProperties(self, patchset=None):
-    """Returns a buildbucket.v2.GerritChange message for the current issue."""
+    """Returns dictionary of properties to launch a tryjob."""
-    host = urlparse.urlparse(self.GetCodereviewServer()).hostname
-    issue = self.GetIssue()
-    patchset = int(patchset or self.GetPatchset())
     data = self._GetChangeDetail(['ALL_REVISIONS'])
-
+    patchset = int(patchset or self.GetPatchset())
-    assert host and issue and patchset, 'CL must be uploaded first'
+    assert patchset
-
+    revision_data = None  # Pylint wants it to be defined.
-    has_patchset = any(
+    for revision_data in data['revisions'].itervalues():
-        int(revision_data['_number']) == patchset
+      if int(revision_data['_number']) == patchset:
-        for revision_data in data['revisions'].itervalues())
+        break
-    if not has_patchset:
+    else:
       raise Exception('Patchset %d is not known in Gerrit change %d' %
                       (patchset, self.GetIssue()))
-
     return {
-        'host': host,
+      'patch_issue': self.GetIssue(),
-        'change': issue,
+      'patch_set': patchset or self.GetPatchset(),
-        'project': data['project'],
+      'patch_project': data['project'],
-        'patchset': patchset,
+      'patch_storage': 'gerrit',
+      'patch_ref': revision_data['fetch']['http']['ref'],
+      'patch_repository_url': revision_data['fetch']['http']['url'],
+      'patch_gerrit_url': self.GetCodereviewServer(),
     }
 
   def GetIssueOwner(self):
@@ -4739,7 +4740,12 @@ def CMDtry(parser, args):
       return 1
 
   patchset = cl.GetMostRecentPatchset()
-  return _trigger_try_jobs(auth_config, cl, buckets, options, patchset)
+  try:
+    _trigger_try_jobs(auth_config, cl, buckets, options, patchset)
+  except BuildbucketResponseException as ex:
+    print('ERROR: %s' % ex)
+    return 1
+  return 0
 
 
 @metrics.collector.collect_metrics('git cl try-results')

tests/git_cl_test.py

@@ -2381,42 +2381,6 @@ class TestGitCl(TestCase):
   def test_git_cl_try_buildbucket_with_properties_gerrit(self):
     self.mock(git_cl.Changelist, 'GetMostRecentPatchset', lambda _: 7)
     self.mock(git_cl.uuid, 'uuid4', lambda: 'uuid4')
-    def mocked_call_with_stdin(*a, **kw):
-      args = list(a)
-      if kw.get('stdin'):
-        args.append(kw['stdin'])
-      return ([self._mocked_call(*args), ''], 0)
-
-    self.mock(subprocess2, 'communicate', mocked_call_with_stdin)
-
-    bb_request = json.dumps({
-        "requests": [{
-            "scheduleBuild": {
-                "requestId": "uuid4",
-                "builder": {
-                    "project": "luci",
-                    "builder": "win",
-                    "bucket": "chromium.try",
-                },
-                "gerritChanges": [{
-                    "project": "depot_tools",
-                    "host": "chromium-review.googlesource.com",
-                    "patchset": 7,
-                    "change": 123456,
-                }],
-                "properties": {
-                    "category": "git_cl_try",
-                    "json": [{"a": 1}, None],
-                    "key": "val",
-                },
-                "tags": [
-                    {"value": "win", "key": "builder"},
-                    {"value": "git_cl_try", "key": "user_agent"},
-                ],
-            },
-        }],
-    }, sort_keys=True)
-
 
     self.calls = [
         ((['git', 'symbolic-ref', 'HEAD'],), 'feature'),
@@ -2446,12 +2410,46 @@ class TestGitCl(TestCase):
             },
           },
         }),
-        ((['bb', 'auth-info'],), ''),
-        ((['git', 'config', 'branch.feature.gerritpatchset'],), '7'),
-        ((['bb', 'batch', '-host', 'cr-buildbucket.appspot.com'],
-          bb_request,), ''),
     ]
 
+    def _buildbucket_retry(*_, **kw):
+      # self.maxDiff = 10000
+      body = json.loads(kw['body'])
+      self.assertEqual(len(body['builds']), 1)
+      build = body['builds'][0]
+      params = json.loads(build.pop('parameters_json'))
+      self.assertEqual(params, {
+        u'builder_name': u'win',
+        u'changes': [{u'author': {u'email': u'owner@e.mail'},
+                      u'revision': None}],
+        u'properties': {
+          u'category': u'git_cl_try',
+           u'key': u'val',
+           u'json': [{u'a': 1}, None],
+
+           u'patch_gerrit_url':
+             u'https://chromium-review.googlesource.com',
+           u'patch_issue': 123456,
+           u'patch_project': u'depot_tools',
+           u'patch_ref': u'refs/changes/56/123456/7',
+           u'patch_repository_url':
+             u'https://chromium.googlesource.com/depot_tools',
+           u'patch_set': 7,
+           u'patch_storage': u'gerrit',
+        }
+      })
+      self.assertEqual(build, {
+        u'bucket': u'luci.chromium.try',
+        u'client_operation_id': u'uuid4',
+        u'tags': [
+          u'builder:win',
+          u'buildset:patch/gerrit/chromium-review.googlesource.com/123456/7',
+          u'user_agent:git_cl_try',
+        ],
+      })
+
+    self.mock(git_cl, '_buildbucket_retry', _buildbucket_retry)
+
     self.mock(git_cl.sys, 'stdout', StringIO.StringIO())
     self.assertEqual(0, git_cl.main([
         'try', '-B', 'luci.chromium.try', '-b', 'win',