diff --git a/git_cl.py b/git_cl.py index 18dc96340..c78207bdc 100755 --- a/git_cl.py +++ b/git_cl.py @@ -75,6 +75,9 @@ REFS_THAT_ALIAS_TO_OTHER_REFS = { 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 @@ -281,10 +284,22 @@ def _prefix_master(master): (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): @@ -324,7 +339,7 @@ def _buildbucket_retry(operation_name, http, *args, **kwargs): 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() @@ -356,9 +371,11 @@ def _trigger_try_jobs(auth_config, changelist, masters, 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 = None + if bucket.startswith(MASTER_PREFIX): + master = _unprefix_master(bucket) for builder, tests in sorted(builders_and_tests.iteritems()): print_text.append(' %s: %s' % (builder, tests)) parameters = { @@ -370,7 +387,6 @@ def _trigger_try_jobs(auth_config, changelist, masters, options, 'properties': { 'category': category, 'issue': changelist.GetIssue(), - 'master': master, 'patch_project': project, 'patch_storage': 'rietveld', 'patchset': patchset, @@ -386,15 +402,22 @@ def _trigger_try_jobs(auth_config, changelist, masters, options, parameters['properties'].update(extra_properties) if options.clobber: parameters['properties']['clobber'] = True + + tags = [ + 'builder:%s' % builder, + 'buildset:%s' % buildset, + 'user_agent:git_cl_try', + ] + if master: + parameters['properties']['master'] = master + tags.append('master:%s' % master) + batch_req_body['builds'].append( { 'bucket': bucket, 'parameters_json': json.dumps(parameters), 'client_operation_id': str(uuid.uuid4()), - 'tags': ['builder:%s' % builder, - 'buildset:%s' % buildset, - 'master:%s' % master, - 'user_agent:git_cl_try'] + 'tags': tags, } ) @@ -4718,10 +4741,12 @@ def CMDtry(parser, args): '"-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 ' @@ -4779,7 +4804,10 @@ def CMDtry(parser, args): 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' @@ -4836,33 +4864,40 @@ def CMDtry(parser, args): # 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} - - 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 - - for builders in masters.itervalues(): + bucket = '' + 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} + + if options.bucket: + buckets = {options.bucket: {b: [] for b in options.bot}} + 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 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). ' @@ -4879,8 +4914,8 @@ def CMDtry(parser, args): '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 diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index 8ef92d301..4813edb9d 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -1941,8 +1941,8 @@ class TestGitCl(TestCase): u'client_operation_id': u'uuid4', u'tags': [u'builder:win', u'buildset:patch/rietveld/codereview.chromium.org/123/20001', - u'master:tryserver.chromium', - u'user_agent:git_cl_try'], + u'user_agent:git_cl_try', + u'master:tryserver.chromium'], }) self.mock(git_cl, '_buildbucket_retry', _buildbucket_retry) @@ -1953,7 +1953,59 @@ class TestGitCl(TestCase): '-p', 'key=val', '-p', 'json=[{"a":1}, null]'])) self.assertRegexpMatches( git_cl.sys.stdout.getvalue(), - 'Tried jobs on:\nMaster: tryserver.chromium') + 'Tried jobs on:\nBucket: master.tryserver.chromium') + + def test_git_cl_try_buildbucket_bucket_flag(self): + self.mock(git_cl.Changelist, 'GetMostRecentPatchset', lambda _: 20001) + self.mock(git_cl.Changelist, 'GetIssueOwner', lambda _: 'owner@e.mail') + self.mock(git_cl.Changelist, 'GetIssueProject', lambda _: 'depot_tools') + self.mock(git_cl.uuid, 'uuid4', lambda: 'uuid4') + self.calls = [ + ((['git', 'symbolic-ref', 'HEAD'],), 'feature'), + ((['git', 'config', 'branch.feature.rietveldissue'],), '123'), + ((['git', 'config', 'rietveld.autoupdate'],), CERR1), + ((['git', 'config', 'rietveld.server'],), + 'https://codereview.chromium.org'), + ((['git', 'config', 'branch.feature.rietveldserver'],), CERR1), + ((['git', 'config', 'branch.feature.rietveldpatchset'],), '20001'), + ] + + 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'issue': 123, + u'patch_project': u'depot_tools', + u'patch_storage': u'rietveld', + u'patchset': 20001, + u'reason': u'feature', # This is a branch name, but why? + u'rietveld': u'https://codereview.chromium.org', + } + }) + self.assertEqual(build, { + u'bucket': u'test.bucket', + u'client_operation_id': u'uuid4', + u'tags': [u'builder:win', + u'buildset:patch/rietveld/codereview.chromium.org/123/20001', + 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', 'test.bucket', '-b', 'win'])) + self.assertRegexpMatches( + git_cl.sys.stdout.getvalue(), + 'Tried jobs on:\nBucket: test.bucket') def _common_GerritCommitMsgHookCheck(self): self.mock(git_cl.sys, 'stdout', StringIO.StringIO())