[Jython-checkins] jython: os.system now uses a simpler wrapping of ProcessBuilder

[jim.baker]

https://hg.python.org/jython/rev/7ea0c8aa8b50
changeset:   7515:7ea0c8aa8b50
user:        Jim Baker <jim.baker at rackspace.com>
date:        Tue Jan 06 17:50:03 2015 -0700
summary:
  os.system now uses a simpler wrapping of ProcessBuilder

Previously os.system used subprocess.call, via subprocess.Popen,
following the recipe described by
https://docs.python.org/2/library/subprocess.html#replacing-os-system

However, Popen is substantially (!) more complex than what os.system
needs and may be causing issues on systems like HPUX. Simplified
os.system accordingly so that it uses subprocess to determine shell
executable and support for shell parseability, but otherwise only uses
ProcessBuilder.

Part of a fix for http://bugs.jython.org/issue2238

files:
  Lib/os.py                                     |   6 ++-
  Lib/subprocess.py                             |  23 ++++++++++
  src/org/python/modules/posix/PosixModule.java |   9 ---
  3 files changed, 28 insertions(+), 10 deletions(-)


diff --git a/Lib/os.py b/Lib/os.py
--- a/Lib/os.py
+++ b/Lib/os.py
@@ -219,7 +219,7 @@
         except error:
             pass
 
-__all__.extend(["makedirs", "removedirs", "renames"])
+__all__.extend(["makedirs", "removedirs", "renames", "system"])
 
 def walk(top, topdown=True, onerror=None, followlinks=False):
     """Directory tree generator.
@@ -719,3 +719,7 @@
         return getattr(self._stream, name)
     def __iter__(self):
         return iter(self._stream)
+
+
+# Recursive import! So need to put it at the end
+from subprocess import _os_system as system
diff --git a/Lib/subprocess.py b/Lib/subprocess.py
--- a/Lib/subprocess.py
+++ b/Lib/subprocess.py
@@ -1824,6 +1824,29 @@
             self.send_signal(signal.SIGKILL)
 
 
+# we need some functionality from subprocess given brokenness for ProcessBuilder,
+# but need to avoid recursive imports
+
+def _os_system(command):
+    """system(command) -> exit_status
+
+    Execute the command (a string) in a subshell."""
+    args = _cmdline2listimpl(command)
+    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)
+    try:
+        return builder.start().waitFor()
+    except (java.io.IOException,
+            java.lang.IllegalArgumentException), e:
+        raise OSError(e.getMessage() or e)
+
+
 def _demo_posix():
     #
     # Example 1: Simple redirection: Get process list
diff --git a/src/org/python/modules/posix/PosixModule.java b/src/org/python/modules/posix/PosixModule.java
--- a/src/org/python/modules/posix/PosixModule.java
+++ b/src/org/python/modules/posix/PosixModule.java
@@ -709,15 +709,6 @@
         }
     }
 
-    public static PyString __doc__system = new PyString(
-        "system(command) -> exit_status\n\n" +
-        "Execute the command (a string) in a subshell.");
-    public static PyObject system(PyObject command) {
-        // import subprocess; return subprocess.call(command, shell=True)
-        return imp.load("subprocess").invoke("call", command, new PyObject[] {Py.True},
-                                             new String[] {"shell"});
-    }
-
     public static PyString __doc__umask = new PyString(
         "umask(new_mask) -> old_mask\n\n" +
         "Set the current numeric umask and return the previous umask.");

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