From 31c836f6cac27aa2095378b1d9d12df234ee8f7c Mon Sep 17 00:00:00 2001 From: Laurens Bezemer | DodoTech Date: Wed, 26 Aug 2026 15:02:58 +0200 Subject: [PATCH] Stop the RPM verification failing on packages that are correct MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI run 161 failed the linux packaging step with The package was built and /opt/DodoSSH/DodoSSH is not in it. and then listed the package contents, which contain /opt/DodoSSH/DodoSSH. The package was fine. The check was not. ◆ A READER THAT STOPS EARLY, UNDER pipefail. if ! printf '%s\n' "$files" | grep -qxF "$expected"; then grep -q exits on its first match. printf is still writing thousands of paths, so it dies of SIGPIPE with status 141; pipefail makes 141 the pipeline's status; `!` inverts it; and a check that found what it was looking for reports the opposite. The exit code names the second instance of the same mistake. The diagnostic on the way out is printf '%s\n' "$files" | head -40 >&2 exit 1 and head stops at forty lines, so printf takes SIGPIPE again and `set -e` ends the script there. Which is why the job exited 141 rather than the 1 the script asked for, and why the listing in the log stops mid-alphabet. ◆ WHY IT LOOKED LIKE SOMEBODY ELSE'S COMMIT. It is a threshold, not a constant. The writer only blocks once it has more than a pipe buffer left to write — about 64 KB on Linux — so for as long as the published tree was small the whole list fitted, printf finished, and the pipeline returned grep's own 0. The check has been wrong since it was written and correct in effect until the path text crossed 64 KB, which is what the commit before this one did. Reproduced both sides of the threshold locally: 4,440 bytes -> pipeline status 0 90,940 bytes -> pipeline status 141 Every reader in that block now consumes all of its input: `grep pattern >/dev/null` in place of `grep -q pattern`, same exit status, reads to the end; and `awk 'NR<=40'` in place of `head -40`, same forty lines, reads to the end. `|| true` would have silenced the symptom and taken a real failure of the writer with it. The other three checks in the same block had the same shape with small inputs — requires and provides are dozens of lines — so they were latent rather than failing. Fixed anyway, because the next thing to grow is not predictable from here. Same for the three publish-verification loops in ci.yml, which grep a release's JSON for each asset name it should carry. Small today; the loops exist because a run once created a release, uploaded nothing and reported success, so a false answer from them is the one thing they must not give. `ensure skia's native dependency` is left alone deliberately. `ldconfig -p | grep -q` is the same shape over an input that genuinely exceeds a pipe buffer, but that step sets `-eu` without `pipefail`, so the pipeline's status is grep's and the SIGPIPE is invisible. Changing it would be churn; the reason it is safe is worth knowing. Verified by lifting the patched loop out of the script and running it against a synthetic 89 KB listing: complete package passes, a package genuinely missing /opt/DodoSSH/DodoSSH exits 1 with its forty lines of diagnostic intact. The real proof is the next run of the job. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 13 ++++++++++--- scripts/ci-linux-rpm.sh | 29 ++++++++++++++++++++++++----- 2 files changed, 34 insertions(+), 8 deletions(-) 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