Add -B/--bucket flag to git-cl try

git_cl.py

 #!/usr/bin/env python
 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 # Copyright (C) 2008 Evan Martin <martine@danga.com>
 
 """A git-command for integrating reviews on Rietveld and Gerrit."""
 
 from __future__ import print_function
@@ skipping 58 matching lines @@
 GIT_INSTRUCTIONS_URL = 'http://code.google.com/p/chromium/wiki/UsingGit'
 REFS_THAT_ALIAS_TO_OTHER_REFS = {
     'refs/remotes/origin/lkgr': 'refs/remotes/origin/master',
     'refs/remotes/origin/lkcr': 'refs/remotes/origin/master',
 }
 
 # Valid extensions for files we want to lint.
 DEFAULT_LINT_REGEX = r"(.*\.cpp|.*\.cc|.*\.h)"
 DEFAULT_LINT_IGNORE_REGEX = r"$^"
 
+# Buildbucket master name prefix.
+MASTER_PREFIX = 'master.'
+
 # Shortcut since it quickly becomes redundant.
 Fore = colorama.Fore
 
 # Initialized in main()
 settings = None
 
 
 def DieWithError(message):
   print(message, file=sys.stderr)
   sys.exit(1)
@@ skipping 179 matching lines @@
 
 
 def _prefix_master(master):
   """Convert user-specified master name to full master name.
 
   Buildbucket uses full master name(master.tryserver.chromium.linux) as bucket
   name, while the developers always use shortened master name
   (tryserver.chromium.linux) by stripping off the prefix 'master.'. This
   function does the conversion for buildbucket migration.
   """
-  prefix = 'master.'
-  if master.startswith(prefix):
+  if master.startswith(MASTER_PREFIX):
     return master
-  return '%s%s' % (prefix, master)
+  return '%s%s' % (MASTER_PREFIX, master)
+
+
+def _unprefix_master(bucket):
+  """Convert bucket name to shortened master name.
+
+  Buildbucket uses full master name(master.tryserver.chromium.linux) as bucket
+  name, while the developers always use shortened master name
+  (tryserver.chromium.linux) by stripping off the prefix 'master.'. This
+  function does the conversion for buildbucket migration.
+  """
+  if bucket.startswith(MASTER_PREFIX):
+    return bucket[len(MASTER_PREFIX):]
+  return bucket
 
 
 def _buildbucket_retry(operation_name, http, *args, **kwargs):
   """Retries requests to buildbucket service and returns parsed json content."""
   try_count = 0
   while True:
     response, content = http.request(*args, **kwargs)
     try:
       content_json = json.loads(content)
     except ValueError:
@@ skipping 19 matching lines @@
     if response.status < 500 or try_count >= 2:
       raise httplib2.HttpLib2Error(content)
 
     # status >= 500 means transient failures.
     logging.debug('Transient errors when %s. Will retry.', operation_name)
     time.sleep(0.5 + 1.5*try_count)
     try_count += 1
   assert False, 'unreachable'
 
 
