From 5004f81df2b064ca290518d130d4c80bef0adce5 Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Wed, 9 Sep 2026 01:45:29 +0000 Subject: [PATCH 1/7] test: verify the binary is built from this source, and that the server runs it jd's design: one controller arm per batch of tests, so a stale .so can never be measured and nothing has to rebuild per test. TWO QUESTIONS THE HARNESS COULD NOT ANSWER. selftest 110 compares the INSTALLED .so against the one built in this tree, so a missed install and a foreign overwrite were already caught. Nothing derived anything from the SOURCE TEXT, so both copies could agree with each other while both were stale against edited source. PGC_SKIP_BUILD opens that hole widest, because not rebuilding is its whole purpose. And a cp is not enough. shared_preload_libraries maps the library at postmaster start, so make install over a running instance changes the file and nothing else: every backend keeps executing the code it already mapped. A binary can match the source exactly while the server runs something older. THE SHAPE. Whoever builds records a fingerprint of the build inputs -- src/*.c, src/*.h, the Makefile, the control file, the shipped SQL. Every suite in the batch recomputes it and compares, which is one build per batch and one hash per suite. Then once the cluster is up, the suite compares the binary's mtime against pg_postmaster_start_time(). -- source: 28b66bd0ac0c matches the binary under test -- server: started after the binary was installed A stale source fingerprint and a server predating the binary are both FATAL, because every check that followed would be about code that is not running. A missing stamp is UNVERIFIED and said plainly rather than failed: a person who ran make install by hand has no stamp, and refusing would break a documented workflow. MEASURED, three arms: normal build, install, run source matches, server fresh, PASSED source edited + PGC_SKIP_BUILD=1 FATAL: not built from this source, exit 1 binary newer than the postmaster FATAL: server already running, exit 1 The two verdict functions are pure and take their inputs as arguments, for the same reason pgc_build_needs_clean does, so selftest 340 exercises fresh, stale, unknown, predates and the non-numeric cases without a build. It also requires that the fingerprint MOVES when a build input moves, STAYS when nothing does, and ignores a file that is not a build input -- a fingerprint that never changes reports fresh forever, which is this file's own failure mode one level down. WHAT WROTE THE STAMP IN THE WRONG PLACE, AND WHAT CAUGHT IT. The first revision wrote it in the skip-build branch, so every run recorded the source it was about to compare against and a suite measuring an edited tree reported "matches the binary under test". The arm that requires `stale` is what caught it. The stamp is now written only where the install succeeded, and the comment there says why. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a --- test/lib.sh | 147 ++++++++++++++++++ .../340-the-binary-must-be-built-from.sh | 84 ++++++++++ 2 files changed, 231 insertions(+) create mode 100644 test/selftest/340-the-binary-must-be-built-from.sh diff --git a/test/lib.sh b/test/lib.sh index f4f8638f..be1d3c49 100755 --- a/test/lib.sh +++ b/test/lib.sh @@ -222,6 +222,17 @@ pgc_setup() { echo " (refusing to report checks against the previously installed .so)" >&2 exit 1 fi + + # THIS RUN IS THE CONTROLLER for whatever follows in this batch: it built + # and installed, so record what the binary was built from. The stamp is + # written HERE and nowhere else. An earlier revision wrote it in the + # skip-build branch instead, which made the check tautological -- every run + # recorded the source it was about to compare against, so a suite measuring + # an edited tree reported "matches the binary under test". My own red arm + # caught it, which is the only reason this comment exists. + pgc_write_source_stamp \ + "$(pgc_source_stamp_path "$PGC_SRCDIR" "$PGC_MAJOR")" \ + "$(pgc_source_fingerprint "$PGC_SRCDIR")" else # Named because the variable is not what it says. It reads as "skip the # build" and means "skip the build AND the install, and test whatever is @@ -234,6 +245,29 @@ pgc_setup() { pgc_so_line + # And verify it, whether this run built or skipped. A skipped build is exactly + # when the binary can be older than the source. + _pgc_fresh_recorded="$(pgc_read_source_stamp \ + "$(pgc_source_stamp_path "$PGC_SRCDIR" "$PGC_MAJOR")")" + _pgc_fresh_current="$(pgc_source_fingerprint "$PGC_SRCDIR")" + case "$(pgc_freshness_verdict "$_pgc_fresh_recorded" "$_pgc_fresh_current")" in + fresh) + echo "-- source: $_pgc_fresh_current matches the binary under test" + ;; + stale) + echo "FATAL: the binary under test was not built from this source" >&2 + echo " source now $_pgc_fresh_current, binary built from $_pgc_fresh_recorded" >&2 + echo " (refusing to report checks about code that is not installed)" >&2 + exit 1 + ;; + unknown) + # Not a failure: a person who ran make install by hand has no stamp, and + # refusing would break a documented workflow. Said plainly so the reader + # knows which question was not answered. + echo "-- source: $_pgc_fresh_current, freshness UNVERIFIED (no stamp for major $PGC_MAJOR)" + ;; + esac + echo "-- initdb" pgc_pg "initdb -D '$PGC_PGDATA' -A trust" >/dev/null 2>&1 { @@ -342,6 +376,33 @@ pgc_setup() { # "already exists" looks exactly like a suite that landed on someone else's # cluster. Removing the noise is most of the value here; failing loudly on # the impossible case is the rest. + # The server is up, so now ask whether it is RUNNING the binary we verified on + # disk. A cp is not enough: shared_preload_libraries maps the .so at start. + { + local _so_path _so_epoch _pm_epoch + _so_path="$("$PGC_PG_CONFIG" --pkglibdir)/pgcolumnar.so" + _so_epoch="$(stat -c %Y "$_so_path" 2>/dev/null || echo '')" + _pm_epoch="$(psql_admin_scalar \ + "SELECT floor(extract(epoch from pg_postmaster_start_time()))::bigint;" \ + 2>/dev/null | tr -dc '0-9')" + case "$(pgc_running_binary_verdict "$_so_epoch" "$_pm_epoch")" in + fresh) + echo "-- server: started after the binary was installed" + ;; + predates) + echo "FATAL: this server was already running when the binary changed" >&2 + echo " .so installed at epoch $_so_epoch, postmaster started $_pm_epoch" >&2 + echo " shared_preload_libraries maps the library at start, so the" >&2 + echo " backends are executing older code than the file on disk." >&2 + echo " Restart the cluster; a reinstall alone does not reload it." >&2 + exit 1 + ;; + unknown) + echo "-- server: could not compare binary and postmaster timestamps" + ;; + esac + } + { local _exists @@ -524,6 +585,92 @@ pgc_build_needs_clean() { # backslash inside single quotes -- which emits the four bytes `1 9 \ n`. That # passed unnoticed because the reader does tr -dc '0-9' and strips the junk; a # direct comparison against the major failed. Found in review, not by a check. +# ---- is the binary under test built from the source in this tree? ----------- +# +# THE GAP THIS CLOSES, AND WHAT ALREADY COVERED THE REST. +# +# selftest 110 compares the INSTALLED .so against the one built in this tree, so a +# missed install and a foreign overwrite are already caught. Neither that check nor +# pgc_so_line can see the case where BOTH copies agree with each other and both are +# stale against edited source: nothing in the harness derives anything from the +# source text. That is the hole, and it is the one PGC_SKIP_BUILD opens widest, +# because its whole purpose is not to rebuild. +# +# Measured cost of the miss: a probe run under PGC_SKIP_BUILD=1 that asserted its fix +# was "present" by grepping the SOURCE while measuring a .so another worktree had +# installed. The control failed and the failure read as a product defect. +# +# THE CONTROLLER SHAPE. Whoever builds records a fingerprint of the build inputs +# beside the install. Every suite in that batch recomputes the fingerprint and +# compares. One hash per suite, one build per batch, and a stale binary can no longer +# report a plausible list of checks. +# +# pgc_freshness_verdict is a pure function of two strings so it can be tested without +# a build, the same reason pgc_build_needs_clean is. + +# The build inputs, hashed. Sources, headers, the Makefile, the control file and the +# SQL that ships: anything whose change should invalidate a binary. Sorted, because a +# directory listing is not ordered and an unordered input makes the hash unstable. +pgc_source_fingerprint() { # pgc_source_fingerprint DIR -> hash + local dir="${1:-.}" + { + find "$dir/src" -maxdepth 1 -type f \( -name '*.c' -o -name '*.h' \) -print0 2>/dev/null + find "$dir" -maxdepth 1 -type f \( -name 'Makefile' -o -name '*.control' \ + -o -name '*.sql' \) -print0 2>/dev/null + } | sort -z | xargs -0 cat 2>/dev/null | md5sum | cut -c1-12 +} + +# fresh the binary was built from this source +# stale it was not, and every check that follows would be about the wrong code +# unknown nobody in this batch recorded a fingerprint, so this cannot be answered +pgc_freshness_verdict() { # pgc_freshness_verdict RECORDED CURRENT -> verdict + local recorded="${1:-}" current="${2:-}" + [ -z "$recorded" ] && { echo unknown; return; } + [ -z "$current" ] && { echo unknown; return; } + [ "$recorded" = "$current" ] && echo fresh || echo stale +} + +# AND A cp IS NOT ENOUGH: THE POSTMASTER MAPS THE .so AT START. +# +# shared_preload_libraries='pgcolumnar' means the library is loaded once, when the +# postmaster starts. `make install` over a running instance changes the file and +# nothing else: every backend keeps executing the code it already mapped. So a +# binary can match the source exactly and the server can still be running something +# older, which the source check above cannot see. +# +# The harness normally escapes this because each suite initdb's and starts its own +# cluster after the install. It stops escaping it the moment a cluster outlives an +# install: a persistent cluster reused between runs, a bench rig left up, or a second +# batch installing into a prefix whose server is already serving. +# +# So compare when the binary was installed against when the server started. A +# postmaster older than the binary has the old code mapped, whatever the file says. +# +# fresh the server started after the binary was installed +# predates the binary is newer than the server, so the server has older code +# unknown one of the two timestamps could not be read +pgc_running_binary_verdict() { # pgc_running_binary_verdict SO_EPOCH PM_EPOCH + local so="${1:-}" pm="${2:-}" + case "$so" in '' | *[!0-9]*) echo unknown; return ;; esac + case "$pm" in '' | *[!0-9]*) echo unknown; return ;; esac + [ "$pm" -ge "$so" ] && echo fresh || echo predates +} + +pgc_write_source_stamp() { # pgc_write_source_stamp FILE HASH + printf '%s\n' "${2:-}" > "${1:-/dev/null}" 2>/dev/null || true +} + +pgc_read_source_stamp() { # pgc_read_source_stamp FILE -> hash or empty + [ -r "${1:-}" ] || { echo ""; return; } + tr -dc 'a-f0-9' < "$1" | head -c 12 +} + +# The stamp lives beside the tree that built the binary, keyed by major, because one +# tree installs into several prefixes and each has its own binary. +pgc_source_stamp_path() { # pgc_source_stamp_path DIR MAJOR + printf '%s/.pgc_source_stamp.%s\n' "${1:-.}" "${2:-0}" +} + pgc_write_build_stamp() { printf '%s\n' "${2:-}" > "${1:-/dev/null}" 2>/dev/null || true } diff --git a/test/selftest/340-the-binary-must-be-built-from.sh b/test/selftest/340-the-binary-must-be-built-from.sh new file mode 100644 index 00000000..e861bcc9 --- /dev/null +++ b/test/selftest/340-the-binary-must-be-built-from.sh @@ -0,0 +1,84 @@ +# ---- the binary under test must be built from this source, and the server must +# ---- be running it (#432 follow-up, jd) +# +# TWO QUESTIONS, AND THE HARNESS COULD ANSWER NEITHER. +# +# selftest 110 compares the INSTALLED .so against the one built in this tree, so a +# missed install and a foreign overwrite are caught. Nothing derived anything from +# the SOURCE TEXT, so both copies could agree with each other while both were stale +# against edited source. PGC_SKIP_BUILD opens that hole widest, because not +# rebuilding is its entire purpose. +# +# And a cp is not enough. shared_preload_libraries='pgcolumnar' maps the library at +# postmaster start, so `make install` over a running instance changes the file and +# nothing else: every backend keeps executing the code it already mapped. A binary +# can match the source exactly while the server runs something older. +# +# THE CONTROLLER SHAPE. Whoever builds records a fingerprint of the build inputs. +# Every suite in the batch recomputes it and compares, so it is one build per batch +# and one hash per suite rather than a rebuild per suite. Then, once the cluster is +# up, the suite compares the binary's mtime against pg_postmaster_start_time(). +# +# The verdict functions are pure, taking their inputs as arguments, for the same +# reason pgc_build_needs_clean is: they can be exercised here without a build. +# +# THE MEASURED COST OF NOT HAVING THIS. A probe run under PGC_SKIP_BUILD=1 asserted +# its fix was "present" by grepping the SOURCE while measuring a .so a different +# worktree had installed. Its control failed, and the failure read as a product +# defect for several minutes. +# +# AND ONE FROM WRITING IT. The first revision wrote the stamp in the skip-build +# branch, so every run recorded the source it was about to compare against and a +# suite measuring an edited tree reported "matches the binary under test". The arm +# below that requires `stale` is what caught it. + +check "a fingerprint equal to the record is fresh" \ + "$(pgc_freshness_verdict abc123abc123 abc123abc123)" "fresh" +check "a fingerprint different from the record is stale" \ + "$(pgc_freshness_verdict abc123abc123 def456def456)" "stale" +check "no record at all is unknown, not fresh" \ + "$(pgc_freshness_verdict "" abc123abc123)" "unknown" +check "and an uncomputable current fingerprint is unknown, not stale" \ + "$(pgc_freshness_verdict abc123abc123 "")" "unknown" + +check "a server started after the binary is fresh" \ + "$(pgc_running_binary_verdict 1000 2000)" "fresh" +check "a server started at the same second is fresh" \ + "$(pgc_running_binary_verdict 1000 1000)" "fresh" +check "a server older than the binary predates it" \ + "$(pgc_running_binary_verdict 2000 1000)" "predates" +check "a missing binary timestamp is unknown, not predates" \ + "$(pgc_running_binary_verdict "" 1000)" "unknown" +check "a missing postmaster timestamp is unknown, not predates" \ + "$(pgc_running_binary_verdict 1000 "")" "unknown" +check "and a non-numeric timestamp is unknown rather than compared as text" \ + "$(pgc_running_binary_verdict 1000 "not-a-time")" "unknown" + +# The fingerprint has to MOVE when a build input moves and STAY when nothing does. +# A fingerprint that never changes reports fresh forever, which is the failure this +# whole file exists to prevent, one level down. +_fp_dir="$(mktemp -d)" +mkdir -p "$_fp_dir/src" +printf 'int x;\n' > "$_fp_dir/src/a.c" +printf 'PG_CONFIG = pg_config\n' > "$_fp_dir/Makefile" +_fp_one="$(pgc_source_fingerprint "$_fp_dir")" +check "a fingerprint is 12 hex characters" \ + "$(printf '%s' "$_fp_one" | grep -cE '^[0-9a-f]{12}$')" "1" +check "the same tree fingerprints the same twice" \ + "$(pgc_source_fingerprint "$_fp_dir")" "$_fp_one" +printf 'int x; int y;\n' > "$_fp_dir/src/a.c" +check "editing a source file moves the fingerprint" \ + "$([ "$(pgc_source_fingerprint "$_fp_dir")" != "$_fp_one" ] && echo moved || echo same)" \ + "moved" +_fp_two="$(pgc_source_fingerprint "$_fp_dir")" +printf 'int x;\n' > "$_fp_dir/src/a.c" +check "and restoring it restores the fingerprint" \ + "$(pgc_source_fingerprint "$_fp_dir")" "$_fp_one" +printf 'notes\n' > "$_fp_dir/README-not-a-build-input" +check "a file that is not a build input does not move it" \ + "$(pgc_source_fingerprint "$_fp_dir")" "$_fp_one" +printf 'int z;\n' > "$_fp_dir/src/b.c" +check "adding a source file moves it" \ + "$([ "$(pgc_source_fingerprint "$_fp_dir")" != "$_fp_one" ] && echo moved || echo same)" \ + "moved" +rm -rf "$_fp_dir" From 36b09f12285465b674724363c07e96c0208b20df Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Wed, 9 Sep 2026 03:19:41 +0000 Subject: [PATCH 2/7] test: the matrix controller records the fingerprint its batch was built from jd's ask: a controller arm that verifies the build is fresh against the batch of tests being run, so we never measure a stale .so but also do not rebuild for every test. run_all_versions.sh is that controller. It builds and installs once per major and then runs every suite with PGC_SKIP_BUILD=1, so the suites have no way of their own to tell whether the binary they measure came from this tree. It now records the fingerprint of the build inputs after a successful install, and lib.sh checks it in every suite whether that suite built or skipped. In a subshell sourcing lib.sh rather than recomputing the hash inline: two implementations of one fingerprint drift, and the suites compare against exactly what this writes. Not `|| true`. If the stamp cannot be written, every suite in the batch reports "freshness UNVERIFIED" and the controller silently stops being a controller -- the batch degrades to the state this exists to prevent, with nothing saying so. A failure now prints what it means for the run below it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a --- test/run_all_versions.sh | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/test/run_all_versions.sh b/test/run_all_versions.sh index 97e3ae5c..4371be84 100755 --- a/test/run_all_versions.sh +++ b/test/run_all_versions.sh @@ -710,6 +710,31 @@ for pgc in "${CONFIGS[@]}"; do continue fi + # THIS RUNNER IS THE CONTROLLER FOR THIS MAJOR'S BATCH. It built and installed + # once; the suites below all run with PGC_SKIP_BUILD=1 and would otherwise have + # no way to tell whether the binary they measure came from this tree. Record the + # fingerprint of the build inputs so each of them can check it. + # + # In a subshell sourcing lib.sh rather than recomputing the hash here: two + # implementations of one fingerprint would drift, and the suites compare against + # whatever this writes. lib.sh's top level is assignments and function + # definitions only, so sourcing it costs nothing and starts nothing. + # NOT `|| true`. If the stamp cannot be written, every suite in this batch + # reports "freshness UNVERIFIED" and the controller arm silently stops being a + # controller arm -- the whole batch degrades to the state this exists to + # prevent, and nothing says so. Say so. + if ( + . "$builddir/test/lib.sh" + pgc_write_source_stamp \ + "$(pgc_source_stamp_path "$builddir" "$major")" \ + "$(pgc_source_fingerprint "$builddir")" + ); then + : + else + echo "WARNING: could not record the source stamp for major $major." >&2 + echo " Every suite below will report freshness UNVERIFIED." >&2 + fi + verfail=0 results="" maxjobs="${PGC_JOBS:-6}" From 5036dea46101044e70cf10dcc54320618f871bc3 Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Wed, 9 Sep 2026 03:22:25 +0000 Subject: [PATCH 3/7] test: ignore the source stamp the harness writes pgc_setup writes .pgc_source_stamp. beside the source it fingerprints, so running any suite from a checkout left an untracked file in `git status`. This project gates on a clean tree, so a harness artifact that dirties one is a recurring false alarm rather than a cosmetic issue. Placed next to .pgc_built_for_major, which is the same kind of file written for the same reason. Verified the new rule is the one that matches -- the file was NOT ignored before, and `git check-ignore -v` now names .gitignore:10 -- and that the existing rule still matches its own file. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 442128df..b672072e 100644 --- a/.gitignore +++ b/.gitignore @@ -7,5 +7,6 @@ results/ regression.diffs regression.out .pgc_built_for_major +.pgc_source_stamp.* __pycache__/ *.pyc From 295a3d299af98530c9163c1bf3bdb99807480855 Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Wed, 9 Sep 2026 14:19:09 +0000 Subject: [PATCH 4/7] docs: record the build-freshness check in the changelog Same omission as #897: this project records test-infrastructure changes in CHANGELOG.md and I opened the PR without an entry. The entry says what both checks refuse, and says that neither fails when it cannot answer -- an unstamped tree prints "freshness UNVERIFIED" and names the question it did not answer, rather than printing nothing and reading as a pass. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a --- CHANGELOG.md | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c4bce3a..35e92b5e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -222,6 +222,33 @@ true until the next version shipped. ## [1.0-alpha3] - 2026-09-02 ### Added +- The test harness refuses to measure a binary that was not built from the source + under test. + + `test/run_all_versions.sh` builds and installs once per major and then runs every + suite with `PGC_SKIP_BUILD=1`, so no suite could tell whether the binary it + measured came from this tree. The controller now records a fingerprint of the + build inputs after a successful install, and `pgc_setup` checks it in every + suite, whether that suite built or skipped. A mismatch is fatal and names both + fingerprints. + + A second check covers the other half. `make install` does not reload anything: + `shared_preload_libraries` maps the library at postmaster start, so a reinstall + under a running server leaves the backends executing older code than the file on + disk. The `.so` mtime is compared against `pg_postmaster_start_time()`, and the + message says to restart rather than only that something is wrong. + + Neither check fails when it cannot answer. Someone who ran `make install` by hand + has no stamp, so that case prints `freshness UNVERIFIED` and says which question + went unanswered, rather than printing nothing and letting a reader assume it + passed. + + `pgc_freshness_verdict` and `pgc_running_binary_verdict` are pure functions of + strings, so `test/selftest/340-the-binary-must-be-built-from.sh` drives them + directly: 16 arms including both empty inputs, a non-numeric epoch, equal + timestamps on the boundary, and sensitivity to each fingerprint input class. + `harness_selftest` goes from 261 checks to 277. + - A nanosecond Arrow import says how many values lost precision, and still imports every row. From 9058515eaaf910c7f1f0e82dcac9c32c0717d0df Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Wed, 9 Sep 2026 15:53:44 +0000 Subject: [PATCH 5/7] test: close three false assurances in the freshness check All three from @jdatcmd's review. Each failed the same way: the check reported an assurance it had not earned, which is worse than reporting nothing, because the line is what stops the next person looking. THE FINGERPRINT DID NOT SEE objstore/. It read $dir/src only, and objstore/columnar_objstore_module.c is a build input -- the top-level Makefile builds and installs it by recursion as a separate shared library. Editing it left the fingerprint unchanged, so objstore_module, objstore_sink_write and objstore_stash_recovery could measure a stale module while the run printed "matches the binary under test". Reproduced with his control: baseline 1449dc9dba17 edit objstore/columnar_objstore_module.c c85763de5bb5 CHANGED edit src/columnar_projection.c (control) 4fffcff2b65f CHANGED edit objstore/Makefile 75d57e06199d CHANGED restored 1449dc9dba17 back to baseline Naming objstore/ would fix today and fail the next time a module is added, so pgc_source_build_dirs DERIVES the set: every directory with its own Makefile, the same rule the build follows. The selftest asserts the property rather than the list -- it parses the `$(MAKE) -C` recursion out of the Makefile and requires every target to be covered -- with a premise check that the parse found something, because a guard that found nothing to check has abstained rather than passed. Proved by removal: reverting to src-only reddens both arms with `missing: objstore`. THE POSTMASTER ARM COULD NOT FIRE THROUGH ANY SHIPPED PATH. pgc_setup always initdb's a fresh cluster and starts it after the install, so the postmaster is always newer than the .so and `predates` was unreachable. selftest 340 fed the verdict function fixture values, which proves its arithmetic and not its call site: the call could have been deleted with every check still passing. pgc_check_running_binary is now extracted from pgc_setup and takes the library path as an argument. The selftest points it at a file it has just touched, so the stat, the pg_postmaster_start_time() query, the verdict and the refusal all run for real against the live cluster and only the path is redirected -- nothing has to touch the installed library to prove the guard fires. Proved by removal: neutering the `return 1` reddens "a library newer than the running server is REFUSED" with got [0] want [1]. devloop.sh GOT UNVERIFIED, NOT ENFORCED, and it is the loop a human uses while editing C. It builds and installs and then runs suites with PGC_SKIP_BUILD=1, and only run_all_versions.sh wrote the stamp -- so edit-a-file-and-forget-to- rebuild was unprotected in exactly the place it happens. devloop is a controller and now records what it installed. Measured end to end: A devloop, clean tree -- source: 1449dc9dba17 matches the binary under test B edit C, no rebuild exit 1, FATAL, source now c636277e827f, binary built from 1449dc9dba17 C restore, same command exit 0, matches, PASSED pgc_major_of is extracted alongside it because devloop writes a stamp whose PATH is keyed on the major and pgc_setup reads it back. Two copies of that sed would be two answers to "which major", and the failure would be a stamp written where nothing looks for it: a silent UNVERIFIED rather than an error. harness_selftest goes from 277 checks to 288. One process note. My first removal proof for the objstore fix SILENTLY FAILED TO APPLY -- nested-heredoc escaping -- and printed PASS. The assert inside the mutation caught it. Without that I would have reported a guard as proven when it had never been exercised. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a --- test/devloop.sh | 27 +++++ test/lib.sh | 111 ++++++++++++++---- .../340-the-binary-must-be-built-from.sh | 96 +++++++++++++++ 3 files changed, 208 insertions(+), 26 deletions(-) diff --git a/test/devloop.sh b/test/devloop.sh index 64adf09a..5be67552 100755 --- a/test/devloop.sh +++ b/test/devloop.sh @@ -77,6 +77,33 @@ if ! bash test/rebuild.sh "$PGC"; then exit 1 fi +# THIS SCRIPT IS A CONTROLLER, so it records what it just installed. +# +# It builds and installs, then runs every suite with PGC_SKIP_BUILD=1. Without a +# stamp those suites print "freshness UNVERIFIED" and continue, so the class this +# check exists to catch -- edit a file, forget to rebuild, measure the old binary +# -- was unprotected in exactly the loop where a human does it (@jdatcmd, #898 +# review). run_all_versions.sh was the only writer, and nobody edits C inside the +# matrix. +# +# In a subshell sourcing lib.sh rather than recomputing the hash here: two +# implementations of one fingerprint drift, and the suites compare against +# whatever this writes. +# +# NOT `|| true`. If the stamp cannot be written, every suite below reports +# UNVERIFIED and this stops being a controller with nothing saying so. +if ( + . "$BUILD/test/lib.sh" + pgc_write_source_stamp \ + "$(pgc_source_stamp_path "$BUILD" "$(pgc_major_of "$PGC")")" \ + "$(pgc_source_fingerprint "$BUILD")" +); then + : +else + echo "devloop: could not record the source stamp; the suites below will" >&2 + echo " report freshness UNVERIFIED rather than checking it" >&2 +fi + rc=0 for s in "$@"; do echo "====================================================================" diff --git a/test/lib.sh b/test/lib.sh index be1d3c49..7eb50d45 100755 --- a/test/lib.sh +++ b/test/lib.sh @@ -141,7 +141,7 @@ pgc_setup() { # A behavior that exists only from some major is core's, not this extension's, # and a check written against the newer one fails on the older ones for a # reason that is not a defect. Branch on this rather than deriving it again. - PGC_MAJOR="$("$PGC_PG_CONFIG" --version | sed -E 's/^[^0-9]*([0-9]+).*/\1/')" + PGC_MAJOR="$(pgc_major_of "$PGC_PG_CONFIG")" # Derived from this process rather than a fixed 54329: two suites run at # once on one box otherwise start on the same port, and the loser reports a # wall of ERROR: database "regress" already exists with no named check @@ -378,30 +378,7 @@ pgc_setup() { # the impossible case is the rest. # The server is up, so now ask whether it is RUNNING the binary we verified on # disk. A cp is not enough: shared_preload_libraries maps the .so at start. - { - local _so_path _so_epoch _pm_epoch - _so_path="$("$PGC_PG_CONFIG" --pkglibdir)/pgcolumnar.so" - _so_epoch="$(stat -c %Y "$_so_path" 2>/dev/null || echo '')" - _pm_epoch="$(psql_admin_scalar \ - "SELECT floor(extract(epoch from pg_postmaster_start_time()))::bigint;" \ - 2>/dev/null | tr -dc '0-9')" - case "$(pgc_running_binary_verdict "$_so_epoch" "$_pm_epoch")" in - fresh) - echo "-- server: started after the binary was installed" - ;; - predates) - echo "FATAL: this server was already running when the binary changed" >&2 - echo " .so installed at epoch $_so_epoch, postmaster started $_pm_epoch" >&2 - echo " shared_preload_libraries maps the library at start, so the" >&2 - echo " backends are executing older code than the file on disk." >&2 - echo " Restart the cluster; a reinstall alone does not reload it." >&2 - exit 1 - ;; - unknown) - echo "-- server: could not compare binary and postmaster timestamps" - ;; - esac - } + pgc_check_running_binary "$("$PGC_PG_CONFIG" --pkglibdir)/pgcolumnar.so" || exit 1 { local _exists @@ -611,10 +588,36 @@ pgc_build_needs_clean() { # The build inputs, hashed. Sources, headers, the Makefile, the control file and the # SQL that ships: anything whose change should invalidate a binary. Sorted, because a # directory listing is not ordered and an unordered input makes the hash unstable. +# pgc_source_build_dirs DIR -> one source directory per line +# +# DERIVED, NOT LISTED. The first version read $dir/src only, and objstore/ is a +# SEPARATE shared library that the top-level Makefile builds and installs by +# recursion. Editing objstore/columnar_objstore_module.c left the fingerprint +# unchanged, so objstore_module, objstore_sink_write and objstore_stash_recovery +# could measure a stale module while the suite printed "matches the binary under +# test" (@jdatcmd, #898 review). A stale binary under an explicit assurance is +# worse than one under no assurance, because the line is what stops the next +# person checking. +# +# A remembered list would have the same defect again the next time a module is +# added, so this returns every directory that has its own Makefile. That is the +# same rule the build itself follows. +pgc_source_build_dirs() { # pgc_source_build_dirs DIR -> dirs + local dir="${1:-.}" + printf '%s\n' "$dir/src" + find "$dir" -mindepth 2 -maxdepth 2 -type f -name Makefile \ + -printf '%h\n' 2>/dev/null | grep -v "^$dir/src$" || true +} + pgc_source_fingerprint() { # pgc_source_fingerprint DIR -> hash local dir="${1:-.}" + local d { - find "$dir/src" -maxdepth 1 -type f \( -name '*.c' -o -name '*.h' \) -print0 2>/dev/null + while IFS= read -r d; do + [ -n "$d" ] || continue + find "$d" -maxdepth 1 -type f \( -name '*.c' -o -name '*.h' \ + -o -name 'Makefile' \) -print0 2>/dev/null + done < <(pgc_source_build_dirs "$dir") find "$dir" -maxdepth 1 -type f \( -name 'Makefile' -o -name '*.control' \ -o -name '*.sql' \) -print0 2>/dev/null } | sort -z | xargs -0 cat 2>/dev/null | md5sum | cut -c1-12 @@ -649,6 +652,52 @@ pgc_freshness_verdict() { # pgc_freshness_verdict RECORDED CURRENT -> verdict # fresh the server started after the binary was installed # predates the binary is newer than the server, so the server has older code # unknown one of the two timestamps could not be read +# pgc_check_running_binary SO_PATH +# +# The runtime half of the freshness check: does the RUNNING server postdate the +# library on disk? Extracted from pgc_setup so it can be driven, because the +# verdict function alone could not be. +# +# WHY THAT MATTERS. Every suite initdb's a fresh cluster and starts it after the +# install, so through any shipped path the postmaster is always newer than the +# .so and `predates` is UNREACHABLE. selftest 340 fed the verdict function +# fixture values and proved its arithmetic; the CALL SITE could have been deleted +# with every check still passing (@jdatcmd, #898 review). That is this project's +# own rule about a helper a suite merely sources. +# +# Taking the path as an argument is what makes it reachable: the selftest points +# it at a file it has just touched, so the stat, the pg_postmaster_start_time() +# query, the verdict and the refusal all run for real and only the path is +# redirected. Nothing has to touch the installed library to prove the guard +# fires. +# +# Returns non-zero rather than calling exit, so a caller that is not a suite can +# turn it into its own kind of failure. pgc_setup passes the status through. +pgc_check_running_binary() { # pgc_check_running_binary SO_PATH -> 0|1 + local _so_path="$1" _so_epoch _pm_epoch + _so_epoch="$(stat -c %Y "$_so_path" 2>/dev/null || echo '')" + _pm_epoch="$(psql_admin_scalar \ + "SELECT floor(extract(epoch from pg_postmaster_start_time()))::bigint;" \ + 2>/dev/null | tr -dc '0-9')" + case "$(pgc_running_binary_verdict "$_so_epoch" "$_pm_epoch")" in + fresh) + echo "-- server: started after the binary was installed" + ;; + predates) + echo "FATAL: this server was already running when the binary changed" >&2 + echo " .so installed at epoch $_so_epoch, postmaster started $_pm_epoch" >&2 + echo " shared_preload_libraries maps the library at start, so the" >&2 + echo " backends are executing older code than the file on disk." >&2 + echo " Restart the cluster; a reinstall alone does not reload it." >&2 + return 1 + ;; + unknown) + echo "-- server: could not compare binary and postmaster timestamps" + ;; + esac + return 0 +} + pgc_running_binary_verdict() { # pgc_running_binary_verdict SO_EPOCH PM_EPOCH local so="${1:-}" pm="${2:-}" case "$so" in '' | *[!0-9]*) echo unknown; return ;; esac @@ -667,6 +716,16 @@ pgc_read_source_stamp() { # pgc_read_source_stamp FILE -> hash or empty # The stamp lives beside the tree that built the binary, keyed by major, because one # tree installs into several prefixes and each has its own binary. +# pgc_major_of PG_CONFIG -> major version number +# +# One definition, because devloop.sh now writes a stamp whose PATH is keyed on +# the major and pgc_setup reads it back. Two copies of this sed would be two +# answers to "which major", and the failure would be a stamp written where +# nothing looks for it -- a silent UNVERIFIED rather than an error. +pgc_major_of() { # pgc_major_of PG_CONFIG -> major + "$1" --version | sed -E 's/^[^0-9]*([0-9]+).*/\1/' +} + pgc_source_stamp_path() { # pgc_source_stamp_path DIR MAJOR printf '%s/.pgc_source_stamp.%s\n' "${1:-.}" "${2:-0}" } diff --git a/test/selftest/340-the-binary-must-be-built-from.sh b/test/selftest/340-the-binary-must-be-built-from.sh index e861bcc9..282023d4 100644 --- a/test/selftest/340-the-binary-must-be-built-from.sh +++ b/test/selftest/340-the-binary-must-be-built-from.sh @@ -82,3 +82,99 @@ check "adding a source file moves it" \ "$([ "$(pgc_source_fingerprint "$_fp_dir")" != "$_fp_one" ] && echo moved || echo same)" \ "moved" rm -rf "$_fp_dir" + +# ---- every directory the build recurses into is in the fingerprint ----------- +# +# THE PROPERTY, NOT A LIST. The first fingerprint read $dir/src only, so editing +# objstore/columnar_objstore_module.c -- a SEPARATE shared library the top-level +# Makefile builds and installs by recursion -- left it unchanged, and three +# objstore suites could measure a stale module while the run printed "matches +# the binary under test" (@jdatcmd, #898 review). +# +# Naming objstore/ here would fix today and fail the next time a module is added, +# so this arm reads the recursion out of the Makefile and requires every target +# directory to be covered. If someone adds `$(MAKE) -C $(FOO_DIR)` and FOO_DIR +# has no Makefile of its own, or is otherwise missed, this reddens. +_bd_root="$PGC_SRCDIR" +_bd_covered="$(pgc_source_build_dirs "$_bd_root" | sed "s|^$_bd_root/||" | sort)" + +check "PREMISE the fingerprint covers at least src" \ + "$(printf '%s\n' "$_bd_covered" | grep -cx 'src')" "1" + +# Every `$(MAKE) -C $(SOMETHING_DIR)` in the top-level Makefile, resolved to the +# directory name that variable ends with. +_bd_missing="" +_bd_seen=0 +while IFS= read -r _bd_var; do + [ -n "$_bd_var" ] || continue + _bd_val="$(sed -n "s/^[[:space:]]*$_bd_var[[:space:]]*=[[:space:]]*\(.*\)$/\1/p" \ + "$_bd_root/Makefile" | head -1)" + _bd_name="${_bd_val##*/}" + [ -n "$_bd_name" ] || continue + _bd_seen=$(( _bd_seen + 1 )) + printf '%s\n' "$_bd_covered" | grep -qx "$_bd_name" || \ + _bd_missing="$_bd_missing $_bd_name" +done < "$_bd_probe" +check "a new source file under objstore moves the fingerprint" \ + "$([ "$(pgc_source_fingerprint "$_bd_root")" != "$_bd_before" ] && echo moved || echo same)" \ + "moved" +rm -f "$_bd_probe" +check "and removing it restores the fingerprint" \ + "$(pgc_source_fingerprint "$_bd_root")" "$_bd_before" + +# ---- the postmaster arm must be able to FIRE, not just to compute ----------- +# +# The arms above feed pgc_running_binary_verdict fixture values and prove its +# arithmetic. They do not prove anything calls it. Every suite initdb's a fresh +# cluster and starts it AFTER the install, so through any shipped path the +# postmaster is always newer than the .so: `predates` is unreachable, and the +# call site could have been deleted with every check still passing +# (@jdatcmd, #898 review). +# +# These arms drive the real call site against the REAL running cluster. The stat, +# the pg_postmaster_start_time() query, the verdict and the refusal all execute; +# only the path is redirected, so nothing has to touch the installed library to +# prove the guard fires. +_rb_dir="$(mktemp -d)" +_rb_so="$_rb_dir/pgcolumnar.so" + +# A file written now is newer than a postmaster that started earlier: predates. +: > "$_rb_so" +touch -d "+1 hour" "$_rb_so" +_rb_out="$(pgc_check_running_binary "$_rb_so" 2>&1)"; _rb_rc=$? +check "a library newer than the running server is REFUSED" "$_rb_rc" "1" +check "and the refusal says the server must be restarted" \ + "$(printf '%s\n' "$_rb_out" | grep -c 'Restart the cluster')" "1" + +# The control: same call, same server, a library older than the postmaster. +touch -d "-1 hour" "$_rb_so" +_rb_out2="$(pgc_check_running_binary "$_rb_so" 2>&1)"; _rb_rc2=$? +check "a library older than the running server is accepted" "$_rb_rc2" "0" +check "and it says so rather than staying silent" \ + "$(printf '%s\n' "$_rb_out2" | grep -c 'started after the binary was installed')" "1" + +# A path that does not exist must read unknown, not fresh and not a crash. +rm -f "$_rb_so" +_rb_out3="$(pgc_check_running_binary "$_rb_so" 2>&1)"; _rb_rc3=$? +check "an unreadable library is not a failure" "$_rb_rc3" "0" +check "but it says which question went unanswered" \ + "$(printf '%s\n' "$_rb_out3" | grep -c 'could not compare')" "1" +rm -rf "$_rb_dir" From b4a77c923eb4ac74253c65d1c5d41ed67b5cd75e Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Wed, 9 Sep 2026 16:27:31 +0000 Subject: [PATCH 6/7] test: brace the variable in the Makefile-recursion parser (shellcheck SC1087) `$_bd_var[[:space:]]` reads to shellcheck as an array expansion, so `shellcheck -S error` -- a CI-only job the local gate never runs -- failed on the selftest arm added in the last push. Braced to `${_bd_var}`. Semantically identical in bash; proved by running the suite rather than by inspection. harness_selftest.sh: 288 passed + 0 failed + 0 unrunnable, including `PREMISE the Makefile's recursion was actually parsed`, which is the check that goes red if the sed stops resolving the variable name. `shellcheck -S error -s bash test/*.sh test/selftest/*.sh` now exits 0. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a --- test/selftest/340-the-binary-must-be-built-from.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/selftest/340-the-binary-must-be-built-from.sh b/test/selftest/340-the-binary-must-be-built-from.sh index 282023d4..8064fdff 100644 --- a/test/selftest/340-the-binary-must-be-built-from.sh +++ b/test/selftest/340-the-binary-must-be-built-from.sh @@ -107,7 +107,7 @@ _bd_missing="" _bd_seen=0 while IFS= read -r _bd_var; do [ -n "$_bd_var" ] || continue - _bd_val="$(sed -n "s/^[[:space:]]*$_bd_var[[:space:]]*=[[:space:]]*\(.*\)$/\1/p" \ + _bd_val="$(sed -n "s/^[[:space:]]*${_bd_var}[[:space:]]*=[[:space:]]*\(.*\)$/\1/p" \ "$_bd_root/Makefile" | head -1)" _bd_name="${_bd_val##*/}" [ -n "$_bd_name" ] || continue From 6939bba366790dde4eecafdc5b15886ee199beed Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Wed, 9 Sep 2026 17:09:31 +0000 Subject: [PATCH 7/7] docs: the changelog entry was in the wrong release, and both its counts were stale Found reviewing my own PR before asking for another look. THE ENTRY WAS UNDER A RELEASED SECTION. It sat at CHANGELOG.md:225, under `## [1.0-alpha3] - 2026-09-02`, not under `## [Unreleased]` at line 17. So this PR, which is not merged, claimed the freshness controller shipped in a release tagged a week ago. A reader of the alpha3 notes would have believed it was in the tarball they have. Moved to [Unreleased]. BOTH COUNTS IN IT WERE STALE, and both were stale by the same cause: the entry was written before the review, and the rework that answered the review grew what it describes without the entry moving. "16 arms" -> 27 (grep -c '^check ' on selftest 340) "goes from 261 checks to 277" -> 288 Measured rather than derived, because a count is a claim: main f2af0809, a fresh worktree accounting: 261 passed + 0 failed = 261 this branch accounting: 288 passed + 0 failed = 288 I nearly published 288 - 27 = 261 as the baseline instead of measuring it. That subtraction assumes selftest 340 is the only thing in this PR that changes the count, and several arms in this suite SWEEP the tree rather than stating a fixed number of checks, so a lib.sh change can move a count without adding a `check` line. The arithmetic happened to agree with the measurement; it was not entitled to. The sentence now also names the three arms the review added -- the derived build-directory set, the running-binary check driven against a live cluster, and devloop writing the stamp -- because "27 arms" with no account of where 11 of them came from is a number a reader cannot check. Verified after the change: docs_style 9 checks PASSED (it reads CHANGELOG.md at two sites, so it runs whole; the em/en dash arm and the VERSION-citation arms both pass) harness_selftest 288 passed + 0 failed + 0 unrunnable PASSED shellcheck -S error -s bash test/*.sh test/selftest/*.sh exit 0 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a --- CHANGELOG.md | 57 +++++++++++++++++++++++++++------------------------- 1 file changed, 30 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 35e92b5e..7ca1850f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,36 @@ true until the next version shipped. ### Added +- The test harness refuses to measure a binary that was not built from the source + under test. + + `test/run_all_versions.sh` builds and installs once per major and then runs every + suite with `PGC_SKIP_BUILD=1`, so no suite could tell whether the binary it + measured came from this tree. The controller now records a fingerprint of the + build inputs after a successful install, and `pgc_setup` checks it in every + suite, whether that suite built or skipped. A mismatch is fatal and names both + fingerprints. + + A second check covers the other half. `make install` does not reload anything: + `shared_preload_libraries` maps the library at postmaster start, so a reinstall + under a running server leaves the backends executing older code than the file on + disk. The `.so` mtime is compared against `pg_postmaster_start_time()`, and the + message says to restart rather than only that something is wrong. + + Neither check fails when it cannot answer. Someone who ran `make install` by hand + has no stamp, so that case prints `freshness UNVERIFIED` and says which question + went unanswered, rather than printing nothing and letting a reader assume it + passed. + + `pgc_freshness_verdict` and `pgc_running_binary_verdict` are pure functions of + strings, so `test/selftest/340-the-binary-must-be-built-from.sh` drives them + directly: 27 arms including both empty inputs, a non-numeric epoch, equal + timestamps on the boundary, sensitivity to each fingerprint input class, and + the three the review added -- the derived build-directory set, the running + binary check driven against a live cluster, and devloop writing the stamp. + `harness_selftest` goes from 261 checks to 288, both measured. + + - Hilbert clustering: `pgcolumnar.cluster_hilbert` and `pgcolumnar.recluster_hilbert` (#889). @@ -222,33 +252,6 @@ true until the next version shipped. ## [1.0-alpha3] - 2026-09-02 ### Added -- The test harness refuses to measure a binary that was not built from the source - under test. - - `test/run_all_versions.sh` builds and installs once per major and then runs every - suite with `PGC_SKIP_BUILD=1`, so no suite could tell whether the binary it - measured came from this tree. The controller now records a fingerprint of the - build inputs after a successful install, and `pgc_setup` checks it in every - suite, whether that suite built or skipped. A mismatch is fatal and names both - fingerprints. - - A second check covers the other half. `make install` does not reload anything: - `shared_preload_libraries` maps the library at postmaster start, so a reinstall - under a running server leaves the backends executing older code than the file on - disk. The `.so` mtime is compared against `pg_postmaster_start_time()`, and the - message says to restart rather than only that something is wrong. - - Neither check fails when it cannot answer. Someone who ran `make install` by hand - has no stamp, so that case prints `freshness UNVERIFIED` and says which question - went unanswered, rather than printing nothing and letting a reader assume it - passed. - - `pgc_freshness_verdict` and `pgc_running_binary_verdict` are pure functions of - strings, so `test/selftest/340-the-binary-must-be-built-from.sh` drives them - directly: 16 arms including both empty inputs, a non-numeric epoch, equal - timestamps on the boundary, and sensitivity to each fingerprint input class. - `harness_selftest` goes from 261 checks to 277. - - A nanosecond Arrow import says how many values lost precision, and still imports every row.