Add streaming output and improve process group handling - #3
Merged
Merged
Conversation
Two things a build wants that this did not do: cancelling a chain left the
compiler that chain had started still running, and nothing was printed
until a chain had finished.
Cancelling now kills the chain's process group rather than its shell alone,
which takes down what the chain started however deep it goes. Job control
is what puts a job in a group of its own, and a non-interactive shell is
allowed not to have any: `set -m` is accepted and ignored by dash and
busybox ash, and zsh refuses it outright, and fatally, without a terminal
to hand a group the foreground with. So the library asks rather than
assumes. At the first `chain` it starts one job under `set -m` and asks
whether a process group by that pid exists: a job that was given its own
group leads it, a job that was not is in the shell's group, and nothing
else can hold that id while the job itself holds the pid, so `kill -0` is
the whole of the test and `ps` is not needed for it. bash, ksh93 and mksh
pass it, zsh passes it when it has a terminal, and dash and busybox ash do
not and get exactly what they had before.
The probe runs in a subshell for zsh's sake, since a special built-in that
fails takes a non-interactive shell down with it and would end the build
script rather than report that job control is not available. Monitor mode
is only on across the fork itself, because that is when the group is
decided and because this is the calling script's shell; the chain turns it
off in itself, so that the background commands it runs stay in the group
that cancelling kills. The kill goes to the chain first and to the group
once the chain has gone: a chain still there to see its own command killed
reports it, which put a `Terminated` in the middle of a cancelled chain's
output under bash and mksh and a line about the job from ksh93. The exit
trap skips that wait, because nothing it kills will be printed and a chain
that made itself deaf to the signal must not be able to hold the trap open.
STREAM=1 prints every line as it arrives instead, under the label of the
chain that wrote it, right aligned so the bars line up:
npm │ added 214 packages in 8s
composer │ Installing dependencies from lock file
The poll that watches for finished chains reads the logs on the same trip
round, so the one process already doing the printing is the only one that
prints and two chains cannot mix into one line. A chain's position is a
count of lines: `tail` starts from the line after it, a line with no
newline yet is left where it is until it has one, and the last line of a
chain that ended without one goes out at the end. The report is then one
line a chain, since the output itself has already gone by. STREAM_SEP is
the bar, and follows the locale: a box drawing character where it says
UTF-8 and an ASCII pipe where it does not.
The README's `POLL=0.5 . ./parallel.sh` was wrong in two of the shells this
supports. An assignment in front of a special built-in persists in a POSIX
shell and is undone in bash and zsh, so the setting was quietly dropped
there; it now says to set it before sourcing, or in the environment.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WWgsYpG4sTSefc6FQURumD
The FreeBSD runner failed the example that asks whether cancelling a chain takes what the chain started with it. The kill was not the problem: the probe was answering for a subshell. `set` is a special built-in, and a special built-in that fails takes a non-interactive shell down with it, so the probe ran in a subshell to keep zsh's refusal of `set -m` from ending the build script. But a subshell is a different place to ask from. mksh already showed that much — it reports the group is there and then will not kill it — and FreeBSD's sh answered for the subshell what was not true of the script, so the library believed it had process groups, did not, and left the chain's children running. The example ran because the spec asked the same question its own way and got the other answer. Only the question of whether the option can be set at all needs a subshell now, and the rest of the probe runs where the chains will be started, which is the only place whose answer matters. The spec no longer keeps a copy of the probe either: it asks the library, so the two cannot disagree about which shells the example is for. Two shells had something to say about all this on the way past — dash announces that it has no terminal for job control, and job control announces the job the probe starts — and neither is the build script's news. The chain drops job control on what the library turned on rather than on what `$-` reports, since dash leaves `m` out of `$-` with monitor mode set and the ash-derived shells it is one of are exactly the ones that would be wrong about. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WWgsYpG4sTSefc6FQURumD
A build is easier to follow while it is running than after it has finished, so labelled output is what `run` does now and grouping is what a build asks for. `STREAM=0` gets the old behaviour: nothing printed until a chain has finished, and nothing a chain wrote anywhere but under its own heading. The report a streamed run ends with says how each chain ended, and now says it in the lines the grouped report ends a chain on: `--- label` for a chain that finished, `[!] label exited 3` for one that failed, `[.] label cancelled` for one that was cancelled. A build that greps its output for one of them finds it whichever way the output was printed. The specs follow the default: the two opening examples are what a build sees without asking for anything, and grouped output has a Describe of its own that asks for it. Two of them assert on lines rather than on a pattern of several parts, which is a better assertion here anyway — grouped output is in declaration order all the way down, so every line of it is known — and avoids ksh93, which would not match such a pattern against a subject the length of a report. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WWgsYpG4sTSefc6FQURumD
A streamed run ended on a line a chain, and the line for a chain that finished was a heading with nothing under it: `--- npm` reads as the start of a block in grouped output, and in streamed output the block is already above it, spread through everything else that was running at the time. What is worth saying at the end is what the output does not already say: which chain failed, with what status, and which chains were cancelled with it. A run where nothing failed now ends on its last line of output, blank line included, since there is nothing to put under it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WWgsYpG4sTSefc6FQURumD
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This change adds streaming output support to
parallel.shand improves how cancelled chains and their child processes are terminated. The library can now print each line of output as it arrives (labeled by chain) instead of holding all output until chains complete, and it properly kills entire process groups when cancelling chains on shells that support job control.Key Changes
Streaming output mode: Added
STREAMenvironment variable to enable line-by-line output printing as chains run, with labels right-aligned for visual alignment. Output is controlled bySTREAM_SEP(defaults to│in UTF-8 locales,|otherwise).Process group handling: Implemented job control probing (
_probe_groups) to detect whether the shell can put background jobs in their own process groups. When supported, chains are started withset -mto ensure they get their own process group, allowing_kill_chainto terminate the entire group and all descendant processes.Improved chain termination: Replaced simple
killcalls with_kill_chainfunction that:Output handling refactoring:
_pumpto read new output during polling_flushto capture remaining output after chains finish_emitto label and print individual lines_prefixesto calculate label padding_reportto show summary-only output in streaming modeJob control management: Carefully manages
set -macross the parent script and chain subshells to ensure chains get process groups while the parent script's own job control state is preserved.Notable Implementation Details
set -mfailures from terminating the build scripthttps://claude.ai/code/session_01WWgsYpG4sTSefc6FQURumD