Added errors handler for forked PTY process #275
Conversation
Here is the issue reproduction: ```python import invoke invoke.run(u'echo \xff', pty=True) ```
invoke/runners.py
Outdated
There was a problem hiding this comment.
- E501 line too long (81 > 79 characters)
|
@bitprophet I discovered this bug while improving integration tests for my #274 |
invoke/runners.py
Outdated
There was a problem hiding this comment.
- W293 blank line contains whitespace
|
Everything is tested and Travis is happy. Please, merge. |
|
@bitprophet You seem to neglect PRs which target stability. This is why I decided to provide even more obvious reproduction: import invoke
try:
invoke.run(u'echo \xff', pty=True)
except:
pass
print("This must be printed ONCE!")$ env LANG=C python bug_275.py
This must be printed ONCE!
This must be printed ONCE!The text "This must be printed ONCE!" is printed TWICE because of an exception which happens when File "/tmp/env/lib/python2.7/site-packages/invoke/runners.py", line 741, in start
os.execv('/bin/bash', ['/bin/bash', '-c', command])
TypeError: execv() arg 2 must contain only strings |
Casting shade on the maintainer is not good feedback. You don't seem to value volunteer time very much going by this and past comments. I get that my slow pace is frustrating, but you need to understand this is all a labor of love and try dialing back the entitlement a little bit please! :) thanks! (And thanks also for the detailed repro! I don't think I had tried reproducing this yet but the extra details make it much clearer.) |
|
@bitprophet It is frustrating to see that my volunteer time (and other PRs also) is left behind, while I see new commits are made. Please, prioritize PRs reviews and either close them or merge before you proceed. Otherwise, PRs are just useless. I have some time for open-source, but I want to make it better, not to waste my time on it. |
|
I have merged this into #274 since the tests failing badly otherwise. |
3 participants
Here is the issue reproduction: