aiden
/
depot_tools
mirror of https://chromium.googlesource.com/chromium/tools/depot_tools
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Reland "Remove Python 2 support for PRESUBMIT.py"

Browse Source 
This is a reland of commit 8454fc2458

Original change's description:
> 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 <aravindvasudev@google.com>
> Commit-Queue: Bruce Dawson <brucedawson@chromium.org>

Bug: 1207012
Change-Id: If542cac21d8ec8704b28d03fd8407c5c2899ca2c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4317177
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
Commit-Queue: Aravind Vasudevan <aravindvasudev@google.com>
Auto-Submit: Josip Sokcevic <sokcevic@chromium.org>
changes/77/4317177/6
Josip Sokcevic 2 years ago committed by LUCI CQ
parent 902fa9a888
commit e9ece0f581
5 changed files with 19 additions and 188 deletions
Show Stats Download Patch File Download Diff File

42
git_cl.py
Unescape Escape View File

@ -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]

58
presubmit_support.py
Unescape Escape View File

@ -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(

27
tests/PRESUBMIT.py
Unescape Escape View File

@ -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 []

44
tests/git_cl_test.py
Unescape Escape View File

@ -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',

36
tests/presubmit_unittest.py
Unescape Escape View File

@ -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

Write Preview
Loading…
Cancel
Save