From 1833b82b69663720b497dafc0a877287d633e01c Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 03:57:16 +0000 Subject: [PATCH] Harden the library, and test it under more shells Nine ways a build could break, each with an example that fails without the fix: - A chain killed from outside left no status behind, and `run` waited for one for ever. It notices the process is gone and reports the chain as killed instead. - A chain that ignores SIGTERM held cancellation open for ever. It gets GRACE polls and then SIGKILL. - A caller that set -C lost every streamed line after the first poll, and got a complaint a poll instead of output. - A chain took the build script's stdin, on every shell with job control and on none of the ones without. It reads /dev/null now. - mktemp failing left the library writing `1.log` and `1.code` into the working directory. It says so and stops. - A chain command assigning to `n`, `code` or `_work` moved the file the chain reports itself through, which lost the status or hung the run. Everything internal is prefixed now, which also stops the library clobbering names in the calling script. - Grouped output printed the closing marker onto the end of a last line that never got its newline. - `"$@"` with nothing behind it stopped a chain with a label and no commands under posh. - A box drawing character in the source made the file unreadable to a strict shell in the C locale. The sources are ASCII, and `make lint` keeps them that way. The report gained a `killed` line, `GRACE` is a new setting, and the README example called `chain` without the label it needs. Simpler along the way: the chain's status comes from the shell's own exit status rather than a variable it kept alongside; the two reports share one function for the line a chain ends on; a label and a pid are variables rather than files, which also takes a fork per chain per poll out of the wait loop; `&&` chains whose failure would abort a set -e caller are `if` statements. Added to the matrix: posh, bash --posix and zsh --emulate sh on Linux, zsh on macOS. The harness can drive a shell with options now. yash is not there, because shellspec cannot inspect it, though the library does run under it. --- .github/workflows/ci.yml | 22 ++- Makefile | 12 +- README.md | 94 +++++++-- parallel.sh | 386 ++++++++++++++++++++++++------------- spec/parallel_spec.sh | 401 ++++++++++++++++++++++++++++++++++++++- spec/spec_helper.sh | 51 +++-- 6 files changed, 797 insertions(+), 169 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 342eb34..86b0265 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -33,6 +33,10 @@ jobs: - shell: bash package: bash path: /bin/bash + # bash in POSIX mode, which is what /bin/sh is on a lot of machines. + - shell: bash --posix + package: bash + path: /bin/bash --posix - shell: ksh93 package: ksh path: /bin/ksh93 @@ -42,6 +46,17 @@ jobs: - shell: zsh package: zsh path: /bin/zsh + # zsh emulating sh, which turns on word splitting and the POSIX + # builtins and is a different shell for everything here. + - shell: zsh --emulate sh + package: zsh + path: /bin/zsh --emulate sh + # Debian's policy-compliant shell: no more than POSIX, which is + # what makes it worth asking. It is the one that objected to $@ + # with nothing behind it. + - shell: posh + package: posh + path: /bin/posh - shell: busybox ash package: busybox-static path: /usr/local/bin/ash @@ -56,7 +71,8 @@ jobs: if: matrix.package == 'busybox-static' run: sudo ln -sf /bin/busybox /usr/local/bin/ash - run: make install-dev - - run: make test SHELLSPEC_FLAGS="--shell ${{ matrix.path }}" + # Quoted, because a shell here may come with options after it. + - run: make test SHELLSPEC_FLAGS='--shell "${{ matrix.path }}"' # BSD mktemp and BSD sleep, and a /bin/sh that is bash 3.2 in disguise. macos: @@ -66,11 +82,11 @@ jobs: strategy: fail-fast: false matrix: - shell: [/bin/sh, /bin/bash] + shell: [/bin/sh, /bin/bash, /bin/zsh] steps: - uses: actions/checkout@v7 - run: make install-dev - - run: make test SHELLSPEC_FLAGS="--shell ${{ matrix.shell }}" + - run: make test SHELLSPEC_FLAGS='--shell "${{ matrix.shell }}"' # The benchmarks are a report, and a hosted runner is a noisy place to # take timings on, so the job only fails when a scenario was not faster in diff --git a/Makefile b/Makefile index 7b48b7b..74a44a3 100644 --- a/Makefile +++ b/Makefile @@ -16,15 +16,23 @@ SCRIPTS = parallel.sh examples/build.sh bench/bench.sh # nesting to keep and flattens the whole file to one column. SPECS = spec/parallel_spec.sh spec/spec_helper.sh -.PHONY: check lint test bench install-dev +.PHONY: check lint ascii test bench install-dev check: lint test -lint: +lint: ascii shellcheck --shell=sh $(SCRIPTS) $(SPECS) checkbashisms $(SCRIPTS) shfmt -ln posix -d $(SCRIPTS) +# Nothing here is anything but printable ASCII, comments included. A shell +# reads a script through the locale, and yash in the C locale refuses one +# carrying a byte sequence that locale cannot make a character of, which is +# the locale a build container has when nobody set one. The box drawing bar +# is written as its bytes in the one place it is needed. +ascii: + ! LC_ALL=C grep -n '[^[:print:][:blank:]]' $(SCRIPTS) $(SPECS) + test: @[ -x $(SHELLSPEC) ] || { echo "$(SHELLSPEC) is missing: make install-dev" >&2; exit 1; } $(SHELLSPEC) $(SHELLSPEC_FLAGS) diff --git a/README.md b/README.md index 6e18f84..35768e3 100644 --- a/README.md +++ b/README.md @@ -24,12 +24,15 @@ After: set -e . ./parallel.sh -chain "composer install --no-dev --no-interaction --prefer-dist --optimize-autoloader" -chain "npm ci --audit false" "npm run build" +chain "composer install" \ + "composer install --no-dev --no-interaction --prefer-dist --optimize-autoloader" +chain "npm build" "npm ci --audit false" "npm run build" run ``` +The first argument is what the chain is called; the rest are what it runs. + Drawn out, with the times off a middling Laravel app. The second one finishes when its longest chain does rather than when its last command does: @@ -81,7 +84,7 @@ composer install │ Generating optimized autoload files npm build │ ERR! Build failed in 4.21s [!] npm build exited 1 -[.] assets cancelled +[.] composer install cancelled ``` A line is printed once it is whole, so a command that writes a line in two @@ -121,8 +124,32 @@ that code, it can be the last line of a build script: run ``` -Nothing else is public. `chain` and `run` are meant to be called once each, -in that order. +Nothing else is public. `chain` may be called as often as a build has chains +to declare, and `run` once, after them. + +A chain that did not simply work ends on one of three lines, and both layouts +use the same ones, so a build that greps for one finds it either way: + +- `[!] npm build exited 1` — the chain failed, and this is the code it failed + with. +- `[.] npm build cancelled` — it was still running when another chain failed. +- `[!] npm build killed` — it was taken away before it could report: the + signal nothing can catch, or the machine running out of memory. `run` + returns 137 for it, which is what a shell reports for a child SIGKILL took. + +Some smaller things worth knowing: + +- A chain reads its input from `/dev/null`, so nothing it runs can take what + the build script was going to read. +- `chain` with no label at all writes to stderr and returns 2, which stops a + `set -e` build where it stands. +- The library takes the `EXIT`, `INT` and `TERM` traps for itself, which is + how it takes the chains down with it and clears up after them. A build with + traps of its own should set them before sourcing, and expect them to be + replaced. +- Every name it defines starts with an underscore, apart from `chain`, `run` + and the settings below. Nothing else in the calling script or in a chain's + commands is touched. ## POLL @@ -143,6 +170,22 @@ depending on where it runs. The environment works everywhere too, so `POLL=0.5 ./build.sh` on a script that sources the library is the other way to do it. +## GRACE + +A cancelled chain is asked to stop and then waited for, so that it is gone +before the process group it leads is taken down and there is nobody left to +announce the killing. `GRACE` is how many polls it gets before the signal +nothing can ignore, a hundred of them by default, which at the default `POLL` +is about ten seconds: + +```sh +GRACE=300 +. ./parallel.sh +``` + +Without a bound, a build step that ignores `SIGTERM` would hold `run` open +with no way out but Ctrl-C. + ## STREAM Output is labelled and printed as it arrives. `STREAM=0` holds each chain's @@ -163,9 +206,6 @@ Generating optimized autoload files added 214 packages building for production... [!] npm build exited 1 - -... assets -[.] assets cancelled ``` `STREAM_SEP` is the bar between a label and its line. It defaults to `│` @@ -249,15 +289,45 @@ Fractional `sleep` is not in POSIX, though both GNU coreutils and BSD accept it. Whole seconds are the fallback, not the default, so a build on a shell without fractional sleep finishes up to a second later than it might. +The file itself is ASCII, comments included, and the box drawing bar is +written as its bytes in the one place it is needed. A shell reads a script +through the locale, and a strict one in the C locale refuses a file carrying +a byte sequence that locale cannot make a character of, which is the locale a +build container has when nobody has set one. `make lint` checks it. + Under zsh, a script with `set -e` whose `run` fails leaves the temp directory behind: zsh skips `EXIT` traps when errexit is triggered by a function returning non-zero. Every other shell, and every other exit path under zsh, removes it. -The library has no `local`, because POSIX sh has none. It keeps its own -state in names starting with `_`, but its loops use `i`, `n`, `cmd`, `code`, -`label`, `mark`, `seen`, `line` and `pending`, and sourcing it will clobber -those in the calling script. +The library has no `local`, because POSIX sh has none. Every name it uses +starts with an underscore instead, including its loop counters, so a calling +script and a chain's commands can use whatever they like as long as it does +not start with one. + +A chain that something else kills is announced by bash as well as by the +report: bash prints a line about the job to stderr when it reaps one that a +signal ended, and there is no asking it not to. No other shell here says +anything, and it only happens when a chain is taken from outside, which is +worth a line either way. + +## Shells + +The suite runs under every shell below, on every push. The library is written +for what they have in common, which is POSIX and not much more: + +| Where | Shells | +| --- | --- | +| Linux | dash, bash, bash `--posix`, ksh93, mksh, zsh, zsh `--emulate sh`, posh, busybox ash | +| macOS | `/bin/sh` (bash 3.2), bash, zsh | +| FreeBSD | `/bin/sh` | + +posh is Debian's policy-compliant shell, which is POSIX and deliberately +nothing else; it is the one that objected to `"$@"` with no arguments behind +it. bash in POSIX mode and zsh emulating sh are there because that is what +`/bin/sh` is on a good many machines, and both differ from their own default +mode in ways this library can feel. yash is not in the list: it is stricter +still, and the library does run under it, but shellspec cannot drive it here. ## Prior art diff --git a/parallel.sh b/parallel.sh index 300cd00..7557536 100644 --- a/parallel.sh +++ b/parallel.sh @@ -1,6 +1,6 @@ #!/bin/sh # shellcheck shell=sh -# Run chains of commands in parallel. POSIX sh — no bashisms, no arrays. +# Run chains of commands in parallel. POSIX sh: no bashisms, no arrays. # # chain