Fix the rebundle step: a non-matching grep must not kill it #130

Merged
bvandeusen merged 1 commits from dev into main 2026-09-10 10:49:40 -04:00
2 changed files with 60 additions and 4 deletions
+14 -4
View File
@@ -421,7 +421,10 @@ jobs:
# the sidecar published beside it, so what the server reports is what
# that build actually recorded rather than something re-derived here.
# Degrades to an empty client/ (404 update channel) — never a wrong
# version — if no release or APK asset can be resolved.
# version — if no release or APK asset can be resolved. That
# degradation only actually works because the greps below carry
# `|| true`; under the runner's default pipefail a non-matching grep
# kills the step instead of falling through to the empty-case branch.
if: steps.guard.outputs.ready == 'true' && github.ref == 'refs/heads/main'
shell: bash
env:
@@ -434,8 +437,15 @@ jobs:
if [ -z "${REL_JSON}" ]; then
echo "::notice::no published release — image ships without bundled APK"; exit 0
fi
TAG="$(printf '%s' "${REL_JSON}" | grep -oP '"tag_name":\s*"\K[^"]+' | head -1)"
APK_URL="$(printf '%s' "${REL_JSON}" | grep -oP '"browser_download_url":\s*"\K[^"]+' | grep -E '\.apk$' | head -1)"
# `|| true` on every one of these, and it is load-bearing rather
# than defensive habit. The runner already invokes this shell as
# `bash -e -o pipefail`, so a pipeline whose grep matches NOTHING
# exits non-zero even though `head` succeeded — and the step dies at
# the assignment, before ever reaching the `if` written to handle the
# empty case. Every "degrades gracefully" branch below is unreachable
# without this.
TAG="$(printf '%s' "${REL_JSON}" | grep -oP '"tag_name":\s*"\K[^"]+' | head -1)" || true
APK_URL="$(printf '%s' "${REL_JSON}" | grep -oP '"browser_download_url":\s*"\K[^"]+' | grep -E '\.apk$' | head -1)" || true
if [ -z "${TAG}" ] || [ -z "${APK_URL}" ]; then
echo "::notice::latest release '${TAG:-?}' has no APK asset — image ships without bundled APK"; exit 0
fi
@@ -446,7 +456,7 @@ jobs:
# the formula lived in two files that had to be kept in step, and it
# could only ever recover the name — the ordering key is build-time
# minutes and does not exist anywhere after that build ends.
SIDECAR_URL="$(printf '%s' "${REL_JSON}" | grep -oP '"browser_download_url":\s*"\K[^"]+' | grep -E '\.apk\.version$' | head -1)"
SIDECAR_URL="$(printf '%s' "${REL_JSON}" | grep -oP '"browser_download_url":\s*"\K[^"]+' | grep -E '\.apk\.version$' | head -1)" || true
if [ -n "${SIDECAR_URL}" ]; then
curl -fsSL -H "Authorization: token ${CI_TOKEN}" -o client/minstrel.apk.version "${SIDECAR_URL}"
cat client/minstrel.apk.version
+46
View File
@@ -240,3 +240,49 @@ func TestDevChannel_RebundlePathIsMainOnly(t *testing.T) {
t.Errorf("the rebundle step is not gated to main; a dev push would overwrite its own APK\n%s", step)
}
}
// The runner invokes `shell: bash` as `bash -e -o pipefail`. That makes any
// command substitution whose grep matches NOTHING fatal at the assignment —
// pipefail reports the grep's non-zero status even though `head` succeeded —
// so the step dies before reaching the `if` written to handle the empty case.
//
// This is not hypothetical. It took down the first `main` build after the
// version rework: the newest release predated sidecar assets, its
// `.apk.version` grep matched nothing, and the step aborted instead of
// falling through to the name-only branch that exists precisely for it. The
// same latent hazard sat on the other two assignments and had simply never
// fired, because their greps always matched.
//
// Pins the property that makes every "degrades gracefully" branch in that
// step reachable at all.
func TestBundleStep_GrepsCannotKillTheStep(t *testing.T) {
body, err := os.ReadFile(filepath.Join(repoRoot(t), ".gitea", "workflows", "release.yml"))
if err != nil {
t.Fatal(err)
}
yaml := string(body)
i := strings.Index(yaml, "- name: Bundle latest release APK")
if i < 0 {
t.Fatal("no 'Bundle latest release APK' step")
}
step := yaml[i:]
if j := strings.Index(step, "\n - name:"); j >= 0 {
step = step[:j]
}
for _, line := range strings.Split(step, "\n") {
trimmed := strings.TrimSpace(line)
if strings.HasPrefix(trimmed, "#") || !strings.Contains(trimmed, "grep") {
continue
}
// Only assignments from a command substitution can abort the step.
if !strings.Contains(trimmed, `="$(`) {
continue
}
if !strings.HasSuffix(trimmed, "|| true") {
t.Errorf("this assignment dies under pipefail when its grep matches nothing, "+
"skipping the empty-case branch below it:\n %s", trimmed)
}
}
}