Revert "Remove Python 2 support for PRESUBMIT.py"

This reverts commit 8454fc2458.

Reason for revert: post submit hooks failing
https://crbug.com/1422416

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
Bug: 1422416
Change-Id: Iaf3102e63ec3c698d0258fac5746dbd92c30edbb
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4317176
Auto-Submit: Josip Sokcevic <sokcevic@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
changes/76/4317176/2
Josip Sokcevic 2 years ago committed by LUCI CQ
parent 8454fc2458
commit d4c6cbeb61

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

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

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

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

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

Loading…
Cancel
Save