[Jython-checkins] jython: os.system should use changes to os.environ in subprocesses

[jim.baker]

https://hg.python.org/jython/rev/cbdeff5f17f5
changeset:   7768:cbdeff5f17f5
user:        Jim Baker <jim.baker at rackspace.com>
date:        Mon Oct 26 22:55:51 2015 -0600
summary:
  os.system should use changes to os.environ in subprocesses

Fixes http://bugs.jython.org/issue2416

files:
  Lib/subprocess.py      |  55 ++++++++++++++++-------------
  Lib/test/test_os_jy.py |  19 +++++++++-
  2 files changed, 49 insertions(+), 25 deletions(-)


diff --git a/Lib/subprocess.py b/Lib/subprocess.py
--- a/Lib/subprocess.py
+++ b/Lib/subprocess.py
@@ -768,6 +768,31 @@
                     raise
 
 
+if jython:
+    def _setup_env(env, builder_env):
+        """Carefully merge env with ProcessBuilder's only
+        overwriting key/values that differ
+
+        System.getenv (Map<String, String>) may be backed by
+        <byte[], byte[]> on UNIX platforms where these are really
+        bytes. ProcessBuilder's env inherits its contents and will
+        maintain those byte values (which may be butchered as
+        Strings) for the subprocess if they haven't been modified.
+        """
+        # Determine what's safe to merge
+        merge_env = dict((key, value) for key, value in env.iteritems()
+                         if key not in builder_env or
+                         builder_env.get(key) != value)
+
+        # Prune anything not in env
+        entries = builder_env.entrySet().iterator()
+        for entry in entries:
+            if entry.getKey() not in env:
+                entries.remove()
+
+        builder_env.putAll(merge_env)
+
+
 class Popen(object):
     def __init__(self, args, bufsize=0, executable=None,
                  stdin=None, stdout=None, stderr=None,
@@ -1268,28 +1293,6 @@
             return _CouplerThread(*args, **kwargs)
 
 
-        def _setup_env(self, env, builder_env):
-            """Carefully merge env with ProcessBuilder's only
-            overwriting key/values that differ
-
-            System.getenv (Map<String, String>) may be backed by
-            <byte[], byte[]> on UNIX platforms where these are really
-            bytes. ProcessBuilder's env inherits its contents and will
-            maintain those byte values (which may be butchered as
-            Strings) for the subprocess if they haven't been modified.
-            """
-            # Determine what's safe to merge
-            merge_env = dict((key, value) for key, value in env.iteritems()
-                             if key not in builder_env or
-                             builder_env.get(key) != value)
-
-            # Prune anything not in env
-            entries = builder_env.entrySet().iterator()
-            for entry in entries:
-                if entry.getKey() not in env:
-                    entries.remove()
-
-            builder_env.putAll(merge_env)
 
 
         def _execute_child(self, args, executable, preexec_fn, close_fds,
@@ -1328,8 +1331,8 @@
                 builder.redirectError(java.lang.ProcessBuilder.Redirect.INHERIT)
 
             # os.environ may be inherited for compatibility with CPython
-            self._setup_env(dict(os.environ if env is None else env),
-                            builder.environment())
+            _setup_env(dict(os.environ if env is None else env),
+                       builder.environment())
 
             if cwd is None:
                 cwd = os.getcwd()
@@ -1888,11 +1891,15 @@
     args = _escape_args(args)
     args = _shell_command + args
     cwd = os.getcwd()
+
+
+
     builder = java.lang.ProcessBuilder(args)
     builder.directory(java.io.File(cwd))
     builder.redirectInput(java.lang.ProcessBuilder.Redirect.INHERIT)
     builder.redirectOutput(java.lang.ProcessBuilder.Redirect.INHERIT)
     builder.redirectError(java.lang.ProcessBuilder.Redirect.INHERIT)
+    _setup_env(dict(os.environ), builder.environment())
     try:
         return builder.start().waitFor()
     except (java.io.IOException,
diff --git a/Lib/test/test_os_jy.py b/Lib/test/test_os_jy.py
--- a/Lib/test/test_os_jy.py
+++ b/Lib/test/test_os_jy.py
@@ -359,7 +359,7 @@
         # would fail with an import error due to creating a circular
         # import chain. This root cause is because the os module
         # imports the subprocess module for the system function; but
-        # the subprocess module imports from os. Verrifies that this
+        # the subprocess module imports from os. Verifies that this is
         # managed by making the import late; also verify the
         # monkeypatching optimization is successful by calling
         # os.system twice.
@@ -371,6 +371,23 @@
                 .replace("\r", ""),  # in case of running on Windows
                 "42\n47\n")
 
+    def test_system_uses_os_environ(self):
+        """Writing to os.environ is made available as env vars in os.system subprocesses"""
+        # This test likely requires additional customization for
+        # environments like AS/400, but I do not have current access.
+        # Verifies fix for http://bugs.jython.org/issue2416
+        if os._name == "nt":
+            echo_command = 'echo %TEST_ENVVAR%'
+        else:
+            echo_command = 'echo $TEST_ENVVAR'
+        with test_support.temp_cwd() as temp_cwd:
+            self.assertEqual(
+                subprocess.check_output(
+                    [sys.executable, "-c",
+                     "import os; os.environ['TEST_ENVVAR'] = 'works on 2.7.1+'; os.system('%s')" % echo_command])\
+                .replace("\r", ""),  # in case of running on Windows
+                "works on 2.7.1+\n")
+
 
 @unittest.skipUnless(hasattr(os, 'link'), "os.link not available")
 class LinkTestCase(unittest.TestCase):

-- 
Repository URL: https://hg.python.org/jython