From 8454fc2458b2421e0e339714c6ff7e6fffb70dc4 Mon Sep 17 00:00:00 2001 From: Bruce Dawson Date: Tue, 7 Mar 2023 22:15:25 +0000 Subject: [PATCH] Remove Python 2 support for PRESUBMIT.py The presubmit system still supports invoking PRESUBMIT.py files using Python 2. This has recently been turned off on the bots so this change removes support more completely. There are still some python3 parameters being passed around - it seemed better to do the simplest possible removal now, with a follow-up change to remove more support code after this has sat for a while. Tests run from PRESUBMIT.py files could still be run using Python 2, but those should also have been addressed already. Removing support for that will be done in a subsequent change. Bug: 1207012 Change-Id: Id244d547a04438f83734dba269c3cc180c148b37 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4315183 Reviewed-by: Aravind Vasudevan Commit-Queue: Bruce Dawson --- git_cl.py | 20 ++++--------- presubmit_support.py | 58 +++++-------------------------------- tests/PRESUBMIT.py | 27 ----------------- tests/git_cl_test.py | 19 ------------ tests/presubmit_unittest.py | 13 +-------- 5 files changed, 13 insertions(+), 124 deletions(-) delete mode 100644 tests/PRESUBMIT.py diff --git a/git_cl.py b/git_cl.py index 51a57085f..e68951e00 100755 --- a/git_cl.py +++ b/git_cl.py @@ -1435,21 +1435,11 @@ class Changelist(object): ' was not specified. To enable ResultDB, please run the command' ' again with the --realm argument to specify the LUCI 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) + return self._RunPresubmit(args, + description, + use_python3=True, + resultdb=resultdb, + realm=realm) def _RunPresubmit(self, args, diff --git a/presubmit_support.py b/presubmit_support.py index fe6e819aa..43d815163 100755 --- a/presubmit_support.py +++ b/presubmit_support.py @@ -303,33 +303,6 @@ 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): @@ -1450,14 +1423,9 @@ 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 _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 verbose: + sys.stdout.write('Running %s\n' % filename) + results.extend(executer.ExecPresubmitScript(presubmit_script, filename)) if not results: return 0 @@ -1732,29 +1700,18 @@ 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') - if _ShouldRunPresubmit(default_presubmit, use_python3): - results += executer.ExecPresubmitScript(default_presubmit, fake_path) - else: - skipped_count += 1 + results += executer.ExecPresubmitScript(default_presubmit, fake_path) 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 _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 + if verbose: + sys.stdout.write('Running %s\n' % filename) + results += executer.ExecPresubmitScript(presubmit_script, filename) results += thread_pool.RunAsync() @@ -1825,7 +1782,6 @@ 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 deleted file mode 100644 index 5c0a3bd4b..000000000 --- a/tests/PRESUBMIT.py +++ /dev/null @@ -1,27 +0,0 @@ -# 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 a2ce1054e..f23b52559 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -3690,25 +3690,6 @@ 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 5a1ee9dfb..eeea919b8 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -592,11 +592,6 @@ 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): @@ -628,17 +623,12 @@ 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' % extra) + '\n') self.assertRegexpMatches(sys.stdout.getvalue(), expected) def testDoPresubmitChecksNoWarningsOrErrors(self): @@ -734,7 +724,6 @@ def CheckChangeOnCommit(input_api, output_api): } ], 'more_cc': ['me@example.com'], - 'skipped_presubmits': 0, } fake_result_json = json.dumps(fake_result, sort_keys=True)