diff --git a/test/lib.sh b/test/lib.sh index 9fc7baf3..8b3b8349 100755 --- a/test/lib.sh +++ b/test/lib.sh @@ -294,6 +294,13 @@ pgc_setup() { echo "FATAL: the binary under test was not built from this source" >&2 echo " source now $_pgc_fresh_current, binary built from $_pgc_fresh_recorded" >&2 echo " (refusing to report checks about code that is not installed)" >&2 + # WHAT IT HASHED, not just what it hashed TO. Two CI failures reported + # this pair of hashes and nothing else, identically, across two + # branches and two majors -- which made the second occurrence another + # sample rather than an answer. Twelve characters cannot name a file; + # the manifest can, and a diff against the next occurrence's manifest + # says whether something appeared, vanished or changed. + pgc_freshness_report "$PGC_SRCDIR" >&2 exit 1 ;; unknown) @@ -645,9 +652,51 @@ pgc_source_build_dirs() { # pgc_source_build_dirs DIR -> dirs -printf '%h\n' 2>/dev/null | grep -v "^$dir/src$" || true } -pgc_source_fingerprint() { # pgc_source_fingerprint DIR -> hash +# A value no md5sum digest can be, so it cannot be confused with one. +_pgc_fp_failed='PGC_FINGERPRINT_DIGEST_FAILED' + +# What the fingerprint was taken over, for the FATAL path. A FUNCTION rather than +# three lines inline, so an arm can drive it: a dump nobody can run is a dump +# nobody knows is empty. Two CI failures printed only the pair of hashes, and the +# second occurrence was therefore another sample rather than an answer. +pgc_freshness_report() { # pgc_freshness_report DIR -> the manifest, annotated + local dir="${1:-.}" n + n="$(pgc_source_manifest "$dir" | wc -l)" + echo " the manifest this fingerprint was taken over:" + if [ "$n" -eq 0 ]; then + echo " | (empty -- nothing under $dir matched, which is itself the finding)" + else + pgc_source_manifest "$dir" | sed 's/^/ | /' + fi + echo " ($n files, under $dir)" +} + +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 @@ -674,9 +723,54 @@ pgc_source_fingerprint() { # pgc_source_fingerprint DIR -> hash # 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 + # + # 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 +} + +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 } # fresh the binary was built from this source diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index 1b455763..a3a4f811 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. -**78 tests in 6 files.** Sixty-three of them test the harness rather than the +**90 tests in 6 files.** Seventy-five 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. @@ -429,6 +429,152 @@ an unreadable postmaster start time, or a non-numeric epoch all produce. The boundary arm is separate on purpose: mtime resolution is one second, so a run fast enough to install and start within the same second must not refuse itself. +### The fingerprint's own integrity, and why it needed six more arms + +Six tests, added with the fix that closed three defects in `pgc_source_fingerprint` +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_gives_unverified_and_never_a_false_stale` is the property +that matters. `stale` is the FATAL; `unknown` prints `freshness UNVERIFIED` and +runs the suites. The asymmetry is the whole argument for the change: a false +UNVERIFIED costs a line of output, a false FATAL costs a matrix **and** teaches +people to re-run past a freshness check, which is the failure this controller +exists to prevent. + +`test_one_tree_hashes_one_way_however_the_path_is_spelled` pins five spellings — +trailing slash, `/./`, `/src/..`, a symlink, and a relative `.` — against the plain +path. Three of them disagreed before the fix, because `${f#"$dir"/}` strips a +prefix that has to match character for character. + +`test_the_fix_does_not_rebaseline_stamps_already_on_disk` is a **compatibility** +assertion rather than a tidiness one, and it is the arm that would have caught the +worst version of this change. Detecting a failed digest means capturing the +per-file lines to inspect them, and `$(...)` strips the trailing newline that the +old straight pipe into `md5sum` included. Without restoring it, the same unchanged +tree hashes differently before and after the fix, every stamp already on disk reads +`stale`, and a fix for false FATALs becomes a false FATAL for everyone holding a +built worktree. The matrix cannot catch that: it copies a fresh tree and re-stamps +every run, so it lands on developers and on nobody's CI. The arm transcribes the +previous implementation and requires the same answer. + +`test_the_fingerprint_still_moves_on_a_real_change` is the control without which +the spelling arms are vacuous — "every spelling agrees" is satisfied perfectly by a +fingerprint that ignores its input. + +`test_a_tree_with_nothing_hashable_reports_no_fingerprint` closes the last one: the +hash of an empty stream is a stable, comparable value, so two trees with no source +would have *matched*. + +**That last arm is the only one in this set with a real observation behind it +rather than a model, and it was not the case it was written for.** It shipped as +"a legitimate empty tree". @OffgridwithJD then observed the WHOLE manifest coming +back empty under process pressure, on a read-only bind mount where content was +excluded by construction: two distinct fingerprints over a tree incapable of +changing, and the deviant value was `d41d8cd98f00`, which is md5 of the empty +string — not a corrupted manifest but *no* manifest, hashed confidently. Measured +here as an A/B with the real `md5sum` and no stub, 20 samples per cell: + + true fingerprint = eebe35d6eaed ; md5("") = d41d8cd98f00 + + OLD ulimit -u 45 correct=19 md5("")=1 refused=0 other=0 | sum=20 of 20 + OLD ulimit -u 40 correct=8 md5("")=11 refused=0 other=1 | sum=20 of 20 + NEW ulimit -u 45 correct=20 md5("")=0 refused=0 other=0 | sum=20 of 20 + NEW ulimit -u 40 correct=20 md5("")=0 refused=0 other=0 | sum=20 of 20 + +At `ulimit -u 40` the old function returns a confident answer about nothing in 11 +runs of 20. The guard covers it structurally rather than statistically: a +non-empty `out` has at least one line, so `md5("")` is not a reachable return +value. + +### When it refuses, it says what it hashed + +Six more tests, added after two CI failures reported *the same pair of hashes and +nothing else* — `source now a735c673b129, binary built from 6d122a7158d5`, +identically, across two branches, two majors and two build directories, with the +fingerprint fix present in one of them. **A bare hash made the second occurrence +another sample rather than an answer.** + +So the manifest is a function in its own right, `pgc_source_fingerprint` is +defined as its hash — the two cannot drift apart, and one test asserts exactly +that — and the FATAL path prints it through `pgc_freshness_report`. + +`test_an_added_file_is_named_rather_than_merely_changing_the_hash` is the arm +aimed at the open question. An addition is the only class that explains one +deviant value from two different build directories, because the manifest carries +the path RELATIVE to the tree: the same file appearing under `matrix-17` and +`matrix-18` contributes the same line and therefore the same hash. The test +requires the diff to name the file rather than report that something changed. + +`test_the_manifest_names_what_the_fingerprint_hashed` pins the shape of each line +— a tree-relative path and a 32-character digest, never an absolute path, because +an absolute path in the digest is the spelling defect returning by another route. +`test_the_fingerprint_is_the_hash_of_the_manifest` is the arm that keeps the two +from drifting, and `test_an_empty_manifest_is_reported_as_empty_not_as_silence` +covers the case the report exists for. + +`test_the_fatal_report_can_be_run_rather_than_grepped_for` exists because the +alternative was asserting that the source calls the function, which is the shape +this suite refuses everywhere else. The report is a function so an arm can drive +it, and the empty case says `(empty -- nothing under ...)` rather than printing +nothing, because a silent empty dump reads as *the manifest was fine*. + +### The suite that wrote into the tree the other suites were reading + +`test_no_selftest_part_writes_into_the_live_source_tree` scans the parts for a +redirection aimed at the live tree. + +`test/selftest/340` used to write `objstore/.pgc_fingerprint_probe.c` into +`$PGC_SRCDIR`, to prove that a new file under a recursed directory moves the +fingerprint. `harness_selftest` runs IN the matrix, so at `PGC_JOBS=4` it created +that file in the shared build directory while sibling suites fingerprinted +concurrently, and whichever sampled inside that window reported `FATAL: the binary +under test was not built from this source` against a tree that was correct. The +path is tree-relative and the content fixed, so the deviant value was *identical* +across majors, build directories and branches — which is what made it look like a +real staleness. It cost four pull requests and two wrong diagnoses before +@linuxhikerpm found it by reading the suite. + +The arm now probes a hardlinked COPY of the tree. The intent survives, because the +defect it was written for was that the *real* tree's `objstore/` was not being +read, and a hand-built fixture could not have caught that — so a premise requires +the copy to discover the same build directories as the real tree. **That premise +immediately earned itself**: the first fix hardlinked across a filesystem +boundary, `cp -al` failed after creating the destination, `cp -a` then copied the +tree *inside* it, and the copy's build directories came out as `bfix src` rather +than `objstore src`. + +**A BEFORE/AFTER RUN CANNOT CATCH THIS, and that is worth recording because it +was my first attempt.** Fingerprint the tree, run the suite, fingerprint again: +the probe was created and `rm -f`'d inside the same suite, so the tree is +byte-identical by the time the run ends and the comparison passes. The damage is +done to whoever samples DURING the window, and an after-the-fact observer is blind +to it by construction. Sampling concurrently instead would make the arm racy — it +would pass whenever the timing missed. So the observable property is the one in +the source: no part directs a write at the live tree. + +Its limit is stated in the test: it recognises a redirection whose target mentions +the tree-root variables the parts actually use, and a write reaching the tree by +another route would evade it. It carries three premises of its own — that the scan +recognises a write at `$PGC_SRCDIR`, that it recognises one through `$_bd_root`, +and that a write into a COPY is *not* flagged — because a pattern that matches +nothing would otherwise pass this arm silently. + +**What is still not guarded**, named here rather than left for someone to find: a +TRUNCATED manifest — `find` returning fewer files rather than none — would produce +a plausible wrong hash that neither the per-file sentinel nor the empty-manifest +guard can see. It has not been observed. The boundary of this change is "the three +observed variants are closed", not "the function is now infallible". + ## 6. test_docs_cover_the_corpus.py: this document, checked The file you are reading is checked mechanically, because it went stale inside a diff --git a/test/pytest/test_build_refusal.py b/test/pytest/test_build_refusal.py index 71cc9773..f0d976b5 100644 --- a/test/pytest/test_build_refusal.py +++ b/test/pytest/test_build_refusal.py @@ -20,7 +20,9 @@ cannot reach and which is where a wrong quote would hide. """ +import os import pathlib +import re import subprocess import pytest @@ -455,3 +457,336 @@ 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") + + +# --------------------------------------------------------------------------- +# THE TWIN of test/selftest/340's fingerprint-integrity arms. Two observers, one +# implementation: these drive the real shell functions through bash rather than +# reimplementing them, for the reason four fingerprint defects in one day proved +# — a second implementation of one idea drifts, and the drift is invisible until +# someone diffs the two. + + +def _sh_fp(expr, path_env=None, env=None): + """Evaluate a lib.sh expression, optionally with a stubbed PATH.""" + script = f'. "{SRCDIR}/test/lib.sh" || exit 1; {expr}' + e = dict(os.environ) + if env: + e.update(env) + if path_env: + e["PATH"] = path_env + ":" + e.get("PATH", "") + p = subprocess.run(["bash", "-c", script], capture_output=True, text=True, env=e) + return p.stdout.strip(), p.returncode + + +def _fp_tree(tmp_path, name="t"): + 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 / "src" / "b.c").write_text("int b;\n") + (t / "src" / "c.c").write_text("int c;\n") + (t / "objstore" / "m.c").write_text("int m;\n") + (t / "objstore" / "Makefile").write_text("all:\n\ttrue\n") + (t / "Makefile").write_text("all:\n\ttrue\n") + (t / "pgcolumnar.control").write_text("x\n") + return t + + +def _md5_stub(tmp_path, fail_on): + """A stub md5sum: real, except that its Nth call fails with no output. + + 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. + """ + 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) + + +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. + + One failed invocation among many silently changed the whole hash, so the + function gave three different confident answers for one unchanged tree. + """ + 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") + + 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_gives_unverified_and_never_a_false_stale(tmp_path, 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. + """ + 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") + + +def test_one_tree_hashes_one_way_however_the_path_is_spelled(tmp_path, expect): + """`${f#"$dir"/}` strips a prefix that must match character for character.""" + t = _fp_tree(tmp_path, "s") + link = tmp_path / "s_link" + link.symlink_to(t) + plain, _ = _sh_fp(f'pgc_source_fingerprint "{t}"') + expect.at_least(len(plain), 12, "premise: the fixture fingerprints at all") + for label, spelling in ( + ("a trailing slash", f"{t}/"), + ("a /./ segment", f"{t}/./"), + ("a /src/.. segment", f"{t}/src/.."), + ("a symlink", str(link)), + ): + got, _ = _sh_fp(f'pgc_source_fingerprint "{spelling}"') + expect.text(got, plain, f"{label} hashes the same tree the same way") + rel, _ = _sh_fp(f'cd "{t}" && pgc_source_fingerprint .') + expect.text(rel, plain, "a relative path hashes the same tree the same way") + + +def test_the_fix_does_not_rebaseline_stamps_already_on_disk(tmp_path, expect): + """A COMPATIBILITY assertion, not a tidiness one. + + Capturing the per-file lines to inspect them strips the trailing newline the + old straight-pipe into md5sum included. Without restoring it the same + unchanged tree hashes differently before and after the fix, every stamp on + disk reads `stale`, and a fix for false FATALs becomes a false FATAL for + everyone holding a built worktree. The matrix cannot catch this: it copies a + fresh tree and re-stamps every run. + """ + t = _fp_tree(tmp_path, "c") + previous = r''' +_prev() { local dir="$1" d + { 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 '' _f; do + printf '%s %s\n' "${_f#"$dir"/}" "$(md5sum < "$_f" 2>/dev/null | cut -d' ' -f1)" + done | md5sum | cut -c1-12; } +''' + now, _ = _sh_fp(f'pgc_source_fingerprint "{t}"') + before, _ = _sh_fp(previous + f'_prev "{t}"') + expect.text(now, before, + "the fixed fingerprint equals what the previous implementation produced") + + +def test_the_fingerprint_still_moves_on_a_real_change(tmp_path, expect): + """Without this the spelling arms are vacuous. + + "Every spelling agrees" is satisfied perfectly by a fingerprint that ignores + its input, so the set needs one arm proving the hash still moves. + """ + t = _fp_tree(tmp_path, "m") + before, _ = _sh_fp(f'pgc_source_fingerprint "{t}"') + (t / "src" / "a.c").write_text("int a = 2;\n") + after, _ = _sh_fp(f'pgc_source_fingerprint "{t}"') + expect.text(str(after != before), "True", "a real content change moves the fingerprint") + (t / "src" / "a.c").write_text("int a;\n") + restored, _ = _sh_fp(f'pgc_source_fingerprint "{t}"') + expect.text(restored, before, "and restoring the content restores it") + + +def test_a_tree_with_nothing_hashable_reports_no_fingerprint(tmp_path, expect): + """The hash of an empty stream is a stable, comparable value: two empty + trees would have "matched". + + THE PREMISE IS LOAD-BEARING AND WAS MISSING. This arm asserts an EMPTY + result, and an empty result is also what a harness that cannot run at all + produces: point the helper at a `lib.sh` that does not exist and stdout is + `''`, so `got or "empty"` is `"empty"` and the arm passes green over a + completely broken tree. That is the same shape as the `expect.refusal` + defect @OffgridwithJD found on main -- a failure that produces exactly the + value the test expects. The populated-tree premise below fails first when + the harness is broken, which is what makes the empty assertion mean + anything. + """ + live = _fp_tree(tmp_path, "hollow_premise") + base, _ = _sh_fp(f'pgc_source_fingerprint "{live}"') + expect.at_least(len(base), 12, + "premise: the same helper returns a fingerprint for a real tree") + empty = tmp_path / "hollow" + empty.mkdir() + got, _ = _sh_fp(f'pgc_source_fingerprint "{empty}"') + expect.text(got or "empty", "empty", "an unhashable tree yields no fingerprint") + + +def _mf_tree(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 / "src" / "h.h").write_text("void h(void);\n") + (t / "objstore" / "m.c").write_text("int m;\n") + (t / "objstore" / "Makefile").write_text("all:\n\ttrue\n") + (t / "Makefile").write_text("all:\n\ttrue\n") + (t / "pgcolumnar.control").write_text("x\n") + return t + + +def test_the_manifest_names_what_the_fingerprint_hashed(tmp_path, expect): + """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, two + majors and two build directories, with the fingerprint fix present in one of + them. A bare hash made the second occurrence another sample rather than an + answer. + """ + t = _mf_tree(tmp_path, "mf") + out, _ = _sh_fp(f'pgc_source_manifest "{t}"') + lines = [l for l in out.splitlines() if l.strip()] + expect.num(len(lines), 6, "the manifest names every file the fingerprint hashes") + shaped = [l for l in lines if re.fullmatch(r"[A-Za-z0-9_./-]+ [0-9a-f]{32}", l)] + expect.num(len(shaped), 6, "each line is a tree-relative path and a digest") + expect.num(len([l for l in lines if l.startswith("/")]), 0, + "the manifest is tree-relative, never absolute") + + +def test_the_fingerprint_is_the_hash_of_the_manifest(tmp_path, expect): + """One is defined as the other, so the two cannot drift apart.""" + t = _mf_tree(tmp_path, "mh") + fp, _ = _sh_fp(f'pgc_source_fingerprint "{t}"') + via, _ = _sh_fp(f'pgc_source_manifest "{t}" | md5sum | cut -c1-12') + expect.text(fp, via, "the fingerprint is the hash of the manifest") + + +def test_an_added_file_is_named_rather_than_merely_changing_the_hash(tmp_path, expect): + """The class the two CI failures fall into, and the one a hash cannot report. + + An addition is the only class that explains one deviant value from two + different build directories: the manifest carries the path RELATIVE to the + tree, so the same file added under matrix-17 and matrix-18 contributes the + same line. + """ + t = _mf_tree(tmp_path, "add") + before, _ = _sh_fp(f'pgc_source_manifest "{t}"') + (t / "src" / "zz_appeared.c").write_text("int zz;\n") + after, _ = _sh_fp(f'pgc_source_manifest "{t}"') + gained = set(after.splitlines()) - set(before.splitlines()) + expect.num(len(gained), 1, "exactly one manifest line appears") + expect.text(sorted(gained)[0].split()[0], "src/zz_appeared.c", + "and it names the file that appeared") + + +def test_the_fatal_report_can_be_run_rather_than_grepped_for(tmp_path, expect): + """A dump nobody can run is a dump nobody knows is empty.""" + t = _mf_tree(tmp_path, "rep") + out, _ = _sh_fp(f'pgc_freshness_report "{t}"') + expect.num(len([l for l in out.splitlines() if l.strip().startswith("| src/a.c ")]), 1, + "the report names each hashed file") + expect.num(len([l for l in out.splitlines() if "(6 files, under" in l]), 1, + "the report states how many files it hashed") + + +def test_an_empty_manifest_is_reported_as_empty_not_as_silence(tmp_path, expect): + """A silent empty dump reads as "the manifest was fine", which is the failure + this report exists to end.""" + hollow = tmp_path / "hollow_report" + hollow.mkdir() + out, _ = _sh_fp(f'pgc_freshness_report "{hollow}"') + expect.num(len([l for l in out.splitlines() if "empty -- nothing under" in l]), 1, + "an empty manifest says so") + + +def test_no_selftest_part_writes_into_the_live_source_tree(expect): + """A suite that runs beside others must not write into the tree they read. + + `test/selftest/340` used to write `objstore/.pgc_fingerprint_probe.c` into + $PGC_SRCDIR to prove that a new file under a recursed directory moves the + fingerprint. `harness_selftest` runs IN the matrix, so at PGC_JOBS=4 it + created that file in the shared build directory while sibling suites + fingerprinted concurrently, and whichever sampled inside the window reported + + FATAL: the binary under test was not built from this source + source now a735c673b129, binary built from 6d122a7158d5 + + against a tree that was correct. Four pull requests and two wrong diagnoses + before @linuxhikerpm found it by reading the suite. + + WHY THIS IS A STATIC SCAN AND NOT A BEFORE/AFTER RUN. I wrote the behavioural + version first: fingerprint the tree, run the suite, fingerprint again. It + CANNOT CATCH THIS DEFECT. The probe was created and `rm -f`'d inside the same + suite, so the tree is byte-identical by the time the run ends and the + comparison passes. The damage is done to whoever samples DURING the window, + and an after-the-fact observer is blind to it by construction. Sampling + concurrently instead would make the arm racy -- it would pass whenever the + timing missed. So the observable property is the one in the source: no part + directs a write at the live tree. + + ITS LIMIT, stated because I have spent today objecting to guards that catch + one spelling: this recognises a redirection whose target mentions the tree + root variables the parts actually use. A write reaching the tree by some other + route -- a `cp` destination, a path assembled elsewhere -- would evade it. The + `.sh` half asserts the concrete probe path is outside the tree and reddens + with `got [INSIDE ...]`; between them they cover this instance and the obvious + generalisations of it, not every conceivable one. + """ + parts = sorted((SRCDIR / "test" / "selftest").glob("*.sh")) + expect.at_least(len(parts), 5, "premise: the selftest parts were found") + + def offenders_in(text): + """Redirections that land in the live tree, following one indirection. + + The real defect is written in two steps -- `_bd_probe="$_bd_root/..."` + and then `printf ... > "$_bd_probe"` -- so a scan that only looks for the + tree root INSIDE a redirection target misses it entirely. That was this + arm's first version, and it passed against the reverted defect. + """ + root = r"(?:PGC_SRCDIR|_bd_root)" + tainted = set(re.findall(r'^\s*([A-Za-z_][A-Za-z0-9_]*)=\"?\$\{?' + root + r'\b', + text, re.M)) + names = "|".join([root] + sorted(re.escape(t) for t in tainted)) + redirect = re.compile(r'>\s*"?\$\{?(?:' + names + r')\b') + found = [] + for n, line in enumerate(text.splitlines(), 1): + if line.lstrip().startswith("#"): + continue + if redirect.search(line): + found.append(n) + return found + + bad = [] + for part in parts: + bad += [f"{part.name}:{n}" for n in offenders_in(part.read_text())] + expect.text(", ".join(bad) or "none", "none", + "no selftest part directs a write at the live source tree") + + # PREMISES: the scan must be able to see each shape it claims to cover, or it + # passes on a pattern that matches nothing -- the shape it exists to refuse. + expect.num(len(offenders_in('printf x > "$PGC_SRCDIR/objstore/p.c"\n')), 1, + "premise: a direct write at the tree root is seen") + expect.num(len(offenders_in('_p="$_bd_root/objstore/p.c"\nprintf x > "$_p"\n')), 1, + "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") 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 14366f49..b272dc49 100644 --- a/test/selftest/340-the-binary-must-be-built-from.sh +++ b/test/selftest/340-the-binary-must-be-built-from.sh @@ -128,17 +128,67 @@ check "every directory the Makefile builds from is in the fingerprint" \ "$([ -z "$_bd_missing" ] && echo none || echo "missing:$_bd_missing")" "none" # And the consequence, end to end: a change under a recursed directory must move -# the fingerprint. Asserted against the real tree rather than a fixture, because -# the defect was that the real tree's objstore/ was not being read. -_bd_probe="$_bd_root/objstore/.pgc_fingerprint_probe.c" -_bd_before="$(pgc_source_fingerprint "$_bd_root")" +# the fingerprint. Against a tree with the REAL tree's SHAPE -- because the defect +# was that the real tree's objstore/ was not being read, and a hand-built fixture +# could not have caught it -- but NOT against the real tree itself. +# +# THIS AROSE FROM WRITING INTO THE LIVE SOURCE TREE, AND IT COST FOUR PULL +# REQUESTS. This arm used to write `objstore/.pgc_fingerprint_probe.c` into +# $PGC_SRCDIR. harness_selftest runs IN the matrix, so at PGC_JOBS=4 it created +# that file in the shared build directory while up to three sibling suites +# fingerprinted concurrently, and whichever sampled inside the window reported +# +# FATAL: the binary under test was not built from this source +# source now a735c673b129, binary built from 6d122a7158d5 +# +# on a tree that was correct. The path is tree-RELATIVE and the content fixed, so +# the deviant value was IDENTICAL across majors, build directories and branches -- +# which is what made it look deterministic enough to be a real staleness, and what +# sent two agents chasing a transient md5sum failure and then a path-spelling +# defect. @linuxhikerpm found it by reading, and it reproduces exactly: +# +# clean tree 6d122a7158d5 +# with objstore/.pgc_fingerprint_probe.c a735c673b129 <- what CI reported +# +# A hardlinked copy costs no data and keeps the structure exact. Writing a NEW +# file into it cannot touch the original, because only existing files are shared. +# Entry by entry, and hardlink-first. `cp -al SRC DST` is NOT safe as a fallback +# pair: /tmp is a different filesystem from the tree here, so the hardlink copy +# failed AFTER creating DST, and `cp -a` then copied the tree INSIDE it -- the +# copy's build dirs came out as `bfix src` instead of `objstore src`. The PREMISE +# below caught that, which is the entire reason it is written as a premise rather +# than assumed. `*` also skips `.git`, which no part of this question needs. +_bd_copy="$(mktemp -d "${TMPDIR:-/tmp}/pgc-bdcopy.XXXXXX")/tree" +mkdir -p "$_bd_copy" +for _bd_entry in "$_bd_root"/*; do + [ -e "$_bd_entry" ] || continue + cp -al "$_bd_entry" "$_bd_copy/" 2>/dev/null || cp -a "$_bd_entry" "$_bd_copy/" +done +_bd_probe="$_bd_copy/objstore/.pgc_fingerprint_probe.c" + +# THE ARM THAT WOULD HAVE CAUGHT THE ORIGINAL. A suite that runs beside others +# must not write into the tree they are reading. +check "the probe is written outside the live source tree" \ + "$(case "$_bd_probe" in "$_bd_root"/*) echo "INSIDE $_bd_root" ;; *) echo outside ;; esac)" \ + "outside" + +# PREMISE: the copy is equivalent for the question being asked. If the copy did +# not carry objstore/, "a new file under objstore moves the fingerprint" would be +# measuring a directory the fingerprint never covered, and would pass for the +# wrong reason. +check "PREMISE the copy discovers the same build directories as the real tree" \ + "$(pgc_source_build_dirs "$_bd_copy" | sed "s|^$_bd_copy/||" | sort | tr '\n' ' ')" \ + "$(pgc_source_build_dirs "$_bd_root" | sed "s|^$_bd_root/||" | sort | tr '\n' ' ')" + +_bd_before="$(pgc_source_fingerprint "$_bd_copy")" printf 'int pgc_fingerprint_probe;\n' > "$_bd_probe" check "a new source file under objstore moves the fingerprint" \ - "$([ "$(pgc_source_fingerprint "$_bd_root")" != "$_bd_before" ] && echo moved || echo same)" \ + "$([ "$(pgc_source_fingerprint "$_bd_copy")" != "$_bd_before" ] && echo moved || echo same)" \ "moved" rm -f "$_bd_probe" check "and removing it restores the fingerprint" \ - "$(pgc_source_fingerprint "$_bd_root")" "$_bd_before" + "$(pgc_source_fingerprint "$_bd_copy")" "$_bd_before" +rm -rf "${_bd_copy%/tree}" # ---- the postmaster arm must be able to FIRE, not just to compute ----------- # @@ -435,3 +485,284 @@ check "so the verdict is fresh, not unknown" \ "$(pgc_source_fingerprint "$_wr")")" "fresh" unset _wr _wr_rc _wr_written _wr_expected _wr_f + +# --------------------------------------------------------------------------- +# A FINGERPRINT THAT COULD NOT BE COMPUTED MUST NOT LOOK LIKE ONE THAT WAS. +# +# `$(md5sum < "$f" 2>/dev/null | cut -d' ' -f1)` substitutes an EMPTY digest +# when md5sum fails, so one transient failure -- a fork that hits EAGAIN, an +# OOM kill, a loaded runner -- silently changes the whole hash and the function +# still returns 0. Modelled with a stub md5sum that fails on its Nth call and is +# otherwise the real one, three different confident answers over ONE unchanged +# tree: +# +# baseline c8e6b23db1c9 +# one digest empty (call 2) 22897add806e +# one digest empty (call 3) 58c76fdab962 +# exit status 0 +# +# On #902's PG18 leg a suite reported FATAL "the binary under test was not built +# from this source" against a tree that was correct, and the stamp it disagreed +# with equalled a clean local fingerprint of the same head. Whatever moved, the +# WRITE was right and one READ was not. +# +# The requirement is not that the computation cannot fail. It is that a failure +# is reported as one: EMPTY, which pgc_freshness_verdict already turns into +# `unknown` and the controller already prints as "freshness UNVERIFIED" and +# 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. + +_fp="$(mktemp -d "${TMPDIR:-/tmp}/pgc-fpfail.XXXXXX")" +mkdir -p "$_fp/tree/src" "$_fp/bin" +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" + +# 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 + +# --------------------------------------------------------------------------- +# ONE TREE HASHES ONE WAY, HOWEVER THE PATH TO IT IS SPELLED. +# +# @OffgridwithJD's finding, reproduced and widened here. `${f#"$dir"/}` strips a +# prefix that must match CHARACTER FOR CHARACTER, so `$dir` with a trailing +# slash, or reached through a symlink, puts the FULL ABSOLUTE PATH into the +# digest instead of the tree-relative one: +# +# plain 92410d0598d6 +# trailing slash bf101efc7c10 differs +# dot segment /./ 774152fff929 differs +# via symlink 3f3c0e36905a differs +# dot-dot /src/.. 92410d0598d6 +# relative . 92410d0598d6 +# +# The `/./` case is the one to keep: it is what a `$(dirname X)/./` composition +# produces and it reads as harmless. Two spellings of one tree must not be able +# to disagree, because the writer and the reader reach the tree by different +# routes and a disagreement there is a FATAL about nothing. + +_sp="$(mktemp -d "${TMPDIR:-/tmp}/pgc-spell.XXXXXX")" +mkdir -p "$_sp/tree/src" "$_sp/tree/objstore" +printf 'int a;\n' > "$_sp/tree/src/a.c" +printf 'int b;\n' > "$_sp/tree/objstore/b.c" +printf 'all:\n\ttrue\n' > "$_sp/tree/objstore/Makefile" +printf 'all:\n\ttrue\n' > "$_sp/tree/Makefile" +printf 'x\n' > "$_sp/tree/pgcolumnar.control" +ln -s "$_sp/tree" "$_sp/link" + +_sp_plain="$(pgc_source_fingerprint "$_sp/tree")" +check "premise: the spelling fixture fingerprints at all" \ + "$([ -n "$_sp_plain" ] && echo yes || echo empty)" "yes" + +check "a trailing slash hashes the same tree the same way" \ + "$(pgc_source_fingerprint "$_sp/tree/")" "$_sp_plain" +check "a /./ segment hashes the same tree the same way" \ + "$(pgc_source_fingerprint "$_sp/tree/./")" "$_sp_plain" +check "a /src/.. segment hashes the same tree the same way" \ + "$(pgc_source_fingerprint "$_sp/tree/src/..")" "$_sp_plain" +check "a symlink to the tree hashes it the same way" \ + "$(pgc_source_fingerprint "$_sp/link")" "$_sp_plain" +check "a relative path hashes the same tree the same way" \ + "$(cd "$_sp/tree" && pgc_source_fingerprint .)" "$_sp_plain" + +unset _fp _fp_base _fp_got _fp_n _sp _sp_plain + +# --------------------------------------------------------------------------- +# THE FIX MUST NOT RE-BASELINE EVERY STAMP ALREADY ON DISK. +# +# Detecting a failed digest means capturing the per-file lines into a variable to +# inspect them, and `$(...)` STRIPS THE TRAILING NEWLINE that the old code's +# straight pipe into md5sum included. The same unchanged tree then hashes +# differently before and after the change, every stamp 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 cannot catch +# this: it copies a fresh tree and re-stamps every run, so it lands on developers +# and on nobody's CI. +# +# This arm is a COMPATIBILITY assertion, not a tidiness one. It recomputes the +# tree the way the previous implementation did and requires the same answer. + +_bc="$(mktemp -d "${TMPDIR:-/tmp}/pgc-compat.XXXXXX")" +mkdir -p "$_bc/src" "$_bc/objstore" +printf 'int a;\n' > "$_bc/src/a.c" +printf 'int b;\n' > "$_bc/src/b.c" +printf 'void h(void);\n' > "$_bc/src/h.h" +printf 'int m;\n' > "$_bc/objstore/m.c" +printf 'all:\n\ttrue\n' > "$_bc/objstore/Makefile" +printf 'all:\n\ttrue\n' > "$_bc/Makefile" +printf 'x\n' > "$_bc/pgcolumnar.control" +printf 'SELECT 1;\n' > "$_bc/pgcolumnar--9.9.sql" + +# The PREVIOUS implementation, transcribed: the same input set and the same +# per-file `path digest` lines, piped straight into md5sum as it was. +_pgc_fp_previous() { + local dir="${1:-.}" d + { + 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 '' _f; do + printf '%s %s\n' "${_f#"$dir"/}" \ + "$(md5sum < "$_f" 2>/dev/null | cut -d' ' -f1)" + done | md5sum | cut -c1-12 +} + +check "the fixed fingerprint equals what the previous implementation produced" \ + "$(pgc_source_fingerprint "$_bc")" "$(_pgc_fp_previous "$_bc")" + +# THE CONTROL WITHOUT WHICH THE SPELLING ARMS ARE VACUOUS. "Every spelling +# agrees" is satisfied perfectly by a fingerprint that ignores its input, so the +# set needs one arm proving the hash still MOVES on a real change. +_bc_before="$(pgc_source_fingerprint "$_bc")" +printf 'int a = 2;\n' > "$_bc/src/a.c" +check "control: a real content change still moves the fingerprint" \ + "$([ "$(pgc_source_fingerprint "$_bc")" != "$_bc_before" ] && echo moved || echo SAME)" \ + "moved" +printf 'int a;\n' > "$_bc/src/a.c" +check "control: and restoring the content restores the fingerprint" \ + "$(pgc_source_fingerprint "$_bc")" "$_bc_before" + +# A tree with nothing hashable cannot be verified, so it reports no fingerprint +# rather than the hash of an empty stream -- which is a stable, comparable value +# and would have made two empty trees "match". +# THE PREMISE IS LOAD-BEARING. This arm asserts an EMPTY result, and empty is +# also what a harness that cannot run produces -- so without a premise that the +# SAME function returns something for a real tree, it passes green over a broken +# one. Same shape as the expect.refusal defect @OffgridwithJD found on main: a +# failure that produces exactly the value the test expects. +_bc_empty="$(mktemp -d "${TMPDIR:-/tmp}/pgc-empty.XXXXXX")" +check "premise: the same function returns a fingerprint for a real tree" \ + "$([ -n "$(pgc_source_fingerprint "$_bc")" ] && echo yes || echo empty)" "yes" +check "a tree with no hashable file yields no fingerprint" \ + "$([ -z "$(pgc_source_fingerprint "$_bc_empty")" ] && echo empty || echo hashed)" "empty" + +unset _bc _bc_before _bc_empty + +# --------------------------------------------------------------------------- +# WHEN IT REFUSES, IT MUST SAY WHAT IT HASHED. +# +# Two CI failures reported the same pair of hashes and nothing else: +# +# source now a735c673b129, binary built from 6d122a7158d5 +# +# Identical across #902/PG18/iceberg_rest and #909/PG17/iceberg_rest_server -- +# two branches, two majors, two build directories, and one of the two runs +# carried the fingerprint fix. That rules out a transient digest failure (which +# gives a different wrong hash every time) and a path-dependent one (two build +# directories would give two values). It is a deterministic, content-derived +# state, and neither agent has reproduced it locally. +# +# Twelve characters cannot say which file moved. A manifest can, and the +# difference between "the hash changed" and "objstore/x.c appeared" is the +# difference between another sample and an answer. So the manifest is a function +# in its own right, the fingerprint is its hash, and the FATAL path prints it. + +_mf="$(mktemp -d "${TMPDIR:-/tmp}/pgc-manifest.XXXXXX")" +mkdir -p "$_mf/src" "$_mf/objstore" +printf 'int a;\n' > "$_mf/src/a.c" +printf 'void h(void);\n'> "$_mf/src/h.h" +printf 'int m;\n' > "$_mf/objstore/m.c" +printf 'all:\n\ttrue\n' > "$_mf/objstore/Makefile" +printf 'all:\n\ttrue\n' > "$_mf/Makefile" +printf 'x\n' > "$_mf/pgcolumnar.control" + +_mf_lines="$(pgc_source_manifest "$_mf" | wc -l)" +check "the manifest names every file the fingerprint hashes" "$_mf_lines" "6" + +check "each manifest line is a tree-relative path and a digest" \ + "$(pgc_source_manifest "$_mf" | grep -cE '^[A-Za-z0-9_./-]+ [0-9a-f]{32}$')" "6" + +check "the manifest is tree-relative, never absolute" \ + "$(pgc_source_manifest "$_mf" | grep -c '^/')" "0" + +# The fingerprint IS the manifest's hash, so the two cannot drift apart. +check "the fingerprint is the hash of the manifest" \ + "$(pgc_source_fingerprint "$_mf")" \ + "$(pgc_source_manifest "$_mf" | md5sum | cut -c1-12)" + +# THE ARM THAT MATTERS: an ADDED file is named, not merely counted. This is the +# class the two CI failures fall into and the one a bare hash cannot report. +printf 'int zz;\n' > "$_mf/src/zz_appeared.c" +check "an added file appears in the manifest by name" \ + "$(pgc_source_manifest "$_mf" | grep -c '^src/zz_appeared.c ')" "1" +check "and comparing two manifests names it rather than saying 'changed'" \ + "$(diff <(pgc_source_manifest "$_mf" | grep -v zz_appeared) \ + <(pgc_source_manifest "$_mf") | grep -oE 'src/zz_appeared\.c')" \ + "src/zz_appeared.c" +rm -f "$_mf/src/zz_appeared.c" + +# A manifest for a tree that cannot be hashed is empty, matching the fingerprint's +# own refusal rather than inventing a second convention. +_mf_hollow="$(mktemp -d "${TMPDIR:-/tmp}/pgc-hollow.XXXXXX")" +check "an unhashable tree has an empty manifest" \ + "$(pgc_source_manifest "$_mf_hollow" | wc -l)" "0" + +unset _mf _mf_lines _mf_hollow + +# The FATAL path's dump, driven rather than grepped for. A report nobody can run +# is a report nobody knows is empty, and "the source says it calls it" is the kind +# of claim this suite exists to refuse. +_fr="$(mktemp -d "${TMPDIR:-/tmp}/pgc-report.XXXXXX")" +mkdir -p "$_fr/src" +printf 'int a;\n' > "$_fr/src/a.c" +printf 'all:\n\ttrue\n' > "$_fr/Makefile" +printf 'x\n' > "$_fr/pgcolumnar.control" + +check "the report names each hashed file" \ + "$(pgc_freshness_report "$_fr" | grep -c '^ | src/a.c ')" "1" +check "the report states how many files it hashed" \ + "$(pgc_freshness_report "$_fr" | grep -cE '^ \(3 files, under ')" "1" +check "an added file shows up in the report" \ + "$(printf 'int z;\n' > "$_fr/src/z.c"; pgc_freshness_report "$_fr" | grep -c '^ | src/z.c ')" "1" +rm -f "$_fr/src/z.c" +# The empty case says so rather than printing nothing, because a silent empty +# dump reads as "the manifest was fine" -- the failure this report exists to end. +_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