<div dir="ltr">Ok, so on the structure of the patch:<div><br></div><div>1. It would be nice if this functionality wasn't in the Libcxx test format, but a utility that was part of lit proper.</div><div><br></div><div>2. This:</div><div><div>--</div><div>+            out = LibcxxTestFormat._read_and_close_pty(out_pty)</div><div>+            err = LibcxxTestFormat._read_and_close_pty(err_pty)</div></div><div>--</div><div>is not safe, if a command writes enough output to stderr that the pipe is full it will block, and then the system will be deadlocked since it will never finish writing to stdout. Also, you probably shouldn't assume that all data is returned in a single .read() call (even though that is likely true). See notes in subprocess module, e.g., <a href="https://docs.python.org/2/library/subprocess.html#subprocess.Popen.communicate">https://docs.python.org/2/library/subprocess.html#subprocess.Popen.communicate</a>.</div><div><br></div><div> - Daniel</div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Oct 27, 2014 at 8:00 PM, Eric Fiselier <span dir="ltr"><<a href="mailto:eric@efcs.ca" target="_blank">eric@efcs.ca</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Only use pseudo-terminal if `sys.stdout.isatty()` is true. Otherwise fall back to the old method.<br>
<br>
<a href="http://reviews.llvm.org/D6010" target="_blank">http://reviews.llvm.org/D6010</a><br>
<br>
Files:<br>
  test/lit.cfg<br>
</blockquote></div><br></div>