diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 390eabd..7ccc0a9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -915,7 +915,10 @@ jobs: # never made. published="$(curl -fsS -H "$auth" "$api/releases/$release")" for name in $names; do - if ! printf '%s' "$published" | grep -qF "\"name\":\"$name\""; then + # grep without -q, which has the same exit status and reads to the end of its input. A + # reader that stops early under pipefail turns the writer's SIGPIPE into this condition's + # answer; see the note in scripts/ci-linux-rpm.sh, where that cost a green build. + if ! printf '%s' "$published" | grep -F "\"name\":\"$name\"" >/dev/null; then echo "The release was created but $name is not on it:" >&2 printf '%s\n' "$published" >&2 exit 1 @@ -1226,7 +1229,10 @@ jobs: published="$(curl -fsS -H "$auth" "$api/releases/tags/$TAG")" for name in releases.win-nightly.json DodoSSH.Desktop.Nightly-win-nightly-Setup.exe; do - if ! printf '%s' "$published" | grep -qF "\"name\":\"$name\""; then + # grep without -q, which has the same exit status and reads to the end of its input. A + # reader that stops early under pipefail turns the writer's SIGPIPE into this condition's + # answer; see the note in scripts/ci-linux-rpm.sh, where that cost a green build. + if ! printf '%s' "$published" | grep -F "\"name\":\"$name\"" >/dev/null; then echo "The release was created but $name is not on it:" >&2 printf '%s\n' "$published" >&2 exit 1 @@ -1279,7 +1285,8 @@ jobs: # upload it never made. withRpm="$(curl -fsS -H "$auth" "$api/releases/$release")" for name in $linuxNames; do - if ! printf '%s' "$withRpm" | grep -qF "\"name\":\"$name\""; then + # grep without -q; see the sibling loops above and scripts/ci-linux-rpm.sh. + if ! printf '%s' "$withRpm" | grep -F "\"name\":\"$name\"" >/dev/null; then echo "The release was created but $name is not on it:" >&2 printf '%s\n' "$withRpm" >&2 exit 1 diff --git a/scripts/ci-linux-rpm.sh b/scripts/ci-linux-rpm.sh index 15e0d97..f12e98f 100755 --- a/scripts/ci-linux-rpm.sh +++ b/scripts/ci-linux-rpm.sh @@ -223,6 +223,25 @@ echo "==> $(basename "$rpm_file"), $(wc -c < "$rpm_file") bytes" # # Everything above can succeed and still produce something nobody can install, and none of the ways # that happens are visible in an exit code. Each check below is one of them. +# +# ◆ EVERY READER IN A PIPELINE HERE MUST CONSUME ALL OF ITS INPUT, AND THAT IS NOT A STYLE RULE. +# +# `set -o pipefail` at the top of this file means a pipeline's status is the rightmost non-zero one. +# A reader that stops early — `grep -q`, `head -n` — closes the pipe while the writer is still +# writing, the writer dies of SIGPIPE with status 141, and pipefail hands that 141 to the `if`. So a +# check that found what it was looking for reports the opposite, and a diagnostic printed on the way +# to `exit 1` kills the script under `set -e` before it gets there. +# +# It is a threshold rather than a constant, which is what made it look like somebody else's bug: the +# writer only blocks once it has more than a pipe buffer left to write, about 64 KB on Linux. `$files` +# is one line per packaged file, so this passed for as long as the published tree was small and then +# failed on the commit that pushed the path text past 64 KB — reporting `/opt/DodoSSH/DodoSSH` missing +# from a package that plainly contained it, and exiting 141 rather than the 1 it meant to. +# +# Hence `grep pattern >/dev/null` rather than `grep -q pattern`, which has the same exit status and +# reads to the end; and `awk 'NR<=40'` rather than `head -40`, which prints the same lines and reads +# to the end. Nothing here is allowed to stop early, and `|| true` is not the fix — it would hide a +# real failure of the writer alongside the imaginary one. requires="$(rpm -qp --requires "$rpm_file")" provides="$(rpm -qp --provides "$rpm_file")" @@ -236,7 +255,7 @@ files="$(rpm -qp --list "$rpm_file")" carried="$(printf '%s\n' "$files" | sed -n 's|^/opt/.*/\(lib[^/]*\.so[^/]*\)$|\1|p' | sort -u)" unsatisfiable="" for soname in $carried; do - if printf '%s\n' "$requires" | grep -qF "$soname("; then + if printf '%s\n' "$requires" | grep -F "$soname(" >/dev/null; then unsatisfiable="$unsatisfiable $soname" fi done @@ -251,7 +270,7 @@ fi # 2. The provides exclusion still doing its job. A package advertising libcoreclr.so to the whole # system is the thing that exclusion exists to prevent, and a future rpm that stopped honouring # the macro would break it silently and in the direction nobody looks. -if printf '%s\n' "$provides" | grep -qE '^lib.*\.so'; then +if printf '%s\n' "$provides" | grep -E '^lib.*\.so' >/dev/null; then echo "This package advertises bundled libraries to the system:" >&2 printf '%s\n' "$provides" | grep -E '^lib.*\.so' >&2 echo "__provides_exclude_from in build/linux/dodossh.spec is meant to stop that." >&2 @@ -262,7 +281,7 @@ fi # without libelf would do exactly that — the package would carry no glibc requirement at all and # would install cheerfully onto a host too old to run it. libc.so.6 is the one dependency a # self-contained .NET tree cannot fail to have. -if ! printf '%s\n' "$requires" | grep -q '^libc\.so\.6'; then +if ! printf '%s\n' "$requires" | grep '^libc\.so\.6' >/dev/null; then echo "This package does not require libc, which a self-contained .NET tree cannot be true of." >&2 echo "The automatic ELF scan produced nothing; rpm here is probably built without elfdeps." >&2 printf '%s\n' "$requires" >&2 @@ -279,9 +298,9 @@ for expected in \ "/usr/share/icons/hicolor/256x256/apps/$bin.png" \ "/usr/lib/udev/rules.d/71-dodossh-tpm.rules" do - if ! printf '%s\n' "$files" | grep -qxF "$expected"; then + if ! printf '%s\n' "$files" | grep -xF "$expected" >/dev/null; then echo "The package was built and $expected is not in it." >&2 - printf '%s\n' "$files" | head -40 >&2 + printf '%s\n' "$files" | awk 'NR<=40' >&2 exit 1 fi done