Change arguments and semantics to better match subprocess.Popen

src/netns/protocol.py

@@ -437,17 +437,19 @@ class Client(object):
             raise
         self._read_and_check_reply()
 
-    def spawn(self, executable, argv = None, cwd = None, env = None,
-            stdin = None, stdout = None, stderr = None, user = None):
+    def spawn(self, argv, executable = None,
+            stdin = None, stdout = None, stderr = None,
+            cwd = None, env = None, user = None):
         """Start a subprocess in the slave; the interface resembles
         subprocess.Popen, but with less functionality. In particular
         stdin/stdout/stderr can only be None or a open file descriptor.
         See netns.subprocess.spawn for details."""
 
+        if executable == None:
+            executable = argv[0]
         params = ["PROC", "CRTE", _b64(executable)]
-        if argv != None:
-            for i in argv:
-                params.append(_b64(i))
+        for i in argv:
+            params.append(_b64(i))
 
         self._send_cmd(*params)
         self._read_and_check_reply()

src/netns/subprocess.py

@@ -14,8 +14,9 @@ class Subprocess(object):
     interface."""
     # FIXME
     default_user = None
-    def __init__(self, node, executable, argv = None, cwd = None, env = None,
-            stdin = None, stdout = None, stderr = None, user = None):
+    def __init__(self, node, argv, executable = None,
+            stdin = None, stdout = None, stderr = None,
+            shell = False, cwd = None, env = None, user = None):
         self._slave = node._slave
         """Forks and execs a program, with stdio redirection and user
         switching.
@@ -48,12 +49,16 @@ class Subprocess(object):
         if user == None:
             user = Subprocess.default_user
 
+        if isinstance(argv, str):
+            argv = [ argv ]
+        if shell:
+            argv = [ '/bin/sh', '-c' ] + argv
         # confusingly enough, to go to the function at the top of this file,
         # I need to call it thru the communications protocol: remember that
         # happens in another process!
-        self._pid = self._slave.spawn(executable, argv = argv, cwd = cwd,
-                env = env, stdin = stdin, stdout = stdout, stderr = stderr,
-                user = user)
+        self._pid = self._slave.spawn(argv, executable = executable,
+                stdin = stdin, stdout = stdout, stderr = stderr,
+                cwd = cwd, env = env, user = user)
 
         node._add_subprocess(self)
         self._returncode = None
@@ -107,9 +112,9 @@ class Popen(Subprocess):
     """Higher-level interface for executing processes, that tries to emulate
     the stdlib's subprocess.Popen as much as possible."""
 
-    def __init__(self, node, executable, argv = None, cwd = None, env = None,
-            stdin = None, stdout = None, stderr = None, user = None,
-            bufsize = 0):
+    def __init__(self, node, argv, executable = None,
+            stdin = None, stdout = None, stderr = None, bufsize = 0,
+            shell = False, cwd = None, env = None, user = None):
         """As in Subprocess, `node' specifies the netns Node to run in.
 
         The `stdin', `stdout', and `stderr' parameters also accept the special
@@ -139,9 +144,10 @@ class Popen(Subprocess):
         if stderr == STDOUT:
             fdmap['stderr'] = fdmap['stdout']
 
-        super(Popen, self).__init__(node, executable, argv = argv, cwd = cwd,
-                env = env, stdin = fdmap['stdin'], stdout = fdmap['stdout'],
-                stderr = fdmap['stderr'], user = user)
+        super(Popen, self).__init__(node, argv, executable = executable,
+                stdin = fdmap['stdin'], stdout = fdmap['stdout'],
+                stderr = fdmap['stderr'],
+                shell = shell, cwd = cwd, env = env, user = user)
 
         # Close pipes, they have been dup()ed to the child
         for k, v in fdmap.items():
@@ -208,24 +214,21 @@ class Popen(Subprocess):
 def system(node, args):
     """Emulates system() function, if `args' is an string, it uses `/bin/sh' to
     exexecute it, otherwise is interpreted as the argv array to call execve."""
-    if isinstance(args, str):
-        args = [ '/bin/sh', '/bin/sh', '-c', args ]
-    return Popen(node, args[0], args[1:]).wait()
+    shell = isinstance(args, str)
+    return Popen(node, args, shell = shell).wait()
 
 def backticks(node, args):
     """Emulates shell backticks, if `args' is an string, it uses `/bin/sh' to
     exexecute it, otherwise is interpreted as the argv array to call execve."""
-    if isinstance(args, str):
-        args = [ '/bin/sh', '/bin/sh', '-c', args ]
-    return Popen(node, args[0], args[1:], stdout = PIPE).communicate()[0]
+    shell = isinstance(args, str)
+    return Popen(node, args, shell = shell, stdout = PIPE).communicate()[0]
 
 def backticks_raise(node, args):
     """Emulates shell backticks, if `args' is an string, it uses `/bin/sh' to
     exexecute it, otherwise is interpreted as the argv array to call execve.
     Raises an RuntimeError if the return value is not 0."""
-    if isinstance(args, str):
-        args = [ '/bin/sh', '/bin/sh', '-c', args ]
-    p = Popen(node, args[0], args[1:], stdout = PIPE)
+    shell = isinstance(args, str)
+    p = Popen(node, args, shell = shell, stdout = PIPE)
     out = p.communicate()[0]
     ret = p.returncode
     if ret > 0:
@@ -238,8 +241,8 @@ def backticks_raise(node, args):
 #
 # Server-side code, called from netns.protocol.Server
 
-def spawn(executable, argv = None, cwd = None, env = None, stdin = None,
-        stdout = None, stderr = None, close_fds = False, user = None):
+def spawn(executable, argv = None, cwd = None, env = None, close_fds = False,
+        stdin = None, stdout = None, stderr = None, user = None):
     """Internal function that performs all the dirty work for Subprocess, Popen
     and friends. This is executed in the slave process, directly from the
     protocol.Server class.

t/test_subprocess.py

@@ -81,7 +81,7 @@ class TestSubprocess(unittest.TestCase):
     def test_Subprocess_chuser(self):
         node = netns.Node(nonetns = True)
         user = 'nobody'
-        p = Subprocess(node, '/bin/sleep', ['/bin/sleep', '1000'], user = user)
+        p = Subprocess(node, ['/bin/sleep', '1000'], user = user)
         self._check_ownership(user, p.pid)
         p.signal()
         self.assertEquals(p.wait(), -signal.SIGTERM)
@@ -126,9 +126,9 @@ class TestSubprocess(unittest.TestCase):
         node = netns.Node(nonetns = True)
         # User does not exist
         self.assertRaises(RuntimeError, Subprocess, node,
-                '/bin/sleep', ['/bin/sleep', '1000'], user = self.nouser)
+                ['/bin/sleep', '1000'], user = self.nouser)
         self.assertRaises(RuntimeError, Subprocess, node,
-                '/bin/sleep', ['/bin/sleep', '1000'], user = self.nouid)
+                ['/bin/sleep', '1000'], user = self.nouid)
         # Invalid CWD: it is a file
         self.assertRaises(RuntimeError, Subprocess, node,
                 '/bin/sleep', cwd = '/bin/sleep')
@@ -141,15 +141,21 @@ class TestSubprocess(unittest.TestCase):
         self.assertRaises(RuntimeError, Subprocess, node,
                 'sleep', env = {'PATH': ''})
 
+        # Argv
+        self.assertRaises(RuntimeError, Subprocess, node, 'true; false')
+        self.assertEquals(Subprocess(node, 'true').wait(), 0)
+        self.assertEquals(Subprocess(node, 'true; false', shell = True).wait(),
+                1)
+
         # Piping
         r, w = os.pipe()
-        p = Subprocess(node, 'echo', ['echo', 'hello world'], stdout = w)
+        p = Subprocess(node, ['echo', 'hello world'], stdout = w)
         os.close(w)
         self.assertEquals(_readall(r), "hello world\n")
         os.close(r)
         p.wait()
 
-        p = Subprocess(node, 'sleep', ['sleep', '100'])
+        p = Subprocess(node, ['sleep', '100'])
         self.assertTrue(p.pid > 0)
         self.assertEquals(p.poll(), None) # not finished
         p.signal()
@@ -158,14 +164,14 @@ class TestSubprocess(unittest.TestCase):
         self.assertEquals(p.wait(), -signal.SIGTERM) # no-op
         self.assertEquals(p.poll(), -signal.SIGTERM) # no-op
 
-        p = Subprocess(node, 'sleep', ['sleep', '100'])
+        p = Subprocess(node, ['sleep', '100'])
         os.kill(p.pid, signal.SIGTERM)
         time.sleep(0.2)
         p.signal() # since it has not been waited for, it should not raise
         self.assertEquals(p.wait(), -signal.SIGTERM)
 
     def test_Popen(self):
-        node = netns.Node(nonetns = True, debug=0)
+        node = netns.Node(nonetns = True, debug = 0)
 
         # repeat test with Popen interface
         r0, w0 = os.pipe()
@@ -204,20 +210,18 @@ class TestSubprocess(unittest.TestCase):
         p = Popen(node, 'cat', stdin = PIPE)
         self.assertEquals(p.communicate(), (None, None))
 
-        p = Popen(node, '/bin/sh', ['sh', '-c', 'cat >&2'],
-                stdin = PIPE, stderr = PIPE)
+        p = Popen(node, 'cat >&2', shell = True, stdin = PIPE, stderr = PIPE)
         p.stdin.write("hello world\n")
         p.stdin.close()
         self.assertEquals(p.stderr.readlines(), ["hello world\n"])
         self.assertEquals(p.stdout, None)
         self.assertEquals(p.wait(), 0)
 
-        p = Popen(node, '/bin/sh', ['sh', '-c', 'cat >&2'],
-                stdin = PIPE, stderr = PIPE)
+        p = Popen(node, ['sh', '-c', 'cat >&2'], stdin = PIPE, stderr = PIPE)
         self.assertEquals(p.communicate(_longstring), (None, _longstring))
 
         #
-        p = Popen(node, '/bin/sh', ['sh', '-c', 'cat >&2'],
+        p = Popen(node, ['sh', '-c', 'cat >&2'],
                 stdin = PIPE, stdout = PIPE, stderr = STDOUT)
         p.stdin.write("hello world\n")
         p.stdin.close()
@@ -225,12 +229,12 @@ class TestSubprocess(unittest.TestCase):
         self.assertEquals(p.stderr, None)
         self.assertEquals(p.wait(), 0)
 
-        p = Popen(node, '/bin/sh', ['sh', '-c', 'cat >&2'],
+        p = Popen(node, ['sh', '-c', 'cat >&2'],
                 stdin = PIPE, stdout = PIPE, stderr = STDOUT)
         self.assertEquals(p.communicate(_longstring), (_longstring, None))
 
         #
-        p = Popen(node, 'tee', ['tee', '/dev/stderr'],
+        p = Popen(node, ['tee', '/dev/stderr'],
                 stdin = PIPE, stdout = PIPE, stderr = STDOUT)
         p.stdin.write("hello world\n")
         p.stdin.close()
@@ -238,13 +242,13 @@ class TestSubprocess(unittest.TestCase):
         self.assertEquals(p.stderr, None)
         self.assertEquals(p.wait(), 0)
 
-        p = Popen(node, 'tee', ['tee', '/dev/stderr'],
+        p = Popen(node, ['tee', '/dev/stderr'],
                 stdin = PIPE, stdout = PIPE, stderr = STDOUT)
         self.assertEquals(p.communicate(_longstring[0:512]),
                 (_longstring[0:512] * 2, None))
 
         #
-        p = Popen(node, 'tee', ['tee', '/dev/stderr'],
+        p = Popen(node, ['tee', '/dev/stderr'],
                 stdin = PIPE, stdout = PIPE, stderr = PIPE)
         p.stdin.write("hello world\n")
         p.stdin.close()
@@ -252,16 +256,16 @@ class TestSubprocess(unittest.TestCase):
         self.assertEquals(p.stderr.readlines(), ["hello world\n"])
         self.assertEquals(p.wait(), 0)
 
-        p = Popen(node, 'tee', ['tee', '/dev/stderr'],
+        p = Popen(node, ['tee', '/dev/stderr'],
                 stdin = PIPE, stdout = PIPE, stderr = PIPE)
         self.assertEquals(p.communicate(_longstring), (_longstring, ) * 2)
 
     def test_backticks(self): 
-        node = netns.Node(nonetns = True, debug=0)
+        node = netns.Node(nonetns = True, debug = 0)
         self.assertEquals(backticks(node, "echo hello world"), "hello world\n")
         self.assertEquals(backticks(node, r"echo hello\ \ world"),
                 "hello  world\n")
-        self.assertEquals(backticks(node, ["echo", "echo", "hello", "world"]),
+        self.assertEquals(backticks(node, ["echo", "hello", "world"]),
                 "hello world\n")
         self.assertEquals(backticks(node, "echo hello world > /dev/null"), "")
         self.assertEquals(backticks_raise(node, "true"), "")
@@ -269,7 +273,7 @@ class TestSubprocess(unittest.TestCase):
         self.assertRaises(RuntimeError, backticks_raise, node, "kill $$")
 
     def test_system(self): 
-        node = netns.Node(nonetns = True, debug=0)
+        node = netns.Node(nonetns = True, debug = 0)
         self.assertEquals(system(node, "true"), 0)
         self.assertEquals(system(node, "false"), 1)
 

t/test_util.py

@@ -2,6 +2,7 @@
 # vim:ts=4:sw=4:et:ai:sts=4
 
 import re, subprocess, sys
+import netns.subprocess
 
 def process_ipcmd(str):
     cur = None
@@ -55,11 +56,11 @@ def get_devs():
     ipcmd = subprocess.Popen(["ip", "addr", "list"],
             stdout = subprocess.PIPE)
     (outdata, errdata) = ipcmd.communicate()
-    ipcmd.wait()
     return process_ipcmd(outdata)
 
 def get_devs_netns(node):
-    (outdata, errdata) = node.run_process(["ip", "addr", "list"])
+    (outdata, errdata) = netns.subprocess.backticks_raise(node,
+            ["ip", "addr", "list"])
     return process_ipcmd(outdata)
 
 # Unittest from Python 2.6 doesn't have these decorators