disposition of std pipes should be notes on the header and keepStdinOpen should default to True

[zooko]

This is two issues, but they both arose from the same use and are related:

The darcs developers are getting their first buildbot going --  http://darcs.net:8008 -- and they have a problem with some 'make test' (running unit tests written in Perl) halting and eventually timing out when invoked from a buildslave, but not when invoked from the command-line. If the disposition of the std pipes was noted in the header, it would have help us notice the existence of the keepStdinOpen option earlier, and if the default value were True, then there would be one less difference for us to wonder about between the buildslave invocation of make test and the local user interactive invocation of make test.

[zooko]

Here is the mailing list discussion:

 http://lists.osuosl.org/pipermail/darcs-devel/2008-March/007525.html

David Roundy had the feeling that buildbot could be changing any number of things about the way it invoked 'make test' and we wouldn't be able to tell how the invocation differed. The first of the two changes suggested in this ticket might have helped him overcome that feeling.

[warner]

Sounds reasonable. stdout and stderr are always just gathered, but the useful things to display are:

• how much data we're sending in on stdin (this is mainly used for 'patch')
• whether we're closing stdin or not
• whether we're using a PTY or not

I've had other problems recently with closing the input side of a PTY causing a SIGHUP to be sent in to the process group, killing off the subprocess. So I'm trying to figure out when exactly it is useful to use a PTY, and (orthogonally) when it is useful to close stdin.

• running a single command that takes input (like 'patch'): no PTY, close stdin
• running a single command that doesn't take input: no PTY, leave stdin open? (shouldn't matter)
• running 'make' or a script that spawns children: yes PTY, leave stdin open

This makes me think that usePTY should be a per-step argument, and that we should never close stdin when we're using a PTY. Also, if we pass anything into stdin, then we should not use a PTY. I'm not sure that PTYs make enough sense to the average programmer that they'd be able to know what to do with a usePTY= option to ShellCommand, so maybe it should just default to False, and when someone wants to find out how to clean up better after child processes, they can discover what conditions need to hold to set it to True safely.

The changes to make usePTY= as an argument are too big for 0.7.7, but the notification parts are doable.

[warner]

[b83038dcb44b32b25351121e12ba3d27611671f7] contains the logging changes. I need to push the rest out to the next release.

[zooko]

I just set up a buildslave on Ubuntu Hardy using the latest Twisted, Buildbot, etc., and I got the "immediate hangup when trying to do a cp -rf" behaviour. It seems to be sporadic. Anyway, I went and changed usepty to False in my buildbot.tac. Just a reminder that we should set the default to False in buildbot.

related tickets: #77, #158, #255

[dustin]

Conceptual +1 from me!

Dan Locks and I spend an inordinate amount of time trying to "simulate" bbot's means of running steps. The more information in the header, the merrier!

[zooko]

Here's another case of a user being unable to get his buildslave working after poking at it every now and then for a couple of weeks, and it turning out to be the usePTY=1 default causing the problem:

 http://lists.osuosl.org/pipermail/darcs-users/2008-May/012312.html

[zooko]

Correction: that turns out not to have been that user's problem:

 http://buildbot.darcs.net

shows that he still has sporadic network failure

[cmetcalf]

#284 also related

[dustin]

Brian basically solved this last march. All that's left is in #158