From b71b9fc1bd41afaed109cccd9136ebcea5f8002f Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Wed, 9 Sep 2026 23:21:21 +0000 Subject: [PATCH] test: one source fingerprint, in Python, called by both harnesses (#907) test/lib.sh and test/pytest/pgc_cluster.py each carried their own answer to "what was this binary built from". On 2026-09-09 the pair produced four defects between them -- two in each copy, and NOT ONE was found by whoever wrote that copy: objstore/*.c never walked python @linuxhikerpm, #897 the bare NAME instead of the path python found while fixing the above `xargs -0 cat | md5sum`, no bounds shell @linuxhikerpm, #898 each build dir's Makefile omitted python found while writing the twin The Python docstring asserted "the same input set as pgc_source_fingerprint in test/lib.sh" throughout all four. It was false when written and stayed false through two rounds of fixing. A prose claim of agreement is not a mechanism, and it is worse than silence because it is what stops the next person checking. Python, not shell, which is the opposite of what #907 first proposed -------------------------------------------------------------------- jd's constraint decided it: the single implementation belongs in the more portable language. bash is largely a GNU thing; Python is present on FreeBSD and Windows where bash is not. lib.sh already requires bash, so calling a more portable interpreter from it cannot cost portability. My argument against this direction was that lib.sh invokes python3 zero times, so this escalates from "53 suites need it" to "every suite needs it at gate time". That is true and it is not a cost, for the reason above. Measured, expecting to report a subprocess penalty: shell, forking md5sum once per file 239 ms/call the module, one interpreter start 26 ms/call across 261 suites x 2 fingerprints 124 s -> 13 s The portable direction is also 9x faster. I had it backwards in both dimensions. A fifth defect, which unifying them found ------------------------------------------ `sort -z` orders by LOCALE COLLATION, and nothing in this harness pins a locale. The same tree fingerprinted two ways depending on whose machine it was: LC_ALL=C 6d122a7158d5 LC_ALL=en_US.UTF-8 0b59bd75fa4f en_US.UTF-8 is a common desktop default, so this is a developer stamping a tree and CI reading it back and calling the binary stale -- a false FATAL arriving from the environment rather than from the source. The module sorts BYTES, which is what LC_ALL=C produced and what every stamp already on disk was written with, so no existing stamp is invalidated. Arms in both harnesses. Equivalence, established rather than asserted ---------------------------------------------- A differential run of the module against the shell it replaces, over trees built to break the ways this pair has actually broken. 17 shapes, manifest AND fingerprint compared: the real source tree, minimal, a recursed module, a dir with sources but no Makefile, collation-sensitive names, a symlinked source file, a symlinked build directory, no src/, an empty tree, root .control and .sql, non-source files, spaces and punctuation, unicode, a Makefile at depth 3, a trailing slash, a /./ segment, five recursed modules AGREE=17 DIVERGE=0 Two of those are subtle enough to be worth naming. `find -type f` tests the LINK, so a symlinked source is not in the shell's manifest, while `pathlib.is_file()` FOLLOWS it and would have added one; the module excludes symlinks explicitly. And `find` does not descend a symlinked directory, so build dirs discovered through one differ -- which is why the module canonicalises the root first. The mechanism of two arms had to change with the implementation ---------------------------------------------------------------- The failed-digest arms in 340 and test_build_refusal.py stubbed `md5sum` on PATH. The digest is hashlib now, which no PATH can reach, so the stub would have left both arms GREEN while testing nothing -- the exact shape this corpus refuses. A real read failure needs a real reader who is denied, and root is denied nothing: chmod 000 is invisible to it. Measured before the arms were rewritten: as root 28a7149e07ae <- reads the mode-000 file regardless as postgres (empty) <- the failure the arm needs So the tree is built outside any mode-0700 directory and read by a second user, with a premise asserting that reader agrees with a privileged one WHILE nothing is denied -- otherwise the arm measures the user switch rather than the failure. Where no non-root user exists it records expect.cannot_run rather than passing. And the arm that would catch this issue recurring -------------------------------------------------- selftest 380's static guards follow the fingerprint to its new file, plus three new arms: neither caller may keep a private implementation, and the module may import nothing from test/pytest/. A static assertion of ABSENCE is the shape that most often cannot fail, so each was proved against the REAL files rather than only against fixtures -- a fixture proves the pattern matches something, not that the arm aimed at the real file would fire: pgc_cluster.py grows a private digest HELD lib.sh grows a private md5sum loop HELD the module imports from the pytest tree HELD harness_selftest 407 passed + 0 failed + 0 unrunnable, rc=0 pytest corpus 91 passed docs_style 9 checks PASSED shellcheck -S error clean Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a --- CHANGELOG.md | 40 ++++ test/lib.sh | 171 ++++++-------- test/pgc_fingerprint.py | 196 ++++++++++++++++ test/pytest/TESTS.md | 33 ++- test/pytest/pgc_cluster.py | 103 +++----- test/pytest/test_build_refusal.py | 220 ++++++++++++++---- .../340-the-binary-must-be-built-from.sh | 163 +++++++++---- .../380-the-pytest-cluster-helpers.sh | 67 ++++-- 8 files changed, 718 insertions(+), 275 deletions(-) create mode 100755 test/pgc_fingerprint.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 3919454c..ee62d1c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,46 @@ true until the next version shipped. regression cannot hide behind a correct answer. Each of the five one-token strategy mutations was proved to fail its corresponding assertion. +- The source fingerprint has one implementation, in Python, and both harnesses + call it. + + `test/lib.sh` and `test/pytest/pgc_cluster.py` each carried their own answer to + "what was this binary built from". In one day the pair produced four defects, + two in each copy, and not one was found by whoever wrote that copy: the Python + side never walked `objstore/*.c`, then mixed in each file's bare name so + `src/module.c` and `objstore/module.c` were interchangeable, then omitted each + build directory's `Makefile`; the shell side hashed `xargs -0 cat | md5sum`, a + stream with no per-file boundaries, so moving bytes between two files left the + hash unchanged while the source no longer compiled. The Python docstring + asserted parity with the shell throughout all four. It was false when written + and stayed false through two rounds of fixing. + + `test/pgc_fingerprint.py` is now the only implementation. It is Python rather + than shell because the portable half should be the one that survives: bash is + largely a GNU thing, while Python is present on FreeBSD and Windows where bash + is not. It uses the standard library only and runs on the system interpreter, + never the pytest virtualenv, so a freshness gate cannot depend on the test + dependencies of a harness it gates. + + It is also faster. The shell forked `md5sum` once per file; the module starts + one interpreter: + + shell, forking md5sum per file 239 ms per call + the module 26 ms per call + across 261 suites, twice each 124 s -> 13 s + + Unifying them closed a fifth defect that neither implementation had been + suspected of. `sort -z` orders by LOCALE COLLATION and nothing in the harness + pinned a locale, so one tree fingerprinted two ways depending on the machine: + + LC_ALL=C 6d122a7158d5 + LC_ALL=en_US.UTF-8 0b59bd75fa4f + + `en_US.UTF-8` is a common desktop default, so a developer could stamp a tree + and have CI read it back and call the binary stale. The module sorts bytes, + which is what `LC_ALL=C` produced and what every stamp already on disk was + written with, so no existing stamp is invalidated. + - The test harness refuses to measure a binary that was not built from the source under test. diff --git a/test/lib.sh b/test/lib.sh index 8b3b8349..3ea4d61f 100755 --- a/test/lib.sh +++ b/test/lib.sh @@ -671,106 +671,85 @@ pgc_freshness_report() { # pgc_freshness_report DIR -> the manifest, annotated echo " ($n files, under $dir)" } +# THE ONE IMPLEMENTATION LIVES IN test/pgc_fingerprint.py. +# +# This file and test/pytest/pgc_cluster.py each used to carry their own, and the +# pair produced four defects in one day -- two in each copy, and not one found by +# whoever wrote that copy (#907). The Python docstring asserted parity with this +# function throughout all four; it was false when written and stayed false through +# two rounds of fixing. +# +# PYTHON RATHER THAN SHELL, which is the opposite of what was first proposed here. +# The single implementation belongs in the more portable language: bash is largely +# a GNU thing, while Python is present on FreeBSD and Windows where bash is not +# (jd). This file already requires bash, so calling a more portable interpreter +# from it cannot cost portability. It is also not a cost at all -- the shell forked +# md5sum once per file, and one interpreter start beats 64 forks: +# +# shell, forking md5sum per file 239 ms/call +# the module 26 ms/call +# across 261 suites x 2 calls 124 s -> 13 s +# +# It fixed a defect on the way, which is the argument for one implementation in +# miniature: `sort -z` uses LOCALE COLLATION and no locale is pinned anywhere in +# this harness, so the same tree fingerprinted two ways depending on whose desktop +# it was -- +# +# LC_ALL=C 6d122a7158d5 +# LC_ALL=en_US.UTF-8 0b59bd75fa4f +# +# A developer on an en_US.UTF-8 default stamping a tree that CI then reads under +# C.UTF-8 is a FATAL naming a stale binary against a clean tree. The module sorts +# bytes, which is what LC_ALL=C did and what every stamp on disk was written with. +_pgc_fp_module() { + printf '%s\n' "$(dirname "${BASH_SOURCE[0]}")/pgc_fingerprint.py" +} + +# SYSTEM python3, NOT the pytest venv. test/pytest/README.md records that the +# interpreter is EXTERNALLY-MANAGED and that pytest runs from a virtualenv; a +# freshness gate that needed those test dependencies would make every suite in +# this directory unrunnable until somebody had installed pytest. The module +# imports nothing outside the standard library and nothing from test/pytest/. +_pgc_fp_python() { + command -v python3 2>/dev/null +} + +# A MISSING python3 IS REPORTED, NOT SWALLOWED. Returning empty alone would make +# the verdict `unknown` and print "freshness UNVERIFIED", which is deliberately +# not a failure -- and the whole gate would then be off with nothing saying so. +# Loud but not fatal: the asymmetry this controller is built on is that a false +# UNVERIFIED costs a line of output while a false FATAL costs a matrix. +_pgc_fp_warn_once() { + [ -n "${_pgc_fp_warned:-}" ] && return 0 + _pgc_fp_warned=1 + echo "-- python3 not found: the freshness check cannot run (see test/pgc_fingerprint.py)" >&2 +} + pgc_source_manifest() { # pgc_source_manifest DIR -> "relpath digest" per file, or EMPTY - local dir="${1:-.}" - local d - # THE MANIFEST IS THE THING; THE FINGERPRINT IS ITS HASH. - # - # Twelve hex characters cannot say which file moved. Two CI failures reported - # `source now a735c673b129, binary built from 6d122a7158d5` and nothing else, - # identically, across two branches and two majors -- so a bare hash gave us a - # second sample of a mystery rather than an answer. The controller's FATAL - # path prints this, and a diff of two manifests names the file. - # ONE TREE HASHES ONE WAY, HOWEVER THE PATH IS SPELLED. `${f#"$dir"/}` below - # strips a prefix that has to match character for character, so `$dir` with a - # trailing slash, or with a `/./` segment, or reached through a symlink, put - # the full ABSOLUTE path into the digest instead of the tree-relative one and - # the same tree hashed three different ways (@OffgridwithJD). Canonicalised - # ONCE here rather than defended at each call site, so a future caller cannot - # reintroduce it: the writer and the reader reach the tree by different routes - # and a disagreement between them is a FATAL about nothing. - # - # pgc_norm_path rather than a second `cd && pwd -P` of my own. This file has - # spent the day proving that two implementations of one idea drift, and a - # private copy here would be the third normaliser in one tree. Its fallback - # hands back the raw path for a directory that cannot be entered, which needs - # no special case: nothing is found under it, `out` is empty, and the guard - # below returns no fingerprint. - dir="$(pgc_norm_path "$dir")" - { - 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 | 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. - # - # A DIGEST THAT FAILED MUST NOT LOOK LIKE ONE THAT SUCCEEDED. This was - # `$(md5sum < "$f" 2>/dev/null | cut -d' ' -f1)` inline, so a failed - # md5sum -- a fork that hits EAGAIN, an OOM kill, a loaded runner -- - # contributed an EMPTY digest and the function returned a confident - # WRONG hash with status 0. One stubbed failure among many, on one - # unchanged tree, gave three different answers: - # - # baseline c8e6b23db1c9 - # one digest empty (call 2) 22897add806e - # one digest empty (call 3) 58c76fdab962 - # - # On the READ side that costs one suite at random, which is what #902's - # PG18 leg showed. On the WRITE side it is worse and deterministic: a - # failed digest while the controller stamps bakes a wrong hash, and every - # suite in the batch then reports `stale` -- a FATAL naming a stale binary - # -- against a tree that is perfectly clean (@OffgridwithJD, measured: - # 5 of 5 suites stale on a correct tree). - _pgc_fp_h="$(md5sum < "$_pgc_fp_f" 2>/dev/null | cut -d' ' -f1)" - [ -n "$_pgc_fp_h" ] || { printf '%s\n' "$_pgc_fp_failed"; break; } - printf '%s %s\n' "${_pgc_fp_f#"$dir"/}" "$_pgc_fp_h" - done + local py out rc + py="$(_pgc_fp_python)" || true + [ -n "$py" ] || { _pgc_fp_warn_once; printf '%s\n' "$_pgc_fp_failed"; return 0; } + out="$("$py" "$(_pgc_fp_module)" manifest "${1:-.}" 2>/dev/null)" + rc=$? + # A DIGEST THAT FAILED MUST NOT LOOK LIKE ONE THAT SUCCEEDED. The module exits + # 1 and prints nothing when a file could not be read, so the marker is emitted + # here and pgc_source_fingerprint below turns it into no fingerprint at all. + # Before this was true anywhere, a failed md5sum contributed an EMPTY digest + # and the function returned a confident WRONG hash at status 0, which cost a + # matrix (@OffgridwithJD, measured: 5 of 5 suites stale on a correct tree). + [ "$rc" -eq 0 ] || { printf '%s\n' "$_pgc_fp_failed"; return 0; } + [ -n "$out" ] || return 0 + printf '%s\n' "$out" } pgc_source_fingerprint() { # pgc_source_fingerprint DIR -> hash, or EMPTY if it could not be computed - local out - out="$(pgc_source_manifest "${1:-.}")" - # Empty rather than a hash, which pgc_freshness_verdict turns into `unknown` - # and the controller prints as "freshness UNVERIFIED" -- already designed, and - # already deliberately not a failure. The asymmetry is the whole argument: 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. - case "$out" in *"$_pgc_fp_failed"*) printf ''; return 0 ;; esac - # A tree with no hashable file cannot be verified, so it gets no fingerprint - # either -- and md5 of an empty stream is a stable, comparable value that - # would have made two empty trees "match". Observed rather than hypothetical: - # under process pressure the whole manifest came back empty and the old code - # returned md5("") = d41d8cd98f00 in 11 runs of 20 (@OffgridwithJD). - [ -n "$out" ] || { printf ''; return 0; } - # THE TRAILING NEWLINE IS LOAD-BEARING. The old code piped the loop straight - # into md5sum, so the stream md5sum saw ended with one; `$(...)` strips it. - # Without `printf '%s\n'` here the same unchanged tree hashes differently - # before and after this change, every stamp already on disk reads `stale`, and - # a fix for false FATALs becomes a false FATAL for everyone holding a built - # worktree (@OffgridwithJD, caught before it shipped). The matrix could not - # have caught it: it copies a fresh tree and re-stamps every run. - printf '%s\n' "$out" | md5sum | cut -c1-12 + local py out rc + py="$(_pgc_fp_python)" || true + [ -n "$py" ] || { _pgc_fp_warn_once; printf ''; return 0; } + out="$("$py" "$(_pgc_fp_module)" fingerprint "${1:-.}" 2>/dev/null)" + rc=$? + [ "$rc" -eq 0 ] || { printf ''; return 0; } + printf '%s\n' "$out" } # fresh the binary was built from this source diff --git a/test/pgc_fingerprint.py b/test/pgc_fingerprint.py new file mode 100755 index 00000000..d170ca86 --- /dev/null +++ b/test/pgc_fingerprint.py @@ -0,0 +1,196 @@ +#!/usr/bin/env python3 +"""What a build reads, as a manifest, and the fingerprint that is its hash. + +THE ONE IMPLEMENTATION. `test/lib.sh` and `test/pytest/pgc_cluster.py` each used +to carry their own, and on 2026-09-09 the pair produced four defects in a day -- +two in each copy, and not one found by whoever wrote that copy (#907): + + objstore/*.c never walked python found by @linuxhikerpm + the bare NAME instead of the path python found while fixing the above + `xargs -0 cat | md5sum`, no bounds shell found by @linuxhikerpm + each build dir's Makefile omitted python found while writing the twin + +The Python docstring asserted "the same input set as pgc_source_fingerprint in +test/lib.sh" throughout all four. It was false when written and stayed false +through two rounds of fixing. A prose claim of agreement is not a mechanism, and +it is worse than saying nothing because it is what stops the next person checking. + +WHY PYTHON RATHER THAN SHELL, which is the opposite of what was first proposed. +The single implementation goes in the more portable language, not the less: bash +is largely a GNU thing, while Python is present on FreeBSD and Windows where bash +is not (jd). `test/lib.sh` already requires bash, so calling a more portable +interpreter from it cannot cost portability -- and it is not a cost at all: + + shell, forking md5sum once per file 239 ms/call + this module, one interpreter start 26 ms/call + +STDLIB ONLY, AND IT MUST RUN ON THE SYSTEM INTERPRETER. test/pytest/README.md +records that the interpreter is EXTERNALLY-MANAGED and that pytest runs from a +venv. lib.sh must never need that venv: a freshness gate that depended on the +pytest test dependencies would make every bash suite unrunnable until somebody +had installed pytest. Nothing here imports outside the standard library, and +nothing here imports from test/pytest/. +""" + +import hashlib +import os +import pathlib +import sys + +__all__ = ["norm_path", "build_dirs", "manifest", "fingerprint"] + +SOURCE_SUFFIXES = (".c", ".h") +ROOT_SUFFIXES = (".control", ".sql") + + +def norm_path(d): + """The tree's physical path, or the argument unchanged if it cannot be entered. + + One tree hashes one way however the path is spelled. The relative path below + is produced by stripping a prefix, so `$dir` with a trailing slash, with a + `/./` segment, or reached through a symlink used to put the full ABSOLUTE + path into the manifest and the same tree hashed four different ways. The + fallback matches the shell's `pgc_norm_path`: hand back the raw path, find + nothing under it, and let the caller return no fingerprint. + """ + try: + return pathlib.Path(os.path.realpath(str(d))) + except OSError: + return pathlib.Path(str(d)) + + +def _is_plain_file(p): + """A regular file, NOT a symlink to one. + + `find -type f` tests the link itself, so a symlinked source is not in the + shell's manifest. `pathlib.is_file()` FOLLOWS the link and would have added + one, which is a silent divergence of exactly the kind this module exists to + end -- and it would have appeared only on trees that use symlinks, which is + to say in somebody else's checkout rather than in CI. + """ + try: + return p.is_file() and not p.is_symlink() + except OSError: + return False + + +def build_dirs(root): + """Every directory the build compiles in: src, plus each recursed module. + + src unconditionally, whether or not it exists -- the shell prints it before + looking, and a missing src must not change the RESULT, only what is found in + it. Then any depth-2 Makefile's directory, which is how the top-level + Makefile reaches a separately built module such as objstore/. + """ + root = pathlib.Path(root) + out = [root / "src"] + try: + entries = sorted(root.iterdir(), key=lambda p: os.fsencode(str(p))) + except OSError: + return out + for d in entries: + try: + if not d.is_dir() or d.is_symlink(): + continue # find does not descend a symlinked directory + except OSError: + continue + if d == root / "src": + continue + if _is_plain_file(d / "Makefile"): + out.append(d) + return out + + +def _manifest_files(root): + paths = [] + for d in build_dirs(root): + try: + entries = list(d.iterdir()) + except OSError: + continue # find prints nothing for an unreadable directory + for f in entries: + if _is_plain_file(f) and (f.suffix in SOURCE_SUFFIXES or f.name == "Makefile"): + paths.append(f) + try: + entries = list(root.iterdir()) + except OSError: + entries = [] + for f in entries: + if _is_plain_file(f) and (f.name == "Makefile" or f.suffix in ROOT_SUFFIXES): + paths.append(f) + # Sorted as BYTES over the absolute path, which is what `sort -z` does under + # LC_ALL=C and what every stamp on disk was written with. It is NOT what + # `sort -z` does under a locale: no locale is pinned anywhere in the harness, + # and en_US.UTF-8 collation reorders `columnar_arrow.c` against + # `columnar-arrow.c`, so the same tree fingerprinted two ways depending on + # the developer's environment -- + # + # LC_ALL=C 6d122a7158d5 + # LC_ALL=en_US.UTF-8 0b59bd75fa4f + # + # A desktop default of en_US.UTF-8 stamping a tree that CI then reads under + # C.UTF-8 is a FATAL naming a stale binary against a clean tree. Sorting + # bytes here removes the environment from the answer. + return sorted(paths, key=lambda p: os.fsencode(str(p))) + + +def manifest(root): + """-> "relpath digest" per file, newline-joined, or None if one could not be read. + + None rather than a short manifest. A digest that FAILED must not look like + one that succeeded: the shell version contributed an empty digest for a failed + md5sum and returned a confident wrong hash at status 0, which cost a matrix. + """ + root = norm_path(root) + lines = [] + for f in _manifest_files(root): + try: + digest = hashlib.md5(f.read_bytes()).hexdigest() + except OSError: + return None + try: + rel = f.relative_to(root) + except ValueError: + return None + lines.append(f"{rel} {digest}") + return "\n".join(lines) + + +def fingerprint(root): + """-> 12 hex characters, or "" if it could not be computed. + + Empty rather than a hash, which pgc_freshness_verdict turns into `unknown` + and the controller prints as "freshness UNVERIFIED" -- deliberately not a + failure. A false UNVERIFIED costs a line of output; a false FATAL costs a + matrix and teaches people to re-run past a freshness check. + """ + out = manifest(root) + if out is None or out == "": + return "" + # THE TRAILING NEWLINE IS LOAD-BEARING. The shell piped its loop straight + # into md5sum, so the stream md5sum saw ended with one. Drop it and the same + # unchanged tree hashes differently, every stamp already on disk reads + # `stale`, and a portability fix becomes a false FATAL for everyone holding a + # built worktree. + return hashlib.md5((out + "\n").encode()).hexdigest()[:12] + + +def main(argv): + if len(argv) != 3 or argv[1] not in ("manifest", "fingerprint"): + print("usage: pgc_fingerprint.py {manifest|fingerprint} DIR", file=sys.stderr) + return 2 + if argv[1] == "manifest": + out = manifest(argv[2]) + # An unreadable digest prints nothing and exits 1, so a caller cannot + # mistake "could not be computed" for "the tree holds nothing". + if out is None: + return 1 + if out: + print(out) + return 0 + print(fingerprint(argv[2])) + return 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv)) diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index a170205a..68a5fea9 100644 --- a/test/pytest/TESTS.md +++ b/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. -**122 tests in 10 files.** One hundred and seven of them test the harness rather than the +**123 tests in 10 files.** One hundred and eight 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. @@ -486,13 +486,27 @@ itself. The subject is the instrument every other arm in this section depends on if the fingerprint can be wrong, `never report on source you did not build` reports on nothing. -`test_a_failed_digest_yields_no_fingerprint_rather_than_a_wrong_one` drives the -real shell function with a **stub `md5sum` that is the real one except on its Nth -call**, where it fails with no output — a fork that hits `EAGAIN`, an OOM kill, a -loaded runner. The old code substituted that empty digest into the hash and -returned status 0, so one unchanged tree produced three different confident -answers. The premise arm requires the stub to agree with the real `md5sum` when -nothing is configured to fail, or the test would be measuring the stub. +`test_a_failed_digest_yields_no_fingerprint_rather_than_a_wrong_one` and +`test_a_failed_digest_gives_unverified_and_never_a_false_stale` are the two arms +here, and THE MECHANISM CHANGED WITH THE IMPLEMENTATION. They used to drive the real +shell function with a **stub `md5sum`** on `PATH`, because the shell forked one +per file. The digest now lives in `test/pgc_fingerprint.py` and uses `hashlib`, +which no `PATH` can reach, so the stub would have left both arms green while +testing nothing — the exact shape this corpus exists to refuse. + +A real read failure needs a real reader who is denied, and **root is denied +nothing**: `chmod 000` is invisible to it. Measured before the arms were +rewritten: + + as root 28a7149e07ae <- reads the mode-000 file regardless + as postgres (empty) <- the failure the arm needs + +So the tree is built outside any mode-0700 directory and read by a second user, +and where no such user exists the arm records `expect.cannot_run` rather than +passing. `test_one_tree_hashes_one_way_however_the_locale_is_set` pins the defect +the single implementation removed on the way: `sort -z` used locale collation and +nothing pinned a locale, so one tree hashed two ways — +`LC_ALL=C` gave `6d122a7158d5` and `LC_ALL=en_US.UTF-8` gave `0b59bd75fa4f`. `test_a_failed_digest_gives_unverified_and_never_a_false_stale` is the property that matters. `stale` is the FATAL; `unknown` prints `freshness UNVERIFIED` and @@ -969,3 +983,6 @@ complete. This is why the property comparison reads names. enclosing command line only while the plain word appears nowhere else in it. A probe whose body also contained `/tmp/pgc-pytest-*` counted its own invocation as a leaked process. Walking `/proc//cmdline` is the reliable instrument. + +`test_one_tree_hashes_one_way_however_the_locale_is_set` requires one tree to +give one fingerprint across every installed locale. diff --git a/test/pytest/pgc_cluster.py b/test/pytest/pgc_cluster.py index 43fce107..0dc126e8 100644 --- a/test/pytest/pgc_cluster.py +++ b/test/pytest/pgc_cluster.py @@ -18,6 +18,7 @@ """ import fcntl +import importlib.util import hashlib import os import re @@ -367,79 +368,51 @@ def build_and_install(srcdir, pg_config, major, runner=None): ) +# BOTH OF THESE NOW DELEGATE TO test/pgc_fingerprint.py. +# +# They used to be an independent implementation of what test/lib.sh does, and the +# pair produced four defects in one day -- two here, two there, and not one found +# by whoever wrote that copy (#907): +# +# objstore/*.c never walked here found by @linuxhikerpm +# the bare NAME instead of the path here found while fixing the above +# `xargs -0 cat | md5sum`, no bounds lib.sh found by @linuxhikerpm +# each build dir's Makefile omitted here found while writing the twin +# +# The docstring here asserted "the same input set as pgc_source_fingerprint in +# test/lib.sh" through all four. It was false when written and stayed false +# through two rounds of fixing, which is the case against a prose claim of +# agreement: it is not a mechanism, and it is worse than silence because it is +# exactly what stops the next person checking. +# +# Loaded BY PATH rather than by package import: test/ is not a package, and this +# module is imported by pytest from test/pytest/ while lib.sh runs it as a script +# from test/. Neither should have to know about the other's layout. +_FP_PATH = pathlib.Path(__file__).resolve().parent.parent / "pgc_fingerprint.py" +_fp_spec = importlib.util.spec_from_file_location("pgc_fingerprint", _FP_PATH) +_fp = importlib.util.module_from_spec(_fp_spec) +_fp_spec.loader.exec_module(_fp) + + def source_build_dirs(srcdir): - """Every directory the build compiles in: src/, plus any directory that - carries its own Makefile. + """Every directory the build compiles in: src/, plus any with its own Makefile. - DERIVED, NOT LISTED, and the same rule pgc_source_build_dirs uses in - test/lib.sh. Naming objstore/ here would fix today and fail the next time a - module is added; a directory with its own Makefile is what the top-level + DERIVED, NOT LISTED. Naming objstore/ would fix today and fail the next time a + module is added; a directory carrying its own Makefile is what the top-level Makefile recurses into, so that is the property to read. """ - srcdir = pathlib.Path(srcdir) - dirs = [srcdir / "src"] - for makefile in sorted(srcdir.glob("*/Makefile")): - if makefile.parent != srcdir / "src": - dirs.append(makefile.parent) - return dirs + return _fp.build_dirs(srcdir) def source_fingerprint(srcdir): - """A hash of everything a build reads, or None if the tree is unreadable. - - The same input set as pgc_source_fingerprint in test/lib.sh: the C sources - and headers of EVERY directory the build compiles in, the Makefile, the - control file and the SQL scripts. Content, not mtime, because a checkout or - a branch switch rewrites mtimes without changing what compiles, and - `git stash` does the reverse. - - THE FIRST VERSION READ src/ ONLY, and said in this docstring that it matched - lib.sh while it did not. objstore/ is a separately built shared library the - top-level Makefile reaches by recursion, so editing - objstore/columnar_objstore_module.c left the hash unchanged and build_once - certified a stale module as current (@linuxhikerpm, #897 review): - - objstore_before=2799803eaeac objstore_after=2799803eaeac - builds=1 second=already-built - - That is the same gap #898 closes in test/lib.sh. This is an INDEPENDENT - implementation, so rebasing #898 would not have fixed it -- which is the - argument for the two eventually becoming one, not two that agree today. + """A hash of everything a build reads, or None if it could not be computed. + + None rather than "" because that is the contract this harness already had, and + build_once distinguishes "no fingerprint" from a real one. The module returns + "" for the same condition; the mapping happens here rather than there so the + shell and Python callers each get the shape they already expect. """ - srcdir = pathlib.Path(srcdir) - paths = [] - for d in source_build_dirs(srcdir): - # 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()] - + sorted(srcdir.glob("*.control")) + sorted(srcdir.glob("*.sql")) - ) - if not paths: - return None - h = hashlib.md5() - for path in paths: - try: - # The path relative to the tree, not just the name: with two build - # directories, src/module.c and objstore/module.c are different - # inputs and a bare name would make them interchangeable. - h.update(str(path.relative_to(srcdir)).encode()) - h.update(path.read_bytes()) - except (OSError, ValueError): - return None - return h.hexdigest()[:12] + return _fp.fingerprint(srcdir) or None def build_once(srcdir, pg_config, major, lock_path=None, runner=None): diff --git a/test/pytest/test_build_refusal.py b/test/pytest/test_build_refusal.py index 813c6137..ddd16a4e 100644 --- a/test/pytest/test_build_refusal.py +++ b/test/pytest/test_build_refusal.py @@ -21,6 +21,9 @@ """ import os +import tempfile +import shutil +import pwd import pathlib import re import subprocess @@ -442,10 +445,12 @@ def test_the_two_fingerprint_implementations_cover_the_same_inputs(tmp_path, exp 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. + THEY ARE NOW ONE IMPLEMENTATION (#907), so the requirement has strengthened + from "the same edit moves both" to "both are the same value". That is worth + asserting rather than merely allowing: equality is what makes the shell's + stamp readable by the Python harness and back, and if someone reintroduces a + private copy on either side this arm reddens on the first edit rather than on + the first edit that happens to diverge. """ t = _tree_with_module(tmp_path, "cover") for edit, path, body in ( @@ -464,6 +469,8 @@ def test_the_two_fingerprint_implementations_cover_the_same_inputs(tmp_path, exp path.write_text(old) expect.text(f"{sh_after != sh_before} {py_after != py_before}", "True True", f"editing {edit} moves both fingerprints") + expect.text(f"{sh_before} {sh_after}", f"{py_before} {py_after}", + f"and one implementation gives one value, editing {edit}") # --------------------------------------------------------------------------- @@ -500,66 +507,126 @@ def _fp_tree(tmp_path, name="t"): return t -def _md5_stub(tmp_path, fail_on): - """A stub md5sum: real, except that its Nth call fails with no output. +def _unprivileged_user(): + """A user that is not root, or None. - Models one transient failure -- a fork that hits EAGAIN, an OOM kill, a - loaded runner -- rather than a permanently broken md5sum, because the - permanent case is not the one that produced a wrong answer in CI. + THE MECHANISM HAD TO CHANGE WHEN THE IMPLEMENTATION DID. These arms used to + stub `md5sum` on PATH, because the shell forked it once per file. The digest + is now hashlib inside test/pgc_fingerprint.py, which no PATH can reach, so a + stub would have left both arms passing while testing nothing -- the exact + shape this suite exists to refuse. + + A real read failure needs a real reader who is denied, and root is denied + nothing: chmod 000 is invisible to it. Measured, before this was written: + + as root : 28a7149e07ae <- reads the mode-000 file anyway + as postgres : '' <- the failure the arm needs """ - b = tmp_path / f"bin{fail_on}" - b.mkdir(exist_ok=True) - counter = tmp_path / f"count{fail_on}" - counter.write_text("0") - (b / "md5sum").write_text( - "#!/bin/bash\n" - f'_n=$(( $(cat "{counter}" 2>/dev/null || echo 0) + 1 ))\n' - f'echo "$_n" > "{counter}"\n' - f'[ "$_n" = "{fail_on}" ] && exit 1\n' - 'exec /usr/bin/md5sum "$@"\n' - ) - (b / "md5sum").chmod(0o755) - return str(b) + if os.geteuid() != 0: + return "" # already unprivileged; read in this process + for name in ("postgres", "nobody"): + try: + pwd.getpwnam(name) + return name + except KeyError: + continue + return None + + +def _sh_fp_as(user, expr): + """Evaluate a lib.sh expression as USER ("" means this process).""" + script = f'. "{SRCDIR}/test/lib.sh" || exit 1; {expr}' + argv = ["bash", "-c", script] if not user else \ + ["runuser", "-u", user, "--", "bash", "-c", script] + p = subprocess.run(argv, capture_output=True, text=True) + return p.stdout.strip(), p.returncode -def test_a_failed_digest_yields_no_fingerprint_rather_than_a_wrong_one(tmp_path, expect): - """`$(md5sum ... 2>/dev/null)` substituted an EMPTY digest and returned rc=0. +def _readable_tree(name): + """A fingerprintable tree an unprivileged user can traverse. - One failed invocation among many silently changed the whole hash, so the - function gave three different confident answers for one unchanged tree. + Not under tmp_path: pytest's directories are mode 0700 and owned by the user + running the session, so a second user cannot walk into them and every arm + below would fail for the wrong reason. """ - t = _fp_tree(tmp_path) - base, rc = _sh_fp(f'pgc_source_fingerprint "{t}"') - expect.at_least(len(base), 12, "premise: the tree fingerprints at all") - - # Premise for the stub: with nothing configured to fail it must agree with - # the real md5sum, or the arms below measure the stub and not the fix. - quiet = _md5_stub(tmp_path, 0) - got, _ = _sh_fp(f'pgc_source_fingerprint "{t}"', path_env=quiet) - expect.text(got, base, "premise: the stub agrees with md5sum when nothing fails") + root = pathlib.Path(tempfile.mkdtemp(prefix="pgc-fpfail-")) + t = root / name + (t / "src").mkdir(parents=True) + for n, body in (("a.c", "int a;\n"), ("b.c", "int b;\n"), ("c.c", "int c;\n")): + (t / "src" / n).write_text(body) + (t / "Makefile").write_text("all:\n\ttrue\n") + (t / "pgcolumnar.control").write_text("x\n") + for d in (root, t, t / "src"): + d.chmod(0o755) + for f in t.rglob("*"): + if f.is_file(): + f.chmod(0o644) + return root, t - for n in (2, 3): - stub = _md5_stub(tmp_path, n) - got, _ = _sh_fp(f'pgc_source_fingerprint "{t}"', path_env=stub) - expect.text(got or "empty", "empty", - f"a failed digest on file {n} yields no fingerprint") +def test_a_failed_digest_yields_no_fingerprint_rather_than_a_wrong_one(expect): + """A digest that FAILED must not look like one that succeeded. -def test_a_failed_digest_gives_unverified_and_never_a_false_stale(tmp_path, expect): + The shell substituted an EMPTY digest for a failed md5sum and returned rc=0, + so one failed read among many silently changed the whole hash and the + function gave three different confident answers for one unchanged tree. + """ + user = _unprivileged_user() + if user is None: + expect.cannot_run("MISSING_DEPENDENCY", + "no non-root user to read as; root ignores chmod 000") + return + root, t = _readable_tree("t") + try: + base, _ = _sh_fp_as(user, f'pgc_source_fingerprint "{t}"') + expect.at_least(len(base), 12, "premise: the tree fingerprints at all") + + # Premise for the mechanism itself: the unprivileged reader must agree + # with a privileged one while nothing is denied, or the arm below would + # be measuring the user switch rather than the failure. + mine, _ = _sh_fp_as("", f'pgc_source_fingerprint "{t}"') + expect.text(base, mine, "premise: the unprivileged read agrees while readable") + + for name in ("b.c", "c.c"): + f = t / "src" / name + f.chmod(0o000) + got, _ = _sh_fp_as(user, f'pgc_source_fingerprint "{t}"') + f.chmod(0o644) + expect.text(got or "empty", "empty", + f"an unreadable {name} yields no fingerprint, not a wrong one") + + after, _ = _sh_fp_as(user, f'pgc_source_fingerprint "{t}"') + expect.text(after, base, "control: and the tree fingerprints again once readable") + finally: + shutil.rmtree(root, ignore_errors=True) + + +def test_a_failed_digest_gives_unverified_and_never_a_false_stale(expect): """The property that matters. `stale` is the FATAL; `unknown` is UNVERIFIED. A false UNVERIFIED costs a line of output. A false FATAL costs a matrix and - teaches people to re-run past a freshness check. + teaches people to re-run past a freshness check, which is the failure this + controller exists to prevent. """ - t = _fp_tree(tmp_path, "v") - base, _ = _sh_fp(f'pgc_source_fingerprint "{t}"') - stub = _md5_stub(tmp_path, 2) - verdict, _ = _sh_fp( - f'pgc_freshness_verdict "{base}" "$(pgc_source_fingerprint "{t}")"', - path_env=stub) - expect.text(verdict, "unknown", "a failed digest reads as unknown, not stale") - healthy, _ = _sh_fp(f'pgc_freshness_verdict "{base}" "$(pgc_source_fingerprint "{t}")"') - expect.text(healthy, "fresh", "control: an unstubbed run still reads fresh") + user = _unprivileged_user() + if user is None: + expect.cannot_run("MISSING_DEPENDENCY", + "no non-root user to read as; root ignores chmod 000") + return + root, t = _readable_tree("v") + try: + base, _ = _sh_fp_as(user, f'pgc_source_fingerprint "{t}"') + expect.at_least(len(base), 12, "premise: the tree fingerprints at all") + (t / "src" / "b.c").chmod(0o000) + verdict, _ = _sh_fp_as( + user, f'pgc_freshness_verdict "{base}" "$(pgc_source_fingerprint "{t}")"') + expect.text(verdict, "unknown", "a failed digest reads as unknown, not stale") + (t / "src" / "b.c").chmod(0o644) + healthy, _ = _sh_fp_as( + user, f'pgc_freshness_verdict "{base}" "$(pgc_source_fingerprint "{t}")"') + expect.text(healthy, "fresh", "control: a readable run still reads fresh") + finally: + shutil.rmtree(root, ignore_errors=True) def test_one_tree_hashes_one_way_however_the_path_is_spelled(tmp_path, expect): @@ -797,3 +864,54 @@ def offenders_in(text): "premise: and the INDIRECT form, which is the defect's own shape") expect.num(len(offenders_in('_p="$_bd_copy/objstore/p.c"\nprintf x > "$_p"\n')), 0, "control: a write into a COPY is not an offender") + + +def test_one_tree_hashes_one_way_however_the_locale_is_set(expect): + """The defect the single implementation removed on the way. + + The shell sorted its manifest with `sort -z`, which uses LOCALE COLLATION, + and no locale is pinned anywhere in this harness. So the same tree + fingerprinted differently depending on whose machine it was: + + LC_ALL=C 6d122a7158d5 + LC_ALL=en_US.UTF-8 0b59bd75fa4f + + en_US.UTF-8 is a common desktop default. A developer stamping a tree there + and a CI runner reading it under C.UTF-8 disagree, and the disagreement is a + FATAL naming a stale binary against a tree that is perfectly clean -- the + exact false FATAL this controller exists to prevent, arriving from the + environment rather than from the source. + + The filenames matter: `_` and `-` are what the two collations order + differently, and `columnar_arrow.c` beside `columnar-arrow.c` is not a + contrived pair in this tree. + """ + root = pathlib.Path(tempfile.mkdtemp(prefix="pgc-locale-")) + try: + t = root / "t" + (t / "src").mkdir(parents=True) + for n in ("columnar_arrow.c", "columnar-arrow.c", "columnarXarrow.c", + "Columnar.c", "columnar.c"): + (t / "src" / n).write_text(f"int x; /* {n} */\n") + (t / "Makefile").write_text("all:\n\ttrue\n") + (t / "pgcolumnar.control").write_text("x\n") + + available = subprocess.run(["locale", "-a"], capture_output=True, + text=True).stdout.lower() + wanted = [l for l in ("c", "c.utf8", "en_us.utf8") if l in available] + if len(wanted) < 2: + expect.cannot_run("MISSING_DEPENDENCY", + f"fewer than two locales installed: {wanted}") + return + + seen = {} + for loc in wanted: + got, _ = _sh_fp(f'pgc_source_fingerprint "{t}"', + env={"LC_ALL": loc, "LANG": loc}) + seen[loc] = got + expect.at_least(len(seen[wanted[0]]), 12, + "premise: the tree fingerprints at all") + expect.num(len(set(seen.values())), 1, + f"one tree, one fingerprint, across {len(wanted)} locales: {seen}") + finally: + shutil.rmtree(root, ignore_errors=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 index b272dc49..18ed61c2 100644 --- a/test/selftest/340-the-binary-must-be-built-from.sh +++ b/test/selftest/340-the-binary-must-be-built-from.sh @@ -512,55 +512,86 @@ unset _wr _wr_rc _wr_written _wr_expected _wr_f # deliberately does not fail. A false UNVERIFIED costs a line of output. A false # FATAL costs a red matrix and teaches people to re-run past the check. +# THE MECHANISM HAD TO CHANGE WHEN THE IMPLEMENTATION DID (#907). +# +# This block used to stub `md5sum` on PATH, because the shell forked it once per +# file. The digest is now hashlib inside test/pgc_fingerprint.py, which no PATH +# can reach -- so the stub would have left every arm below GREEN while testing +# nothing at all, which is the precise failure this file exists to refuse. +# +# A real read failure needs a real reader who is denied, and root is denied +# nothing: chmod 000 is invisible to it. Measured before this was written: +# +# as root 28a7149e07ae <- reads the mode-000 file regardless +# as postgres (empty) <- the failure these arms need +# +# The tree therefore lives outside any 0700 directory and is read by a second +# user. Where no such user exists the arms SKIP loudly rather than pass quietly: +# a guard that cannot run is not a guard that held. _fp="$(mktemp -d "${TMPDIR:-/tmp}/pgc-fpfail.XXXXXX")" -mkdir -p "$_fp/tree/src" "$_fp/bin" +mkdir -p "$_fp/tree/src" printf 'int a;\n' > "$_fp/tree/src/a.c" printf 'int b;\n' > "$_fp/tree/src/b.c" printf 'int c;\n' > "$_fp/tree/src/c.c" printf 'all:\n\ttrue\n' > "$_fp/tree/Makefile" printf 'x\n' > "$_fp/tree/pgcolumnar.control" +chmod -R a+rX "$_fp" + +_fp_user="" +if [ "$(id -u)" -ne 0 ]; then + _fp_user="-" # already unprivileged; read in this shell +else + for _fp_u in postgres nobody; do + id -u "$_fp_u" >/dev/null 2>&1 && { _fp_user="$_fp_u"; break; } + done +fi + +# _fp_as reads as the unprivileged user, or in this shell when already one. +_fp_as() { # _fp_as EXPR -> stdout + if [ "$_fp_user" = "-" ]; then + bash -c ". \"$PGC_TESTDIR/lib.sh\" || exit 1; $1" + else + runuser -u "$_fp_user" -- bash -c ". \"$PGC_TESTDIR/lib.sh\" || exit 1; $1" + fi +} -# A stub that behaves exactly like md5sum except on its Nth invocation, where it -# fails the way a fork failure or an OOM kill does: no output, non-zero status. -cat > "$_fp/bin/md5sum" <<'STUB' -#!/bin/bash -_n=$(( $(cat "$PGC_FP_COUNT" 2>/dev/null || echo 0) + 1 )) -echo "$_n" > "$PGC_FP_COUNT" -[ "$_n" = "${PGC_FP_FAIL_ON:-0}" ] && exit 1 -exec /usr/bin/md5sum "$@" -STUB -chmod +x "$_fp/bin/md5sum" - -_fp_base="$(pgc_source_fingerprint "$_fp/tree")" -check "premise: the tree fingerprints to something on an unstubbed run" \ - "$([ -n "$_fp_base" ] && echo yes || echo empty)" "yes" - -# Premise for the stub itself: with no failure configured it must agree with the -# real thing, or the arms below would be measuring the stub rather than the fix. -PGC_FP_COUNT="$_fp/count"; export PGC_FP_COUNT -PGC_FP_FAIL_ON=0; export PGC_FP_FAIL_ON -echo 0 > "$PGC_FP_COUNT" -check "premise: the stub agrees with the real md5sum when nothing fails" \ - "$(PATH="$_fp/bin:$PATH" pgc_source_fingerprint "$_fp/tree")" "$_fp_base" - -# THE ARM. One failed digest, and the answer must be EMPTY rather than a hash. -for _fp_n in 2 3; do - PGC_FP_FAIL_ON="$_fp_n"; export PGC_FP_FAIL_ON - echo 0 > "$PGC_FP_COUNT" - _fp_got="$(PATH="$_fp/bin:$PATH" pgc_source_fingerprint "$_fp/tree")" - check "a failed digest on file $_fp_n yields no fingerprint, not a wrong one" \ - "$([ -z "$_fp_got" ] && echo empty || echo "$_fp_got")" "empty" -done - -# And the verdict that follows from it, which is the property that matters: the -# controller must say UNVERIFIED, never `stale`. `stale` is the FATAL. -PGC_FP_FAIL_ON=2; export PGC_FP_FAIL_ON -echo 0 > "$PGC_FP_COUNT" -check "so the verdict is unknown -- UNVERIFIED -- and never stale" \ - "$(pgc_freshness_verdict "$_fp_base" \ - "$(PATH="$_fp/bin:$PATH" pgc_source_fingerprint "$_fp/tree")")" "unknown" - -unset PGC_FP_FAIL_ON PGC_FP_COUNT +if [ -z "$_fp_user" ]; then + echo "SKIP no non-root user to read as; root ignores chmod 000" +else + _fp_base="$(_fp_as "pgc_source_fingerprint \"$_fp/tree\"")" + check "premise: the tree fingerprints to something when it is readable" \ + "$([ -n "$_fp_base" ] && echo yes || echo empty)" "yes" + + # Premise for the mechanism: the unprivileged reader must agree with this + # shell while nothing is denied, or the arms below measure the user switch. + check "premise: the unprivileged read agrees while everything is readable" \ + "$_fp_base" "$(pgc_source_fingerprint "$_fp/tree")" + + # THE ARM. One unreadable file, and the answer must be EMPTY, not a hash. + for _fp_n in b.c c.c; do + chmod 000 "$_fp/tree/src/$_fp_n" + _fp_got="$(_fp_as "pgc_source_fingerprint \"$_fp/tree\"")" + chmod 644 "$_fp/tree/src/$_fp_n" + check "an unreadable $_fp_n yields no fingerprint, not a wrong one" \ + "$([ -z "$_fp_got" ] && echo empty || echo "$_fp_got")" "empty" + done + + check "control: and the tree fingerprints again once it is readable" \ + "$(_fp_as "pgc_source_fingerprint \"$_fp/tree\"")" "$_fp_base" + + # And the verdict that follows, which is the property that matters: the + # controller must say UNVERIFIED, never `stale`. `stale` is the FATAL. + chmod 000 "$_fp/tree/src/b.c" + check "so the verdict is unknown -- UNVERIFIED -- and never stale" \ + "$(pgc_freshness_verdict "$_fp_base" \ + "$(_fp_as "pgc_source_fingerprint \"$_fp/tree\"")")" "unknown" + chmod 644 "$_fp/tree/src/b.c" + check "control: a readable run still reads fresh" \ + "$(pgc_freshness_verdict "$_fp_base" \ + "$(_fp_as "pgc_source_fingerprint \"$_fp/tree\"")")" "fresh" +fi +unset _fp_user _fp_u _fp_got _fp_n +unset -f _fp_as # --------------------------------------------------------------------------- # ONE TREE HASHES ONE WAY, HOWEVER THE PATH TO IT IS SPELLED. @@ -766,3 +797,51 @@ _fr_hollow="$(mktemp -d "${TMPDIR:-/tmp}/pgc-rhollow.XXXXXX")" check "an empty manifest is reported as empty, not as silence" \ "$(pgc_freshness_report "$_fr_hollow" | grep -c 'empty -- nothing under')" "1" unset _fr _fr_hollow + + +# ---- one tree hashes one way, however the LOCALE is set --------------------- +# +# The defect the single implementation removed on the way (#907). The shell +# sorted its manifest with `sort -z`, which uses LOCALE COLLATION, and nothing in +# this harness pins a locale. So the same tree fingerprinted two ways depending +# on whose machine it was: +# +# LC_ALL=C 6d122a7158d5 +# LC_ALL=en_US.UTF-8 0b59bd75fa4f +# +# en_US.UTF-8 is a common desktop default, so this was a developer stamping a +# tree and CI reading it back under C.UTF-8 and calling the binary stale. The +# module sorts BYTES, which is what LC_ALL=C did and what every stamp already on +# disk was written with. +# +# `_` against `-` is what the two collations order differently, and +# columnar_arrow.c beside columnar-arrow.c is not a contrived pair in this tree. +_lc="$(mktemp -d "${TMPDIR:-/tmp}/pgc-locale.XXXXXX")" +mkdir -p "$_lc/tree/src" +for _lc_n in columnar_arrow.c columnar-arrow.c columnarXarrow.c Columnar.c columnar.c; do + printf 'int x; /* %s */\n' "$_lc_n" > "$_lc/tree/src/$_lc_n" +done +printf 'all:\n\ttrue\n' > "$_lc/tree/Makefile" +printf 'x\n' > "$_lc/tree/pgcolumnar.control" + +_lc_have="" +for _lc_l in C C.utf8 en_US.utf8; do + locale -a 2>/dev/null | grep -qx "$_lc_l" && _lc_have="$_lc_have $_lc_l" +done +_lc_count="$(printf '%s\n' $_lc_have | grep -c .)" + +check "premise: at least two locales are installed to compare" \ + "$([ "$_lc_count" -ge 2 ] && echo enough || echo "$_lc_count")" "enough" + +if [ "$_lc_count" -ge 2 ]; then + _lc_vals="" + for _lc_l in $_lc_have; do + _lc_vals="$_lc_vals $(LC_ALL="$_lc_l" LANG="$_lc_l" pgc_source_fingerprint "$_lc/tree")" + done + check "premise: the fixture fingerprints at all" \ + "$([ -n "$(printf '%s' $_lc_vals)" ] && echo yes || echo empty)" "yes" + check "one tree, one fingerprint, whatever the locale" \ + "$(printf '%s\n' $_lc_vals | sort -u | grep -c .)" "1" +fi +rm -rf "$_lc" +unset _lc _lc_n _lc_l _lc_have _lc_count _lc_vals diff --git a/test/selftest/380-the-pytest-cluster-helpers.sh b/test/selftest/380-the-pytest-cluster-helpers.sh index aa0eec0d..9eeec064 100644 --- a/test/selftest/380-the-pytest-cluster-helpers.sh +++ b/test/selftest/380-the-pytest-cluster-helpers.sh @@ -29,21 +29,47 @@ _pc_cl="$PGC_TESTDIR/pytest/pgc_cluster.py" check "premise: the pytest cluster helper is where this part thinks it is" \ "$([ -f "$_pc_cl" ] && echo yes || echo no)" "yes" -check "the fingerprint derives its build directories from a Makefile glob" \ - "$(grep -c 'glob("\*/Makefile")' "$_pc_cl")" "1" +# THE FINGERPRINT MOVED (#907). It was an independent Python implementation in +# pgc_cluster.py and an independent shell one in lib.sh; the pair produced four +# defects in a day. Both now delegate to test/pgc_fingerprint.py, so these arms +# follow the property to where it lives. Left pointed at pgc_cluster.py they +# would have gone GREEN by finding nothing to object to, which is why each one +# below is mirrored against a fixture that must make it red. +_pc_fp="$PGC_TESTDIR/pgc_fingerprint.py" -# A list would pass the arm above if someone added the glob AND kept a name. +check "premise: the one fingerprint implementation is where this part thinks it is" \ + "$([ -f "$_pc_fp" ] && echo yes || echo no)" "yes" + +check "the fingerprint derives its build directories from a Makefile on disk" \ + "$(grep -c '_is_plain_file(d / "Makefile")' "$_pc_fp")" "1" + +# A list would pass the arm above if someone added the derivation AND kept a name. check "and it names no module directory, so it is a derivation and not a list" \ - "$(grep -c '"objstore"' "$_pc_cl")" "0" + "$(grep -c '"objstore"' "$_pc_fp")" "0" # src/module.c and objstore/module.c are different inputs. Hashing the bare name -# would make them interchangeable, which is a collision the glob itself cannot -# prevent. +# would make them interchangeable, which is a collision the derivation cannot +# prevent by itself. check "the hash mixes in each file's path relative to the tree, not its name" \ - "$(grep -c 'relative_to(srcdir)' "$_pc_cl")" "1" + "$(grep -c 'relative_to(root)' "$_pc_fp")" "1" check "and no longer mixes in the bare filename" \ - "$(grep -c 'h.update(path.name.encode())' "$_pc_cl")" "0" + "$(grep -c 'h.update(path.name.encode())' "$_pc_fp")" "0" + +# AND NEITHER CALLER MAY KEEP A PRIVATE COPY. This is the arm that would catch +# #907 recurring: the whole point is one implementation, so a second one +# reappearing in either caller is the defect, not a detail. +check "the pytest helper keeps no private fingerprint implementation" \ + "$(grep -cE 'hashlib\.md5|glob\("\*/Makefile"\)' "$_pc_cl")" "0" + +check "and the shell keeps none either" \ + "$(grep -cE 'md5sum < |xargs -0 cat' "$PGC_TESTDIR/lib.sh")" "0" + +# STDLIB ONLY, AND NOTHING FROM test/pytest/. lib.sh runs this module with the +# SYSTEM interpreter; an import from the pytest tree or a third-party package +# would make every bash suite unrunnable until somebody installed pytest. +check "the module imports nothing from the pytest tree" \ + "$(grep -cE '^(import|from) +(pgc_|conftest|pytest|psycopg)' "$_pc_fp")" "0" # ---- and the lifecycle ------------------------------------------------------ # @@ -80,9 +106,19 @@ printf 'def make_cluster(a, b):\n root = mkdtemp()\n return cluster, root\ check "a make_cluster with no cleanup is caught" \ "$(grep -c 'shutil.rmtree(root' "$_pc_fix/noguard.py")" "0" -printf ' dirs = [srcdir / "src"]\n' > "$_pc_fix/srconly.py" +printf ' dirs = [root / "src"]\n' > "$_pc_fix/srconly.py" check "a fingerprint that reads src only is caught" \ - "$(grep -c 'glob("\*/Makefile")' "$_pc_fix/srconly.py")" "0" + "$(grep -c '_is_plain_file(d / "Makefile")' "$_pc_fix/srconly.py")" "0" + +# And the private-copy arms must be able to redden too, or "0" above means only +# that the pattern matches nothing anywhere. +printf 'import hashlib\nh = hashlib.md5(b"")\n' > "$_pc_fix/privatecopy.py" +check "a caller that reimplements the digest is caught" \ + "$(grep -cE 'hashlib\.md5' "$_pc_fix/privatecopy.py")" "1" + +printf 'from pgc_cluster import x\n' > "$_pc_fix/badimport.py" +check "an import from the pytest tree is caught" \ + "$(grep -cE '^(import|from) +(pgc_|conftest|pytest|psycopg)' "$_pc_fix/badimport.py")" "1" printf ' dirs = [srcdir / "src", srcdir / "objstore"]\n' > "$_pc_fix/listed.py" check "and a hard-coded module list is caught by the name arm" \ @@ -90,7 +126,12 @@ check "and a hard-coded module list is caught by the name arm" \ # The mirror: the same three greps on the REAL file, so a zero above is a missing # guard rather than a pattern that matches nothing anywhere. -check "premise: while the real helper satisfies the two it must" \ - "$(grep -cE 'glob\("\*/Makefile"\)|shutil.rmtree\(root' "$_pc_cl")" "2" +# The mirror: the same greps on the REAL files, so a zero above is a missing +# guard rather than a pattern that matches nothing anywhere. +check "premise: while the real module satisfies the derivation arm" \ + "$(grep -cE '_is_plain_file\(d / "Makefile"\)' "$_pc_fp")" "1" + +check "premise: and the real helper still carries its cleanup" \ + "$(grep -cE 'shutil.rmtree\(root' "$_pc_cl")" "1" -unset _pc_cl _pc_body _pc_fix +unset _pc_cl _pc_fp _pc_body _pc_fix