From 5025893cb6061b8a219750155cccd67b55079311 Mon Sep 17 00:00:00 2001 From: Robert Iannucci Date: Mon, 19 Mar 2018 10:30:59 -0700 Subject: [PATCH] [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/961865 Reviewed-by: Dirk Pranke Reviewed-by: Andrii Shyshkalov Commit-Queue: Robbie Iannucci --- presubmit_canned_checks.py | 6 +----- presubmit_support.py | 24 ++++++++++++++++++++---- tests/presubmit_unittest.py | 4 +++- 3 files changed, 24 insertions(+), 10 deletions(-) diff --git a/presubmit_canned_checks.py b/presubmit_canned_checks.py index 39c491f7c4..f8a0a4eaa8 100644 --- a/presubmit_canned_checks.py +++ b/presubmit_canned_checks.py @@ -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()} diff --git a/presubmit_support.py b/presubmit_support.py index 8493d273c5..5b89a335d8 100755 --- a/presubmit_support.py +++ b/presubmit_support.py @@ -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 and 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 diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index d9ac3e8fda..e94b5c7d98 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -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()