release: META.json named a script the distribution does not contain - #1086
Conversation
OffgridwithJD
left a comment
There was a problem hiding this comment.
Approving. I verified the finding, the fix and the removal proof independently rather than reading them, and all three hold. Two small notes below, neither blocking.
The finding is real, and the filename half is the part that mattered
Checked against the distribution rather than the tree, which is the right question and the one the PR asks:
$ git archive main | tar -t | grep -cx 'pgcolumnar--1.0-alpha3.sql' 0
$ git archive pr/1086 | tar -t | grep -cx 'pgcolumnar--1.0-alpha4.sql' 1
Main's published metadata pointed at a file PGXN would never receive. The install scripts actually shipped are pgcolumnar--1.0-alpha4.sql plus the four upgrade scripts, so 1.0-alpha3 was not merely a lagging string — it was a dangling reference. Your framing that the two version fields were "the visible symptom" is right, and I'd have made the same misread if I'd only diffed them against VERSION.
Removal proof reproduced, both harnesses
Restored META.json byte-identical to main's (md5 5a865604…, confirmed equal to git show main:META.json) in a worktree at your head, ran both sides, then restored and confirmed byte-identical.
Shell, docs_style.sh — baseline all six arms green, suite PASSED:
FAIL META.json version is the version VERSION holds, in PGXN form: got [1.0.0-alpha.3] want [1.0.0-alpha.4]
FAIL META.json provides.pgcolumnar.version is the version VERSION holds, in PGXN form: got [1.0.0-alpha.3] want [1.0.0-alpha.4]
FAIL the script META.json names is in the published distribution: got [0] want [1]
PASS premise: META.json is present and parses
PASS premise: the PGXN form was derived from VERSION, not empty
PASS premise: META.json names a base install script
pytest, test_pgxn_metadata.py — 4 passed at baseline, then:
FAILED test_meta_json_states_the_version_the_VERSION_file_holds
FAILED test_the_script_meta_json_names_is_in_the_published_distribution
2 failed, 2 passed
Every premise stayed green on both sides, exactly as you claimed — the file still parses and still names a script, which is the distinction the arms draw. The derivation arm is worth the space: it is the one thing that, if wrong, would make every other arm compare against a wrong expectation while looking green.
I also confirmed the harness boundary holds. test_pgxn_metadata.py shells out only to git archive and tar; it does not touch docs_style.sh, and nothing in docs_style.sh references test/pytest/. Parallel in function, independent in call.
And no gate exposure: registered stays 253, the ledger and budget are untouched, and docs_style has no ledger rows, so no new check can be refused.
Note 1 — a count in the CHANGELOG is off by one on the shell side
The entry says the removal proof reddens "the two substantive arms ... on each side". That is right for pytest and wrong for the shell, where it reddens three, because docs_style.sh splits the version property across two arms with its _mk loop (top-level version and provides.pgcolumnar.version) while pytest asserts both inside one test.
Cosmetic, but a count is a claim, and this one is in the file a future reader will re-derive the proof from.
Note 2 — the fourth arm exists only in pytest
test_every_sql_file_meta_json_could_name_is_shipped is the arm covering what provides.file cannot: one filename can't notice an export-ignore dropping a different install script. docs_style.sh has no counterpart — its five check calls produce the six arms listed above and none of them is that property.
That is not a defect, and I'm not asking you to duplicate it. Worth knowing, though, that the upgrade-script property is gated in CI only, while the release gate is the five-major shell matrix — so the one arm protecting ALTER EXTENSION ... UPDATE for PGXN installs is not among the arms that gate the tag. A sentence in the suite header saying the shell side deliberately stops at the base script would stop the next person concluding it was an oversight.
CI is green 14/14. Nothing here blocks the alpha4 tag.
c8ab574 to
cbe69a4
Compare
|
Both notes taken, pushed at Note 1 — the count was wrong and my own table said soYou are right, and the part worth recording is that the evidence contradicting it I wrote the prose from memory of the shape and the table from the run, and never Fixed, and the entry now says why they differ rather than just stating the right Note 2 — I did not take your advice, I took the finding under itYou said you were not asking me to duplicate the fourth arm. But the reason you
That is not a documentation gap, it is coverage in the wrong harness, and the
The A sentinel rather than an empty expectation on the compared string, since
|
Re-read at
|
`META.json` is the PGXN distribution metadata. It hardcodes the version twice and
names the base install script by filename, and NO suite, Makefile rule or CI step
had ever read it. It went stale for the whole alpha4 cycle:
version 1.0.0-alpha.3 while VERSION said 1.0-alpha4
provides.pgcolumnar.file pgcolumnar--1.0-alpha3.sql
THE FILENAME IS THE HALF THAT MATTERS. `pgcolumnar--1.0-alpha3.sql` does not
exist: this repository opens each cycle by RENAMING the base script to the new
version. So the published metadata pointed at a file PGXN would not receive.
git archive HEAD | tar -t | grep -cx pgcolumnar--1.0-alpha3.sql 0
git archive HEAD | tar -t | grep -cx pgcolumnar--1.0-alpha4.sql 1
Gated in both harnesses now, because nothing reading it is why it went stale.
`git archive` rather than a filesystem test throughout: the question is what the
DISTRIBUTION contains, and an `export-ignore` can drop a file plainly present on
disk.
THE UPGRADE-SCRIPT ARM IS ON BOTH SIDES, not python alone. `provides.file` is ONE
filename and cannot notice an `export-ignore` dropping a DIFFERENT install script,
which breaks `ALTER EXTENSION ... UPDATE` for PGXN installs. The pytest guards run
in CI; the five-major shell matrix is the RELEASE GATE, so an arm protecting the
upgrade path for published installs belongs in the gate that runs before a tag.
Raised in review by @OffgridwithJD.
ITS FIRST SHELL VERSION TRIPPED selftest/080 AND TURNED harness_selftest RED, which
is the rule working on arrival rather than an argument against the move: the arm was
fine in the pytest twin and only became subject to the no-pipe sweep on entering the
shell harness. The line was
printf '%s\n' "$_meta_ship" | grep -qx "$_ms"
a builtin writing a captured string into a reader that exits on its first match,
which is #486 exactly, and `$_meta_ship` is the whole `git archive | tar -t`
listing -- 4999 bytes here, right at the pipe-buffer boundary where the shape works
almost every time and then does not. Replaced with a `case` over newline-sentinelled
text: the anchored match `grep -x` provided, with no process started.
The failure direction was a false RED, a shipped script reported missing, so noisy
rather than dangerous. It was red in the gate the arm had just been moved into,
which is the part that mattered. Caught by @OffgridwithJD at the previous head; I
pushed and did not re-check CI, which is the process miss behind it.
Removal proof for the new arm uses the real mechanism, `export-ignore` on a tracked
upgrade script, and is unchanged by the rewrite:
archive holds 4 of 5 scripts
FAIL every pgcolumnar--*.sql in the tree is in the published distribution:
got [pgcolumnar--1.0-alpha2--1.0-alpha3.sql] want [all shipped]
PASS the script META.json names is in the published distribution
The `provides.file` arm stays green there, which is the gap the new one covers.
The PGXN form is DERIVED from VERSION rather than hardcoded, and the transform has
its own arm because a derivation is a claim too:
1.0-alpha4 -> 1.0.0-alpha.4 1.0-beta1 -> 1.0.0-beta.1
1.0 -> 1.0.0 2.1-rc2 -> 2.1.0-rc.2
Removal proof for the version and filename arms, restoring META.json as it shipped:
docs_style.sh 3 arms red, every premise green
pytest 2 tests red, 9 pass + 2 fail + 0 unrun = 11
THE TWO COUNTS DIFFER AND THAT IS NOT DRIFT. `docs_style.sh` splits the version
property across two arms with its `_mk` loop while the twin asserts both inside one
test. An earlier revision said "two on each side", which the table printed beside it
already contradicted (@OffgridwithJD).
Verified on this head: docs_style.sh PASSED, harness_selftest 967 passed + 0 failed,
selftest/080 green. `guard_tests` 346, `cluster_tests` 410, `docs_style` has 0
ledger rows.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NhwXKAgSmYDUjteWkfajHK
cbe69a4 to
4aba713
Compare
|
Fixed at The line, and why it is the one I reviewed someone else forA builtin writing a captured string into a reader that exits on its first match. Replaced with your form: case $'\n'"$_meta_ship"$'\n' in
*$'\n'"$_ms"$'\n'*) ;;
*) _meta_missing="$_meta_missing $_ms" ;;
esacNewline sentinels on both sides give the anchored match Verified on the new headThe only remaining match for the pipe pattern in the file is inside the comment Removal proof re-run on the rewritten arm, same real mechanism, unchanged: And your framing of it is the part I have recorded
That is the right way round. The move was correct and the cost of it was a defect Approval at |
OffgridwithJD
left a comment
There was a problem hiding this comment.
Approving at 4aba713. This supersedes my approval at c8ab5741, which this repository would otherwise have carried forward across two substantive pushes — the head I first approved did not contain the fourth arm at all.
Verified on my own runs rather than citing yours:
docs_style.sh PASSED — all 7 META arms green, including the new fourth
harness_selftest 967 passed + 0 failed + 0 unrunnable
selftest/080 PASS
The only remaining match for the banned pipe shape is inside the comment that explains it, which 080 excludes by design. The case $'\n'…$'\n' rewrite gives the anchored match grep -qx was providing, with no pipe and no subprocess.
The fourth arm's removal proof, re-run
PREMISE — the archive must now hold 4 of 5: it holds 4
PASS the script META.json names is in the published distribution
FAIL every pgcolumnar--*.sql in the tree is in the published distribution:
got [pgcolumnar--1.0-alpha2--1.0-alpha3.sql] want [all shipped]
provides.file staying green beside the failure is the gap, demonstrated rather than argued. Restored to 4aba713 afterwards, tree clean, 5 of 5 again.
Worth recording that my first attempt at this proof was vacuous: I appended the export-ignore line to the working-tree .gitattributes and got 5 of 5 with both arms passing. git archive HEAD resolves attributes from the commit, not the working tree, so nothing was dropped — and two PASSes there read exactly like "the arm does not fire." Committing the probe is what made it decisive. Anyone re-deriving this proof later needs that detail, which is why it is here and not only in my notes.
CI green 14/14 at this head.
Nothing outstanding from either of my earlier reviews.
CHANGELOG.md only, from #1086 landing. Both sides add to `### Fixed`. conflict markers left 0 each entry present exactly once 4 checked by name bodiless headings in [Unreleased] 0 The four docs and test files are untouched by the merge: each one's patch md5 against main is unchanged, and hilbert_cluster.sh's sorted check-name list still hashes 72eb1c4e4f42. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NhwXKAgSmYDUjteWkfajHK
Found checking alpha4 release readiness.
META.jsonis the PGXN distributionmetadata, and no suite, Makefile rule or CI step had ever read it. It went
stale for the whole alpha4 cycle:
The filename is the half that matters
pgcolumnar--1.0-alpha3.sqldoes not exist. This repository opens each cycle byrenaming the base script to the new version — that is how
pgcolumnar--1.0-alpha4.sqlcame to be. So the published metadata pointed at a filePGXN would not receive.
The two version strings were the visible symptom. On their own they read as a
cosmetic lag; the filename is a broken distribution.
Verified against what is published, not the working tree:
Gated in both harnesses now
Nothing reading it is why it went stale, so the fix without a guard would be half
a fix.
docs_style.shalready carried a section for this exact class — documentsthat hardcode a version beside a citation of the file holding it — and
META.jsonis that shape with a filename added.
test/pytest/test_pgxn_metadata.pyasserts thesame properties through its own parse and its own
git archive; neither names theother.
git archiverather than a filesystem test, because the question is what thedistribution contains. An
export-ignorecan drop a file plainly present on disk,which this repository has been bitten by before.
A fourth arm covers what the third cannot see.
provides.fileis one filename,so an
export-ignoredropping a different install script would leave it greenwhile
ALTER EXTENSION ... UPDATEbroke for anyone who installed from PGXN.The version form is derived, not hardcoded
so the next release does not have to edit the arms. The transform has its own arm,
because a derivation is a claim too: were it wrong, every other arm would compare
against a wrong expectation and could pass or fail for reasons unrelated to
META.json.Removal proof, both harnesses
Restore
META.jsonas it shipped:The premises hold because the file still parses and still names a script. It names
the wrong one, which is the distinction the arms exist to draw.
Bookkeeping
guard_testsre-derived by collection 342 → 346.cluster_testsre-derived in thesame run and did not move (410) — checked rather than assumed, since the new
file needs no cluster and
NO_CLUSTERmembership is computed and asserted in bothdirections.
Blocks the alpha4 tag: publishing with this would put broken metadata on PGXN.
🤖 Generated with Claude Code
https://claude.ai/code/session_01NhwXKAgSmYDUjteWkfajHK