From c1bc4291a6232632a57f9c9385c5f2b87c2a0068 Mon Sep 17 00:00:00 2001 From: JC-000 <3798556+JC-000@users.noreply.github.com> Date: Fri, 14 Aug 2026 09:58:51 -0500 Subject: [PATCH] fix(package): produce a partial release + a legible blocker instead of aborting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported by LaneCerts and reproduced here on a fresh checkout of 21f2ce5: at the nistcurves v0.9.1 pin both onchip variants fail to link, which took the entire packaging pipeline down. `make package` aborted inside build_prgs.sh, so no disk images, no listener and no manifest were ever produced, and `make package-verify` had nothing coherent to report. One broken variant left the operator with no artifacts and no written record of what broke. That is the wrong failure mode for exactly the situation it shows up in. A library bump breaking one profile is common, and the useful outcome is "here are the three that work, here is the error for the fourth" — the release still cannot be cut, but the blocker is legible and the good artifacts are testable. - build_prgs.sh is now three-valued: 0 = all built, 2 = partial, 1 = nothing built. It also greps the first ld65/ca65 diagnostic out of each failed variant's log into dist/build-info.txt, so downstream can state WHY a variant is missing without anyone opening a log. - build_d64.sh skips PRGs that are absent and creates no image when none of its inputs exist, rather than erroring on the first gap. - write_manifest.sh still runs, and opens with an "!! INCOMPLETE RELEASE !!" block naming each missing variant, its make line and its exact error. Missing variants are marked in the guidance list and show FAILED in the size table. A partial release announces itself at the top rather than being a checksum list with fewer lines than it should have. - `make package` runs the pipeline to completion on a partial matrix, then prints the blocker and exits 1. - verify_release.py leads with a BLOCKER section, reports failed variants as [n/a ] in the reproducibility check instead of as fabricated failures, and ends "RELEASE INCOMPLETE" with exit 1. Measured against the real failure at 21f2ce5: make package -> uci-reu + ip65-reu built, both onchip variants recorded with their ld65 error, manifest written with the INCOMPLETE banner, exit 1 make package-verify -> 12/12 present-artifact checks pass (both REU PRGs reproduce byte-for-byte, 4 disk images boot to the correct banner, listener selftest 4/4), 2 variants reported as blockers, exit 1 The underlying link failure is NOT addressed here — it lives in src/lib_contract_asserts.s, which this lane does not own. Diagnosis for whoever picks it up: the assert hardcodes LIB_NISTCURVES_SHARED_PRIMITIVES = $0007, measured at v0.6.0 when both profiles reported that. Upstream has since split ownership from consumption (contract v0.5.0, lib-contract #44), and the FP_ONCHIP_MUL profile now correctly reports $0005 for both masks — od65 on our archives confirms $0007 for the REU build and $0005/$0005 for onchip, exactly matching the table in libs/nistcurves/CHANGELOG.md. The expected value needs to become profile-dependent. Note the assert's own message misdirects: it tells you to re-derive the archive member drops in build_nistcurves_p256.sh, and nothing about the archive surgery changed. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 13 +++++++++++ Makefile | 20 ++++++++++++++++- tools/package/build_d64.sh | 21 +++++++++++++---- tools/package/build_prgs.sh | 40 ++++++++++++++++++++++++++++----- tools/package/verify_release.py | 38 ++++++++++++++++++++++++++++++- tools/package/write_manifest.sh | 33 +++++++++++++++++++++++++++ 6 files changed, 154 insertions(+), 11 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 858b3da..e018612 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1515,6 +1515,19 @@ line per shipped PRG, every other script derives from it), Nothing is version-specific; re-running `make package` after a submodule bump regenerates every artifact with zero edits. +**A variant that fails to build no longer takes the pipeline down.** +`build_prgs.sh` is three-valued (0 all built / 2 partial / 1 nothing), +records the first ld65/ca65 diagnostic per failed variant into +`dist/build-info.txt`, and `make package` deliberately runs to +completion on a partial matrix: disks are made from what exists, +`MANIFEST.txt` is still written and opens with an `!! INCOMPLETE +RELEASE !!` block naming each missing variant with its make line and +exact error, and the target then exits 1. `package-verify` leads with a +BLOCKER section and ends `RELEASE INCOMPLETE`. The point is that a +library bump breaking one profile should leave a legible blocker plus +testable artifacts for the profiles that still work — not an empty +`dist/` and an aborted make. + `make package-verify` is the acceptance gate (`tools/package/ verify_release.py`): rebuilds every variant and compares **PRG** hashes (object hashes are not evidence — ca65 stamps build time into every diff --git a/Makefile b/Makefile index 68d4c74..b977b57 100644 --- a/Makefile +++ b/Makefile @@ -455,12 +455,30 @@ clean: # # PACKAGE_PYTHON must be an interpreter that can run the listener's own # selftest — see `make package-verify`. +# +# build_prgs.sh is three-valued: 0 = all variants built, 2 = PARTIAL, 1 = none. +# On PARTIAL the pipeline deliberately runs to completion anyway, so a single +# broken variant still yields disks, a listener and a manifest that names what +# is missing and why — then the target fails, because a partial matrix must +# never be mistaken for a release. `-` on the first line lets make continue; +# the status is recovered from the build-info records rather than from $?, +# which `-` discards. PACKAGE_PYTHON ?= python3 package: - bash tools/package/build_prgs.sh + -bash tools/package/build_prgs.sh + @grep -q 'result=OK' dist/build-info.txt 2>/dev/null \ + || { echo "[package] no variant built at all — nothing to package" >&2; exit 1; } bash tools/package/build_d64.sh $(PACKAGE_PYTHON) tools/package/build_listener.py bash tools/package/write_manifest.sh + @if grep -q '^failreason=' dist/build-info.txt; then \ + echo ""; \ + echo "[package] ***** INCOMPLETE: the following variants did NOT build *****"; \ + grep '^failreason=' dist/build-info.txt | cut -d= -f2- | sed 's/^/[package] /'; \ + echo "[package] dist/ holds the variants that DID build; see MANIFEST.txt."; \ + echo "[package] Do not tag a release from this."; \ + exit 1; \ + fi # Acceptance gate for the release artifacts: rebuild every PRG a second time # and compare PRG hashes, boot every D64 in VICE and assert the banner, and run diff --git a/tools/package/build_d64.sh b/tools/package/build_d64.sh index 17fab6e..c7f8f69 100755 --- a/tools/package/build_d64.sh +++ b/tools/package/build_d64.sh @@ -49,16 +49,27 @@ c1541_list() { : > "$D64_LIST" # make_disk <1541-name> [...] +# Silently drops PRGs that are not present and makes no disk at all when none +# of its inputs exist. A missing PRG means its variant failed to build, which +# build_prgs.sh has already reported and recorded; erroring out a second time +# here would only stop the surviving variants from getting disks. make_disk() { local image="$1" label="$2" id="$3"; shift 3 - rm -f "$image" - "$C1541" -format "$label,$id" d64 "$image" >/dev/null local -a writes=() while [ "$#" -gt 0 ]; do - [ -f "$1" ] || { echo "ERROR: missing $1 — run build_prgs.sh first." >&2; exit 1; } - writes+=(-write "$1" "$2,p") + if [ -f "$1" ]; then + writes+=(-write "$1" "$2,p") + else + echo "[package] skipping $(basename "$1") on $(basename "$image") — not built" + fi shift 2 done + if [ "${#writes[@]}" -eq 0 ]; then + echo "[package] $(basename "$image"): no PRGs available, image not created" + return 0 + fi + rm -f "$image" + "$C1541" -format "$label,$id" d64 "$image" >/dev/null "$C1541" -attach "$image" "${writes[@]}" >/dev/null local listing listing="$(c1541_list "$image")" @@ -72,6 +83,8 @@ make_disk() { } # --- One image per variant ---------------------------------------------------- +rm -f "$DIST"/c64-https-*.d64 # stale images from a previous, fuller run + for line in "${PACKAGE_VARIANTS[@]}"; do key="$(variant_field "$line" 1)" prg="$(variant_field "$line" 2)" diff --git a/tools/package/build_prgs.sh b/tools/package/build_prgs.sh index 2f4725a..2e1cddc 100755 --- a/tools/package/build_prgs.sh +++ b/tools/package/build_prgs.sh @@ -94,14 +94,26 @@ for line in "${PACKAGE_VARIANTS[@]}"; do if ! make $args >"$log" 2>&1; then echo "[package] BUILD FAILED for $key — see $log" >&2 tail -n 15 "$log" >&2 - echo "variant=$key prg=$prg args=$args result=FAILED log=$(basename "$log")" \ - >> "$BUILD_INFO" - failed=1 + # Pull the first ca65/ld65 diagnostic out of the log so the manifest + # can state WHY a variant is missing without anyone opening the log. + # Falls back to the last line for failures that are not toolchain + # diagnostics (a missing submodule, a full disk). + reason="$(grep -m1 -E '^(ld65|ca65|ar65|od65):|Error:' "$log" || true)" + [ -n "$reason" ] || reason="$(tail -n1 "$log")" + { + echo "variant=$key prg=$prg args=$args result=FAILED log=$(basename "$log")" + echo "failreason=$key $reason" + } >> "$BUILD_INFO" + failed=$((failed + 1)) continue fi if [ ! -f "$BUILT_PRG" ]; then echo "[package] ERROR: make $args exited 0 but $BUILT_PRG is missing" >&2 - failed=1 + { + echo "variant=$key prg=$prg args=$args result=FAILED log=(none)" + echo "failreason=$key make exited 0 but produced no PRG" + } >> "$BUILD_INFO" + failed=$((failed + 1)) continue fi cp "$BUILT_PRG" "$DIST/$prg" @@ -113,8 +125,26 @@ for line in "${PACKAGE_VARIANTS[@]}"; do printf '[package] wrote dist/%s %s bytes %s\n' "$prg" "$bytes" "$sha" done +# Exit status is three-valued on purpose, and the Makefile depends on it: +# +# 0 every variant built +# 2 PARTIAL — some built, some did not +# 1 nothing built at all +# +# A hard `exit 1` on the first failure used to abort `make package` before the +# disk images, the listener and the manifest were ever produced, which meant a +# single broken variant left the operator with no artifacts AND no written +# record of what broke. Partial is the common case during a library bump (one +# profile's archive trips a link assert while the other is fine), and the +# useful outcome there is "here are the three that work, here is the error for +# the fourth" — the release still cannot be cut, but the blocker is legible +# and the good artifacts are testable. The non-zero status is what stops +# anyone mistaking a partial run for a complete one. +built=$(( ${#PACKAGE_VARIANTS[@]} - failed )) if [ "$failed" -ne 0 ]; then - echo "[package] PRG matrix INCOMPLETE — at least one variant failed to build." >&2 + echo "[package] PRG matrix INCOMPLETE — $built/${#PACKAGE_VARIANTS[@]} variants built," \ + "$failed FAILED (see dist/build-*.log and the manifest)." >&2 + [ "$built" -gt 0 ] && exit 2 exit 1 fi echo "[package] PRG matrix complete (${#PACKAGE_VARIANTS[@]} variants)." diff --git a/tools/package/verify_release.py b/tools/package/verify_release.py index 9e64082..e771033 100755 --- a/tools/package/verify_release.py +++ b/tools/package/verify_release.py @@ -94,7 +94,9 @@ def check_reproducible(variants: list[dict]) -> None: print("\n=== 1. PRG byte-reproducibility (second build from clean) ===") for rec in variants: if rec.get("result") != "OK": - record(f"{rec['key']} rebuild", False, "first build had already failed") + # Not a reproducibility failure — there is nothing to reproduce. + # Reported once, up front, by report_missing_variants(). + print(f" [n/a ] {rec['key']} — did not build; see the blocker above") continue subprocess.run(["make", "clean"], cwd=REPO_ROOT, check=True, stdout=subprocess.DEVNULL) @@ -272,10 +274,39 @@ def check_listener() -> None: f"found {leftovers}" if leftovers else "clean") +def report_missing_variants(variants: list[dict]) -> int: + """Surface variants that never built, with the toolchain's own reason. + + These are release blockers, but they are not verification failures: there + is no artifact to verify. Counting them as failed checks would bury the + one line that says what to fix under a pile of consequential noise, so + they get their own section and their own exit path. + """ + missing = [r for r in variants if r.get("result") != "OK"] + if not missing: + return 0 + reasons = {} + if BUILD_INFO.is_file(): + for line in BUILD_INFO.read_text().splitlines(): + if line.startswith("failreason="): + key, _, why = line[len("failreason="):].partition(" ") + reasons[key] = why + print("\n" + "=" * 78) + print(f" BLOCKER — {len(missing)} of {len(variants)} variants did not build") + print("=" * 78) + for rec in missing: + print(f"\n {rec['prg']} (make {rec['args']})") + print(f" {reasons.get(rec['key'], 'no reason recorded')}") + print("\n dist/ holds only the variants that did build. This is not a" + "\n releasable matrix; fix the build before tagging.") + return len(missing) + + def main() -> int: variants = parse_build_info() print(f"Verifying {len(variants)} PRG variants and " f"{len(d64_images())} disk images in {DIST}") + missing = report_missing_variants(variants) if os.environ.get("SKIP_REBUILD") != "1": check_reproducible(variants) @@ -301,6 +332,11 @@ def main() -> int: for name in failed: print(f" - {name}") return 1 + if missing: + print(f"Everything present verifies, but {missing} variant(s) are " + f"MISSING — see the blocker above.") + print("RELEASE INCOMPLETE") + return 1 print("RELEASE ARTIFACTS VERIFIED") return 0 diff --git a/tools/package/write_manifest.sh b/tools/package/write_manifest.sh index b5f54de..a228556 100755 --- a/tools/package/write_manifest.sh +++ b/tools/package/write_manifest.sh @@ -38,6 +38,30 @@ echo " WARNING: built from a DIRTY working tree, not a clean checkout fi echo "ip65 blob : $(info ip65_blob_bytes) bytes, sha256 $(info ip65_blob_sha256)" echo +# A partial release must announce itself at the top, not bury the gap in a +# checksum list that simply has fewer lines than it should. Anyone diffing two +# manifests would otherwise have to notice an absence. +if grep -q '^failreason=' "$BUILD_INFO"; then +echo "!! INCOMPLETE RELEASE — some variants did not build !!" +echo +echo " The artifacts below are real and usable, but this is NOT the full" +echo " matrix and must not be tagged as one. Missing:" +echo +grep '^failreason=' "$BUILD_INFO" | cut -d= -f2- | while read -r key reason; do + prg="" + for line in "${PACKAGE_VARIANTS[@]}"; do + [ "$(variant_field "$line" 1)" = "$key" ] || continue + prg="$(variant_field "$line" 2)" + echo " $prg" + echo " make $(variant_field "$line" 3)" + done + [ -n "$prg" ] || echo " $key" + echo " $reason" + echo +done +echo "------------------------------------------------------------------------------" +echo +fi echo "submodule pins:" grep '^submodule=' "$BUILD_INFO" | cut -d= -f2- | while read -r sub sha tag; do printf ' %-18s %s %s\n' "$sub" "$tag" "$sha" @@ -69,6 +93,12 @@ for line in "${PACKAGE_VARIANTS[@]}"; do key="$(variant_field "$line" 1)" prg="$(variant_field "$line" 2)" note="$(variant_field "$line" 6)" + if grep -q "^failreason=$key " "$BUILD_INFO"; then + echo " $prg — NOT IN THIS RELEASE (failed to build)" + echo " $note" + echo + continue + fi echo " $prg" echo " $note" echo " disk: c64-https-$key.d64 (also on c64-https-$(variant_field "$line" 5).d64)" @@ -90,6 +120,9 @@ grep '^variant=' "$BUILD_INFO" | while read -r rec; do esac done args="$(printf '%s' "$rec" | sed -n 's/.*args=\(.*\) result=.*/\1/p')" + case "$rec" in + *" result=FAILED"*) bytes="FAILED" ;; + esac printf ' %-28s %8s make %s\n' "$prg" "$bytes" "$args" done echo