diff --git a/git_cl.py b/git_cl.py index 51a57085f..dae123dec 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, @@ -1489,19 +1479,6 @@ class Changelist(object): json_results = gclient_utils.FileRead(json_output) return json.loads(json_results) - def _MergePresubmitResults(self, py2_results, py3_results): - return { - 'more_cc': sorted(set(py2_results.get('more_cc', []) + - py3_results.get('more_cc', []))), - 'errors': ( - py2_results.get('errors', []) + py3_results.get('errors', [])), - 'notifications': ( - py2_results.get('notifications', []) + - py3_results.get('notifications', [])), - 'warnings': ( - py2_results.get('warnings', []) + py3_results.get('warnings', [])) - } - def RunPostUploadHook(self, verbose, upstream, description, py3_only): args = self._GetCommonPresubmitArgs(verbose, upstream) args.append('--post_upload') @@ -1509,13 +1486,8 @@ class Changelist(object): with gclient_utils.temporary_file() as description_file: gclient_utils.FileWrite(description_file, description) args.extend(['--description_file', description_file]) - if not py3_only: - p_py2 = subprocess2.Popen(['vpython', PRESUBMIT_SUPPORT] + args) - p_py3 = subprocess2.Popen(['vpython3', PRESUBMIT_SUPPORT] + args + - ['--use-python3']) - if not py3_only: - p_py2.wait() - p_py3.wait() + subprocess2.Popen(['vpython3', PRESUBMIT_SUPPORT] + args + + ['--use-python3']).wait() def _GetDescriptionForUpload(self, options, git_diff_args, files): # type: (optparse.Values, Sequence[str], Sequence[str] 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..b01d05b95 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', { @@ -3880,31 +3861,6 @@ class ChangelistTest(unittest.TestCase): cl = git_cl.Changelist() cl.RunPostUploadHook(2, 'upstream', 'description', False) - 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', - '--post_upload', - '--description_file', - '/tmp/fake-temp1', - ]) subprocess2.Popen.assert_called_with([ 'vpython3', 'PRESUBMIT_SUPPORT', diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index 5a1ee9dfb..2583a5070 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -164,10 +164,7 @@ index fe3de7b..54ae6e1 100755 # limit set. self.maxDiff = None - # TODO: remove once py2 no longer supported - self.presubmit_text_prefix = ('USE_PYTHON3 = ' + - str(sys.version_info.major == 3) + '\n') - self.presubmit_text = self.presubmit_text_prefix + self.presubmit_text + self.presubmit_text = self.presubmit_text class FakeChange(object): def __init__(self, obj): @@ -443,7 +440,6 @@ class PresubmitUnittest(PresubmitTestsBase): # No error if no on-upload entry point self.assertFalse( executer.ExecPresubmitScript( - self.presubmit_text_prefix + ('def CheckChangeOnCommit(input_api, output_api):\n' ' return (output_api.PresubmitError("!!"))\n'), fake_presubmit)) @@ -452,13 +448,11 @@ class PresubmitUnittest(PresubmitTestsBase): # No error if no on-commit entry point self.assertFalse( executer.ExecPresubmitScript( - self.presubmit_text_prefix + ('def CheckChangeOnUpload(input_api, output_api):\n' ' return (output_api.PresubmitError("!!"))\n'), fake_presubmit)) self.assertFalse( executer.ExecPresubmitScript( - self.presubmit_text_prefix + ('def CheckChangeOnUpload(input_api, output_api):\n' ' if not input_api.change.BugsFromDescription():\n' ' return (output_api.PresubmitError("!!"))\n' @@ -467,7 +461,6 @@ class PresubmitUnittest(PresubmitTestsBase): self.assertFalse( executer.ExecPresubmitScript( - self.presubmit_text_prefix + 'def CheckChangeOnCommit(input_api, output_api):\n' ' results = []\n' ' results.extend(input_api.canned_checks.CheckChangeHasBugField(\n' @@ -492,7 +485,6 @@ class PresubmitUnittest(PresubmitTestsBase): # STATUS_PASS on success executer.ExecPresubmitScript( - self.presubmit_text_prefix + 'def CheckChangeOnCommit(input_api, output_api):\n' ' return [output_api.PresubmitResult("test")]\n', fake_presubmit) sink.report.assert_called_with('CheckChangeOnCommit', @@ -501,7 +493,6 @@ class PresubmitUnittest(PresubmitTestsBase): # STATUS_FAIL on fatal error sink.reset_mock() executer.ExecPresubmitScript( - self.presubmit_text_prefix + 'def CheckChangeOnCommit(input_api, output_api):\n' ' return [output_api.PresubmitError("error")]\n', fake_presubmit) sink.report.assert_called_with('CheckChangeOnCommit', @@ -519,14 +510,12 @@ class PresubmitUnittest(PresubmitTestsBase): self.assertEqual([], executer.ExecPresubmitScript( - self.presubmit_text_prefix + ('def CheckChangeOnUpload(input_api, output_api):\n' ' if len(input_api._named_temporary_files):\n' ' return (output_api.PresubmitError("!!"),)\n' ' return ()\n'), fake_presubmit)) result = executer.ExecPresubmitScript( - self.presubmit_text_prefix + ('def CheckChangeOnUpload(input_api, output_api):\n' ' with input_api.CreateTemporaryFile():\n' ' pass\n' @@ -552,7 +541,7 @@ class PresubmitUnittest(PresubmitTestsBase): executer = presubmit.PresubmitExecuter(change, False, None, presubmit.GerritAccessor()) - executer.ExecPresubmitScript(self.presubmit_text_prefix, fake_presubmit) + executer.ExecPresubmitScript('', fake_presubmit) # Check that the executer switched to the directory of the script and back. self.assertEqual(os.chdir.call_args_list, [ @@ -592,11 +581,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 +612,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): @@ -677,8 +656,7 @@ class PresubmitUnittest(PresubmitTestsBase): fake_notify = 'This is a dry run' fake_notify_items = '["N"]' fake_notify_long_text = 'Notification long text...' - always_fail_presubmit_script = ('USE_PYTHON3 = ' + - str(sys.version_info.major == 3) + """\n + always_fail_presubmit_script = ("""\n def CheckChangeOnUpload(input_api, output_api): output_api.more_cc = ['me@example.com'] return [ @@ -734,7 +712,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) @@ -834,8 +811,7 @@ def CheckChangeOnCommit(input_api, output_api): self.assertEqual(sys.stdout.getvalue().count(RUNNING_PY_CHECKS_TEXT), 1) def testDoDefaultPresubmitChecksAndFeedback(self): - always_fail_presubmit_script = ('USE_PYTHON3 = ' + - str(sys.version_info.major == 3) + """\n + always_fail_presubmit_script = ("""\n def CheckChangeOnUpload(input_api, output_api): return [output_api.PresubmitError("!!")] def CheckChangeOnCommit(input_api, output_api): @@ -912,7 +888,6 @@ def CheckChangeOnCommit(input_api, output_api): os.path.isfile.side_effect = lambda f: 'PRESUBMIT.py' in f os.listdir.return_value = ['PRESUBMIT.py'] gclient_utils.FileRead.return_value = ( - 'USE_PYTHON3 = ' + str(sys.version_info.major == 3) + '\n' 'def PostUploadHook(gerrit, change, output_api):\n' ' return ()\n') scm.determine_scm.return_value = None @@ -937,7 +912,6 @@ def CheckChangeOnCommit(input_api, output_api): @mock.patch('lib.utils.ListRelevantFilesInSourceCheckout') def testMainUnversionedChecksFail(self, *_mocks): gclient_utils.FileRead.return_value = ( - 'USE_PYTHON3 = ' + str(sys.version_info.major == 3) + '\n' 'def CheckChangeOnUpload(input_api, output_api):\n' ' return [output_api.PresubmitError("!!")]\n') scm.determine_scm.return_value = None