diff --git a/CHANGELOG.md b/CHANGELOG.md index ccf5f590..790e4bc3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1332,6 +1332,41 @@ true until the next version shipped. refused, since absence is not a contradiction; what holds the committed budget to naming both numbers is a separate arm in each harness. +- The harness no longer reports that the installed library matches when it has not + looked at the library (#959). + + `lib.sh` compared a recorded source fingerprint against the current one and then + printed "source matches the binary under test". That is a claim about the + binary drawn from evidence about the source, and it is false whenever another + process has written the shared prefix: a second worktree, a timing run, a manual + install. The stamp could not see it, because the stamp is keyed per source tree, + so two trees installing into one prefix keep two stamps and each records only what + its own tree built. + + Measured on two trees whose sources differ by five files. One built and installed + through the harness, the other installed its own library into the same prefix, and + the first then ran a suite with PGC_SKIP_BUILD=1: the run printed the library's + fingerprint, asserted that the source matched the binary, and failed nine checks + of a feature the installed library did not contain. + + The stamp now records the installed library's digest beside the source + fingerprint, and the claim requires both to match what is on disk. A library that + changed under the stamp is refused the way a changed source already was, naming + both digests and the prefix another build wrote. A stamp written before this change + records no digest, so it reports the source claim it earned and says the library is + unverified rather than implying it was checked. + + The digest cannot be predicted from the source, because the build path is compiled + in: one commit built in two directories produces two different libraries. So what + is recorded is the digest installed at the moment the stamp was written. + + All three places that write a stamp record it: the build function, the matrix + runner and the development loop. Without that the matrix, which builds once per + major and then sets PGC_SKIP_BUILD, would have reported every suite as unverified. + + The refusal on a changed source had never been exercised by anyone before this + change, only read. It is now driven end to end, along with the three other states. + ## [1.0-alpha3] - 2026-09-02 ### Added diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index 165fa5e0..e43e615e 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -335,14 +335,19 @@ harness_selftest 340-the-binary-must-be-built-from a library older than the runn harness_selftest 340-the-binary-must-be-built-from a missing binary timestamp is unknown, not predates never - harness_selftest 340-the-binary-must-be-built-from a missing postmaster timestamp is unknown, not predates never - harness_selftest 340-the-binary-must-be-built-from a new source file under objstore moves the fingerprint never - +harness_selftest 340-the-binary-must-be-built-from a one-line stamp yields its source fingerprint never - +harness_selftest 340-the-binary-must-be-built-from a recorded digest different from the installed one is replaced, not fresh never - +harness_selftest 340-the-binary-must-be-built-from a recorded library digest equal to the installed one is fresh never - harness_selftest 340-the-binary-must-be-built-from a relative path hashes the same tree the same way never - harness_selftest 340-the-binary-must-be-built-from a server older than the binary predates it never - harness_selftest 340-the-binary-must-be-built-from a server started after the binary is fresh never - harness_selftest 340-the-binary-must-be-built-from a server started at the same second is fresh never - +harness_selftest 340-the-binary-must-be-built-from a stale source is refused whatever the binary says never - harness_selftest 340-the-binary-must-be-built-from a symlink to the tree hashes it the same way never - harness_selftest 340-the-binary-must-be-built-from a symlinked src contributes nothing, as find -P contributes nothing never - harness_selftest 340-the-binary-must-be-built-from a trailing slash hashes the same tree the same way never - harness_selftest 340-the-binary-must-be-built-from a tree with no hashable file yields no fingerprint never - +harness_selftest 340-the-binary-must-be-built-from a two-line stamp still yields the source fingerprint first never - harness_selftest 340-the-binary-must-be-built-from adding a source file moves it never - harness_selftest 340-the-binary-must-be-built-from an added file appears in the manifest by name never - harness_selftest 340-the-binary-must-be-built-from an added file shows up in the report never - @@ -352,7 +357,9 @@ harness_selftest 340-the-binary-must-be-built-from an unreadable b.c yields no f harness_selftest 340-the-binary-must-be-built-from an unreadable c.c yields no fingerprint, not a wrong one never - harness_selftest 340-the-binary-must-be-built-from an unreadable library is not a failure never - harness_selftest 340-the-binary-must-be-built-from and a non-numeric timestamp is unknown rather than compared as text never - +harness_selftest 340-the-binary-must-be-built-from and a stale source with a replaced binary is still refused for the source never - harness_selftest 340-the-binary-must-be-built-from and an uncomputable current fingerprint is unknown, not stale never - +harness_selftest 340-the-binary-must-be-built-from and an unreadable installed library is unknown, not replaced never - harness_selftest 340-the-binary-must-be-built-from and comparing two manifests names it rather than saying 'changed' never - harness_selftest 340-the-binary-must-be-built-from and it says so rather than staying silent never - harness_selftest 340-the-binary-must-be-built-from and removing it restores the fingerprint never - @@ -362,6 +369,8 @@ harness_selftest 340-the-binary-must-be-built-from and the major is still readab harness_selftest 340-the-binary-must-be-built-from and the reader reads back the fingerprint the writer recorded never - harness_selftest 340-the-binary-must-be-built-from and the refusal says the server must be restarted never - harness_selftest 340-the-binary-must-be-built-from and two pg_configs for one prefix share a stamp, keyed on pkglibdir never - +harness_selftest 340-the-binary-must-be-built-from and yields NO library digest, rather than reusing the source hash never - +harness_selftest 340-the-binary-must-be-built-from and yields the library digest second never - harness_selftest 340-the-binary-must-be-built-from but it says which question went unanswered never - harness_selftest 340-the-binary-must-be-built-from control: a caller passing a pg_config is not flagged never - harness_selftest 340-the-binary-must-be-built-from control: a readable run still reads fresh never - @@ -378,6 +387,8 @@ harness_selftest 340-the-binary-must-be-built-from every directory the Makefile harness_selftest 340-the-binary-must-be-built-from moving bytes between files moves the fingerprint never - harness_selftest 340-the-binary-must-be-built-from no caller passes a major where a pg_config belongs never - harness_selftest 340-the-binary-must-be-built-from no record at all is unknown, not fresh never - +harness_selftest 340-the-binary-must-be-built-from no recorded digest is unknown, not fresh never - +harness_selftest 340-the-binary-must-be-built-from no source record is unverified, not verified never - harness_selftest 340-the-binary-must-be-built-from one tree, one fingerprint, whatever the locale never - harness_selftest 340-the-binary-must-be-built-from premise: and the stamp really was not written, so the arm is not vacuous never - harness_selftest 340-the-binary-must-be-built-from premise: at least two locales are installed to compare never - @@ -396,6 +407,9 @@ harness_selftest 340-the-binary-must-be-built-from renaming a source file moves harness_selftest 340-the-binary-must-be-built-from so the tree still fingerprints from its root files alone never - harness_selftest 340-the-binary-must-be-built-from so the verdict is fresh, not unknown never - harness_selftest 340-the-binary-must-be-built-from so the verdict is unknown -- UNVERIFIED -- and never stale never - +harness_selftest 340-the-binary-must-be-built-from source fresh and binary fresh is the only state that earns the claim never - +harness_selftest 340-the-binary-must-be-built-from source fresh with a replaced binary is refused never - +harness_selftest 340-the-binary-must-be-built-from source fresh with an unverifiable binary does not earn it never - harness_selftest 340-the-binary-must-be-built-from the fingerprint is the hash of the manifest never - harness_selftest 340-the-binary-must-be-built-from the fixed fingerprint equals what the previous implementation produced never - harness_selftest 340-the-binary-must-be-built-from the manifest is tree-relative, never absolute never - diff --git a/test/check_ledger_budget.txt b/test/check_ledger_budget.txt index a2175691..43d884cc 100644 --- a/test/check_ledger_budget.txt +++ b/test/check_ledger_budget.txt @@ -34,4 +34,4 @@ suites_not_covered 250 # Without that it is a hand-maintained count that drifts, which is the failure # this repository has spent a day proving. It is not a ceiling; it is a # measurement that must be true. -checks_never_observed_red 829 +checks_never_observed_red 843 diff --git a/test/devloop.sh b/test/devloop.sh index 2d995863..a6087272 100755 --- a/test/devloop.sh +++ b/test/devloop.sh @@ -96,7 +96,8 @@ if ( . "$BUILD/test/lib.sh" pgc_write_source_stamp \ "$(pgc_source_stamp_path "$BUILD" "$PGC")" \ - "$(pgc_source_fingerprint "$BUILD")" + "$(pgc_source_fingerprint "$BUILD")" \ + "$(pgc_installed_library_digest "$PGC")" ); then : else diff --git a/test/lib.sh b/test/lib.sh index 16947cb0..af59479e 100755 --- a/test/lib.sh +++ b/test/lib.sh @@ -209,9 +209,13 @@ pgc_build_and_install() { # 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". A red arm caught it, which is the only reason this exists. + # The digest is read AFTER the install, because the install is what writes the + # library. Taken before, it would record the previous one and certify exactly + # the state this check exists to refuse. pgc_write_source_stamp \ "$(pgc_source_stamp_path "$_pgc_bi_src" "$_pgc_bi_cfg")" \ - "$(pgc_source_fingerprint "$_pgc_bi_src")" + "$(pgc_source_fingerprint "$_pgc_bi_src")" \ + "$(pgc_installed_library_digest "$_pgc_bi_cfg")" return 0 } @@ -293,14 +297,34 @@ pgc_setup() { # 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_PG_CONFIG")")" + _pgc_stamp_file="$(pgc_source_stamp_path "$PGC_SRCDIR" "$PGC_PG_CONFIG")" + _pgc_fresh_recorded="$(pgc_read_source_stamp "$_pgc_stamp_file")" _pgc_fresh_current="$(pgc_source_fingerprint "$PGC_SRCDIR")" - case "$(pgc_freshness_verdict "$_pgc_fresh_recorded" "$_pgc_fresh_current")" in - fresh) + # AND THE ARTIFACT, not only the source (#959). The sentence below is about the + # BINARY, so the binary has to be evidence in it. + _pgc_bin_recorded="$(pgc_read_installed_stamp "$_pgc_stamp_file")" + _pgc_bin_current="$(pgc_installed_library_digest "$PGC_PG_CONFIG")" + case "$(pgc_freshness_claim \ + "$(pgc_freshness_verdict "$_pgc_fresh_recorded" "$_pgc_fresh_current")" \ + "$(pgc_binary_identity_verdict "$_pgc_bin_recorded" "$_pgc_bin_current")")" in + verified) echo "-- source: $_pgc_fresh_current matches the binary under test" ;; - stale) + source-only) + # The stamp predates #959, so it records the source and not the library. + # The source claim is still earned; the binary claim is not, and saying + # the first while implying the second is the defect this removes. + echo "-- source: $_pgc_fresh_current matches what this tree last built;" + echo " the installed library was not recorded, so it is UNVERIFIED" + ;; + refuse-binary) + echo "FATAL: the installed library is not the one this tree built" >&2 + echo " library now $_pgc_bin_current, this tree installed $_pgc_bin_recorded" >&2 + echo " (another build wrote $("$PGC_PG_CONFIG" --pkglibdir 2>/dev/null)," >&2 + echo " so these checks would measure somebody else's binary)" >&2 + exit 1 + ;; + refuse-source) 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 @@ -313,7 +337,7 @@ pgc_setup() { pgc_freshness_report "$PGC_SRCDIR" >&2 exit 1 ;; - unknown) + unverified) # 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. @@ -844,7 +868,55 @@ pgc_running_binary_verdict() { # pgc_running_binary_verdict SO_EPOCH PM_EPOCH [ "$pm" -ge "$so" ] && echo fresh || echo predates } -pgc_write_source_stamp() { # pgc_write_source_stamp FILE HASH +# THE BINARY, NOT ONLY THE SOURCE (#959). `pgc_freshness_verdict` compares two +# SOURCE fingerprints, and the caller printed "source X matches the binary under +# test" -- a claim about the BINARY from evidence about the SOURCE. It is false +# whenever another process has written the shared prefix, and it is false in the +# POSITIVE branch, which is the only one that asserts anything. +# +# Measured: two trees whose src/ differs by five files. B built and installed +# through this function, A then installed its own library into the same prefix, and +# B ran #945's suite under PGC_SKIP_BUILD=1 -- ".so: d312a10c0cfb" beside "source: +# a0e6afc3e13e matches the binary under test", then nine failures with the code +# entirely innocent. @jdatcmd measured the same sentence above two different +# libraries on PG 17, 25+19 failed against 44+0. +# +# The stamp cannot see it because it is keyed per SOURCE TREE: two trees installing +# into one prefix have two stamp files, and each records only what its own tree +# built. So the evidence has to be the artifact itself. +pgc_installed_library_digest() { # pgc_installed_library_digest PG_CONFIG -> 12 hex or empty + local so + so="$("${1:-}" --pkglibdir 2>/dev/null)/pgcolumnar.so" + [ -r "$so" ] || { echo ""; return; } + md5sum "$so" 2>/dev/null | cut -c1-12 +} + +pgc_binary_identity_verdict() { # pgc_binary_identity_verdict RECORDED CURRENT -> verdict + local recorded="${1:-}" current="${2:-}" + [ -z "$recorded" ] && { echo unknown; return; } + [ -z "$current" ] && { echo unknown; return; } + [ "$recorded" = "$current" ] && echo fresh || echo replaced +} + +# ONE DECISION, so no caller can claim the binary on source evidence alone. Pure, +# like its two siblings, because that is what let them be exercised without a build. +# +# A stale source outranks everything: the tree has moved, so nothing installed can +# be what it would now produce, and saying which of two reasons came first is less +# useful than refusing. +pgc_freshness_claim() { # pgc_freshness_claim SOURCE_VERDICT BINARY_VERDICT -> decision + case "${1:-}" in + stale) echo refuse-source; return ;; + unknown) echo unverified; return ;; + esac + case "${2:-}" in + replaced) echo refuse-binary ;; + fresh) echo verified ;; + *) echo source-only ;; + esac +} + +pgc_write_source_stamp() { # pgc_write_source_stamp FILE SOURCE_HASH [LIBRARY_DIGEST] # NO `|| true`. It was there, and it made both controllers' warning branches # UNREACHABLE: run_all_versions.sh and devloop.sh each wrap this in `if (...)` # and promise to say so when the stamp cannot be written, and each carries a @@ -856,7 +928,22 @@ pgc_write_source_stamp() { # pgc_write_source_stamp FILE HASH # The stamp absent, nothing warned, every child suite degraded to UNVERIFIED. # A comment that argues for a guarantee the code does not provide is worse # than no comment, because it stops the next person checking. - printf '%s\n' "${2:-}" > "${1:-/dev/null}" 2>/dev/null + # Two lines when a digest is known, one when it is not. A one-line stamp stays + # valid and reads as "source recorded, library unrecorded", which is what every + # stamp written before #959 is -- so the migration needs no special case. + if [ -n "${3:-}" ]; then + printf '%s\n%s\n' "${2:-}" "$3" > "${1:-/dev/null}" 2>/dev/null + else + printf '%s\n' "${2:-}" > "${1:-/dev/null}" 2>/dev/null + fi +} + +pgc_read_installed_stamp() { # pgc_read_installed_stamp FILE -> digest or empty + # THE SECOND LINE ONLY. Reading hex from the whole file would return the SOURCE + # fingerprint for every pre-#959 stamp, certifying a source hash as a library + # digest -- the exact confusion this change exists to remove. + [ -r "${1:-}" ] || { echo ""; return; } + sed -n '2p' "$1" | tr -dc 'a-f0-9' | head -c 12 } pgc_read_source_stamp() { # pgc_read_source_stamp FILE -> hash or empty diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index a77278c6..4f8e4b30 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -634,8 +634,13 @@ nothing pinned a locale, so one tree hashed two ways — `LC_ALL=C` gave `6d122a7158d5` and `LC_ALL=en_US.UTF-8` gave `0b59bd75fa4f`. The verdict is the property that matters, and the reason it is worth an arm at all -is the asymmetry. `stale` is the FATAL; `unknown` prints `freshness UNVERIFIED` and -runs the suites. The asymmetry is the whole argument for the change: a false +is the asymmetry. `stale` is a FATAL; `unknown` prints `freshness UNVERIFIED` and +runs the suites. Since #959 there are two more: a **replaced library** is also a +FATAL — the source can be unchanged while another tree has overwritten the shared +prefix, which is how `matches the binary under test` came to be printed above +somebody else's binary — and a stamp written before #959, which records no library +digest, reports the source claim it earned and says the library is UNVERIFIED rather +than implying it was checked. The asymmetry is the whole argument for the change: a false UNVERIFIED costs a line of output, a false FATAL costs a matrix **and** teaches people to re-run past a freshness check, which is the failure this controller exists to prevent. diff --git a/test/run_all_versions.sh b/test/run_all_versions.sh index 74ef7d0e..42bbe63c 100755 --- a/test/run_all_versions.sh +++ b/test/run_all_versions.sh @@ -730,7 +730,8 @@ for pgc in "${CONFIGS[@]}"; do . "$builddir/test/lib.sh" pgc_write_source_stamp \ "$(pgc_source_stamp_path "$builddir" "$pgc")" \ - "$(pgc_source_fingerprint "$builddir")" + "$(pgc_source_fingerprint "$builddir")" \ + "$(pgc_installed_library_digest "$pgc")" ); then : else 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 5a2bdee7..1e4233b3 100644 --- a/test/selftest/340-the-binary-must-be-built-from.sh +++ b/test/selftest/340-the-binary-must-be-built-from.sh @@ -54,6 +54,69 @@ check "a missing postmaster timestamp is unknown, not predates" \ check "and a non-numeric timestamp is unknown rather than compared as text" \ "$(pgc_running_binary_verdict 1000 "not-a-time")" "unknown" +# ---- the stamp must record the BINARY, not only the source (#959) ------------ +# +# `pgc_freshness_verdict` compares two SOURCE fingerprints and the caller printed +# "source X matches the binary under test" -- a claim about the BINARY from evidence +# about the SOURCE. When another process writes the shared prefix the claim is false, +# and it is false in the POSITIVE branch, which is the one that asserts something. +# +# MEASURED on two trees whose src/ differs by five files: tree B built and installed +# through the harness, tree A then installed its own library into the same prefix, +# and B ran #945's suite under PGC_SKIP_BUILD=1: +# +# -- .so: d312a10c0cfb <- A's build +# -- source: a0e6afc3e13e matches the binary under test +# FAIL plan has runtime coordinator: got [0] want [1] (nine in all) +# +# The stamp is keyed per SOURCE TREE -- /root/wv3/.pgc_source_stamp.18. and +# /root/wfpB/.pgc_source_stamp.18. are two files -- so a tree's stamp records +# only what THAT tree built and is structurally unable to see another tree overwrite +# the prefix. @jdatcmd reproduced the same sentence above two different libraries +# with opposite outcomes on PG 17. +# +# The verdict and the decision are pure, like the two above, so they are exercised +# here without a build. + +check "a recorded library digest equal to the installed one is fresh" \ + "$(pgc_binary_identity_verdict aaa111aaa111 aaa111aaa111)" "fresh" +check "a recorded digest different from the installed one is replaced, not fresh" \ + "$(pgc_binary_identity_verdict aaa111aaa111 bbb222bbb222)" "replaced" +check "no recorded digest is unknown, not fresh" \ + "$(pgc_binary_identity_verdict "" aaa111aaa111)" "unknown" +check "and an unreadable installed library is unknown, not replaced" \ + "$(pgc_binary_identity_verdict aaa111aaa111 "")" "unknown" + +# THE DECISION, so the caller cannot claim the binary on source evidence alone. +check "source fresh and binary fresh is the only state that earns the claim" \ + "$(pgc_freshness_claim fresh fresh)" "verified" +check "source fresh with an unverifiable binary does not earn it" \ + "$(pgc_freshness_claim fresh unknown)" "source-only" +check "source fresh with a replaced binary is refused" \ + "$(pgc_freshness_claim fresh replaced)" "refuse-binary" +check "a stale source is refused whatever the binary says" \ + "$(pgc_freshness_claim stale fresh)" "refuse-source" +check "and a stale source with a replaced binary is still refused for the source" \ + "$(pgc_freshness_claim stale replaced)" "refuse-source" +check "no source record is unverified, not verified" \ + "$(pgc_freshness_claim unknown fresh)" "unverified" + +# THE OLD STAMP FORMAT MUST NOT BE MISREAD. A one-line stamp holds the source +# fingerprint only. Reading a library digest out of it must give nothing, or every +# pre-existing stamp would certify the source hash as a binary digest. +_bs="$PGC_WORKDIR/stamp959"; mkdir -p "$(dirname "$_bs")" +printf '%s\n' "a0e6afc3e13e" > "$_bs" +check "a one-line stamp yields its source fingerprint" \ + "$(pgc_read_source_stamp "$_bs")" "a0e6afc3e13e" +check "and yields NO library digest, rather than reusing the source hash" \ + "$(pgc_read_installed_stamp "$_bs")" "" +pgc_write_source_stamp "$_bs" "a0e6afc3e13e" "d312a10c0cfb" +check "a two-line stamp still yields the source fingerprint first" \ + "$(pgc_read_source_stamp "$_bs")" "a0e6afc3e13e" +check "and yields the library digest second" \ + "$(pgc_read_installed_stamp "$_bs")" "d312a10c0cfb" +unset _bs + # 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.