Skip to content

test: one source fingerprint, in Python, called by both harnesses (#907) - #911

Merged
jdatcmd merged 1 commit into
commandprompt:mainfrom
OffgridwithJD:audit/fingerprint-one-implementation
Sep 10, 2026
Merged

test: one source fingerprint, in Python, called by both harnesses (#907)#911
jdatcmd merged 1 commit into
commandprompt:mainfrom
OffgridwithJD:audit/fingerprint-one-implementation

Conversation

@OffgridwithJD

Copy link
Copy Markdown
Collaborator

Closes #907.

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:

defect in found by
objstore/*.c never walked python @linuxhikerpm (#897)
the bare NAME instead of the path, so src/module.c and objstore/module.c were interchangeable python me, fixing the above
xargs -0 cat | md5sum — no paths, no boundaries shell @linuxhikerpm (#898)
each build directory's Makefile omitted python me, writing the twin

The Python docstring 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.

Python rather than shell, which is the opposite of what #907 proposed

jd's constraint decided the direction: the single implementation belongs in the
more portable language. bash is largely a GNU thing; Python is on FreeBSD and
Windows where bash is not. lib.sh already requires bash, so calling a more
portable interpreter from it cannot cost portability.

I argued against this direction — lib.sh invokes python3 zero times, so it
escalates from "53 suites need it" to "every suite needs it at gate time". True,
and not a cost, for the reason above. Then I measured it expecting 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

9x faster, because the shell paid 64 forks per call. I had it backwards in
both dimensions.

Stdlib only, and it runs on the system interpreter: test/pytest/README.md
records that the interpreter is EXTERNALLY-MANAGED and pytest runs from a venv,
and a freshness gate that needed the pytest test dependencies would make every
bash suite unrunnable until somebody installed pytest.

A fifth defect, which unifying them found

sort -z orders by locale collation and nothing in the harness pins 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 — a false FATAL arriving from the
environment rather than the source. The module sorts bytes, which is what
LC_ALL=C produced and what every stamp already on disk was written with, so
nothing is invalidated. Arms in both harnesses.

Equivalence established, not asserted

A differential run of the module against the shell it replaces, over trees built
to break the ways this pair has actually broken — manifest and fingerprint
compared on each:

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 are subtle enough to name. 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, which is why the module canonicalises the root
before discovering build dirs.

Two arms had to change mechanism, and why that is not weakening them

The failed-digest arms 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 exists to refuse.

A real read failure needs a real reader who is denied, and root is denied nothing:

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 read
failure. Where no non-root user exists it records expect.cannot_run rather than
passing.

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 (@jdatcmd made this a condition, and he was right to):

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, serial and under -n 4
docs_style           9 checks PASSED
shellcheck -S error  clean
shell and python fingerprints of the real tree: both 6d122a7158d5

Note on ordering

Based on main at a9c40b1. #905 and #906 are ahead of it in the merge queue and
both touch TESTS.md, so this will need a recount-not-pick rebase once they land.
I will do that rather than resolve it in anyone else's branch.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a

@OffgridwithJD
OffgridwithJD force-pushed the audit/fingerprint-one-implementation branch from de432fa to 7f55857 Compare September 9, 2026 23:30
@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

Rebased onto 6a1fa1ca (#905 and #906 both merged), now 7f55857d and CLEAN.

One conflict, on TESTS.md's totals line — the seventh today, and neither side
was authorable in its own branch:

ours (main)  **120 tests in 8 files.** One hundred and five of them ...
theirs (911) **91 tests in 6 files.** Seventy-six of them ...
correct      **121 tests in 8 files.** One hundred and six of them ...

Recounted from the corpus with the gate's own function rather than resolved by
picking, and the prose count moved with it — that half is not gated, which is the
half #908 is about.

Re-gated after the rebase rather than before:

harness_selftest   428 passed + 0 failed + 0 unrunnable, rc=0
pytest corpus      121 passed, serial and under -n 4
pytest -k refus     59 passed, 62 deselected  (#906's guard still healthy)
docs_style           9 checks PASSED
shellcheck -S error  clean
shell and python fingerprints of the real tree: both 6d122a7158d5

The last line is the one worth reading twice: those two numbers were never equal
before this branch, because the two implementations used different digest recipes
over the same files. They are now one implementation, so equality is the property
rather than a coincidence, and test_the_two_fingerprint_implementations_cover_the_same_inputs
asserts it directly instead of only asserting that the same edit moves both.

…mmandprompt#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, commandprompt#897
    the bare NAME instead of the path    python   found while fixing the above
    `xargs -0 cat | md5sum`, no bounds   shell    @linuxhikerpm, commandprompt#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 commandprompt#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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
@OffgridwithJD
OffgridwithJD force-pushed the audit/fingerprint-one-implementation branch from 7f55857 to b71b9fc Compare September 9, 2026 23:57
@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

Rebased onto a0aa11ff (#910 and #904 both merged), now b71b9fc1 and CLEAN.

Two conflicts this time. The TESTS.md totals line for the ninth time today,
and CHANGELOG.md, where #904 added an entry beside mine — both entries are real
and additive, so both are kept rather than either being chosen.

TESTS.md  ours 122 in 10 · theirs 121 in 8 · correct **123 in 10**

Recounted from the corpus, not picked. The prose count moved with it to
One hundred and eight, and that is still the half nothing gates.

harness_selftest   428 passed + 0 failed + 0 unrunnable, rc=0
pytest corpus      123 passed, serial and under -n 4
docs_style           9 checks PASSED
shellcheck -S error  clean

shell  pgc_source_fingerprint  213b0077e930
python source_fingerprint      213b0077e930

That value moved from 6d122a7158d5, and it should have: #904 changed
src/columnar_reader.c and src/columnar_customscan.c, which are build inputs.
A content fingerprint that did not move there would be the defect. Both
implementations moved to the same new value, which is the property this PR adds —
they were never equal before it, because the two used different digest recipes
over the same file set.

@jdatcmd jdatcmd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving at b71b9fc1. This replaces the implementation I landed six hours ago, so I verified the properties #909 established rather than trusting that a port preserves them — and one measurement of mine was wrong in a way that nearly cost you a false blocker.

The property that mattered most: stamps on disk stay valid

A port that changes the hash makes every stamp already written read as stale — a false FATAL on every developer's tree, invisible to CI because CI copies a fresh tree and re-stamps. Same tree, both implementations:

old shell implementation (a9c40b1) : 213b0077e930
new python port          (b71b9fc) : 213b0077e930   IDENTICAL

And on a tree built to be awkward — a symlinked FILE, a symlinked DIRECTORY, a .c in a directory with no Makefile, every hashed suffix at the root:

old : b133dc404e36
new : b133dc404e36   IDENTICAL

The symlink cases are the ones I would have got wrong. find -type f tests the LINK, so a symlinked source is not in the shell's manifest; pathlib.is_file() FOLLOWS it and would have silently added one. The module excludes them explicitly and the hashes agree.

The fifth defect is real on THIS tree, not a hypothetical

I checked, because a defect demonstrated only on a constructed filename pair might never bite here:

OLD implementation, the real tree
  LC_ALL=C            6d122a7158d5
  LC_ALL=C.UTF-8      6d122a7158d5
  LC_ALL=en_US.UTF-8  0b59bd75fa4f     <- same tree, different answer

NEW port, same three locales
  213b0077e930 / 213b0077e930 / 213b0077e930

Exactly the two values you quoted. And the tree really does carry the sensitive shape — columnar.h against columnar_arrow.c, where . and _ collate differently:

C order      columnar-arrow.c  columnarZarrow.c  columnar_arrow.c
en_US order  columnar_arrow.c  columnar-arrow.c  columnarZarrow.c

A developer on an en_US desktop stamping a tree that CI then reads under C.UTF-8 is a FATAL naming a stale binary against a clean tree. Sorting bytes removes the environment from the answer.

#909's guarantees, each re-checked

empty tree            -> []          not md5("") = d41d8cd98f00
nonexistent tree      -> []
all six spellings     -> 213b0077e930 (plain, trailing slash, /./, /src/.., symlink, relative .)

And in the module itself, manifest() returns None rather than a short manifest when a digest fails (test/pgc_fingerprint.py:149). That is the distinction that matters and it is the one I would most expect a port to lose: skipping the unreadable file would produce a well-formed hash over fewer files, which is worse than the defect #909 fixed, not better.

The absence arms fire against the real files

This is the shape I said I would look at hardest, because a static assertion of ABSENCE is the one most likely to be unable to fail. I injected into the real files rather than fixtures:

real pgc_cluster.py gains `hashlib.md5`
    427 passed + 1 failed
    FAIL  the pytest helper keeps no private fingerprint implementation: got [1] want [0]

real pgc_fingerprint.py gains `from pgc_cluster import ...`
    400 passed + 28 failed, including
    FAIL  the module imports nothing from the pytest tree: got [1] want [0]

The first is surgical — exactly one arm. The second's blast radius is 28 because that import breaks the module outright under the system interpreter, which is precisely why the arm exists.

I nearly filed a false blocker, and the correction is worth more than the finding would have been

I probed "what if python3 is absent" and measured the fingerprint as the string -- python3 not found: ..., then traced the consequence: both sides carrying that string compare EQUAL, so the verdict would be fresh — a false green on every suite, strictly worse than anything in this family.

It was my instrument. I captured the subshell with 2>&1, and that warning goes to stderr (lib.sh:725). Measured properly:

python3 absent -> value []            verdict unknown -> UNVERIFIED   correct
stamp written with python3, read without -> unknown                   correct
warning appears on stderr, once

What made me re-check was the source reading >&2 while my measurement said otherwise. When the instrument and the code disagree, suspect the instrument — I have spent today saying that to other people.

One residual, not blocking

_manifest_files skips an unreadable directory with continue (pgc_fingerprint.py:109), producing a well-formed hash over fewer files. That is faithful to find, which prints nothing for a directory it cannot read, so it is equivalence-preserving and pre-existing rather than introduced here — and equivalence is this PR's contract, so I would not change the behaviour.

But the module's thesis is fail-closed, and this is the one place it is fail-open by inheritance. The per-file path has a comment explaining why None beats a short manifest; the per-directory path has a comment explaining what find does. A sentence saying the asymmetry is deliberate would stop the next reader treating it as an oversight. I could not exercise it — the tests run as root here and root ignores the mode bits — so this is a reading, not a measurement, and I have labelled it as such.

Verified

harness_selftest   428 passed + 0 failed + 0 unrunnable
CI                 12/12, CLEAN

The direction is right and your measurement settled it against your own prior: one interpreter start at 26 ms beats 64 forks at 239 ms, and 124 s of gate time becomes 13 s. Two implementations of one idea produced four defects in a day and not one was found by whoever wrote that copy; one implementation with two observers is the shape that has been earning its keep all day.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Two implementations of the source fingerprint produced four defects in one day

2 participants