Allow setting arbitrary 'git cl try' properties with buildbucket.

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."""
 
 from distutils.version import LooseVersion
@@ skipping 196 matching lines @@
     else:
       git_set_branch_value('git-find-copies', int(options.find_copies))
 
     print('Using %d%% similarity for rename/copy detection. '
           'Override with --similarity.' % options.similarity)
 
     return options, args
   parser.parse_args = Parse
 
 
+def _get_properties_from_options(options):

[Michael Achenbach, 2015/09/14 14:18:23]

This is partially copied from
https://code.google.com/p/chromium/codesearch#chromium/build/scripts/tools/ru...
but using json instead of python.

+  properties = dict(x.split('=', 1) for x in options.properties)
+  for key, val in properties.iteritems():
+    try:
+      properties[key] = json.loads(val)
+    except ValueError:
+      pass  # If a value couldn't be evaluated, silently ignore it.

[tandrii(chromium), 2015/09/14 14:52:22]

how about:
s/, silently ignore it./, treat it as a string

[Michael Achenbach, 2015/09/15 08:32:57]

Done.

+  return properties
+
+
 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):
     return master
   return '%s%s' % (prefix, master)
 
 
-def trigger_try_jobs(auth_config, changelist, options, masters, category,
-                     override_properties=None):

[Michael Achenbach, 2015/09/14 14:18:23]

This was added by me a while ago in order to hook into the trigger_try_jobs
method from a client. It was never used and is obsolete with the new option.

[tandrii(chromium), 2015/09/14 14:52:22]

Acknowledged.

+def trigger_try_jobs(auth_config, changelist, options, masters, category):
   rietveld_url = settings.GetDefaultServerUrl()
   rietveld_host = urlparse.urlparse(rietveld_url).hostname
   authenticator = auth.get_authenticator_for_host(rietveld_host, auth_config)
   http = authenticator.authorize(httplib2.Http())
   http.force_exception_to_status_code = True
   issue_props = changelist.GetIssueProperties()
   issue = changelist.GetIssue()
   patchset = changelist.GetMostRecentPatchset()
+  properties = _get_properties_from_options(options)
 
   buildbucket_put_url = (
       'https://{hostname}/_ah/api/buildbucket/v1/builds/batch'.format(
           hostname=options.buildbucket_host))
   buildset = 'patch/rietveld/{hostname}/{issue}/{patch}'.format(
       hostname=rietveld_host,
       issue=issue,
       patch=patchset)
 
   batch_req_body = {'builds': []}
@@ skipping 15 matching lines @@
               'master': master,
               'patch_project': issue_props['project'],
               'patch_storage': 'rietveld',
               'patchset': patchset,
               'reason': options.name,
               'revision': options.revision,
               'rietveld': rietveld_url,
               'testfilter': tests,
           },
       }
-      if override_properties:
-        parameters['properties'].update(override_properties)
+      if properties:
+        parameters['properties'].update(properties)

[tandrii(chromium), 2015/09/14 14:52:22]

I'm not sure if overriding all properties is a good thing in terms on the scale
of tradeoffs between customizability and ease of debug. 

[Michael Achenbach, 2015/09/15 08:32:57]

I'd just like to resemble the buildbucket api, where you can set all these
properties to arbitrary values. Nobody needs to make use of this, but can.

       if options.clobber:
         parameters['properties']['clobber'] = True
       batch_req_body['builds'].append(
           {
               'bucket': bucket,
               'parameters_json': json.dumps(parameters),
               'tags': ['builder:%s' % builder,
                        'buildset:%s' % buildset,
                        'master:%s' % master,
                        'user_agent:git_cl_try']
@@ skipping 2778 matching lines @@
            "its waterfall for more info")
   group.add_option(
       "-c", "--clobber", action="store_true", default=False,
       help="Force a clobber before building; e.g. don't do an "
            "incremental build")
   group.add_option(
       "--project",
       help="Override which project to use. Projects are defined "
            "server-side to define what default bot set to use")
   group.add_option(
+      "-p", "--property", dest="properties", action="append", default=[],
+      help="Specify generic properties in the form -p key1=value1 -p "
+           "key2=value2 etc (buildbucket only). The value will treated as"

[tandrii(chromium), 2015/09/14 14:52:22]

nit: missing "be" : will BE treated

[Michael Achenbach, 2015/09/15 08:32:57]

Done.

+           "json if decodable, or as string otherwise.")
+  group.add_option(
       "-n", "--name", help="Try job name; default to current branch name")
   group.add_option(
       "--use-rietveld", action="store_true", default=False,
       help="Use Rietveld to trigger try jobs.")
   group.add_option(
       "--buildbucket-host", default='cr-buildbucket.appspot.com',
       help="Host of buildbucket. The default host is %default.")
   parser.add_option_group(group)
   auth.add_auth_options(parser)
   options, args = parser.parse_args(args)
   auth_config = auth.extract_auth_config_from_options(options)
 
+  if options.use_rietveld and options.properties:
+    parser.error('Properties can only be specified with buildbucket')
+
+  # Make sure that all properties are prop=value pairs.
+  bad_params = [x for x in options.properties if '=' not in x]
+  if bad_params:
+    parser.error('Got properties with missing "=": %s' % bad_params)
+
   if args:
     parser.error('Unknown arguments: %s' % args)
 
   cl = Changelist(auth_config=auth_config)
   if not cl.GetIssue():
     parser.error('Need to upload first')
 
   props = cl.GetIssueProperties()
   if props.get('closed'):
     parser.error('Cannot send tryjobs for a closed CL')
@@ skipping 460 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()
   colorama.init()
   try:
     sys.exit(main(sys.argv[1:]))
   except KeyboardInterrupt:
     sys.stderr.write('interrupted\n')
     sys.exit(1)