Commit e9730d75 authored by Raul Tambre's avatar Raul Tambre Committed by LUCI CQ

subprocess2: Ensure environment keys and values are strings on Python 3

Example error during "git cl upload":
Traceback (most recent call last):
  File "C:\Google\depot_tools\presubmit_support.py", line 220, in CallCommand
    returncode, stdout = self._RunWithTimeout(cmd, test.stdin, test.kwargs)
  File "C:\Google\depot_tools\presubmit_support.py", line 204, in _RunWithTimeout
    p = subprocess.Popen(cmd, **kwargs)
  File "C:\Google\depot_tools\subprocess2.py", line 143, in __init__
    super(Popen, self).__init__(args, **kwargs)
  File "C:\Google\depot_tools\bootstrap-3_8_0b1_chromium_1_bin\python\bin\Lib\subprocess.py", line 390, in __init__
    errread, errwrite)
  File "C:\Google\depot_tools\bootstrap-3_8_0b1_chromium_1_bin\python\bin\Lib\subprocess.py", line 640, in _execute_child
    startupinfo)
TypeError: environment can only contain strings

Bug: 984182
Change-Id: Id996d73a80845aaeaa061107ed123627091cb600
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1815872
Commit-Queue: Raul Tambre <raul@tambre.ee>
Reviewed-by: 's avatarEdward Lesmes <ehmaldonado@chromium.org>
Auto-Submit: Raul Tambre <raul@tambre.ee>
parent 2a048032
......@@ -121,6 +121,17 @@ class Popen(subprocess.Popen):
env = get_english_env(kwargs.get('env'))
if env:
kwargs['env'] = env
if kwargs.get('env') is not None and sys.version_info.major != 2:
# Subprocess expects environment variables to be strings in Python 3.
def ensure_str(value):
if isinstance(value, bytes):
return value.decode()
return value
kwargs['env'] = {
ensure_str(k): ensure_str(v)
for k, v in kwargs['env'].items()
}
if kwargs.get('shell') is None:
# *Sigh*: Windows needs shell=True, or else it won't search %PATH% for
# the executable, but shell=True makes subprocess on Linux fail when it's
......
......@@ -81,6 +81,14 @@ class DefaultsTest(unittest.TestCase):
mockCommunicate.assert_called_with(
['foo'], a=True, stdin=subprocess2.VOID_INPUT, stdout=subprocess2.PIPE)
@mock.patch('subprocess.Popen.__init__')
def test_env_type(self, mockPopen):
if sys.version_info.major != 2:
subprocess2.Popen(['foo'], env={b'key': b'value'})
mockPopen.assert_called_with(['foo'],
env={'key': 'value'},
shell=mock.ANY)
def _run_test(with_subprocess=True):
"""Runs a tests in 12 combinations:
......
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