buildbot/buildbot

add option to join stdout and stderr for ShellCommand

Open

#2999 opened on Mar 9, 2017

View on GitHub
 (6 comments) (0 reactions) (0 assignees)Python (5,101 stars) (1,650 forks)batch import
easyfeaturegood first issuestep

Description

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.

Contributor guide

Tech stack
python
Domain
ci cdbackend
Issue type
feature
DifficultyEstimated implementation difficulty for a new contributor, from 1 for very small changes to 5 for expert-level work.
2
Estimated timeA rough time range for an experienced contributor to investigate, implement, test, and prepare a pull request.
1-3 hours
Activity statusHow available the issue appears right now: fresh, active, stale, blocked, or waiting on maintainer input.
fresh
ClarityHow clearly the issue explains the expected change, acceptance criteria, and next step.
mostly clear
Prerequisites
Basic Python knowledgeUnderstanding of subprocess streams
Newbie friendlinessA 1-100 score estimating how approachable this issue is for first-time contributors.
75
Research direction
The issue requests a new option, `merge stderr` or `merge streams`, for ShellCommand that would cause stdout and stderr to be combined into a single stream (like 2>&1). To implement, look at the ShellCommand class (likely in master/buildbot/steps/shell.py) and the run method where the process is spawned. Currently, the step uses `spawnProcess` or direct subprocess calls with separate stdout/stderr. Adding a parameter that sets `stderr=subprocess.STDOUT` when true would achieve this. Ensure compatibility with existing `want stdout` and `want stderr` options. Consider documenting the behavior and any trade offs regarding stream coloring. Test with a simple build step that outputs to both streams and verify chronological ordering.