Sync pgxntool to 2.4.0

.gitignore

@@ -18,21 +18,16 @@ control.mk
 
 # built targets
 # Note: Version-specific files (sql/*--*.sql) are now tracked in git and should be committed
-# Exception: the `stable` pseudo-version (default_version between releases,
-# see ../ai/RELEASE.md and ../ai/CLAUDE.md's "Version-specific SQL files")
-# is permanently current, not a frozen release -- it would be regenerated
-# and re-diffed on every source edit for zero test-coverage value if
-# tracked. Named explicitly, not `sql/*--stable.sql`: that glob's `*` also
-# matches across the second `--`, so it silently swept up the *update*
-# scripts too (sql/<ext>--<last-release>--stable.sql, which must stay
-# committed -- confirmed the hard way when `git status` didn't show them
-# as untracked after creating them).
-sql/test_factory--stable.sql
-sql/test_factory_pgtap--stable.sql
 
 # Generated by asciidoctor from doc/*.asc; only the .asc source is tracked
 doc/*.html
 
+# Post-tag placeholder version's generated SQL file (see PGXNTOOL_POST_TAG_VERSION
+# in README.asc's "post-tag-version-bump" section). Update both lines if you override that variable.
+# The negation keeps the update script (sql/{ext}--{old}--stable.sql) tracked.
+sql/*--stable.sql
+!sql/*--*--stable.sql
+
 # Test artifacts
 results/
 regression.diffs
@@ -46,6 +41,11 @@ test/build/sql/
 # Created by make when test/install/*.sql files exist.
 test/install/schedule
 
+# test/install/*.out is rewritten by every `make test` run and never
+# meaningfully compared against anything (see base.mk's test/install
+# comments) -- there's no reason to commit it or track changes to it.
+test/install/*.out
+
 # Misc
 tmp/
 .DS_Store

pgxntool/CLAUDE-POSTGRES.md

@@ -0,0 +1,38 @@
+# General PostgreSQL Guidance
+
+Guidance in this file is about **PostgreSQL itself** rather than about pgxntool -- it
+applies just as much in a project that has never heard of pgxntool. `CLAUDE.md`, next to
+this file, covers pgxntool proper.
+
+As with `CLAUDE.md`, an extension project's own CLAUDE.md and instructions take
+precedence over anything here.
+
+The human-facing version of this material is `POSTGRES-NOTES.asc`.
+
+## Never Mention `trusted` Without `superuser = false` and the Security Caveat
+
+Whenever you bring up the `trusted` control-file parameter -- in code, a comment, a
+document, a commit message, a PR, or a chat reply -- you MUST in the same breath also:
+
+1. Offer `superuser = false` as the alternative, and
+2. State that `trusted = true` carries security consequences, pointing at PostgreSQL's
+   [Security Considerations for Extensions](https://www.postgresql.org/docs/current/extend-extensions.html#EXTEND-EXTENSIONS-SECURITY)
+   rather than reassuring the reader yourself.
+
+The rule is one-directional: mentioning `superuser = false` on its own is fine. It is
+specifically `trusted` that must never appear unaccompanied.
+
+**Why**: `trusted = true` is the answer that comes to mind fastest for "how do I let a
+non-superuser install this?", so raising it alone reads as a recommendation. It isn't a
+free switch -- it runs the install script as the bootstrap superuser on behalf of an
+unprivileged caller, which turns any flaw in that script into privilege escalation.
+`superuser = false` solves the same problem while granting nothing: the caller simply
+needs the privileges the script actually uses. When the extension doesn't require
+superuser-only capabilities, that's the better suggestion.
+
+See `POSTGRES-NOTES.asc` for the user-facing explanation of both parameters.
+
+## Metrics and Estimates
+
+Never produce any kind of metrics or estimates unless you have data to back them up. If
+you do have data you MUST reference it.

pgxntool/CLAUDE.md

@@ -12,9 +12,12 @@ sequentially.
 The CI monitor lives in the pgxntool-test checkout: run
 `bash ../pgxntool-test/.claude/skills/ci/scripts/monitor-ci.sh` (the `/ci`
 skill). It monitors both repos and derives the owner from the current repo.
-Pass the exact push SHA when available — `gh run list --branch` has a race
-condition: if two pushes land close together on the same branch, `--branch`
-may pick up the wrong run. `--commit SHA` targets the exact push and avoids it.
+Pass the exact push SHA(s) as positional arguments when available (the
+script takes `[repos] [branch] [sha_pgxntool_test] [sha_pgxntool]`, not a
+`--commit` flag — see the `/ci` skill for exact usage) — `gh run list
+--branch` has a race condition: if two pushes land close together on the
+same branch, `--branch` may pick up the wrong run. An exact SHA targets
+the push directly and avoids this.
 
 ## Scope of This File
 
@@ -30,6 +33,16 @@ development must be done from the **pgxntool-test** repository, not from here. S
 Any agent working in an extension project should always defer to that project's own
 CLAUDE.md and instructions over anything stated here.
 
+Guidance that is about PostgreSQL itself rather than about pgxntool lives in
+`CLAUDE-POSTGRES.md` instead, so that this file stays about pgxntool.
+
+## Read `CLAUDE-POSTGRES.md`
+
+**`CLAUDE-POSTGRES.md` (next to this file) contains rules you must follow.** The `@` line
+below imports it where that is supported; read the file directly if it did not.
+
+@CLAUDE-POSTGRES.md
+
 ## Git Commit Guidelines
 
 **IMPORTANT**: When creating commit messages, do not attribute commits to yourself (Claude). Commit messages should reflect the work being done without AI attribution in the message body. The standard Co-Authored-By trailer is acceptable.
@@ -78,7 +91,7 @@ https://github.com/Postgres-Extensions/pgxntool-test
 Extension projects include pgxntool via git subtree:
 
 ```bash
-git subtree add -P pgxntool --squash git@github.com:decibel/pgxntool.git release
+git subtree add -P pgxntool --squash git@github.com:Postgres-Extensions/pgxntool.git release
 pgxntool/setup.sh
 ```
 
@@ -138,7 +151,7 @@ project/
 │   ├── deps.sql              # Load extension and test dependencies
 │   ├── sql/*.sql             # Test SQL files
 │   └── expected/*.out        # Expected test outputs
-└── doc/                       # Optional docs (*.adoc, *.asciidoc)
+└── doc/                       # Optional docs (*.adoc, *.asciidoc, *.asc)
 ```
 
 ## Commands for Extension Developers (End Users)
@@ -147,7 +160,7 @@ These are the commands extension developers use (documented for context):
 
 ```bash
 make                    # Build extension (generates versioned SQL, docs)
-make test              # Full test: testdeps → install → installcheck → show diffs
+make test              # Full test: testdeps → [test-build] → [install] → installcheck → safeguard checks → show diffs ([...] = when enabled; see Critical Testing Rules)
 make results           # Run tests and update expected output files
 make html              # Generate HTML from Asciidoc sources
 make tag               # Create git tag for current META.json version
@@ -163,17 +176,18 @@ make pgxntool-sync     # Update to latest pgxntool via git subtree pull
 ### Critical Testing Rules
 
 **NEVER use `make installcheck` directly**. Always use `make test` instead. The `make test` target ensures:
-- Correct test dependency installation (`testdeps`, and `test-build` when enabled)
-- Extension is installed before tests run (`install`)
+- Correct test dependency installation (`testdeps`, and `test-build` when enabled); `testdeps`' default `pgtap` prerequisite auto-installs pgtap via `pgxn install`, a no-op when `PGXNTOOL_ENABLE_PGXN_INSTALL=no`
+- Extension is filesystem-installed before tests run (`install`), unless `PGXNTOOL_ENABLE_FS_INSTALL=no`
 - Test comparison via `installcheck`, with diffs shown on failure
+- Safeguard checks that fail the run: the stale-expected-file check (orphaned or non-`.out` files in `test/expected/`, run after `pg_regress`), and, when `test/install` is in use, the check that every `test/install/*.sql` file sets `ON_ERROR_STOP`
 
 Note: `make test` intentionally does *not* depend on `clean` — depending on `clean` caused problems with incremental/watch-based builds (see `base.mk`). If your tests need a clean build to pass, that's a sign of a missing dependency elsewhere, not something to fix by adding `clean` back.
 
 **Database Connection Requirement**: PostgreSQL must be running before executing `make test`. If you get connection errors (e.g., "could not connect to server"), stop and ask the user to start PostgreSQL.
 
-**Claude Code MUST NEVER run `make results`**. This target updates test expected output files and requires manual human verification of test changes before execution.
+**Claude Code MUST NEVER run `make results` or `make results-build`**. Both update test expected output files and require manual human verification of test changes before execution.
 
-**Claude Code MUST NEVER modify files in `test/expected/`**. These are expected test outputs that define correct behavior and must only be updated through the `make results` workflow.
+**Claude Code MUST NEVER modify files in `test/expected/` or `test/build/expected/`**. These are expected test outputs that define correct behavior and must only be updated through the `make results`/`make results-build` workflows.
 
 The workflow is:
 1. Human runs `make test` and examines diffs
@@ -190,6 +204,11 @@ pgxntool uses PostgreSQL's pg_regress test framework:
 
 When tests fail, examine the diff output carefully. The actual test output in `test/results/` shows what your code produced, while `test/expected/` shows what was expected.
 
+**Exceptions to the above** -- `test-build` and `test/install` (both optional, see `README.asc`) don't follow the `test/results` vs `test/expected` model:
+
+- **test-build** runs first, in its own separate `pg_regress` pass over `test/build/*.sql`, and gates the main suite: if it fails, `test/install`/`test/sql` never run at all. It does compare actual vs expected normally (`test/build/results/` vs `test/build/expected/`) -- use `make results-build` to refresh its expected output, not `make results`.
+- **test/install** does NOT get a real diff at all: its actual output is written to the exact same file as its expected output, so a content difference can never fail the build, no matter what changed. The only thing that still fails the build is a hard SQL error, and only if the file has `ON_ERROR_STOP` set (directly, or via `\i test/pgxntool/psql.sql`) -- pgxntool checks for this by default, passing any file that includes `psql.sql` or has its own `\set`/`\unset ON_ERROR_STOP`, whatever the value. If a `test/install/*.sql` file is misbehaving, don't go looking for a diff; check whether it errored, and don't assume a stale-looking `.out` for it means anything.
+
 ## Key Implementation Details
 
 ### PostgreSQL Version Handling
@@ -205,7 +224,7 @@ When tests fail, examine the diff output carefully. The actual test output in `t
 
 ### Document Generation
 - Auto-detects `asciidoctor` or `asciidoc`
-- Generates HTML from `*.adoc` and `*.asciidoc` in `$(DOC_DIRS)`
+- Generates HTML from `*.adoc`, `*.asciidoc` and `*.asc` in `$(DOC_DIRS)`
 - HTML required for `make dist`, optional for `make install`
 - Template-based rules via `ASCIIDOC_template`
 
@@ -288,5 +307,4 @@ may lack such a header.
 
 ## Related Repositories
 
-- **pgxntool-test** - Test harness for validating pgxntool functionality: https://github.com/Postgres-Extensions/pgxntool-test
-- Never produce any kind of metrics or estimates unless you have data to back them up. If you do have data you MUST reference it.
+- **pgxntool-test** - Test harness for validating pgxntool functionality: https://github.com/Postgres-Extensions/pgxntool-test

pgxntool/HISTORY.asc

@@ -1,3 +1,137 @@
+2.4.0
+-----
+== Breaking changes
+
+=== `test/install` now requires `ON_ERROR_STOP`
+`test/install/*.sql` files never get a real diff -- their actual output is
+written to the same file as their expected output, so a content difference
+can never fail the build. Without `ON_ERROR_STOP`, a hard SQL error was
+silently swallowed too, making the file "pass" regardless of what happened.
+`make test` now fails if a `test/install/*.sql` file doesn't set
+`ON_ERROR_STOP`: it must either include `test/pgxntool/psql.sql` (which
+sets it) or have its own `\set ON_ERROR_STOP` or `\unset ON_ERROR_STOP`
+command, to any value; `\include` and `\include_relative` count as including `psql.sql`, the same as
+`\i` and `\ir`; see
+`PGXNTOOL_ENABLE_TEST_INSTALL_ERROR_STOP_CHECK` to disable. Since
+`test/install/*.out` was never really compared against anything, and is
+rewritten by every run, it's now gitignored -- stop committing it.
+
+=== `test-build` now actually runs before the main suite
+`make test` was meant to run `test-build` first and stop if it failed, but
+with `check-stale-expected` enabled (the default) the main suite ran first.
+`test-build` now always runs before `installcheck`, including a direct
+`make installcheck`, so a broken build -- or a stale
+`test/build/expected/*.out` -- stops the run before the main suite starts.
+Use `make results-build` to refresh stale test-build output.
+
+=== `PGXNTOOL_PGTLE_VERSION` must be an exact pg_tle range name
+`make pgtle PGXNTOOL_PGTLE_VERSION=...` (and `pgtle.sh --pgtle-version`) now
+errors unless the value is exactly `1.0.0-1.4.0`, `1.4.0-1.5.0` or `1.5.0+`.
+A plain pg_tle version such as `1.5.0` used to be accepted silently,
+generating a bogus `pg_tle/1.5.0/` directory; the error now names the range
+it belongs to (`did you mean 1.5.0+?`).
+
+=== Reject invalid `PGXNTOOL_VERIFY_RESULTS_MODE` values
+Any value other than exactly `pgtap` silently selected `diffs` mode, so a
+typo like `pgTap` quietly weakened the `make results` safeguard. The value
+is now case-insensitive, and anything other than `pgtap` or `diffs` is a
+hard `make` error.
+
+=== `PGXNTOOL_ENABLE_TEST_INSTALL=yes` errors when `test/install/` is empty
+Explicitly setting `PGXNTOOL_ENABLE_TEST_INSTALL=yes` now fails when
+`test/install/` has no SQL files, as documented and matching
+`PGXNTOOL_ENABLE_TEST_BUILD`; it used to silently run nothing.
+
+=== Rename internal-only variables to the `_PGXNTOOL_` shape
+Several variables that are pure internal plumbing -- never designed as user
+override points -- were named with a bare `PGXNTOOL_` prefix, which reads
+like a supported override point by name alone. Renamed to signal "internal,
+not part of the user-facing surface", matching the existing
+`_CHECK_STALE_EXPECTED_SCRIPT` precedent (itself renamed here too, for the
+same `_PGXNTOOL_` consistency): `PGXNTOOL_BASE_MK_INCLUDED` ->
+`_PGXNTOOL_BASE_MK_INCLUDED`, `PGXNTOOL_CONTROL_FILES` ->
+`_PGXNTOOL_CONTROL_FILES`, `PGXNTOOL_EXTENSIONS` -> `_PGXNTOOL_EXTENSIONS`,
+`PGXNTOOL_INSTALL_SCHEDULE` -> `_PGXNTOOL_INSTALL_SCHEDULE`, and
+`_CHECK_STALE_EXPECTED_SCRIPT` -> `_PGXNTOOL_CHECK_STALE_EXPECTED_SCRIPT`.
+None of these are documented anywhere as override points, but if you happened
+to reference one directly (unsupported, but possible), update to the new
+name. The internal `check-stale-expected` target is likewise renamed to
+`_check-stale-expected`; `make test` runs it as before.
+
+== Behavior changes
+
+=== Guarantee `test/install/*.sql` ordering
+Install files now always run in byte-value (ASCII) filename order,
+regardless of filesystem or locale. If one install file depends on another,
+encode that in the filenames (e.g. a numeric prefix); see README.
+
+== New features
+
+=== Add `make post-tag-version-bump` and `releasing-with-pgxntool.md`
+New target `post-tag-version-bump` sets each extension's `default_version`
+to a placeholder (`stable` by default) after a release, so later `make`
+runs don't overwrite the released versioned SQL file; see README. New
+`releasing-with-pgxntool.md` is a recommended release checklist. New
+`tree-is-clean` target fails on uncommitted or untracked changes; `tag`
+and `post-tag-version-bump` depend on it.
+
+=== Add `PGXNTOOL_ENABLE_FS_INSTALL` and `PGXNTOOL_ENABLE_PGXN_INSTALL`
+`test`/`verify-results` unconditionally filesystem-installed the extension
+(via PGXS's `install`), and `installcheck` unconditionally auto-installed
+the pgtap dependency via `pgxn install pgtap --sudo` when it wasn't already
+present -- with no way to disable either. That defeated "existing mode"
+testing, where the extension (and/or pgtap) was deployed some other way,
+e.g. registered via pg_tle instead of the filesystem: calling `test` or
+`verify-results` would silently filesystem-install as a side effect,
+undermining the point of proving the other deployment path works.
+
+`PGXNTOOL_ENABLE_FS_INSTALL` (default `yes`) now gates `install` as a
+prerequisite of `test`/`verify-results`, and whether `installcheck` depends
+on `install`. `PGXNTOOL_ENABLE_PGXN_INSTALL` (default: follows
+`PGXNTOOL_ENABLE_FS_INSTALL`, but can be set independently) gates the pgtap
+auto-install. Set either to `no` to disable.
+
+=== Add `print-pgtle` target for combined multi-extension pg_tle files
+`make print-pgtle` prints the generated pg_tle registration SQL for each
+extension to stdout, selecting the version directory from the new
+`PGXNTOOL_PGTLE_TARGET_VERSION` variable (an actual pg_tle version, e.g.
+`1.5.2`) or, if unset, the installed pg_tle version. This lets a consumer
+build a combined multi-extension install file (e.g.
+`$(MAKE) -C ../deps/cat_tools print-pgtle >> pgtle-all.sql`) without needing
+to know the extension name or which `pg_tle/` version-range directory
+applies. When invoking it recursively for stdout capture, pass
+`--no-print-directory` -- otherwise GNU Make's own `Entering
+directory`/`Leaving directory` announcements pollute the captured output.
+
+=== Add `make results-build`
+Refreshes `test/build/expected/*.out` from the last `test-build` run's
+actual output, mirroring `make results` for the main suite. Refuses to
+bless any file whose actual output contains an `ERROR:` line, since that
+would defeat the point of test-build; skip such a file and bless it by hand
+if the error is intentional.
+
+=== Warn when two `DOCS` entries install to the same filename
+The new `check-duplicate-docs` target warns about `DOCS` entries that share a
+basename, which would otherwise fail `make install` with `will not overwrite
+just-created`. `all` depends on it, so a plain `make` reports it too.
+
+== Bug fixes
+
+=== Fix `make results` refusing to create a new test's first expected output
+`make verify-results` used to block `make results` anytime
+`regression.diffs` existed, which would be the case when there were no
+`test/expected/` files at all. Switch to actually examining
+`regression.diffs` to determine if tests are broken. Note this is only in
+effect when `PGXNTOOL_VERIFY_RESULTS_MODE=pgtap`.
+
+=== Fix `make test` failing when `test/sql/` doesn't exist
+`installcheck` needs `test/sql/` to exist, but the rule that creates it was
+spelled `test/sql` (no trailing slash), which Make treats as a different
+target. A project without that directory got `No rule to make target
+'test/sql/'`; it now gets created like `test/expected/` and `results/`.
+
+Issues fixed in this release: #20, #21, #55, #87, #90, #97, #108, #111, #115, #119
+
 2.3.0
 -----
 == Rename `PGTLE_VERSION` to `PGXNTOOL_PGTLE_VERSION`