Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Python 3 issues - os.environ bytes vs unicode #6222

Merged
merged 1 commit into from Jul 26, 2018
Merged

Python 3 issues - os.environ bytes vs unicode #6222

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/python/pants/util/contextutil.py
View file
Open in desktop
Expand Up @@ -17,7 +17,7 @@
from contextlib import closing, contextmanager

from colors import green
from future.utils import string_types
from future.utils import PY3, string_types

from pants.util.dirutil import safe_delete
from pants.util.tarutil import TarFile
Expand Down Expand Up @@ -50,7 +50,7 @@ def environment_as(**kwargs):

def setenv(key, val):
if val is not None:
os.environ[key] = _os_encode(val)
os.environ[key] = val if PY3 else _os_encode(val)
else:
if key in os.environ:
del os.environ[key]
Expand Down Expand Up @@ -80,13 +80,13 @@ def _purge_env():

def _restore_env(env):
for k, v in env.items():
os.environ[k] = _os_encode(v)
os.environ[k] = v if PY3 else _os_encode(v)


@contextmanager
def hermetic_environment_as(**kwargs):
"""Set the environment to the supplied values from an empty state."""
old_environment = _copy_and_decode_env(os.environ)
old_environment = os.environ.copy() if PY3 else _copy_and_decode_env(os.environ)
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like it would be cleaner to do the PY3 check in _copy_and_decode_env and _os_encode rather than before calling those methods?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about that. But if we want the function names to stay true to their implementation, we would have to rename _copy_and_decode_env to something like _copy_env and _os_encode to _os_encode_if_py2, which doesn't seem ideal either.

_purge_env()
try:
with environment_as(**kwargs):
Expand Down
10 changes: 6 additions & 4 deletions tests/python/pants_test/util/test_contextutil.py
View file
Open in desktop
Expand Up @@ -16,6 +16,7 @@
from contextlib import contextmanager

import mock
from future.utils import PY3

from pants.util.contextutil import (HardSystemExit, InvalidZipPath, Timer, environment_as,
exception_logging, hard_exit_handler, hermetic_environment_as,
Expand Down Expand Up @@ -67,7 +68,7 @@ def test_hermetic_environment(self):
def test_hermetic_environment_subprocesses(self):
self.assertIn('USER', os.environ)
with hermetic_environment_as(**dict(AAA='333')):
output = subprocess.check_output('env', shell=True)
output = subprocess.check_output('env', shell=True).decode('utf-8')
self.assertNotIn('USER=', output)
self.assertIn('AAA', os.environ)
self.assertEquals(os.environ['AAA'], '333')
Expand All @@ -77,12 +78,13 @@ def test_hermetic_environment_subprocesses(self):
def test_hermetic_environment_unicode(self):
UNICODE_CHAR = '¡'
ENCODED_CHAR = UNICODE_CHAR.encode('utf-8')
expected_output = UNICODE_CHAR if PY3 else ENCODED_CHAR
with environment_as(**dict(XXX=UNICODE_CHAR)):
self.assertEquals(os.environ['XXX'], ENCODED_CHAR)
self.assertEquals(os.environ['XXX'], expected_output)
with hermetic_environment_as(**dict(AAA=UNICODE_CHAR)):
self.assertIn('AAA', os.environ)
self.assertEquals(os.environ['AAA'], ENCODED_CHAR)
self.assertEquals(os.environ['XXX'], ENCODED_CHAR)
self.assertEquals(os.environ['AAA'], expected_output)
self.assertEquals(os.environ['XXX'], expected_output)

def test_simple_pushd(self):
pre_cwd = os.getcwd()
Expand Down