diff --git a/git_cl.py b/git_cl.py index e68951e00..51a57085f 100755 --- a/git_cl.py +++ b/git_cl.py @@ -1435,11 +1435,21 @@ class Changelist(object): ' was not specified. To enable ResultDB, please run the command' ' again with the --realm argument to specify the LUCI realm.') - return self._RunPresubmit(args, - description, - use_python3=True, - resultdb=resultdb, - realm=realm) + py3_results = self._RunPresubmit(args, + description, + use_python3=True, + resultdb=resultdb, + realm=realm) + if py3_results.get('skipped_presubmits', 1) == 0: + print('No more presubmits to run - skipping Python 2 presubmits.') + return py3_results + + py2_results = self._RunPresubmit(args, + description, + use_python3=False, + resultdb=resultdb, + realm=realm) + return self._MergePresubmitResults(py2_results, py3_results) def _RunPresubmit(self, args, diff --git a/presubmit_support.py b/presubmit_support.py index 43d815163..fe6e819aa 100755 --- a/presubmit_support.py +++ b/presubmit_support.py @@ -303,6 +303,33 @@ def prompt_should_continue(prompt_string): response = sys.stdin.readline().strip().lower() return response in ('y', 'yes') + +def _ShouldRunPresubmit(script_text, use_python3): + """Try to figure out whether these presubmit checks should be run under + python2 or python3. We need to do this without actually trying to + compile the text, since the text might compile in one but not the + other. + + Args: + script_text: The text of the presubmit script. + use_python3: if true, will use python3 instead of python2 by default + if USE_PYTHON3 is not specified. + + Return: + A boolean if presubmit should be executed + """ + if os.getenv('LUCI_OMIT_PYTHON2') == 'true': + # If LUCI omits python2, run all presubmits with python3, regardless of + # USE_PYTHON3 variable. + return True + + m = re.search('^USE_PYTHON3 = (True|False)$', script_text, flags=re.MULTILINE) + if m: + use_python3 = m.group(1) == 'True' + + return ((sys.version_info.major == 2) and not use_python3) or \ + ((sys.version_info.major == 3) and use_python3) + # Top level object so multiprocessing can pickle # Public access through OutputApi object. class _PresubmitResult(object): @@ -1423,9 +1450,14 @@ def DoPostUploadExecuter(change, gerrit_obj, verbose, use_python3=False): filename = os.path.abspath(filename) # Accept CRLF presubmit script. presubmit_script = gclient_utils.FileRead(filename).replace('\r\n', '\n') - if verbose: - sys.stdout.write('Running %s\n' % filename) - results.extend(executer.ExecPresubmitScript(presubmit_script, filename)) + if _ShouldRunPresubmit(presubmit_script, use_python3): + if sys.version_info[0] == 2: + sys.stdout.write( + 'Running %s under Python 2. Add USE_PYTHON3 = True to prevent ' + 'this.\n' % filename) + elif verbose: + sys.stdout.write('Running %s\n' % filename) + results.extend(executer.ExecPresubmitScript(presubmit_script, filename)) if not results: return 0 @@ -1700,18 +1732,29 @@ def DoPresubmitChecks(change, executer = PresubmitExecuter(change, committing, verbose, gerrit_obj, dry_run, thread_pool, parallel, use_python3, no_diffs) + skipped_count = 0; if default_presubmit: if verbose: sys.stdout.write('Running default presubmit script.\n') fake_path = os.path.join(change.RepositoryRoot(), 'PRESUBMIT.py') - results += executer.ExecPresubmitScript(default_presubmit, fake_path) + if _ShouldRunPresubmit(default_presubmit, use_python3): + results += executer.ExecPresubmitScript(default_presubmit, fake_path) + else: + skipped_count += 1 for filename in presubmit_files: filename = os.path.abspath(filename) # Accept CRLF presubmit script. presubmit_script = gclient_utils.FileRead(filename).replace('\r\n', '\n') - if verbose: - sys.stdout.write('Running %s\n' % filename) - results += executer.ExecPresubmitScript(presubmit_script, filename) + if _ShouldRunPresubmit(presubmit_script, use_python3): + if sys.version_info[0] == 2: + sys.stdout.write( + 'Running %s under Python 2. Add USE_PYTHON3 = True to prevent ' + 'this.\n' % filename) + elif verbose: + sys.stdout.write('Running %s\n' % filename) + results += executer.ExecPresubmitScript(presubmit_script, filename) + else: + skipped_count += 1 results += thread_pool.RunAsync() @@ -1782,6 +1825,7 @@ def DoPresubmitChecks(change, for warning in messages.get('Warnings', []) ], 'more_cc': executer.more_cc, + 'skipped_presubmits': skipped_count, } gclient_utils.FileWrite( diff --git a/tests/PRESUBMIT.py b/tests/PRESUBMIT.py new file mode 100644 index 000000000..5c0a3bd4b --- /dev/null +++ b/tests/PRESUBMIT.py @@ -0,0 +1,27 @@ +# Copyright (c) 2021 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. + +import sys + +PRESUBMIT_VERSION = '2.0.0' + +# This file can be removed once py2 presubmit is no longer supported. This is +# an integration test to ensure py2 presubmit still works. + + +def CheckPythonVersion(input_api, output_api): + # The tests here are assuming this is not defined, so raise an error + # if it is. + if 'USE_PYTHON3' in globals(): + return [ + output_api.PresubmitError( + 'USE_PYTHON3 is defined; update the tests in //PRESUBMIT.py and ' + '//tests/PRESUBMIT.py.') + ] + if sys.version_info.major != 2: + return [ + output_api.PresubmitError( + 'Did not use Python2 for //PRESUBMIT.py by default.') + ] + return [] diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index f23b52559..a2ce1054e 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -3690,6 +3690,25 @@ class ChangelistTest(unittest.TestCase): '--json_output', '/tmp/fake-temp2', '--description_file', '/tmp/fake-temp1', ]) + subprocess2.Popen.assert_any_call([ + 'vpython', 'PRESUBMIT_SUPPORT', + '--root', 'root', + '--upstream', 'upstream', + '--verbose', '--verbose', + '--gerrit_url', 'https://chromium-review.googlesource.com', + '--gerrit_project', 'project', + '--gerrit_branch', 'refs/heads/main', + '--author', 'author', + '--issue', '123456', + '--patchset', '7', + '--commit', + '--may_prompt', + '--parallel', + '--all_files', + '--no_diffs', + '--json_output', '/tmp/fake-temp4', + '--description_file', '/tmp/fake-temp3', + ]) gclient_utils.FileWrite.assert_any_call( '/tmp/fake-temp1', 'description') metrics.collector.add_repeated('sub_commands', { diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index eeea919b8..5a1ee9dfb 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -592,6 +592,11 @@ class PresubmitUnittest(PresubmitTestsBase): change=change, gerrit_obj=None, verbose=False)) expected = (r'Running Python ' + str(sys.version_info.major) + r' post upload checks \.\.\.\n') + if sys.version_info[0] == 2: + expected += ('Running .*PRESUBMIT.py under Python 2. Add USE_PYTHON3 = ' + 'True to prevent this.\n') + expected += ('Running .*PRESUBMIT.py under Python 2. Add USE_PYTHON3 = ' + 'True to prevent this.\n') self.assertRegexpMatches(sys.stdout.getvalue(), expected) def testDoPostUploadExecuterWarning(self): @@ -623,12 +628,17 @@ class PresubmitUnittest(PresubmitTestsBase): presubmit.DoPostUploadExecuter( change=change, gerrit_obj=None, verbose=False)) + extra = '' + if sys.version_info[0] == 2: + extra = ('Running .*PRESUBMIT.py under Python 2. Add USE_PYTHON3 = True ' + 'to prevent this.\n') expected = ('Running Python ' + str(sys.version_info.major) + ' ' 'post upload checks \.\.\.\n' + '%s' '\n' '\*\* Post Upload Hook Messages \*\*\n' '!!\n' - '\n') + '\n' % extra) self.assertRegexpMatches(sys.stdout.getvalue(), expected) def testDoPresubmitChecksNoWarningsOrErrors(self): @@ -724,6 +734,7 @@ def CheckChangeOnCommit(input_api, output_api): } ], 'more_cc': ['me@example.com'], + 'skipped_presubmits': 0, } fake_result_json = json.dumps(fake_result, sort_keys=True)