diff --git a/git_cl.py b/git_cl.py index 3878b965d..a809e68b8 100755 --- a/git_cl.py +++ b/git_cl.py @@ -1273,7 +1273,7 @@ class Changelist(object): return args def RunHook(self, committing, may_prompt, verbose, parallel, upstream, - description, all_files, resultdb=False): + description, all_files, resultdb=False, realm=None): """Calls sys.exit() if the hook fails; returns a HookResults otherwise.""" args = self._GetCommonPresubmitArgs(verbose, upstream) args.append('--commit' if committing else '--upload') @@ -1292,10 +1292,15 @@ class Changelist(object): args.extend(['--description_file', description_file]) start = time_time() - cmd = ['vpython', PRESUBMIT_SUPPORT] + args - if resultdb: - cmd = ['rdb', 'stream', '-new'] + cmd + if resultdb and realm: + cmd = ['rdb', 'stream', '-new', '-realm', realm, '--'] + cmd + elif resultdb: + # TODO (crbug.com/1113463): store realm somewhere and look it up so + # it is not required to pass the realm flag + print('Note: ResultDB reporting will NOT be performed because --realm' + ' was not specified. To enable ResultDB, please run the command' + ' again with the --realm argument to specify the LUCI realm.') p = subprocess2.Popen(cmd) exit_code = p.wait() @@ -1413,7 +1418,8 @@ class Changelist(object): upstream=base_branch, description=change_desc.description, all_files=False, - resultdb=options.resultdb) + resultdb=options.resultdb, + realm=options.realm) self.ExtendCC(hook_results['more_cc']) print_stats(git_diff_args) @@ -1864,7 +1870,7 @@ class Changelist(object): detail = self._GetChangeDetail(['LABELS']) return u'Commit-Queue' in detail.get('labels', {}) - def CMDLand(self, force, bypass_hooks, verbose, parallel): + def CMDLand(self, force, bypass_hooks, verbose, parallel, resultdb, realm): if git_common.is_dirty_git_tree('land'): return 1 @@ -1905,7 +1911,8 @@ class Changelist(object): upstream=upstream, description=description, all_files=False, - resultdb=False) + resultdb=resultdb, + realm=realm) self.SubmitIssue(wait_for_merge=True) print('Issue %s has been submitted.' % self.GetIssueURL()) @@ -3831,6 +3838,7 @@ def CMDpresubmit(parser, args): parser.add_option('--resultdb', action='store_true', help='Run presubmit checks in the ResultSink environment ' 'and send results to the ResultDB database.') + parser.add_option('--realm', help='LUCI realm if reporting to ResultDB') options, args = parser.parse_args(args) if not options.force and git_common.is_dirty_git_tree('presubmit'): @@ -3857,7 +3865,8 @@ def CMDpresubmit(parser, args): upstream=base_branch, description=description, all_files=options.all, - resultdb=options.resultdb) + resultdb=options.resultdb, + realm=options.realm) return 0 @@ -4071,6 +4080,7 @@ def CMDupload(parser, args): parser.add_option('--resultdb', action='store_true', help='Run presubmit checks in the ResultSink environment ' 'and send results to the ResultDB database.') + parser.add_option('--realm', help='LUCI realm if reporting to ResultDB') orig_args = args (options, args) = parser.parse_args(args) @@ -4217,6 +4227,10 @@ def CMDland(parser, args): parser.add_option('--parallel', action='store_true', help='Run all tests specified by input_api.RunTests in all ' 'PRESUBMIT files in parallel.') + parser.add_option('--resultdb', action='store_true', + help='Run presubmit checks in the ResultSink environment ' + 'and send results to the ResultDB database.') + parser.add_option('--realm', help='LUCI realm if reporting to ResultDB') (options, args) = parser.parse_args(args) cl = Changelist() @@ -4225,8 +4239,8 @@ def CMDland(parser, args): DieWithError('You must upload the change first to Gerrit.\n' ' If you would rather have `git cl land` upload ' 'automatically for you, see http://crbug.com/642759') - return cl.CMDLand(options.force, options.bypass_hooks, - options.verbose, options.parallel) + return cl.CMDLand(options.force, options.bypass_hooks, options.verbose, + options.parallel, options.resultdb, options.realm) @subcommand.usage('') diff --git a/presubmit_support.py b/presubmit_support.py index cd2eb7b08..e44b739e9 100755 --- a/presubmit_support.py +++ b/presubmit_support.py @@ -1496,7 +1496,6 @@ def DoPostUploadExecuter(change, return exit_code - class PresubmitExecuter(object): def __init__(self, change, committing, verbose, gerrit_obj, dry_run=None, thread_pool=None, parallel=False): @@ -1529,6 +1528,7 @@ class PresubmitExecuter(object): Return: A list of result objects, empty if no problems. """ + # Change to the presubmit file's directory to support local imports. main_path = os.getcwd() presubmit_dir = os.path.dirname(presubmit_path) @@ -1541,47 +1541,81 @@ class PresubmitExecuter(object): parallel=self.parallel) output_api = OutputApi(self.committing) context = {} + + PRESUBMIT_VERSION = [0] + def REQUIRE_PRESUBMIT_VERSION(num): + PRESUBMIT_VERSION[0] = num + + context['REQUIRE_PRESUBMIT_VERSION'] = REQUIRE_PRESUBMIT_VERSION try: exec(compile(script_text, 'PRESUBMIT.py', 'exec', dont_inherit=True), context) except Exception as e: raise PresubmitFailure('"%s" had an exception.\n%s' % (presubmit_path, e)) - # These function names must change if we make substantial changes to - # the presubmit API that are not backwards compatible. - if self.committing: - function_name = 'CheckChangeOnCommit' - else: - function_name = 'CheckChangeOnUpload' - if function_name in context: - try: - context['__args'] = (input_api, output_api) - logging.debug('Running %s in %s', function_name, presubmit_path) - - # TODO (crbug.com/1106943): Dive into each of the individual checks - - # Get path of presubmit directory relative to repository root. - # Always use forward slashes, so that path is same in *nix and Windows - root = input_api.change.RepositoryRoot() - rel_path = os.path.relpath(presubmit_dir, root) - rel_path = rel_path.replace(os.path.sep, '/') - - with rdb_wrapper.setup_rdb(function_name, rel_path) as my_status: - result = eval(function_name + '(*__args)', context) - self._check_result_type(result) - if any(res.fatal for res in result): - my_status.status = rdb_wrapper.STATUS_FAIL - logging.debug('Running %s done.', function_name) - self.more_cc.extend(output_api.more_cc) - finally: - for f in input_api._named_temporary_files: - os.remove(f) - else: - result = () # no error since the script doesn't care about current event. + context['__args'] = (input_api, output_api) + + # Get path of presubmit directory relative to repository root + # Always use forward slashes, so that path is same in *nix and Windows + root = input_api.change.RepositoryRoot() + rel_path = os.path.relpath(presubmit_dir, root) + rel_path = rel_path.replace(os.path.sep, '/') + + # Perform all the desired presubmit checks. + results = [] + + try: + if PRESUBMIT_VERSION[0] > 0: + for function_name in context: + if not function_name.startswith('Check'): + continue + if function_name.endswith('Commit') and not self.committing: + continue + if function_name.endswith('Upload') and self.committing: + continue + logging.debug('Running %s in %s', function_name, presubmit_path) + results.extend( + self._run_check_function(function_name, context, rel_path)) + logging.debug('Running %s done.', function_name) + self.more_cc.extend(output_api.more_cc) + + else: # Old format + if self.committing: + function_name = 'CheckChangeOnCommit' + else: + function_name = 'CheckChangeOnUpload' + if function_name in context: + logging.debug('Running %s in %s', function_name, presubmit_path) + results.extend( + self._run_check_function(function_name, context, rel_path)) + logging.debug('Running %s done.', function_name) + self.more_cc.extend(output_api.more_cc) + + finally: + for f in input_api._named_temporary_files: + os.remove(f) # Return the process to the original working directory. os.chdir(main_path) - return result + return results + + def _run_check_function(self, function_name, context, prefix): + """Evaluates a presubmit check function, function_name, in the context + provided. If LUCI_CONTEXT is enabled, it will send the result to ResultSink. + Passes function_name and prefix to rdb_wrapper.setup_rdb. Returns results. + + Args: + function_name: a string representing the name of the function to run + context: a context dictionary in which the function will be evaluated + prefix: a string describing prefix for ResultDB test id + + Returns: Results from evaluating the function call.""" + with rdb_wrapper.setup_rdb(function_name, prefix) as my_status: + result = eval(function_name + '(*__args)', context) + self._check_result_type(result) + if any(res.fatal for res in result): + my_status.status = rdb_wrapper.STATUS_FAIL + return result def _check_result_type(self, result): """Helper function which ensures result is a list, and all elements are diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index 7bcd64091..ff588e7df 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -2152,7 +2152,9 @@ class TestGitCl(unittest.TestCase): self.assertEqual(0, cl.CMDLand(force=True, bypass_hooks=True, verbose=True, - parallel=False)) + parallel=False, + resultdb=False, + realm=None)) self.assertIn( 'Issue chromium-review.googlesource.com/123 has been submitted', sys.stdout.getvalue()) @@ -2794,11 +2796,12 @@ class ChangelistTest(unittest.TestCase): upstream='upstream', description='description', all_files=False, - resultdb=True) + resultdb=True, + realm='chromium:public') self.assertEqual(expected_results, results) subprocess2.Popen.assert_called_once_with([ - 'rdb', 'stream', '-new', + 'rdb', 'stream', '-new', '-realm', 'chromium:public', '--', 'vpython', 'PRESUBMIT_SUPPORT', '--root', 'root', '--upstream', 'upstream', @@ -2937,7 +2940,8 @@ class CMDPresubmitTestCase(CMDTestCaseBase): upstream='upstream', description='fetch description', all_files=None, - resultdb=None) + resultdb=None, + realm=None) def testNoIssue(self): git_cl.Changelist.GetIssue.return_value = None @@ -2950,7 +2954,8 @@ class CMDPresubmitTestCase(CMDTestCaseBase): upstream='upstream', description='get description', all_files=None, - resultdb=None) + resultdb=None, + realm=None) def testCustomBranch(self): self.assertEqual(0, git_cl.main(['presubmit', 'custom_branch'])) @@ -2962,12 +2967,13 @@ class CMDPresubmitTestCase(CMDTestCaseBase): upstream='custom_branch', description='fetch description', all_files=None, - resultdb=None) + resultdb=None, + realm=None) def testOptions(self): self.assertEqual( 0, git_cl.main(['presubmit', '-v', '-v', '--all', '--parallel', '-u', - '--resultdb'])) + '--resultdb', '--realm', 'chromium:public'])) git_cl.Changelist.RunHook.assert_called_once_with( committing=False, may_prompt=False, @@ -2976,7 +2982,8 @@ class CMDPresubmitTestCase(CMDTestCaseBase): upstream='upstream', description='fetch description', all_files=True, - resultdb=True) + resultdb=True, + realm='chromium:public') class CMDTryResultsTestCase(CMDTestCaseBase): _DEFAULT_REQUEST = { diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index e21c957a6..d04e6e0f9 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -555,7 +555,7 @@ class PresubmitUnittest(PresubmitTestsBase): executer = presubmit.PresubmitExecuter( self.fake_change, False, None, False) - self.assertEqual((), executer.ExecPresubmitScript( + self.assertEqual([], executer.ExecPresubmitScript( ('def CheckChangeOnUpload(input_api, output_api):\n' ' if len(input_api._named_temporary_files):\n' ' return (output_api.PresubmitError("!!"),)\n'