Add pass_fds parameter to PtyProcess:spawn() #49
Conversation
|
Hi! Previous travis tests failed because of ubuntu 14.04 dash file descriptor limit. Updated with bash, could you please re-launch travis? |
| @@ -190,6 +190,10 @@ def spawn( | |||
|
|
|||
| Dimensions of the psuedoterminal used for the subprocess can be | |||
| specified as a tuple (rows, cols), or the default (24, 80) will be used. | |||
|
|
|||
| By default, all file descriptors except 0, 1 and 2 are closed. This | |||
| behavior can be overridden with pass_fds, a list of file descriptors to | |||
There was a problem hiding this comment.
Nitpick: you document it as a list, but it will only work with a tuple, because you concatenate it to a tuple (pass_fds + (exec_err_pipe_write,)). Let's cast it to a set, like subprocess does; that should work with any sequence, and also eliminates any odd behaviour with duplicate entries.
| '-c', | ||
| 'printf hello >&{}'.format(temp_file_fd)], | ||
| echo=True, | ||
| pass_fds=(temp_file_fd,)) |
There was a problem hiding this comment.
Is it worth having a test that when you don't specify it in pass_fds, the child process can't access it?
There was a problem hiding this comment.
I think that the more tests, the better :) I've added that one too.
eugpermar
force-pushed
the
master
branch
2 times, most recently
from
2a0cddc to
55cf239
Compare
|
Hi! I think I've addressed both comments, could you take a look? |
|
Thanks |
|
Thank you! As soon as pexpect tests download this feature from pip, I will continue PR pexpect/pexpect#546 😄 |
Same format as subprocess.Popen