From f36ee29818eb4c863035f4da99e1f43c55d1164f Mon Sep 17 00:00:00 2001 From: VizzleTF Date: Mon, 14 Sep 2026 13:36:34 +0300 Subject: [PATCH 1/5] fix(land-updates): fail closed on failed reads inside land land runs as the left side of '|| echo', so POSIX disables errexit for every command in it. A failing 'gh pr list' left pr empty and a branch whose pull request was waiting for a person was fast-forwarded onto main. A failing 'git diff --name-only' in superseded read as 'adds nothing' and deleted the branch; failing fetches continued on stale refs. Each read now checks its own status and refuses to land (or keeps the branch). test-land-updates.sh covers gh pr list, git diff and git fetch failing, and lands the same branch once nothing fails. --- tools/land-updates.sh | 61 ++++++++++++++++++++++--- tools/test-land-updates.sh | 92 +++++++++++++++++++++++++++++++++++++- 2 files changed, 145 insertions(+), 8 deletions(-) diff --git a/tools/land-updates.sh b/tools/land-updates.sh index c8f6876..8c1d146 100755 --- a/tools/land-updates.sh +++ b/tools/land-updates.sh @@ -100,7 +100,14 @@ superseded() { _base="$(git merge-base "refs/remotes/origin/main" "$_sha" 2>/dev/null || true)" [ -n "$_base" ] || return 0 - _own="$(git diff --name-only "$_base..$_sha")" + # Checked by hand, because errexit is off in here: this runs inside `if` and + # inside `land || echo`. A failed diff prints nothing, and nothing is the answer + # one line down that deletes the branch. Said on stderr so stdout stays empty, + # which the caller reads as "keep". + if ! _own="$(git diff --name-only "$_base..$_sha")"; then + echo "cannot tell what $_sha changes: 'git diff --name-only $_base..$_sha' failed; keeping it" >&2 + return 0 + fi [ -n "$_own" ] || { echo "it adds nothing on top of where it left main"; return 0; } # One `git diff` per path, not one call with the whole list: an unquoted list @@ -152,7 +159,18 @@ land() { # Captured, never piped into a test: a pipeline reports its last command, so a # failing `gh pr list` reaching `grep` reads as "no pull request is open" -- # the one answer that makes this script push. - pr="$(gh pr list -R "$SELF" --head "$branch" --state open --json number -q '.[0].number')" + # + # And its status checked right here, because capturing is not enough on its + # own. `land` is the left side of `|| echo` below, and POSIX turns errexit off + # for every command in it: a failed call left `pr` empty and the function + # carried on to push a branch whose pull request was waiting for a person. + # Reproduced with a stub `gh` that exits 1 -- tools/test-land-updates.sh. + if ! pr="$(gh pr list -R "$SELF" --head "$branch" --state open --json number -q '.[0].number')"; then + echo "$branch: could not read its pull requests, so it is not landed this run" + echo " failed: gh pr list -R $SELF --head $branch --state open" + echo " a branch waiting for a person looks like any other until this answers; the next run asks again" + return 1 + fi if [ -n "$pr" ]; then echo "$branch: pull request #$pr is open; a person merges that one" return 0 @@ -175,8 +193,15 @@ land() { # What the required contexts say about THIS commit. Check runs bind to a # commit rather than to an event, so the run `check-updates.sh` dispatched on # the branch reports against the same sha that is about to be pushed. - checks="$(gh api "repos/$SELF/commits/$sha/check-runs?per_page=100" \ - -q '.check_runs[] | "\(.status)/\(.conclusion // "pending")\t\(.name)"')" + # A failed read already falls the safe way -- no runs seen is "no run" -- but it + # would say so as "not green yet", which sends whoever reads the log to the + # checks instead of to the API call that failed. + if ! checks="$(gh api "repos/$SELF/commits/$sha/check-runs?per_page=100" \ + -q '.check_runs[] | "\(.status)/\(.conclusion // "pending")\t\(.name)"')"; then + echo "$branch: could not read its check runs, so it is not landed this run" + echo " failed: gh api repos/$SELF/commits/$sha/check-runs" + return 1 + fi # Every run of a required name has to be completed and successful, not just # the newest one. A cancelled run stays on the commit and GitHub has been @@ -210,8 +235,21 @@ land() { # `main` is read per branch, not once per run: an earlier branch in this same # loop may already have landed, and the fast-forward test below has to be # against where `main` is now rather than where it was when the job started. - git fetch -q origin "+refs/heads/main:refs/remotes/origin/main" - git fetch -q origin "+refs/heads/$branch:refs/remotes/origin/$branch" + # + # Each fetch checked by hand (errexit is off in here, see `gh pr list` above). A + # failed fetch leaves the refs where the last one put them, and every test below + # -- the branch did not move, `main` is its ancestor, the paths it adds -- would + # then be answered about a repository that may no longer exist. + if ! git fetch -q origin "+refs/heads/main:refs/remotes/origin/main"; then + echo "$branch: could not fetch main, so it is not landed this run" + echo " failed: git fetch origin +refs/heads/main:refs/remotes/origin/main" + return 1 + fi + if ! git fetch -q origin "+refs/heads/$branch:refs/remotes/origin/$branch"; then + echo "$branch: could not fetch the branch, so it is not landed this run" + echo " failed: git fetch origin +refs/heads/$branch:refs/remotes/origin/$branch" + return 1 + fi head="$(git rev-parse "refs/remotes/origin/$branch")" if [ "$head" != "$sha" ]; then # The branch moved between the listing and the fetch. The green contexts @@ -256,7 +294,11 @@ land() { # deletes the branch. Kept as a guard rather than as a branch of logic: an # empty `files` would make the pattern check below vacuously true, and "no # paths to object to" must never read as "allowed". - files="$(git diff --name-only "refs/remotes/origin/main..$sha")" + if ! files="$(git diff --name-only "refs/remotes/origin/main..$sha")"; then + echo "$branch: could not list the paths it changes, so it is not landed this run" + echo " failed: git diff --name-only refs/remotes/origin/main..$sha" + return 1 + fi if [ -z "$files" ]; then echo "$branch: nothing to land against main" return 0 @@ -301,6 +343,11 @@ fi # branch and not every branch after it. Under plain `set -eu` the first one would # take the job down with the rest unread, which is the failure mode this repository # keeps re-learning (see the `checkout -` note in check-updates.sh). +# +# The price of `||`: POSIX turns errexit off for everything `land` runs, so nothing +# inside it stops on its own. Every step there whose failure would read as a +# harmless answer checks its own status and returns -- keep it that way when +# adding one. printf '%s\n' "$refs" | while read -r sha ref; do branch="${ref#refs/heads/}" land "$branch" "$sha" || echo "$branch: not landed this run (the step above failed)" diff --git a/tools/test-land-updates.sh b/tools/test-land-updates.sh index e258ce3..7f568fb 100755 --- a/tools/test-land-updates.sh +++ b/tools/test-land-updates.sh @@ -120,7 +120,12 @@ set -eu case "${1:-} ${2:-}" in "pr list") # No pull request is open on any branch here. The human gate has its own - # reason to exist and is not what this test is about. + # reason to exist and is not what this test is about -- except that a call + # which FAILS must never read as "none is open", which the flag below pins. + if [ -f "$GH_STATE/pr-list-fails" ]; then + echo "HTTP 502: Bad Gateway" >&2 + exit 1 + fi ;; "api "*"/git/matching-refs/heads/update/") cat "$GH_STATE/refs" @@ -217,6 +222,91 @@ else no "the second run reports $lines line(s); it should report delta and nothing else" fi +# FAILURES THAT MUST NOT LAND OR DELETE. `land` runs as the left side of `|| echo`, +# and POSIX switches errexit off for every command inside it, so a step that fails +# carries on with an empty answer unless it checks its own status. Each empty answer +# below is the one that does damage: no pull request (push past a person), nothing +# changed (delete the branch), refs that were never refreshed (push on stale state). +# +# One green, landable branch throughout, re-pushed before each run so a case does not +# depend on how the previous one ended. The last run lands it with nothing failing, +# which is what makes "it was kept" mean the failure kept it. +git checkout -q -b update/gamma-1.2.0 refs/remotes/origin/main +pin gamma 1.2.0-r1 +save "gamma: 1.1.0 -> 1.2.0" +gamma2="$(git rev-parse HEAD)" +git checkout -q main +offer() { git push -q origin "$gamma2:refs/heads/update/gamma-1.2.0"; } + +unmoved() { + head="$(git ls-remote origin refs/heads/main | cut -f1)" + if [ "$head" = "$gamma" ]; then + ok "main did not move" + else + no "main moved to $head; it had to stay at $gamma" + fi +} + +# A git that refuses one kind of call, for the run that sets the flag. Only the +# script under test sees it: run() is the one place $work/bin goes on PATH. +realgit="$(command -v git)" +cat >"$work/bin/git" <&2 + exit 128 + ;; + esac +fi +exec "$realgit" "\$@" +SHIM +chmod +x "$work/bin/git" + +echo "--- third run: gh pr list fails" +offer +: >"$GH_STATE/pr-list-fails" +listing +run "$work/out3" +rm -f "$GH_STATE/pr-list-fails" +kept update/gamma-1.2.0 +unmoved +logged "$work/out3" "update/gamma-1.2.0: could not read its pull requests" + +echo "--- fourth run: git diff --name-only fails" +offer +echo "--name-only" >"$GH_STATE/git-fails" +listing +run "$work/out4" +rm -f "$GH_STATE/git-fails" +kept update/gamma-1.2.0 +kept update/delta-3.0.0 +unmoved +logged "$work/out4" "cannot tell what" + +echo "--- fifth run: git fetch fails" +offer +echo "fetch" >"$GH_STATE/git-fails" +listing +run "$work/out5" +rm -f "$GH_STATE/git-fails" +kept update/gamma-1.2.0 +unmoved +logged "$work/out5" "update/gamma-1.2.0: could not fetch" + +echo "--- sixth run: nothing fails" +offer +listing +run "$work/out6" +head="$(git ls-remote origin refs/heads/main | cut -f1)" +if [ "$head" = "$gamma2" ]; then + ok "the same branch lands once nothing fails" +else + no "main is $head, expected $gamma2: the branch kept above was not landable anyway" +fi + if [ "$result" = 0 ]; then echo "PASS" else From 34289c2e175ff4d053621a232e52060aa51ee356 Mon Sep 17 00:00:00 2001 From: VizzleTF Date: Mon, 14 Sep 2026 13:36:34 +0300 Subject: [PATCH 2/5] fix(check-updates): refuse automerge when git reads fail may_automerge runs as an if condition with errexit off. 'git log | wc -l' counted a failed git as 0 recent updates, and a failed 'git diff' piped through '|| true' read as 'only pins moved'; both allowed automerge. Capture git output first and return 1 on failure. In the binaries shape 'awk ... && mv' exempted a failing awk from errexit and committed VERSION/TAG with stale checksums; exit instead. Take sha256sum into a variable so its failure is not hidden inside sed/printf. --- tools/check-updates.sh | 46 ++++++++++++++++++++++++++++++++----- tools/test-check-updates.sh | 31 +++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 6 deletions(-) diff --git a/tools/check-updates.sh b/tools/check-updates.sh index 1656446..77a2180 100755 --- a/tools/check-updates.sh +++ b/tools/check-updates.sh @@ -103,7 +103,17 @@ may_automerge() { # Only this job's own commits count. Counting every commit that touched the file # counts the one that added the package, and every hand edit to it since -- # which in a young repository is enough to refuse the first real update. - _recent="$(git log --since='24 hours ago' --author='owfeed-bot' --oneline -- "$_up" | wc -l | tr -d ' ')" + # + # Read first and counted second. This function runs as an `if` condition, where + # errexit is off, and in `git log | wc -l` a failed git still counts to 0 -- + # under the ceiling, so a failure read as permission. Every failed read in here + # returns 1: the cost is a pull request, never an unreviewed merge. + if ! _log="$(git log --since='24 hours ago' --author='owfeed-bot' --oneline -- "$_up")"; then + echo " could not count recent automatic updates: 'git log --since=\"24 hours ago\" --author=owfeed-bot -- $_up' failed" + return 1 + fi + _recent=0 + [ -z "$_log" ] || _recent="$(printf '%s\n' "$_log" | wc -l | tr -d ' ')" if [ "$_recent" -ge 2 ]; then echo " $_recent automatic updates to $_name in the last day: the next one wants a person" return 1 @@ -122,7 +132,15 @@ may_automerge() { # changed line anywhere else means either a bug here or an upstream.sh that was # edited between the checkout and now -- and SIG_KEY_ID moving would be the # whole verification quietly relaxing itself. - _bad="$(git diff -U0 -- "$_up" \ + # + # The diff is captured on its own for the same reason as the log above: piped + # straight into the filters, a failed `git diff` is an empty list of changed + # lines, and the `|| true` the filters need turns that into "only pins moved". + if ! _diff="$(git diff -U0 -- "$_up")"; then + echo " could not read what changed: 'git diff -U0 -- $_up' failed, so it is not merging itself" + return 1 + fi + _bad="$(printf '%s\n' "$_diff" \ | grep -E '^[+-][^+-]' \ | grep -vE '^[+-](VERSION|TAG|ARTIFACT|ARTIFACT_IPK|SHA256|SHA256_IPK)=' \ | grep -vE '^[+-][a-zA-Z0-9_.-]+ +[0-9a-f]{64} +' || true)" @@ -314,8 +332,12 @@ for up in packages/*/upstream.sh; do apk) file="$(echo "$ARTIFACT" | sed "s/${current}/${latest}/g")" [ -f "$tmp/$file" ] || { echo "$name: $latest_tag publishes no $file" >&2; exit 1; } + # The sum is taken into a variable before it goes near `sed`. Inside the + # sed argument a failed `sha256sum` is invisible -- the command's status + # is sed's -- and the pin is committed as an empty checksum. + sum="$(sha256sum "$tmp/$file")" sed -i "s|^ARTIFACT=.*|ARTIFACT=\"${file}\"|" "$up" - sed -i "s|^SHA256=.*|SHA256=\"$(sha256sum "$tmp/$file" | cut -d' ' -f1)\"|" "$up" + sed -i "s|^SHA256=.*|SHA256=\"${sum%% *}\"|" "$up" # The 24.10 container, when upstream ships one. Leaving it pinned to the # previous version does not fail here -- it fails later, when fetch.sh asks @@ -324,8 +346,9 @@ for up in packages/*/upstream.sh; do if [ -n "${ARTIFACT_IPK:-}" ]; then file_ipk="$(echo "$ARTIFACT_IPK" | sed "s/${current}/${latest}/g")" [ -f "$tmp/$file_ipk" ] || { echo "$name: $latest_tag publishes no $file_ipk" >&2; exit 1; } + sum="$(sha256sum "$tmp/$file_ipk")" sed -i "s|^ARTIFACT_IPK=.*|ARTIFACT_IPK=\"${file_ipk}\"|" "$up" - sed -i "s|^SHA256_IPK=.*|SHA256_IPK=\"$(sha256sum "$tmp/$file_ipk" | cut -d' ' -f1)\"|" "$up" + sed -i "s|^SHA256_IPK=.*|SHA256_IPK=\"${sum%% *}\"|" "$up" fi ;; binaries) @@ -335,13 +358,24 @@ for up in packages/*/upstream.sh; do echo "$ARTIFACTS" | while read -r artifact _ arches; do [ -n "$artifact" ] || continue [ -f "$tmp/$artifact" ] || { echo "$name: $latest_tag publishes no $artifact" >&2; exit 1; } - printf '%s %s %s\n' "$artifact" "$(sha256sum "$tmp/$artifact" | cut -d' ' -f1)" "$arches" + sum="$(sha256sum "$tmp/$artifact")" + printf '%s %s %s\n' "$artifact" "${sum%% *}" "$arches" done > "$tmp/table" + # `|| exit 1`, not `&& mv`. The left side of `&&` is exempt from errexit, + # so a failing awk -- BSD awk refuses the newlines in this `-v` -- left + # the package running with VERSION and TAG already rewritten above and + # the old checksums still in place, and that half-pin was committed, + # pushed and sent to the checks. awk -v table="$(cat "$tmp/table")" ' /^ARTIFACTS="/ { print; print table; inside = 1; next } inside && /^"/ { print; inside = 0; next } !inside { print } - ' "$up" > "$tmp/new" && mv "$tmp/new" "$up" + ' "$up" > "$tmp/new" || { + echo "$name: could not rewrite the checksum table in $up; nothing is committed" >&2 + echo " failed: awk -v table=... $up (its own error is above)" >&2 + exit 1 + } + mv "$tmp/new" "$up" ;; esac diff --git a/tools/test-check-updates.sh b/tools/test-check-updates.sh index 46c1255..aea5b33 100755 --- a/tools/test-check-updates.sh +++ b/tools/test-check-updates.sh @@ -302,6 +302,37 @@ else no "the second run downloaded $((after - before)) release(s); only a0-unfet # that goes green on the next run is a failure nobody ever sees. if [ "$status" -ne 0 ]; then ok "the second run is red too"; else no "the failure stopped being reported"; fi +# `may_automerge` runs as an `if` condition, where POSIX switches errexit off. A git +# call inside it that fails hands the next line an empty answer, and both empty +# answers there mean "yes": no recent updates (under the daily ceiling), no diff +# outside the pins. A failed read has to be a pull request instead. +realgit="$(command -v git)" +cat >"$bin/git" <&2 + exit 128 +fi +exec "$realgit" "\$@" +SHIM +chmod +x "$bin/git" + +echo "--- third run: git log fails while counting recent updates" +release example/d-vprefix v1.2.0 +echo log >"$GH_STATE/git-fails" +run "$work/out3" +rm -f "$GH_STATE/git-fails" +said "$work/out3" "d-vprefix: https://example.invalid/pull/1" +unsaid "$work/out3" "d-vprefix: update/d-vprefix-1.2.0 pushed, no pull request" + +echo "--- fourth run: git diff fails while reading what moved" +release example/c-noprefix 2026.09 +echo diff >"$GH_STATE/git-fails" +run "$work/out4" +rm -f "$GH_STATE/git-fails" +said "$work/out4" "c-noprefix: https://example.invalid/pull/1" +unsaid "$work/out4" "c-noprefix: update/c-noprefix-2026.09 pushed, no pull request" + if [ "$result" = 0 ]; then echo "PASS" else From e4bdad160b74ddd6ebf816e5dc492cbead155e0e Mon Sep 17 00:00:00 2001 From: VizzleTF Date: Mon, 14 Sep 2026 13:36:35 +0300 Subject: [PATCH 3/5] fix(tools): fail when an index cannot be read sources.sh piped 'find -exec jq' into sort with stderr discarded, and check-origin.sh appended with '2>/dev/null || true'. A jq or awk failure on one index silently dropped its packages from the copyleft-source and origin checks while the rest passed. Both now write to a file, check find's status and report the failed command. --- tools/check-origin.sh | 23 +++++++++++++++++++---- tools/sources.sh | 26 +++++++++++++++++++------- tools/test-sources.sh | 16 ++++++++++++++++ 3 files changed, 54 insertions(+), 11 deletions(-) diff --git a/tools/check-origin.sh b/tools/check-origin.sh index f090ef8..cac113c 100755 --- a/tools/check-origin.sh +++ b/tools/check-origin.sh @@ -42,9 +42,19 @@ trap 'rm -f "$rows" "$missing" "$named"' EXIT # package's own pkginfo, so `.url` is what `apk info` prints. A noarch package # appears in every architecture's index and is wanted once, which is what the # sort -u below is for. -find "$OUT" -name index.json -exec \ +# +# No `2>/dev/null || true` on either read. That pair hid a jq or awk that failed on +# one index, and every package in it went unchecked while the rest passed -- a check +# that cannot run counts as failed. `find` matching nothing still exits 0; `find` +# exits non-zero when an `-exec ... {} +` invocation does. +if ! find "$OUT" -name index.json -exec \ jq -r '.packages[]? | ["apk", .name, .version, (.url // "")] | @tsv' {} + \ - >> "$rows" 2>/dev/null || true + >> "$rows"; then + echo "tools/check-origin.sh: could not read every index.json under $OUT" >&2 + echo " failed: find $OUT -name index.json -exec jq -r '.packages[]? | ...' {} +" >&2 + echo " jq's own error is above; rebuild the index (owfeed index) and re-run" >&2 + exit 1 +fi # opkg, from the text index. Both spellings are read because both appear in # practice: a package built by OpenWrt's SDK carries the repository in `URL:` and @@ -54,7 +64,7 @@ find "$OUT" -name index.json -exec \ # # Continuation lines cannot be mistaken for fields here: opkg indents them with a # space, and `Description:` -- the only multi-line field -- is written last. -find "$OUT" -name Packages -type f -exec awk ' +if ! find "$OUT" -name Packages -type f -exec awk ' function flush() { if (name != "") { origin = (url != "") ? url : source @@ -73,7 +83,12 @@ find "$OUT" -name Packages -type f -exec awk ' /^URL: / { url = substr($0, 6) } /^Source: / { source = substr($0, 9) } END { flush() } -' {} + >> "$rows" 2>/dev/null || true +' {} + >> "$rows"; then + echo "tools/check-origin.sh: could not read every Packages index under $OUT" >&2 + echo " failed: find $OUT -name Packages -type f -exec awk ... {} +" >&2 + echo " awk's own error is above; rebuild the index (owfeed index) and re-run" >&2 + exit 1 +fi [ -s "$rows" ] || { echo "tools/check-origin.sh: no package found in any index under $OUT" >&2; exit 1; } diff --git a/tools/sources.sh b/tools/sources.sh index 4fd10be..91913d7 100755 --- a/tools/sources.sh +++ b/tools/sources.sh @@ -78,15 +78,27 @@ fi # Everything the tree publishes, from the JSON index owfeed writes beside each binary # one. A noarch package appears in every architecture's index and is wanted once. -packages="$(find "$OUT" -name index.json -exec \ - jq -r '.packages[]? | [.name, .version, (.license // "")] | @tsv' {} + 2>/dev/null \ - | sort -u)" - -[ -n "$packages" ] || { echo "tools/sources.sh: no package found in any index.json under $OUT" >&2; exit 1; } - +# +# Written to a file and sorted afterwards, never piped. `find ... | sort` reports +# sort's status, and jq's stderr went to /dev/null: an index.json jq could not parse +# dropped that index's packages from this check without a word, while the readable +# indexes beside it passed -- a copyleft package in the broken one was published +# with no source. `find` exits non-zero when any `-exec ... {} +` invocation does. missing="$(mktemp)" listed="$(mktemp)" -trap 'rm -f "$missing" "$listed"' EXIT +rows="$(mktemp)" +trap 'rm -f "$missing" "$listed" "$rows"' EXIT + +if ! find "$OUT" -name index.json -exec \ + jq -r '.packages[]? | [.name, .version, (.license // "")] | @tsv' {} + > "$rows"; then + echo "tools/sources.sh: could not read every index.json under $OUT, so no licence is trusted" >&2 + echo " failed: find $OUT -name index.json -exec jq -r '.packages[]? | ...' {} +" >&2 + echo " jq's own error is above; rebuild the index (owfeed index) and re-run" >&2 + exit 1 +fi +packages="$(sort -u "$rows")" + +[ -n "$packages" ] || { echo "tools/sources.sh: no package found in any index.json under $OUT" >&2; exit 1; } printf '%s\n' "$packages" | while IFS="$TAB" read -r name version licence; do [ -n "${name:-}" ] || continue diff --git a/tools/test-sources.sh b/tools/test-sources.sh index d4bdf57..7ff9f03 100755 --- a/tools/test-sources.sh +++ b/tools/test-sources.sh @@ -219,5 +219,21 @@ else ok "no record: an archive found by name is still source, with no origin claimed" fi +# An index this cannot parse. Its packages are packages whose licence nobody read, so +# the publish has to stop -- the readable index beside it passing on its own is +# exactly how a copyleft package in the broken one used to go out without source. +d="$(scenario unreadable-index)" +index "$d" "example-daemon 1.0-r1 Apache-2.0" +mkdir -p "$d/out/releases/25.12/aarch64_generic" +printf '{"packages":[{"name":"luci-app-example",' \ + > "$d/out/releases/25.12/aarch64_generic/index.json" +if run "$d"; then + bad "unreadable index" "published past an index.json it could not parse" +elif ! says "$d" "could not read every index.json"; then + bad "unreadable index" "refused without saying which step failed: $(cat "$d/stderr")" +else + ok "unreadable index: an index that cannot be read refuses the publish" +fi + [ "$fails" -eq 0 ] || { echo "tools/sources.sh: $fails case(s) failed" >&2; exit 1; } echo "tools/sources.sh: every case passed" From 670f56b83471150afc6bb6a5fe168e481898de9c Mon Sep 17 00:00:00 2001 From: VizzleTF Date: Mon, 14 Sep 2026 13:47:43 +0300 Subject: [PATCH 4/5] fix(check-updates): stop a package when its release cannot be read 'gh release view ... 2>/dev/null || true' read a 401, a 502 or a network error as 'upstream has no releases' and kept the run green. gh exits 1 for all of them; a repository with no releases and a missing repository both print exactly 'release not found' (measured, gh 2.99.0). Treat only that text as no releases, confirmed by 'gh api repos//releases' succeeding; anything else stops the package through the existing per-package failure path. --- tools/check-updates.sh | 44 ++++++++++++++++++++++++-------- tools/test-check-updates.sh | 51 ++++++++++++++++++++++++++++++++++--- 2 files changed, 80 insertions(+), 15 deletions(-) diff --git a/tools/check-updates.sh b/tools/check-updates.sh index 77a2180..920e7dc 100755 --- a/tools/check-updates.sh +++ b/tools/check-updates.sh @@ -225,15 +225,40 @@ for up in packages/*/upstream.sh; do # fetch.sh's, so a package that pins no tag is still compared against # exactly the tag fetch.sh would download for it. current_tag="${TAG:-v${VERSION%-r*}}" - # `|| true` is load-bearing under `set -e`: an assignment takes the exit - # status of its command substitution, so a repository with no releases at - # all -- or a `gh` that could not reach GitHub -- would kill this subshell - # here, before the line below can say so. It used to survive that by - # accident: the old command ended in `| sed`, and a pipeline reports its - # last command. - latest_tag="$(gh release view --repo "$REPO" --json tagName -q .tagName 2>/dev/null || true)" + # Scratch space for this package, removed however the subshell ends. Made + # before the first `gh` call, whose stderr is read below. + tmp="$(mktemp -d)" + trap 'rm -rf "$tmp"' EXIT - [ -n "$latest_tag" ] || { echo "$name: upstream has no releases"; exit 0; } + # "Upstream has no releases" and "could not ask" are different answers, and + # only the first is green. This was `2>/dev/null || true`, which read a 401, + # a 502 or a dropped connection as "no releases" and left the run green with + # the package never checked -- a check that cannot run counts as failed. + # + # gh does not tell them apart by exit code. Measured with gh 2.99.0: a + # repository with no releases (octocat/Hello-World), a repository that does + # not exist, a bad token and an unreachable proxy all exit 1. The first two + # print exactly `release not found` -- both are a 404 on /releases/latest -- + # and the others print the HTTP or transport error. So `release not found` is + # confirmed with a question an existing repository answers and a missing one + # fails, the release list; a renamed or deleted upstream is a REPO to fix, + # not a quiet hour. Every other failure stops this package through the + # handler after the subshell. + if ! latest_tag="$(gh release view --repo "$REPO" --json tagName -q .tagName 2>"$tmp/view.err")"; then + if [ "$(cat "$tmp/view.err")" = "release not found" ] && + gh api "repos/$REPO/releases?per_page=1" -q length >/dev/null 2>"$tmp/list.err"; then + echo "$name: upstream has no releases" + exit 0 + fi + { + echo "$name: could not read the latest release of $REPO" + cat "$tmp/view.err" "$tmp/list.err" 2>/dev/null | sed 's/^/ /' + echo " failed: gh release view --repo $REPO --json tagName, then gh api repos/$REPO/releases" + echo " a renamed or deleted repository needs REPO fixed in $up; an outage clears on a later run" + } >&2 + exit 1 + fi + [ -n "$latest_tag" ] || { echo "$name: gh release view answered an empty tag for $REPO" >&2; exit 1; } # Versions, for what reads a version rather than a tag: the major-bump # refusal, an `apk` shape's artifact names, and anyone reading the branch. @@ -295,9 +320,6 @@ for up in packages/*/upstream.sh; do fi echo "$name: $current -> $latest" - tmp="$(mktemp -d)" - trap 'rm -rf "$tmp"' EXIT - # Only what this shape needs to recompute its pins. A manifest package pins # no checksums at all -- they are in the manifest, under the author's # signature -- so downloading its ninety-odd assets on every run to look at diff --git a/tools/test-check-updates.sh b/tools/test-check-updates.sh index aea5b33..bc7aba4 100755 --- a/tools/test-check-updates.sh +++ b/tools/test-check-updates.sh @@ -7,13 +7,19 @@ # repository under $TMPDIR. No release is ever downloaded for real; the stub records # which tag was asked for, which is most of what these cases are about. # -# WHAT IT PINS. Six pins the scheduled job meets, and where each has to land: +# WHAT IT PINS. Nine pins the scheduled job meets, and where each has to land: # # a-handmapped tag 0.19.17-2, VERSION 0.19.17-r2, upstream now 0.19.17-3 # -> updated: upstream's -3 becomes the feed's -r3, the tag verbatim # a0-unfetchable upstream names a latest release, and downloading it fails # -> this package stops, every package after it is still checked, # and the run is red naming it +# b0-outage asking for the latest release fails with an HTTP error +# -> stops like a0-unfetchable; never read as "no releases" +# b1-missing `release not found`, and the repository itself is gone +# -> stops: a REPO to fix, not an upstream with nothing released +# f-norelease `release not found` from a repository that exists +# -> "upstream has no releases", green # b-current tag 0.19.17-2, VERSION 0.19.17-r2, upstream still 0.19.17-2 # -> current; the run must not propose what is already pinned # c-noprefix tag 2026.07 -> 2026.08, no `v` anywhere @@ -69,7 +75,7 @@ mkdir -p "$bin" GH_STATE="$work/state" export GH_STATE -mkdir -p "$GH_STATE/releases" "$GH_STATE/broken" +mkdir -p "$GH_STATE/releases" "$GH_STATE/broken" "$GH_STATE/outage" "$GH_STATE/missing" : > "$GH_STATE/downloads" # Stand-in for gh(1). It answers the four calls the script makes and fails on @@ -86,10 +92,30 @@ shift 2 case "$what" in "release view") # gh release view --repo --json tagName -q .tagName - f="$GH_STATE/releases/$(slug "$2")" + # + # The three shapes a failure takes, as measured against gh 2.99.0: an HTTP + # error names itself; no releases and no repository both say exactly + # `release not found`. + s="$(slug "$2")" + if [ -f "$GH_STATE/outage/$s" ]; then + echo "HTTP 502: Bad Gateway (https://api.github.com/repos/$2/releases/latest)" >&2 + exit 1 + fi + f="$GH_STATE/releases/$s" [ -f "$f" ] || { echo "release not found" >&2; exit 1; } cat "$f" ;; +"api repos/"*"/releases?per_page=1") + # gh api repos//releases?per_page=1 -q length -- a missing repository + # is a 404 here, an existing one with no releases answers 0. + r="${what#api repos/}" + r="${r%/releases*}" + if [ -f "$GH_STATE/missing/$(slug "$r")" ]; then + echo "gh: Not Found (HTTP 404)" >&2 + exit 1 + fi + if [ -f "$GH_STATE/releases/$(slug "$r")" ]; then echo 1; else echo 0; fi + ;; "release download") # gh release download --repo --dir --pattern printf '%s\n' "$1" >>"$GH_STATE/downloads" @@ -167,6 +193,9 @@ pin b-current 0.19.17-r2 0.19.17-2 pin c-noprefix 2026.07-r1 2026.07 pin d-vprefix 1.0.0-r1 v1.0.0 pin e-retag 1.0.0-r1 v1.0.0 +pin b0-outage 1.0.0-r1 v1.0.0 +pin b1-missing 1.0.0-r1 v1.0.0 +pin f-norelease 1.0.0-r1 v1.0.0 git add -A && git commit -q -m "packages: initial pins" git push -q origin main @@ -177,6 +206,10 @@ release example/b-current 0.19.17-2 release example/c-noprefix 2026.08 release example/d-vprefix v1.1.0 release example/e-retag 1.1.0 +release example/b0-outage v1.0.1 +: > "$GH_STATE/outage/example_b0-outage" +: > "$GH_STATE/missing/example_b1-missing" +# f-norelease: no release at all, and the repository is there. # The branch the guessing code would have left behind for e-retag: the right version # in its name, the wrong tag in its pin. Its content is what decides whether the next @@ -230,7 +263,17 @@ run "$work/out" # a scheduled job reporting that nothing was released when it never asked. if [ "$status" -ne 0 ]; then ok "the run is red"; else no "the run went green with a package it could not check"; fi said "$work/out" "a0-unfetchable: stopped; the remaining packages are still checked" -said "$work/out" "check stopped for: a0-unfetchable" +said "$work/out" "check stopped for: a0-unfetchable b0-outage b1-missing" + +# Not being able to ask is not an answer. Both failures stop their package, and the +# one repository that exists and has released nothing is still the green case. +said "$work/out" "b0-outage: could not read the latest release of example/b0-outage" +said "$work/out" "HTTP 502: Bad Gateway" +unsaid "$work/out" "b0-outage: upstream has no releases" +said "$work/out" "b1-missing: could not read the latest release of example/b1-missing" +unsaid "$work/out" "b1-missing: upstream has no releases" +said "$work/out" "f-norelease: upstream has no releases" +unsaid "$work/out" "f-norelease: stopped" if [ -z "$(git ls-remote --heads origin 'update/a0-unfetchable-*')" ]; then ok "a0-unfetchable pushed nothing" else From ea7d873dc1da986719e664398653d12815f837be Mon Sep 17 00:00:00 2001 From: VizzleTF Date: Mon, 14 Sep 2026 13:47:43 +0300 Subject: [PATCH 5/5] fix(land-updates): exit non-zero when a branch failed to land A step that could not run (return 1 from land) cost that branch but left the scheduled run green. Collect those branches and exit 1 after every branch has been read. Ordinary outcomes (checks pending, pull request open, not a fast-forward, superseded) still return 0. The check and publish jobs already run on '!cancelled()', so a red land job does not skip them. --- .github/workflows/update.yml | 13 ++++-- tools/land-updates.sh | 46 ++++++++++++++------ tools/test-land-updates.sh | 81 +++++++++++++++++++++++++++++++----- 3 files changed, 114 insertions(+), 26 deletions(-) diff --git a/.github/workflows/update.yml b/.github/workflows/update.yml index 58c4d88..f70c280 100644 --- a/.github/workflows/update.yml +++ b/.github/workflows/update.yml @@ -48,9 +48,16 @@ jobs: # # First, so a branch that went green since the last run is on `main` before # `check-updates.sh` compares anything against `main`, and before the publish job - # below asks whether `main` has been published. `!cancelled()` rather than - # `success()`: a failure here is a branch that did not land, and it must not stop - # the scheduled check from finding new releases. + # below asks whether `main` has been published. + # + # This job goes red when a step failed on any branch -- a `gh` or `git` call that + # could not run -- after it has tried every other branch. The jobs below carry + # `!cancelled()` rather than `success()`, which is what makes red here safe: a + # branch that did not land must not stop the scheduled check from finding new + # releases, and `main` may still need publishing for a commit that landed earlier + # in this run or by a human merge. Red is kept rather than swallowed with + # `continue-on-error`, because that would show a green run for an hour in which + # nothing could be landed. land: runs-on: ubuntu-latest permissions: diff --git a/tools/land-updates.sh b/tools/land-updates.sh index 8c1d146..a33aa13 100755 --- a/tools/land-updates.sh +++ b/tools/land-updates.sh @@ -143,10 +143,11 @@ superseded() { echo "it proposes $_theirs, behind the $_ours main publishes" } -# Land one branch, or say why not and return. Never fails the run: one branch that -# cannot land must not stop the next one, and none of the reasons below is an error -# in the first place -- a check still running, a `main` that moved, a human's branch -# that is none of this script's business. +# Land one branch, or say why not and return. Returns 0 for every ordinary reason not +# to land -- a check still running, a `main` that moved, a pull request waiting for a +# person, a branch already spent -- because none of those is an error. Returns 1 only +# when a step could not run; the loop at the bottom carries on to the next branch and +# makes the run red at the end. land() { branch="$1" sha="$2" @@ -338,17 +339,38 @@ if [ -z "$refs" ]; then exit 0 fi -# `|| echo` on the call, not `set +e` around the loop: an unexpected failure inside +# `if ! land` on the call, not `set +e` around the loop: an unexpected failure inside # `land` -- a `gh` outage, a git object that is not there -- must cost that one # branch and not every branch after it. Under plain `set -eu` the first one would # take the job down with the rest unread, which is the failure mode this repository # keeps re-learning (see the `checkout -` note in check-updates.sh). # -# The price of `||`: POSIX turns errexit off for everything `land` runs, so nothing -# inside it stops on its own. Every step there whose failure would read as a -# harmless answer checks its own status and returns -- keep it that way when -# adding one. -printf '%s\n' "$refs" | while read -r sha ref; do +# The price: POSIX turns errexit off for everything `land` runs when it is an `if` +# condition, so nothing inside it stops on its own. Every step there whose failure +# would read as a harmless answer checks its own status and returns 1 -- keep it +# that way when adding one. +# +# A here-document rather than `printf | while`, so the loop runs in this shell and +# `failed` survives it; a pipeline would run the loop in a subshell and lose it. +# `land` reads nothing from stdin, and `&2 + echo " every other branch was still read; the lines above name the command that failed" >&2 + exit 1 +fi diff --git a/tools/test-land-updates.sh b/tools/test-land-updates.sh index 7f568fb..18dd99e 100755 --- a/tools/test-land-updates.sh +++ b/tools/test-land-updates.sh @@ -119,19 +119,35 @@ cat >"$work/gh" <<'STUB' set -eu case "${1:-} ${2:-}" in "pr list") - # No pull request is open on any branch here. The human gate has its own - # reason to exist and is not what this test is about -- except that a call - # which FAILS must never read as "none is open", which the flag below pins. - if [ -f "$GH_STATE/pr-list-fails" ]; then + # gh pr list -R --head --state open ... + # + # No pull request is open unless `pr-open` names the branch. A call that + # FAILS must never read as "none is open": `pr-list-fails` makes it fail for + # the branch it names, or for every branch when it is empty. + head="" + prev="" + for a in "$@"; do + [ "$prev" != "--head" ] || head="$a" + prev="$a" + done + if [ -f "$GH_STATE/pr-list-fails" ] && + { [ ! -s "$GH_STATE/pr-list-fails" ] || [ "$(cat "$GH_STATE/pr-list-fails")" = "$head" ]; }; then echo "HTTP 502: Bad Gateway" >&2 exit 1 fi + if [ -f "$GH_STATE/pr-open" ] && [ "$(cat "$GH_STATE/pr-open")" = "$head" ]; then + echo 42 + fi ;; "api "*"/git/matching-refs/heads/update/") cat "$GH_STATE/refs" ;; "api "*"/check-runs"*) - printf 'completed/success\tcheck / build\ncompleted/success\tcheck / check\n' + if [ -f "$GH_STATE/checks-pending" ]; then + printf 'in_progress/pending\tcheck / build\nqueued/pending\tcheck / check\n' + else + printf 'completed/success\tcheck / build\ncompleted/success\tcheck / check\n' + fi ;; *) echo "stub gh: unexpected call: $*" >&2 @@ -147,13 +163,23 @@ chmod +x "$work/bin/gh" # API sorts it. `tr` because ls-remote separates with a tab and `gh -q` with a space. listing() { git ls-remote --heads origin 'update/*' | tr '\t' ' ' >"$GH_STATE/refs"; } +status=0 run() { - if ! PATH="$work/bin:$PATH" GITHUB_REPOSITORY="owfeed/test" sh "$script" >"$1" 2>&1; then - no "land-updates.sh exited non-zero" - fi + status=0 + PATH="$work/bin:$PATH" GITHUB_REPOSITORY="owfeed/test" sh "$script" >"$1" 2>&1 || status=$? sed 's/^/ | /' "$1" } +# The run's colour, both ways. Only a step that could not run is red; every ordinary +# reason not to land is green, or the scheduled job is red on a normal hour and +# nobody reads it any more. +green() { + if [ "$status" = 0 ]; then ok "the run exits 0: $1"; else no "the run exited $status: $1 is not a failure"; fi +} +red() { + if [ "$status" != 0 ]; then ok "the run is red: $1"; else no "the run exited 0 although $1"; fi +} + logged() { if grep -qF "$2" "$1"; then ok "said: $2" @@ -179,6 +205,7 @@ kept() { echo "--- first run" listing run "$work/out" +green "branches deleted, landed and waiting for a rebuild" gone update/alpha-1.2.0 logged "$work/out" "update/alpha-1.2.0: every file it touches already matches main; deleting it" @@ -215,6 +242,7 @@ fi echo "--- second run" listing run "$work/out2" +green "a branch main moved ahead of" lines="$(wc -l <"$work/out2" | tr -d ' ')" if [ "$lines" = 1 ] && grep -q '^update/delta-3.0.0: ' "$work/out2"; then ok "the second run reports only the branch that is waiting for a rebuild" @@ -271,6 +299,7 @@ offer listing run "$work/out3" rm -f "$GH_STATE/pr-list-fails" +red "gh pr list failed" kept update/gamma-1.2.0 unmoved logged "$work/out3" "update/gamma-1.2.0: could not read its pull requests" @@ -281,10 +310,12 @@ echo "--name-only" >"$GH_STATE/git-fails" listing run "$work/out4" rm -f "$GH_STATE/git-fails" +red "git diff failed" kept update/gamma-1.2.0 kept update/delta-3.0.0 unmoved logged "$work/out4" "cannot tell what" +logged "$work/out4" "update/delta-3.0.0: main moved ahead of it" echo "--- fifth run: git fetch fails" offer @@ -292,21 +323,49 @@ echo "fetch" >"$GH_STATE/git-fails" listing run "$work/out5" rm -f "$GH_STATE/git-fails" +red "git fetch failed" kept update/gamma-1.2.0 unmoved logged "$work/out5" "update/gamma-1.2.0: could not fetch" -echo "--- sixth run: nothing fails" +# One branch fails and the one after it is still read. delta sorts first, so gamma +# landing is the proof the loop went on -- and that the branch kept by every failure +# above was landable all along. +echo "--- sixth run: gh pr list fails for the first branch only" offer +echo update/delta-3.0.0 >"$GH_STATE/pr-list-fails" listing run "$work/out6" +rm -f "$GH_STATE/pr-list-fails" +red "one branch could not be read" +logged "$work/out6" "update/delta-3.0.0: could not read its pull requests" +logged "$work/out6" "not landed because a step failed: update/delta-3.0.0" head="$(git ls-remote origin refs/heads/main | cut -f1)" if [ "$head" = "$gamma2" ]; then - ok "the same branch lands once nothing fails" + ok "the branch after the failure was still read, and landed" else - no "main is $head, expected $gamma2: the branch kept above was not landable anyway" + no "main is $head, expected $gamma2: the loop stopped at the failure, or the branch was not landable" fi +# The ordinary reasons not to land stay green. +echo "--- seventh run: checks still running" +: >"$GH_STATE/checks-pending" +listing +run "$work/out7" +rm -f "$GH_STATE/checks-pending" +green "checks still running" +logged "$work/out7" "update/delta-3.0.0: not green yet" +kept update/delta-3.0.0 + +echo "--- eighth run: a pull request is open" +echo update/delta-3.0.0 >"$GH_STATE/pr-open" +listing +run "$work/out8" +rm -f "$GH_STATE/pr-open" +green "a pull request waiting for a person" +logged "$work/out8" "update/delta-3.0.0: pull request #42 is open" +kept update/delta-3.0.0 + if [ "$result" = 0 ]; then echo "PASS" else