Merge pull request #4260 from takluyver/system-raw-subprocess

Use subprocess for system_raw

This unifies the exit codes from system_raw and system_piped to use the subprocess model on Unix - positive numbers for exit status, negative for terminating signal.

This doesn't change the situation on Windows. system_raw will give the raw exit code (Windows exit statuses don't appear to include the terminating signal), and system_piped will give nothing - our process_win32 machinery never gets the exit code, and I don't want to go messing around with that.

IPython/core/interactiveshell.py

@@ -29,6 +29,7 import runpy
 import sys
 import tempfile
 import types
+import subprocess
 from io import open as io_open
 
 from IPython.config.configurable import SingletonConfigurable
@@ -2232,7 +2233,8 class InteractiveShell(SingletonConfigurable):
         self.user_ns['_exit_code'] = system(self.var_expand(cmd, depth=1))
 
     def system_raw(self, cmd):
-        """Call the given cmd in a subprocess using os.system
+        """Call the given cmd in a subprocess using os.system on Windows or
+        subprocess.call using the system shell on other platforms.
 
         Parameters
         ----------
@@ -2250,11 +2252,10 class InteractiveShell(SingletonConfigurable):
                 ec = os.system(cmd)
         else:
             cmd = py3compat.unicode_to_str(cmd)
-            ec = os.system(cmd)
-            # The high byte is the exit code, the low byte is a signal number
-            # that we discard for now. See the docs for os.wait()
-            if ec > 255:
-                ec >>= 8
+            # Call the cmd using the OS shell, instead of the default /bin/sh, if set.
+            ec = subprocess.call(cmd, shell=True, executable=os.environ.get('SHELL', None))
+            # exit code is positive for program failure, or negative for
+            # terminating signal number.
         
         # We explicitly do NOT return the subprocess status code, because
         # a non-None value would trigger :func:`sys.displayhook` calls.

IPython/core/tests/test_interactiveshell.py

@@ -22,6 +22,7 Authors
 # stdlib
 import ast
 import os
+import signal
 import shutil
 import sys
 import tempfile
@@ -33,7 +34,7 from StringIO import StringIO
 import nose.tools as nt
 
 # Our own
-from IPython.testing.decorators import skipif, onlyif_unicode_paths
+from IPython.testing.decorators import skipif, skip_win32, onlyif_unicode_paths
 from IPython.testing import tools as tt
 from IPython.utils import io
 
@@ -414,19 +415,48 class TestSafeExecfileNonAsciiPath(unittest.TestCase):
         """
         ip.safe_execfile(self.fname, {}, raise_exceptions=True)
 
+class ExitCodeChecks(tt.TempFileMixin):
+    def test_exit_code_ok(self):
+        self.system('exit 0')
+        self.assertEqual(ip.user_ns['_exit_code'], 0)
+
+    def test_exit_code_error(self):
+        self.system('exit 1')
+        self.assertEqual(ip.user_ns['_exit_code'], 1)
+
+    @skipif(not hasattr(signal, 'SIGALRM'))
+    def test_exit_code_signal(self):
+        self.mktmp("import signal, time\n"
+                   "signal.setitimer(signal.ITIMER_REAL, 0.1)\n"
+                   "time.sleep(1)\n")
+        self.system("%s %s" % (sys.executable, self.fname))
+        self.assertEqual(ip.user_ns['_exit_code'], -signal.SIGALRM)
+
+class TestSystemRaw(unittest.TestCase, ExitCodeChecks):
+    system = ip.system_raw
 
-class TestSystemRaw(unittest.TestCase):
     @onlyif_unicode_paths
     def test_1(self):
         """Test system_raw with non-ascii cmd
         """
-        cmd = ur'''python -c "'åäö'"   '''
+        cmd = u'''python -c "'åäö'"   '''
         ip.system_raw(cmd)
-    
-    def test_exit_code(self):
-        """Test that the exit code is parsed correctly."""
-        ip.system_raw('exit 1')
-        self.assertEqual(ip.user_ns['_exit_code'], 1)
+
+# TODO: Exit codes are currently ignored on Windows.
+class TestSystemPipedExitCode(unittest.TestCase, ExitCodeChecks):
+    system = ip.system_piped
+
+    @skip_win32
+    def test_exit_code_ok(self):
+        ExitCodeChecks.test_exit_code_ok(self)
+
+    @skip_win32
+    def test_exit_code_error(self):
+        ExitCodeChecks.test_exit_code_error(self)
+
+    @skip_win32
+    def test_exit_code_signal(self):
+        ExitCodeChecks.test_exit_code_signal(self)
 
 class TestModules(unittest.TestCase, tt.TempFileMixin):
     def test_extraneous_loads(self):

IPython/utils/_process_posix.py

@@ -184,6 +184,12 class ProcessHandler(object):
                 child.terminate(force=True)
         # add isalive check, to ensure exitstatus is set:
         child.isalive()
+
+        # We follow the subprocess pattern, returning either the exit status
+        # as a positive number, or the terminating signal as a negative
+        # number. sh returns 128+n for signals
+        if child.exitstatus > 128:
+            return -(child.exitstatus - 128)
         return child.exitstatus