Passing both _err_to_out=True and _out=sys.stdout to sh.Command raises Exception

[jshilkaitis]

Hi there,

TL;DR

The following code snippet causes an error in sh 1.12.9, but not 1.1:

import sh
import sys
sh.Command("echo")(1, _err_to_out=True, _out=sys.stdout)

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/quid/stacks/shipit/current/build/env/lib/python2.7/site-packages/sh.py", line 1288, in __call__
    return RunningCommand(cmd, call_args, stdin, stdout, stderr)
  File "/quid/stacks/shipit/current/build/env/lib/python2.7/site-packages/sh.py", line 631, in __init__
    self.call_args, pipe, process_assign_lock)
  File "/quid/stacks/shipit/current/build/env/lib/python2.7/site-packages/sh.py", line 1646, in __init__
    self._stderr_read_fd = os.dup(self._stdout_read_fd)
TypeError: an integer is required

Is there a bug to be fixed here? Should I be sending the output of the subcommand to the parent process' stdout a different way?

Full story:

We're using sh 1.1 to run a command from a deploy script a la:

lambda src_path: sh.Command("/opt/sbt/current/bin/sbt")("publish", _cwd=src_path, _err_to_out=True, _out=sys.stdout)

This would stream the output of the deploy command to our deploy logger so we could follow along with its progress in real time instead of tapping our feet impatiently, waiting for sbt to finish. Sweet!

When we upgraded to sh 1.12.9, this line started raising an exception:

"sbt": lambda src_path: sh.Command("/opt/sbt/current/bin/sbt")("publish", _cwd=src_path, _err_to_out=True, _out=sys.stdout)
File "/quid/stacks/shipit/current/build/env/lib/python2.7/site-packages/sh.py", line 1288, in __call__
return RunningCommand(cmd, call_args, stdin, stdout, stderr)
File "/quid/stacks/shipit/current/build/env/lib/python2.7/site-packages/sh.py", line 631, in __init__
self.call_args, pipe, process_assign_lock)
File "/quid/stacks/shipit/current/build/env/lib/python2.7/site-packages/sh.py", line 1646, in __init__
self._stderr_read_fd = os.dup(self._stdout_read_fd)
TypeError: an integer is required

There are only a few assignments to self._stdout_read_fd before that line, so it didn't take much digging to settle on this line:

https://github.com/amoffat/sh/blob/master/sh.py#L1630

which gets hit when the supplied _out param is a tty or pipe, which the deploy script's sys.stdout is.

At this point, I don't know where to go. A narrow fix to only call os.dup(self._stdout_read_fd) when the fd isn't None let's me make progress and doesn't break any of the existing tests, but I don't know if it's the right thing to do.

[amoffat]

@jshilkaitis Thanks for reporting and for the patch. Your instinct is mostly correct about avoiding the call to os.dup(self._stdout_read_fd). I added some comments going into a little more detail. I'll be shipping the new release out tonight or tomorrow

[amoffat]

Your fix is live with 1.12.10 @jshilkaitis, good work