-def _trigger_try_jobs(auth_config, changelist, masters, options,
+def _trigger_try_jobs(auth_config, changelist, buckets, options,
                       category='git_cl_try', patchset=None):
   assert changelist.GetIssue(), 'CL must be uploaded first'
   codereview_url = changelist.GetCodereviewServer()
   assert codereview_url, 'CL must be uploaded first'
   patchset = patchset or changelist.GetMostRecentPatchset()
   assert patchset, 'CL must be uploaded first'
 
   codereview_host = urlparse.urlparse(codereview_url).hostname
   authenticator = auth.get_authenticator_for_host(codereview_host, auth_config)
   http = authenticator.authorize(httplib2.Http())
@@ skipping 11 matching lines @@
   buildset = 'patch/{codereview}/{hostname}/{issue}/{patch}'.format(
       codereview='gerrit' if changelist.IsGerrit() else 'rietveld',
       hostname=codereview_host,
       issue=changelist.GetIssue(),
       patch=patchset)
   extra_properties = _get_properties_from_options(options)
 
   batch_req_body = {'builds': []}
   print_text = []
   print_text.append('Tried jobs on:')
-  for master, builders_and_tests in sorted(masters.iteritems()):
-    print_text.append('Master: %s' % master)
-    bucket = _prefix_master(master)
+  for bucket, builders_and_tests in sorted(buckets.iteritems()):
+    print_text.append('Bucket: %s' % bucket)
+    master = _unprefix_master(bucket)
     for builder, tests in sorted(builders_and_tests.iteritems()):
       print_text.append('  %s: %s' % (builder, tests))
       parameters = {
           'builder_name': builder,
           'changes': [{
               'author': {'email': owner_email},
               'revision': options.revision,
           }],
           'properties': {
               'category': category,
@@ skipping 4332 matching lines @@
   """
   group = optparse.OptionGroup(parser, 'Try job options')
   group.add_option(
       '-b', '--bot', action='append',
       help=('IMPORTANT: specify ONE builder per --bot flag. Use it multiple '
             'times to specify multiple builders. ex: '
             '"-b win_rel -b win_layout". See '
             'the try server waterfall for the builders name and the tests '
             'available.'))
   group.add_option(
+      '-B', '--bucket', default='',
+      help=('Buildbucket bucket to send the try requests.'))
+  group.add_option(
       '-m', '--master', default='',
       help=('Specify a try master where to run the tries.'))
-  # TODO(tandrii,nodir): add -B --bucket flag.
   group.add_option(
       '-r', '--revision',
       help='Revision to use for the try job; default: the revision will '
            'be determined by the try recipe that builder runs, which usually '
            'defaults to HEAD of origin/master')
   group.add_option(
       '-c', '--clobber', action='store_true', default=False,
       help='Force a clobber before building; that is don\'t do an '
            'incremental build')
   group.add_option(
@@ skipping 37 matching lines @@
         'If your project has Commit Queue, dry run is a workaround:\n'
         '  git cl set-commit --dry-run')
 
   error_message = cl.CannotTriggerTryJobReason()
   if error_message:
     parser.error('Can\'t trigger try jobs: %s')
 
   if not options.name:
     options.name = cl.GetBranch()
 
-  if options.bot and not options.master:
+  if options.bucket and options.master:
+    parser.error('Only one of --bucket and --master may be used.')
+
+  if options.bot and not options.master and not options.bucket:
     options.master, err_msg = GetBuilderMaster(options.bot)
     if err_msg:
       parser.error('Tryserver master cannot be found because: %s\n'
                    'Please manually specify the tryserver master'
                    ', e.g. "-m tryserver.chromium.linux".' % err_msg)
 
+  def GetBotsMap(bots):
+    builders_and_tests = {}
+
+    # TODO(machenbach): The old style command-line options don't support
+    # multiple try masters yet.
+    old_style = filter(lambda x: isinstance(x, basestring), bots)
+    new_style = filter(lambda x: isinstance(x, tuple), bots)
+
+    for bot in old_style:

[nodir, 2016/10/14 18:41:18]

let's not support old style stuff in new code paths

[borenet, 2016/10/14 18:46:42]

Unfortunately the old code path goes through here as well, so I don't think I
can remove it without changing behavior.

[nodir, 2016/10/14 18:54:16]

I think it is better copy-paste code that is needed for the new code path and
leave old code untouched. This way we will be able to delete old code later
without touching new code paths.

It is about 4 lines of copying and less branching

[borenet, 2016/10/17 14:25:00]

Done. It seems that when using "-b <bot>" we get the old_style (non-tuple) path.
 Is that correct?

+      if ':' in bot:
+        parser.error('Specifying testfilter is no longer supported')
+      elif ',' in bot:
+        parser.error('Specify one bot per --bot flag')
+      else:
+        builders_and_tests.setdefault(bot, [])
+
+    for bot, tests in new_style:
+      builders_and_tests.setdefault(bot, []).extend(tests)
+    return builders_and_tests
+
   def GetMasterMap():
     # Process --bot.
     if not options.bot:
       change = cl.GetChange(cl.GetCommonAncestorWithUpstream(), None)
 
       # Get try masters from PRESUBMIT.py files.
       masters = presubmit_support.DoGetTryMasters(
           change,
           change.LocalPaths(),
           settings.GetRoot(),
@@ skipping 10 matching lines @@
           change.LocalPaths(),
           settings.GetRoot(),
           None,
           None,
           options.verbose,
           sys.stdout)
 
     if not options.bot:
       return {}
 
-    builders_and_tests = {}
-    # TODO(machenbach): The old style command-line options don't support
-    # multiple try masters yet.
-    old_style = filter(lambda x: isinstance(x, basestring), options.bot)
-    new_style = filter(lambda x: isinstance(x, tuple), options.bot)
-
-    for bot in old_style:
-      if ':' in bot:
-        parser.error('Specifying testfilter is no longer supported')
-      elif ',' in bot:
-        parser.error('Specify one bot per --bot flag')
-      else:
-        builders_and_tests.setdefault(bot, [])
-
-    for bot, tests in new_style:
-      builders_and_tests.setdefault(bot, []).extend(tests)
+    builders_and_tests = GetBotsMap(options.bot)
 
     # Return a master map with one master to be backwards compatible. The
     # master name defaults to an empty string, which will cause the master
     # not to be set on rietveld (deprecated).
-    return {options.master: builders_and_tests}
+    bucket = ""

[nodir, 2016/10/14 18:41:17]

use ''

[borenet, 2016/10/14 18:46:42]

Done.

+    if options.master:
+      # Add the "master." prefix to the master name to obtain the bucket name.
+      bucket = _prefix_master(options.master)
+    return {bucket: builders_and_tests}
 
-  masters = GetMasterMap()
-  if not masters:
-    # Default to triggering Dry Run (see http://crbug.com/625697).
-    if options.verbose:
-      print('git cl try with no bots now defaults to CQ Dry Run.')
-    try:
-      cl.SetCQState(_CQState.DRY_RUN)
-      print('scheduled CQ Dry Run on %s' % cl.GetIssueURL())
-      return 0
-    except KeyboardInterrupt:
-      raise
-    except:
-      print('WARNING: failed to trigger CQ Dry Run.\n'
-            'Either:\n'
-            ' * your project has no CQ\n'
-            ' * you don\'t have permission to trigger Dry Run\n'
-            ' * bug in this code (see stack trace below).\n'
-            'Consider specifying which bots to trigger manually '
-            'or asking your project owners for permissions '
-            'or contacting Chrome Infrastructure team at '
-            'https://www.chromium.org/infra\n\n')
-      # Still raise exception so that stack trace is printed.
-      raise
+  if options.bucket:
+    buckets = {options.bucket: GetBotsMap(options.bot or [])}
+  else:
+    buckets = GetMasterMap()
+    if not buckets:
+      # Default to triggering Dry Run (see http://crbug.com/625697).
+      if options.verbose:
+        print('git cl try with no bots now defaults to CQ Dry Run.')
+      try:
+        cl.SetCQState(_CQState.DRY_RUN)
+        print('scheduled CQ Dry Run on %s' % cl.GetIssueURL())
+        return 0
+      except KeyboardInterrupt:
+        raise
+      except:
+        print('WARNING: failed to trigger CQ Dry Run.\n'
+              'Either:\n'
+              ' * your project has no CQ\n'
+              ' * you don\'t have permission to trigger Dry Run\n'
+              ' * bug in this code (see stack trace below).\n'
+              'Consider specifying which bots to trigger manually '
+              'or asking your project owners for permissions '
+              'or contacting Chrome Infrastructure team at '
+              'https://www.chromium.org/infra\n\n')
+        # Still raise exception so that stack trace is printed.
+        raise
 
-  for builders in masters.itervalues():
+  for builders in buckets.itervalues():
     if any('triggered' in b for b in builders):
       print('ERROR You are trying to send a job to a triggered bot. This type '
             'of bot requires an initial job from a parent (usually a builder). '
             'Instead send your job to the parent.\n'
             'Bot list: %s' % builders, file=sys.stderr)
       return 1
 
   patchset = cl.GetMostRecentPatchset()
   if patchset != cl.GetPatchset():
     print('Warning: Codereview server has newer patchsets (%s) than most '
           'recent upload from local checkout (%s). Did a previous upload '
           'fail?\n'
           'By default, git cl try uses the latest patchset from '
           'codereview, continuing to use patchset %s.\n' %
           (patchset, cl.GetPatchset(), patchset))
   try:
-    _trigger_try_jobs(auth_config, cl, masters, options, 'git_cl_try',
-                     patchset)
+    _trigger_try_jobs(auth_config, cl, buckets, options, 'git_cl_try',
+                      patchset)
   except BuildbucketResponseException as ex:
     print('ERROR: %s' % ex)
     return 1
   return 0
 
 
 def CMDtry_results(parser, args):
   """Prints info about try jobs associated with current CL."""
   group = optparse.OptionGroup(parser, 'Try job results options')
   group.add_option(
@@ skipping 485 matching lines @@
 if __name__ == '__main__':
   # These affect sys.stdout so do it outside of main() to simplify mocks in
   # unit testing.
   fix_encoding.fix_encoding()
   setup_color.init()
   try:
     sys.exit(main(sys.argv[1:]))
   except KeyboardInterrupt:
     sys.stderr.write('interrupted\n')
     sys.exit(1)