Commit 5025893c authored by Robert Iannucci's avatar Robert Iannucci Committed by Commit Bot

[presubmit_support] Prevent depot_tools' virtualenv from leaking into other repos.

Currently in LUCI `git cl` is invoked on chromium_presubmit using vpython. This
means that it operates under the depot_tools .vpython environment (which is
blank).

Some features in presubmit_support allow invocation of python subprocesses, and
previously they would use `sys.executable` (especially on windows, but
occasionally on non-windows as well).

In non-vpython environments, this just picks up the system python and whatever
modules happen to be installed there. Some python unittests work around this on
non-windows systems by having a shebang line which explicitly invokes vpython.

This CL changes presubmit_support so that invocations of 'python', or executions
of '.py' scripts will always end up invoking vpython. In the best case, this will
pick up the invoked scripts' vpython environment. In the worst case, this will
invoke the script under an empty vpython environment (aka "stock python").

R=dpranke@chromium.org, jbudorick@chromium.org, tandrii@chromium.org, tikuta@chromium.org

Bug: 821669
Change-Id: I5d2d5dfd0364022d56833c2c8af4983553a29c7a
Reviewed-on: https://chromium-review.googlesource.com/961865Reviewed-by: 's avatarDirk Pranke <dpranke@chromium.org>
Reviewed-by: 's avatarAndrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Robbie Iannucci <iannucci@chromium.org>
parent c621b21a
......@@ -528,11 +528,7 @@ def GetUnitTests(input_api, output_api, unit_tests, env=None):
results = []
for unit_test in unit_tests:
cmd = []
if input_api.platform == 'win32' and unit_test.endswith('.py'):
# Windows needs some help.
cmd = [input_api.python_executable]
cmd.append(unit_test)
cmd = [unit_test]
if input_api.verbose:
cmd.append('--verbose')
kwargs = {'cwd': input_api.PresubmitLocalPath()}
......
......@@ -438,8 +438,13 @@ class InputApi(object):
self.unittest = unittest
self.urllib2 = urllib2
# To easily fork python.
self.python_executable = sys.executable
self.is_windows = sys.platform == 'win32'
# Set python_executable to 'python'. This is interpreted in CallCommand to
# convert to vpython in order to allow scripts in other repos (e.g. src.git)
# to automatically pick up that repo's .vpython file, instead of inheriting
# the one in depot_tools.
self.python_executable = 'python'
self.environ = os.environ
# InputApi.platform is the platform you're currently running on.
......@@ -468,7 +473,6 @@ class InputApi(object):
fopen=file, os_path=self.os_path)
self.owners_finder = owners_finder.OwnersFinder
self.verbose = verbose
self.is_windows = sys.platform == 'win32'
self.Command = CommandData
# Replace <hash_map> and <hash_set> as headers that need to be included
......@@ -1533,12 +1537,24 @@ def CallCommand(cmd_data):
multiprocessing module.
multiprocessing needs a top level function with a single argument.
This function converts invocation of .py files and invocations of "python" to
vpython invocations.
"""
vpython = 'vpython.bat' if sys.platform == 'win32' else 'vpython'
cmd = cmd_data.cmd
if cmd[0] == 'python':
cmd = list(cmd)
cmd[0] = vpython
elif cmd[0].endswith('.py'):
cmd = [vpython] + cmd
cmd_data.kwargs['stdout'] = subprocess.PIPE
cmd_data.kwargs['stderr'] = subprocess.STDOUT
try:
start = time.time()
(out, _), code = subprocess.communicate(cmd_data.cmd, **cmd_data.kwargs)
(out, _), code = subprocess.communicate(cmd, **cmd_data.kwargs)
duration = time.time() - start
except OSError as e:
duration = time.time() - start
......
......@@ -2786,7 +2786,9 @@ class CannedChecksUnittest(PresubmitTestsBase):
CommHelper(input_api, ['allo', '--verbose'], cwd=self.fake_root_dir)
cmd = ['bar.py', '--verbose']
if input_api.platform == 'win32':
cmd.insert(0, input_api.python_executable)
cmd.insert(0, 'vpython.bat')
else:
cmd.insert(0, 'vpython')
CommHelper(input_api, cmd, cwd=self.fake_root_dir, ret=(('', None), 1))
self.mox.ReplayAll()
......
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