After Bug 29316, bugzilla-tool post-diff inserts unnecessary stderr outputs into patches. Maybe we should add an optional parameter into SCM.run_command to determine how to handle stderr and use the parameter in create_patch()?
http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/modules/scm.py#L133 is the line which is "wrong". We should not be piping stderr to stdout but rather we should rout it to a separate buffer (including to system.stderr) in some circumstances.
Created attachment 43419 [details] proposed patch
Do we really need to proliferate [0] all over the code? That's pretty ugly and error-prone.
No, we don't. We can give back only the stdout by default (with run_command). Currently we use stderr output only in one unittest. So this is also a possible way. If it's better I'll file a new patch at the morning.
Yeah, I think we should just go back to teh "just return stdout" model. The one problem with this new implementation is that places which want both stderr and stdout ouput can't get them interlaced anymore. :( I think that if scripts like run-webkit-tests fail we want to report both outputs. Although I'm not sure that any of our scripts use both, so we could go with this change for now and see what harm it does. I also need to fix run_command to know how to echo to the current stdout and stderr as well.
By the way, we can use [stderr] and [stdout] instead of [1], [0]. It's more clearly.
Created attachment 43493 [details] other solution
Comment on attachment 43493 [details] other solution I think we should do this.
Comment on attachment 43493 [details] other solution Clearing flags on attachment: 43493 Committed r51230: <http://trac.webkit.org/changeset/51230>
All reviewed patches have been landed. Closing bug.
Comment on attachment 43493 [details] other solution So the behavior of this is that stderr will now go to the parent's stderr. Which is OK. And might actually be better. But if you intended for None to mean /dev/null, that is not the POpen behavior. :) I think this is better than what we had before. I think that some of our callsites definitely want the return_stderr behavior. run-webkit-tests calls being one such callsite.