Commit f2b0421f authored by Bruce Dawson's avatar Bruce Dawson Committed by LUCI CQ

Report when vpython (Python 2) is run during presubmits

While all Chromium PRESUBMIT.py scripts have been running on Python 2
for a long time they continue to invoke child scripts under Python 2.
Part of the reason for slow progress on this transition is that it is
not easy to tell that this is happening, and most developers probably
assume that Python 3 presubmits implies a lack of Python 2.

This change adds a warning when it detects Python 2 scripts being run.
Typical output (edited for clarity) looks like this:

  git cl presubmit --files "chrome/updater/tools/*;ppapi/generators/*"
...
  Python 2 scripts were run during Python 3 presubmits. Please ask ??? if help is needed in preventing this.
   "depot_tools\pylint-1.5" --args-on-stdin from chrome\updater\tools \
   "depot_tools\pylint-1.5" --args-on-stdin from chrome\updater\tools \
   idl_tests.py from ppapi\generators

If Python 2 scripts launch child scripts, especially if they use
sys.executable, then they will not be reported. However this is a good
thing because it means that the report focuses on the top-level scripts
that drive Python 2 usage.

This change works by modifying vpython.bat to write invocation
information to a text file. The data in this text file is picked up by
presubmit_support.py when it finishes running a set of presubmits.

Bug: 1313804
Change-Id: Ic632b38eae07eca2e02e94358305cc9c998818e9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3699002Reviewed-by: 's avatarGavin Mak <gavinmak@google.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
parent a9467d82
......@@ -93,3 +93,6 @@ testing_support/google_appengine
# Ignore VsChromium configuration file.
/vs-chromium-project.txt
# Ignore the file that logs Python 2 scripts run during presubmits.
/python2_usage.txt
......@@ -1725,6 +1725,10 @@ def DoPresubmitChecks(change,
if not presubmit_files and verbose:
sys.stdout.write('Warning, no PRESUBMIT.py found.\n')
results = []
depot_tools = os.path.dirname(os.path.abspath(__file__))
python2_usage_log_file = os.path.join(depot_tools, 'python2_usage.txt')
if os.path.exists(python2_usage_log_file):
os.remove(python2_usage_log_file)
thread_pool = ThreadPool()
executer = PresubmitExecuter(change, committing, verbose, gerrit_obj,
dry_run, thread_pool, parallel, use_python3,
......@@ -1751,6 +1755,17 @@ def DoPresubmitChecks(change,
results += thread_pool.RunAsync()
if os.path.exists(python2_usage_log_file):
with open(python2_usage_log_file) as f:
python2_usage = [x.strip() for x in f.readlines()]
results.append(
OutputApi(committing).PresubmitPromptWarning(
'Python 2 scripts were run during %s presubmits. Please see '
'https://bugs.chromium.org/p/chromium/issues/detail?id=1313804'
'#c61 for tips on resolving this.'
% python_version,
items=python2_usage))
messages = {}
should_prompt = False
presubmits_failed = False
......
......@@ -41,6 +41,8 @@ base_dir=$(dirname "$0")
source "$base_dir/cipd_bin_setup.sh"
cipd_bin_setup &> /dev/null
echo $@ from $(pwd) >> "$base_dir/python2_usage.txt"
# If Python bootstrapping is not disabled, make sure Python has been
# bootstrapped and add it to the front of PATH.
if [[ $(uname -s) = MINGW* || $(uname -s) = CYGWIN* ]]; then
......
......@@ -6,4 +6,5 @@
:: See revert instructions in cipd_manifest.txt
call "%~dp0\cipd_bin_setup.bat" > nul 2>&1
echo %* from %cd% >> "%~dp0\python2_usage.txt"
"%~dp0\.cipd_bin\vpython.exe" -vpython-interpreter "%~dp0\python.bat" %*
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment