[MERGE] run_shell_command

[John Arbash Meinel]

Ian Clatworthy wrote:
> This is the first of a series of patches related to enhanced hook
> management. This one adds a wrapper around subprocess.Popen() that
> provides some Bazaar friendly error reporting. This is a foundation API
> used by shell hooks, i.e. out of process command line scripts instead of
> in-process Python hooks.
> 
> Ian C.
> 

+class TestShellCmd(TestCase):
+
+    def test_run_shell_command_api(self):
+        """run_shell_command API test"""
+        stdout, stderr, retcode = run_shell_command('echo',
+            ['hello', 'world'])
+        self.assertEquals('hello world\n', stdout)
+        self.assertEquals('', stderr)
+        self.assertEquals(0, retcode)
+
+    def test_missing_shell_command(self):
+        """Confirm right exception thrown if command missing."""
+        self.assertRaises(ShellCommandNotFound, run_shell_command,
+            'non-existent-cmd')


^- We are not guaranteed to have access to "echo". We've generally 
chosen to use sys.executable, -c, for anything we need to do in this 
regards.

(It will still fail for compiled bzr, but at least it works everywhere 
else.)

+def run_shell_command(command, args=None, stdin=None, require_success=True,
+    label="shell", shell=False, cwd=None, env=None):

*I* really prefer it when all arguments are lined up like:
+def run_shell_command(command, args=None, stdin=None,
+                      require_success=True, label="shell", shell=False,
+                      cwd=None, env=None):

It isn't a requirement, but I've had decent success at convincing others 
to use it, so I keep trying.

...


+    :param require_success: if True, the return code must be zero or the
+    ShellCommandFailed exception is raised
+    :param label: the label to use for the command in error reporting

^- Continuation lines should be indented:
+    :param require_success: if True, the return code must be zero or the
+        ShellCommandFailed exception is raised
+    :param label: the label to use for the command in error reporting

(sorry about Thunderbird's poor choice to wrap your lines.)

+        stdout, stderr = process.communicate(stdin)
+        retcode = process.wait()

I think .wait() after .communicate() is redundant, and you can just use: 
retcode = process.returncode

It probably doesn't hurt anything, though.

BB:tweak