buildbot icon indicating copy to clipboard operation
buildbot copied to clipboard

add option to join stdout and stderr for ShellCommand

Open tardyp opened this issue 8 years ago • 6 comments

This ticket is a migrated Trac ticket 3557

People contributed to the original ticket: kparal@..., @rutsky Ticket created on: Jun 02 2016 Ticket last modified on: Jun 07 2016


Current implementation of [[ShellCommand]] splits stdout and stderr. That has a benefit of being able to color streams, and the drawback of not keeping exact chronological ordering (which seems impossible to achieve with split streams, according to my research).

In our project, we value chronological ordering more than stderr coloring. That's why we wanted to keep stdout and stderr in a single stream (as bash would do by default). We tried to use usePTY=True for that, and it works, however [[your documentation|http://docs.buildbot.net/latest/manual/cfg-buildsteps.html#using-shellcommands]] says:

Using a pseudo-terminal brings lots of compatibility problems and indeed, we encountered [[a weird issue involving doubled newlines|https://phab.qadevel.cloud.fedoraproject.org/T799]].

We don't really want to deal with weird PTY issues, and honestly using PTY is just a workaround for [[BuildBot]] splitting streams by default without any option to disable it. It's an overkill solution.

Another solution is to append 2>&1 to the executed command, but that means we need to run the command through shell (instead of execute it directly) which brings a lot of headaches with shell escaping and security. Again, a lot of unnecessary trouble just to work around a missing feature.

You already have want_stdout and want_stderr options, so you offer some configuration in this reagard. Please implement a new option named e.g. merge_stderr or merge_streams which would not split streams when starting the process. It would essentially do the same as

subprocess.call(args, stderr=subprocess.STDOUT)

That's all we need. It does not split streams (nor color them, obviously), it keeps chronological ordering, it does not need to allocate PTY with its compatibility issues, it does not bring headaches with shell escaping, and it should be hopefully very simple to implement.

Thank you.


tardyp avatar Mar 09 '17 16:03 tardyp

+1 hard here.

karolyi avatar Mar 19 '17 19:03 karolyi

i can do this if you would assign it to me. thanks

samuelgregorovic avatar Jun 24 '21 19:06 samuelgregorovic

@p12tic is this issue still relevant?

aqeelat avatar Jul 01 '23 12:07 aqeelat

I think yes, we don't have this option as far as I see.

p12tic avatar Jul 02 '23 20:07 p12tic

Is this still an issue? I'd like to work on it.

urfriendglenn avatar Dec 23 '23 01:12 urfriendglenn