Enable correlation of stdout and stderr in ProcessRunner
- Dominant language
- C#
- Stars
- 4.2k
- Forks
- 778
- Avg merge
- 1h 15m
- Merged PRs (30d)
- 19
Description
The way `ProcessRunner` is implemented right now, redirected stdout and stderror cannot be correlated. The output is [buffered into two different queues](https://github.com/cake-build/cake/blob/v0.17.0/src/Cake.Core/IO/ProcessRunner.cs#L113-L140).
The API is designed such that you are forced to consume each collection of lines separately. [`IProcess.GetStandardOutput()` and `GetStandardError()`](https://github.com/cake-build/cake/blob/v0.17.0/src/Cake.Core/IO/IProcess.cs#L33-L43)
This means that if any script or module needs to redirect them, there is no way to piece back together the order of events:
```
[stdout] Executing test A
[stderr] Error message
[stdout] Executing test B
[stderr] Error message
[stdout] Executing test C
```
You will have to choose between showing all error output first or all last:
```
[stdout] Executing test A
[stdout] Executing test B
[stdout] Executing test C
[stderr] Error message
[stderr] Error message
```
This is a particular blocker for #156, allowing parallel execution of tasks. My [output buffering prototype](https://github.com/cake-build/cake/issues/156#issuecomment-272663967) works well except that it's impossible to buffer the correct order of error and output lines without me duplicating and fixing the implementation of `ProcessRunner`.
**The important part** though is that this is an issue you'll want to deal with at some point for sure. You'll have to use something like my buffering prototype to make #156 useful. Also, this affects ordinary modules, some of which need to redirect standard output and errors. The sooner this can be resolved the better.
---
I propose changing [`ProcessRunner`](https://github.com/cake-build/cake/blob/v0.17.0/src/Cake.Core/IO/ProcessRunner.cs#L113-L140) to put both stdout and stderr lines in the same queue with an extra bit to indicate which are stdout and which are stderr.
I propose exposing an `IProcess.GetRedirectedLines()` which returns an `IEnumerable` with `struct RedirectedLine` containing `string Line` and `StreamType Type` with `enum StreamType { StdOut, StdErr }`.
Lastly, I propose using `BlockingCollection` as the actual backing queue. This has immense technical benefits. You call `Add` when stdout or stderr events are raised and you call `CompleteAdding` when the process exit event is raised. It becomes super easy to build an enumerator which actually blocks [without burning CPU like the current enumerator does](https://github.com/cake-build/cake/blob/v0.17.0/src/Cake.Core/IO/ProcessWrapper.cs#L80-L85).
`BlockingCollection.TryTake(out item, Timeout.Infinite)` is the best possible way to handle this. It returns true and an item as soon as one is available, it returns false when the blocking collection is complete and there are no items, and it blocks until one of those two things happens. It's a direct translation to the `MoveNext` behavior you want.
I'm happy to write up a PR if you want to see what this looks like in practice.
Contributor guide
Research direction
Start by reading src/Cake.Core/IO/ProcessRunner.cs and IProcess.cs to understand the separate output queues and public enumeration API, then inspect src/Cake.Core/IO/ProcessWrapper.cs for the current blocking behavior. Compare the proposal with the buffering prototype linked from #156; done means redirected stdout and stderr can be enumerated together in their original order and completion is handled when the process exits.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- tooling
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 35/100