Add byte-exact reproduction CI for Knots releases - #2
Conversation
kwsantiago
left a comment
There was a problem hiding this comment.
Went through the reproduction harness, nice work, the core SHA-comparison path is sound (fresh detached worktree at the base tag, driver builds forward, so a partial/failed run can't coincidentally land on the release SHA). A few findings inline. The top one is a framing point about what a green run actually proves. Separately running run-all.sh to reproduce a release end-to-end for a tested-ACK; will follow up with the result.
|
|
||
| This branch adds a GitHub Actions workflow | ||
| (`.github/workflows/reproduce.yml`) that reproduces the most recent | ||
| published Knots release **byte-exact** from public sources only, on |
There was a problem hiding this comment.
from public sources only (and starting from nothing but public upstream sources, L103) slightly overclaims what a green run proves. synthesize-refs.sh recovers each merged branch tip as the second parent of the corresponding merge on the release tag's own first-parent chain, and bootstrap.sh tolerates fork-fetch failures, so the branch content is sourced from the release tag's own object graph, not independently verified against the public forks. Green attests that the driver + recorded resolutions + metadata deterministically replay the release tag's DAG back into the tag; it would not catch a release whose merged commits differ from what the forks actually contained (a backdoored tag still reproduces green). Suggest rewording to: the assembly is deterministically reproducible, and release-time branch tips are recovered from the release tag itself, not re-verified against the forks.
| printf(" target: %s tree=%s\n", y[1], y[2]); | ||
| exit 1 | ||
| }} | ||
| END {if (n == 0) exit 2}' |
There was a problem hiding this comment.
Under set -euo pipefail, this awk pipeline exiting non-zero (exit 1 on divergence, exit 2 on empty) trips set -e and kills the script here, before awk_ec=$? on the next line is read. So on a real Level-1 mismatch the Level 3/4 diagnostics and the GITHUB_STEP_SUMMARY PARTIAL/FAIL table never emit, exactly when they're needed. Level 4 is guarded with set +e/set -e but this one isn't. Wrap the pipeline in set +e/set -e, or append || awk_ec=$?.
| needs_pin=$(perl -ne ' | ||
| s/\s*#.*//; | ||
| next unless /^\s*(?:NM|TM|[am]*)\t\s*(?:[a-z]?\d+|\-|n\/a)\s+(\S+)/; | ||
| next unless $1 eq "'"$branch"'"; |
There was a problem hiding this comment.
$branch (from a release commit's merge subject) is spliced unquoted into a Perl string literal here; a subject containing a double-quote, $, @, or backslash breaks or injects Perl. Same class feeds the generated sed (the last= printf just below, and the cherrypick sub earlier), where a # or / in the value corrupts the sed and silently rewrites SPEC_IN. It's maintainer-controlled history so low likelihood, but it's an untrusted-input path running in CI. Pass values via $ENV{...} into Perl and pick a sed delimiter that can't occur, or validate against ^[A-Za-z0-9._/-]+$ first.
| local latest | ||
| latest=$(cd "$TMP" 2>/dev/null && \ | ||
| ls knots-*.knots[0-9][0-9][0-9][0-9][0-9][0-9][0-9][0-9].spec 2>/dev/null \ | ||
| | sort -V | tail -1) |
There was a problem hiding this comment.
The comment says 'latest YYYYMMDD suffix', but sort -V orders by base version first, then date. A patch release to an older base published after a newer base shipped (e.g. knots-29.2.knots20260601.spec alongside an existing knots-29.3.knots20260508.spec) would auto-select 29.3 despite the older date, so the nightly cron reproduces the wrong 'latest'. If date is the intent, sort on the extracted YYYYMMDD field rather than the whole filename.
There was a problem hiding this comment.
tACK 84e0be9. Reproduced v29.3.knots20260508 byte-exact via ./.ci/run-all.sh: repro HEAD == release tag f41f01e1e6de7025d52a865bef97f2a67277f0f3 (LEVEL 1 PASS).
One caveat inline that's worth fixing for anyone reproducing locally.
| say "linked assemble-knots-resolutions -> $TMP/assemble-knots-resolutions" | ||
|
|
||
| git -C "$REPRO" config user.email "$COMMITTER_EMAIL" | ||
| git -C "$REPRO" config user.name "merge-script" |
There was a problem hiding this comment.
This pins user.email/user.name so committer bytes match, but doesn't disable signing. With an ambient commit.gpgsign=true (a common dev setup) every assembled merge commit gets signed with the local key, so the final SHA diverges and Level 1 FAILs even though the assembly is correct, Level 2 (tree match) still passes, so a local reproducer sees a scary "not byte-exact" that is purely their git config. Recommend also setting commit.gpgsign=false and tag.gpgsign=false on the worktree here so ambient config can't contaminate the result. Verified both ways: signing on -> Level 1 FAIL (tree still matched); same spec in a clean env -> byte-exact.
This adds a GitHub Actions workflow that reproduces the most recent
published Knots release byte-exact from public sources only (no
local artifacts or trusting a prebuilt binary). On pushes to this branch
and on a nightly cron it re-derives the assembled tree from the dated
spec plus the pinned
assemble-knotsdriver, and compares the resultingHEAD SHA against the published release tag.
The README on this branch doubles as the walkthrough, and is the part
worth reading: it explains the spec model (how a Knots
release is derived from a Core tag plus a curated set of PRs, branches,
and cherry-picks) and shows how to reproduce any release
locally with a single command.
What a green run proves: the spec in this repo, replayed through the
historically pinned driver, still produces a tree whose commit SHA is
identical to the release tag's, starting from nothing but public
upstream sources. A red run means either a public artifact drifted or
the harness regressed; either is worth catching early.
All six final dated 29.x releases reproduce byte-exact today:
v29.3.knots20260508v29.3.knots20260507v29.3.knots20260210v29.2.knots20251110v29.2.knots20251010v29.1.knots20250903The workflow auto-detects the newest dated spec (currently
v29.3.knots20260508), so new releases are picked up as soon as theirspec lands; no workflow edits needed. Everything runs through
.ci/run-all.sh [tag], which works the same locally as in CI.While this is under review the runs live on my fork's Actions tab:
https://github.com/chrisguida/knots-assembly/actions/workflows/reproduce.yml
Opening as a draft while I confirm the run is green on the pushed branch.
Note for the maintainer on merge: the workflow's
pushtrigger listsboth
knots-specandci-reproduceon purpose: theci-reproduceentry is only so the CI runs on my fork's PR branch during review. Once
this merges to
knots-spec, that entry is dead weight, so feel free todrop it and leave just: