test: close three false-freshness paths in the build-freshness controller

test/devloop.sh

@@ -95,7 +95,7 @@ fi
 if (
 	. "$BUILD/test/lib.sh"
 	pgc_write_source_stamp \
-		"$(pgc_source_stamp_path "$BUILD" "$(pgc_major_of "$PGC")")" \
+		"$(pgc_source_stamp_path "$BUILD" "$PGC")" \
 		"$(pgc_source_fingerprint "$BUILD")"
 ); then
 	:

test/lib.sh

@@ -200,7 +200,7 @@ pgc_build_and_install() {
 	# 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.
 	pgc_write_source_stamp \
-		"$(pgc_source_stamp_path "$_pgc_bi_src" "$_pgc_bi_major")" \
+		"$(pgc_source_stamp_path "$_pgc_bi_src" "$_pgc_bi_cfg")" \
 		"$(pgc_source_fingerprint "$_pgc_bi_src")"
 	return 0
 }
@@ -284,7 +284,7 @@ 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_MAJOR")")"
+		"$(pgc_source_stamp_path "$PGC_SRCDIR" "$PGC_PG_CONFIG")")"
 	_pgc_fresh_current="$(pgc_source_fingerprint "$PGC_SRCDIR")"
 	case "$(pgc_freshness_verdict "$_pgc_fresh_recorded" "$_pgc_fresh_current")" in
 		fresh)
@@ -656,7 +656,27 @@ pgc_source_fingerprint() {	# pgc_source_fingerprint DIR -> hash
 		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
+	} | sort -z | while IFS= read -r -d '' _pgc_fp_f; do
+		# EACH FILE'S PATH AND ITS OWN DIGEST, not the concatenated stream.
+		#
+		# `xargs -0 cat | md5sum` hashed the bytes of every file run together,
+		# so it could not see a change that PRESERVES the stream while moving
+		# bytes between translation units. Two files, `static int x=1;` and
+		# `static int x=2;`, both compile; move the second into the first and
+		# empty it and the source no longer compiles, while the hash does not
+		# move (@linuxhikerpm, #898 review):
+		#
+		#     before_hash=bfce474cc159 after_hash=bfce474cc159
+		#     initial_compile=0 repartitioned_compile=1
+		#     error: redefinition of 'x'
+		#
+		# A skip-build run then printed "matches the binary under test" for
+		# source that cannot produce any binary at all. The path makes the
+		# partition part of the input, and the per-file digest is an
+		# unambiguous boundary between one file's bytes and the next's.
+		printf '%s %s\n' "${_pgc_fp_f#"$dir"/}" \
+			"$(md5sum < "$_pgc_fp_f" 2>/dev/null | cut -d' ' -f1)"
+	done | md5sum | cut -c1-12
 }
 
 # fresh   the binary was built from this source
@@ -742,7 +762,18 @@ pgc_running_binary_verdict() {	# pgc_running_binary_verdict SO_EPOCH PM_EPOCH
 }
 
 pgc_write_source_stamp() {	# pgc_write_source_stamp FILE HASH
-	printf '%s\n' "${2:-}" > "${1:-/dev/null}" 2>/dev/null || true
+	# 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
+	# comment saying "NOT || true" -- while the function they call swallowed the
+	# status (@linuxhikerpm, #898 review). Driven against an unwritable target:
+	#
+	#     write_rc=0 exists=no
+	#
+	# 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
 }
 
 pgc_read_source_stamp() {	# pgc_read_source_stamp FILE -> hash or empty
@@ -762,8 +793,37 @@ 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}"
+pgc_source_stamp_path() {	# pgc_source_stamp_path DIR PG_CONFIG
+	# KEYED BY THE INSTALLATION, NOT ONLY THE MAJOR. The key was
+	# `.pgc_source_stamp.<major>`, and the comment above it already said that one
+	# tree installs into several prefixes each with its own binary -- so the key
+	# discarded the distinction the comment drew (@linuxhikerpm, #898 review).
+	#
+	# Not hypothetical on this box: pg18a, pg18n and pg18_san are three PG18
+	# installations with different pkglibdirs, and all three resolved to
+	# `.pgc_source_stamp.18`. Build current source into one prefix, then run
+	# PGC_SKIP_BUILD=1 against another, and the fingerprint matches while the
+	# binary is stale -- and the postmaster arm passes too, because the freshly
+	# started server is newer than the old .so. The run then reports fresh while
+	# executing the other prefix's binary, which is this file's whole subject.
+	#
+	# pkglibdir rather than the pg_config path, because that is where the .so
+	# actually lands: two pg_configs pointing at one prefix ARE the same
+	# installation and should share a stamp.
+	local _pgc_sp_dir="${1:-.}" _pgc_sp_cfg="${2:-}"
+	local _pgc_sp_major _pgc_sp_lib _pgc_sp_id
+	_pgc_sp_major="$(pgc_major_of "$_pgc_sp_cfg" 2>/dev/null)"
+	_pgc_sp_lib="$("$_pgc_sp_cfg" --pkglibdir 2>/dev/null)"
+	# An unreadable pg_config gets a key that matches nothing rather than one
+	# every broken config shares: `unknown` would alias them together, which is
+	# the defect being fixed, one level down.
+	if [ -n "$_pgc_sp_lib" ]; then
+		_pgc_sp_id="$(printf '%s' "$_pgc_sp_lib" | md5sum | cut -c1-8)"
+	else
+		_pgc_sp_id="nolib$(printf '%s' "$_pgc_sp_cfg" | md5sum | cut -c1-3)"
+	fi
+	printf '%s/.pgc_source_stamp.%s.%s\n' \
+		"$_pgc_sp_dir" "${_pgc_sp_major:-0}" "$_pgc_sp_id"
 }
 
 pgc_write_build_stamp() {

test/pytest/TESTS.md

@@ -4,7 +4,7 @@ Reference for anyone reading, running, or adding to `test/pytest/`. The design a
 the decisions behind the harness are in `design/ISSUE_432_PYTEST_HARNESS.md`. This
 file covers the tests themselves.
 
-**74 tests in 6 files.** Fifty-nine of them test the harness rather than the
+**78 tests in 6 files.** Sixty-three of them test the harness rather than the
 product, and they come first, because a harness that can report a false green makes
 every other result in this directory worthless.
 
@@ -330,6 +330,10 @@ is where a wrong quote would hide.
 | `test_the_fingerprint_covers_a_separately_built_module` | an `objstore/` edit moves the hash |
 | `test_an_objstore_edit_forces_a_second_build` | and forces a rebuild, end to end |
 | `test_make_cluster_leaves_nothing_behind_when_setup_fails` | a failed setup leaks no directory |
+| `test_the_stamp_writer_reports_failure` | `\|\| true` made both controllers' warnings unreachable |
+| `test_two_installations_of_one_major_do_not_share_a_stamp` | the key names the installation, not just the major |
+| `test_moving_bytes_between_files_moves_the_shell_fingerprint` | the digest sees a repartition |
+| `test_the_two_fingerprint_implementations_cover_the_same_inputs` | **the two implementations move on the same edits** |
 
 ### The build/start ORDER, which is not a detail
 
@@ -344,6 +348,40 @@ after the alpha4 rebase gave 15 cluster-start errors and the second run passed.
 **A flake that clears on a second run is what a stale-binary defect looks like from
 outside.**
 
+### The twin of `selftest/340`, and the fourth instance of one defect
+
+These four drive the **shell** functions through `bash` rather than
+reimplementing them, and they are the pytest half of `test/selftest/340`'s stamp
+arms, owed under the twin rule and payable only once `test/pytest/` reached
+`main` with #897.
+
+The last one is the interesting one. `source_fingerprint` in `pgc_cluster.py`
+says in its own docstring that it uses *"the same input set as
+`pgc_source_fingerprint` in `test/lib.sh`"*. It did not. The shell hashes each
+build directory's `*.c`, `*.h` **and `Makefile`**; this side read only the
+sources, so editing `objstore/Makefile` — which changes how that module builds —
+moved one hash and not the other:
+
+```
+baseline                    shell=45be41a5c47b  python=bea88c7d79ca
+objstore/Makefile edited    shell=cfb8f4553041  python=bea88c7d79ca
+```
+
+`build_once` then certified a stale module as current. **That is
+@linuxhikerpm's finding one layer over**: they found the module's *sources*
+missing from this implementation, and the module's *Makefile* was still missing
+after that was fixed.
+
+The arm asserts the property the docstring always claimed, and not more: the two
+hashes are **not** required to be equal — they are different digests over the
+same files, used independently — but **the same edit must move both**. It walks
+five edits: a source, a module source, a module Makefile, the top-level
+Makefile, and the control file.
+
+Two implementations of one idea have now been separately wrong, separately
+fixed, and a third party had to find each. That is the argument for making them
+one.
+
 ### Two findings from @linuxhikerpm, both about infrastructure rather than coverage
 
 **The fingerprint read `src/` only.** `objstore/` is a separately built shared

test/pytest/pgc_cluster.py

@@ -409,7 +409,19 @@ def source_fingerprint(srcdir):
     srcdir = pathlib.Path(srcdir)
     paths = []
     for d in source_build_dirs(srcdir):
-        paths += list(d.glob("*.c")) + list(d.glob("*.h"))
+        # EACH BUILD DIRECTORY'S Makefile TOO, not only its sources. The shell
+        # implementation hashes `*.c`, `*.h` AND `Makefile` per directory; this
+        # read only the sources, so editing `objstore/Makefile` -- which changes
+        # how that module is built -- moved the shell hash and not this one:
+        #
+        #     baseline                  shell=45be41a5c47b  python=bea88c7d79ca
+        #     objstore/Makefile edited  shell=cfb8f4553041  python=bea88c7d79ca
+        #
+        # `build_once` then certified a stale module as current. That is
+        # @linuxhikerpm's #897 finding one layer over: they found the module's
+        # sources missing here, and the module's Makefile was still missing
+        # after that was fixed. The docstring claimed parity throughout.
+        paths += list(d.glob("*.c")) + list(d.glob("*.h")) + list(d.glob("Makefile"))
     paths = sorted(
         paths
         + [p for p in (srcdir / "Makefile",) if p.exists()]

test/pytest/test_build_refusal.py

@@ -344,3 +344,114 @@ def test_make_cluster_leaves_nothing_behind_when_setup_fails(tmp_path, expect):
     leaked = sorted(set(glob.glob("/tmp/pgc-pytest-*")) - before)
     expect.text(", ".join(leaked) or "none", "none",
                 "a failed make_cluster leaves no directory behind")
+
+
+# ---------------------------------------------------------------------------
+# THE PYTEST TWIN of test/selftest/340's stamp arms, owed under jd's rule of
+# 2026-09-23... 2026-09-09: every test written twice. The .sh half could not
+# have one until test/pytest/ existed on main, which it now does (#897).
+#
+# These drive the SHELL functions through bash rather than reimplementing them,
+# for the reason that keeps being proved this week: a second implementation of
+# one idea drifts, and the drift is invisible until someone diffs the two.
+
+
+# The tree this corpus belongs to, derived the same way conftest.py derives it.
+SRCDIR = pathlib.Path(__file__).resolve().parents[2]
+
+
+def _sh(srcdir, expr):
+    """Evaluate one lib.sh expression against a tree, and return its stdout."""
+    script = f'. "{SRCDIR}/test/lib.sh" || exit 1; {expr}'
+    p = subprocess.run(["bash", "-c", script], capture_output=True, text=True)
+    return p.stdout.strip(), p.returncode
+
+
+def _tree_with_module(tmp_path, name):
+    t = tmp_path / name
+    (t / "src").mkdir(parents=True, exist_ok=True)
+    (t / "objstore").mkdir(parents=True, exist_ok=True)
+    (t / "src" / "a.c").write_text("int a;\n")
+    (t / "objstore" / "b.c").write_text("int b;\n")
+    (t / "objstore" / "Makefile").write_text("all:\n\ttrue\n")
+    (t / "Makefile").write_text("all:\n\t$(MAKE) -C objstore\n")
+    (t / "pgcolumnar.control").write_text("x\n")
+    return t
+
+
+def test_the_stamp_writer_reports_failure(tmp_path, expect):
+    """`|| true` made both controllers' warning branches unreachable."""
+    out, rc = _sh(tmp_path, 'pgc_write_source_stamp "/proc/pgc-twin" "deadbeef"')
+    expect.num(rc, 1, "the writer reports failure on an unwritable target")
+    ok, rc2 = _sh(tmp_path, f'pgc_write_source_stamp "{tmp_path}/s" "cafebabe"')
+    expect.num(rc2, 0, "control: and succeeds on a writable one")
+
+
+def test_two_installations_of_one_major_do_not_share_a_stamp(tmp_path, expect):
+    """The stamp key must name the installation, not only the major."""
+    cfgs = []
+    for n in ("a", "b"):
+        c = tmp_path / f"pg_config.{n}"
+        c.write_text('#!/bin/sh\ncase "$1" in\n'
+                     '  --version) echo "PostgreSQL 18.4" ;;\n'
+                     f'  --pkglibdir) echo "/usr/local/pg18{n}/lib" ;;\nesac\n')
+        c.chmod(0o755)
+        cfgs.append(c)
+    a, _ = _sh(tmp_path, f'pgc_source_stamp_path /tree "{cfgs[0]}"')
+    b, _ = _sh(tmp_path, f'pgc_source_stamp_path /tree "{cfgs[1]}"')
+    expect.text(str(a != b), "True", "two prefixes of one major get different stamps")
+    a2, _ = _sh(tmp_path, f'pgc_source_stamp_path /tree "{cfgs[0]}"')
+    expect.text(a2, a, "control: the same pg_config twice gives the same path")
+
+
+def test_moving_bytes_between_files_moves_the_shell_fingerprint(tmp_path, expect):
+    """`xargs -0 cat | md5sum` could not see a repartition."""
+    t = _tree_with_module(tmp_path, "rp")
+    before, _ = _sh(t, f'pgc_source_fingerprint "{t}"')
+    expect.at_least(len(before), 12, "premise: the tree fingerprints at all")
+    (t / "src" / "a.c").write_text("int a;\nint b;\n")
+    (t / "objstore" / "b.c").write_text("")
+    after, _ = _sh(t, f'pgc_source_fingerprint "{t}"')
+    expect.text(str(after != before), "True",
+                "moving bytes between files moves the fingerprint")
+
+
+def test_the_two_fingerprint_implementations_cover_the_same_inputs(tmp_path, expect):
+    """THE PROPERTY THE TWO IMPLEMENTATIONS MUST SHARE, and the one they did not.
+
+    `source_fingerprint` in pgc_cluster.py says in its own docstring that it uses
+    "the same input set as pgc_source_fingerprint in test/lib.sh". It did not:
+    the shell hashes each build directory's `*.c`, `*.h` AND `Makefile`, while
+    the Python read only `*.c` and `*.h` there. Editing `objstore/Makefile` --
+    which changes how that module builds -- moved the shell hash and not the
+    Python one, so `build_once` certified a stale module as current:
+
+        baseline                    shell=45be41a5c47b  python=bea88c7d79ca
+        objstore/Makefile edited    shell=cfb8f4553041  python=bea88c7d79ca
+
+    That is @linuxhikerpm's finding one layer over: they found the .c files
+    missing from the Python side, and the Makefiles were still missing after it
+    was fixed.
+
+    The two hashes are NOT required to be equal -- they are different digests
+    over the same files, used independently. What is required is that the same
+    edit moves both, which is what "the same input set" means and all the
+    docstring ever claimed.
+    """
+    t = _tree_with_module(tmp_path, "cover")
+    for edit, path, body in (
+        ("a source file", t / "src" / "a.c", "int a = 2;\n"),
+        ("a module source", t / "objstore" / "b.c", "int b = 2;\n"),
+        ("a module Makefile", t / "objstore" / "Makefile", "all:\n\ttrue # x\n"),
+        ("the top-level Makefile", t / "Makefile", "all:\n\t$(MAKE) -C objstore # x\n"),
+        ("the control file", t / "pgcolumnar.control", "y\n"),
+    ):
+        sh_before, _ = _sh(t, f'pgc_source_fingerprint "{t}"')
+        py_before = source_fingerprint(t)
+        old = path.read_text()
+        path.write_text(body)
+        sh_after, _ = _sh(t, f'pgc_source_fingerprint "{t}"')
+        py_after = source_fingerprint(t)
+        path.write_text(old)
+        expect.text(f"{sh_after != sh_before} {py_after != py_before}", "True True",
+                    f"editing {edit} moves both fingerprints")

test/run_all_versions.sh

@@ -726,7 +726,7 @@ for pgc in "${CONFIGS[@]}"; do
 	if (
 		. "$builddir/test/lib.sh"
 		pgc_write_source_stamp \
-			"$(pgc_source_stamp_path "$builddir" "$major")" \
+			"$(pgc_source_stamp_path "$builddir" "$pgc")" \
 			"$(pgc_source_fingerprint "$builddir")"
 	); then
 		: