diff --git a/CLAUDE.md b/CLAUDE.md index e0bab38..0acd2ad 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -39,12 +39,17 @@ Never repeat the same comment verbatim in adjacent code — write it once and re ## References to PRs and issues in committed files -Any reference to a GitHub PR or issue inside a **committed file** (SQL/code comments, +A reference to a GitHub PR or issue inside a **committed file** (SQL/code comments, `.github/workflows/ci.yml` comments, `CLAUDE.md`, `test/install/load.sql`, docs) MUST be a full URL, e.g. `https://github.com/Postgres-Extensions/cat_tools/issues/28` — never a bare -`#28` (a bare number is meaningless when the file is read outside GitHub). Referencing by -number is fine only in GitHub-native text (PR/issue titles and descriptions, review -comments). +`#28` (a bare number is meaningless when the file is read outside GitHub), **if it's +something a reader might still need to act on or look up** — an open TODO, a workaround to +revisit once some other issue lands, a known limitation. For a reference that's purely +historical context (explaining why past code looks the way it does, where the issue is +already resolved and there's nothing left to do), a bare `#28` is fine — readers are less +likely to need to chase it down, and it's less noise. When in doubt, use the full URL. +Referencing by number is always fine in GitHub-native text (PR/issue titles and +descriptions, review comments). ## Pull request descriptions diff --git a/bin/test_existing b/bin/test_existing index 32a4f4c..675d485 100755 --- a/bin/test_existing +++ b/bin/test_existing @@ -37,16 +37,18 @@ # # TEST_EXISTING_DEPLOY=pgtle (env var, checked by run_suite): the extension # under test was deployed via pg_tle, not the filesystem, and must NEVER be -# filesystem-installed by anything this script does. `make test` unconditionally -# depends on `install` (pgxntool/base.mk's TEST_DEPS), and `make verify-results` -# in turn depends on `test` -- so calling either would install cat_tools to disk -# as a side effect, even though the database itself was never touched via the -# filesystem. In this mode run_suite calls `testdeps`+`installcheck` directly -# (skips the `install` prerequisite that only `test` pulls in) and invokes -# verify-results-pgtap.sh directly (skips the `test` prerequisite that -# `verify-results` pulls in) -- same underlying work, no pgxntool changes -# needed. This whole workaround could be simplified away if pgxntool grows an -# install-skip override (requested upstream: +# filesystem-installed by anything this script does. pgxntool 2.3.0 fixed +# installcheck running before install (issue #79) by making `installcheck` +# itself unconditionally depend on `install` in base.mk -- so even calling +# `installcheck` directly (bypassing `test`) now runs `install` too, with no +# opt-out. Redirect its filesystem writes to a throwaway DESTDIR instead of +# skipping install entirely: this mode is only ever checked with pg_regress's +# --use-existing against a database that already has the extension via pg_tle, +# so the CREATE-EXTENSION-from-disk side effect that `install` would otherwise +# produce is never actually exercised, and a scratch DESTDIR keeps the real +# extension directory untouched (see run_suite for a related pgtap-specific +# gotcha with this same DESTDIR). This whole workaround could be simplified +# away if pgxntool grows a real install-skip override (requested upstream: # https://github.com/Postgres-Extensions/pgxntool/issues/55) -- revisit this # block if/when that lands. The CALLER is responsible for having already # registered pg_tle + cat_tools against `template1` before creating any @@ -246,12 +248,35 @@ run_suite() { local existing_args="TEST_LOAD_SOURCE=existing CONTRIB_TESTDB=$db EXTRA_REGRESS_OPTS=--use-existing PGXNTOOL_ENABLE_TEST_BUILD=no" if [ "${TEST_EXISTING_DEPLOY:-}" = pgtle ]; then # pg_tle mode (see the TEST_EXISTING_DEPLOY comment at the top of this - # file): `test` and `verify-results` each unconditionally pull in - # `install`, so run the same underlying work directly instead -- - # `testdeps`+`installcheck` (neither depends on `install`), then the - # pgtap-mode verify-results script directly (skips its `test` prerequisite). + # file): run the same underlying work as `test`/`verify-results` directly + # -- `testdeps`+`installcheck`, then the pgtap-mode verify-results script + # directly (skips its `test` prerequisite) -- but with a throwaway DESTDIR + # so installcheck's own (now unconditional, see the comment above) install + # prerequisite writes nowhere near the real extension directory. + # TODO: drop this DESTDIR redirect once pgxntool grows a real install-skip + # override (https://github.com/Postgres-Extensions/pgxntool/issues/55) and + # use that instead. + pgtle_destdir=$(mktemp -d) + # Double-quoted so $pgtle_destdir expands NOW, baking the actual path into the + # trap as a literal -- the trap then no longer references the variable at all, + # so it can't be affected by whatever value (or lack of one) it holds later when + # the trap actually fires at EXIT. + trap "rm -rf '$pgtle_destdir'" EXIT + # base.mk's `pgtap` target checks for $(DESTDIR)$(datadir)/extension/pgtap.control + # but its recipe (`pgxn install pgtap --sudo`) ignores DESTDIR entirely and always + # installs for real. Against our empty scratch DESTDIR that control file can never + # exist, so without this stub Make would consider `pgtap` stale and re-run a real + # `pgxn install pgtap --sudo` on every call (network + sudo, and -- more importantly + # for this workaround's whole point -- a real write to the actual system extension + # directory, since the recipe itself doesn't honor DESTDIR). pgtap is already + # installed for real by an earlier CI step in every caller of this mode, so a stub + # is all `installcheck`'s prerequisite check needs. + local pgtap_destdir + pgtap_destdir="$pgtle_destdir$(make -s print-datadir | sed -n 's/.*set to "\(.*\)"$/\1/p')/extension" + mkdir -p "$pgtap_destdir" + touch "$pgtap_destdir/pgtap.control" make testdeps $existing_args - make installcheck $existing_args + make installcheck $existing_args DESTDIR="$pgtle_destdir" local testout testout=$(make -s print-TESTOUT 2>/dev/null | sed -n 's/.*set to "\(.*\)"$/\1/p') pgxntool/verify-results-pgtap.sh "$testout" diff --git a/pgxntool/CLAUDE.md b/pgxntool/CLAUDE.md index 5db8c95..65e3bf7 100644 --- a/pgxntool/CLAUDE.md +++ b/pgxntool/CLAUDE.md @@ -91,18 +91,24 @@ include pgxntool/base.mk ### Phase 1: Meta Generation (`build_meta.sh`) - Processes `META.in.json` (template with placeholders/empty values) -- Strips out X_comment fields and empty values +- Strips lines with empty string values (this incidentally removes some, but not all, `X_comment` placeholder lines) - Produces clean `META.json` -### Phase 2: Variable Extraction (`meta.mk.sh`) -- Parses `META.json` using `JSON.sh` (a bash-based JSON parser) +### Phase 2: Distribution Variable Extraction (`meta.mk.sh`) +- Parses `META.json` (PGXN distribution metadata) - Generates `meta.mk` with Make variables: - `PGXN` - distribution name - `PGXNVERSION` - version number +- `base.mk` includes `meta.mk` via `-include` + +### Phase 3: Extension Variable Extraction (`control.mk.sh`) +- Parses each `.control` file (what PostgreSQL actually uses), not `META.json` +- Generates `control.mk` with Make variables and rules: - `EXTENSIONS` - list of extensions provided - - `EXTENSION_*_VERSION` - per-extension versions + - `EXTENSION_*_VERSION` - per-extension versions, from each `.control` file's `default_version` - `EXTENSION__CURRENT_VERSION__FILES` - the current/most-recent auto-generated versioned SQL file for each extension (not all version files) -- `base.mk` includes `meta.mk` via `-include` + - Rules for generating versioned SQL files +- `base.mk` includes `control.mk` via `-include` ### The Magic of base.mk @@ -157,10 +163,11 @@ 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: -- Clean builds before testing -- Proper test isolation -- Correct test dependency installation -- Proper cleanup and result comparison +- Correct test dependency installation (`testdeps`, and `test-build` when enabled) +- Extension is installed before tests run (`install`) +- Test comparison via `installcheck`, with diffs shown on failure + +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. @@ -204,7 +211,7 @@ When tests fail, examine the diff output carefully. The actual test output in `t ### Distribution Packaging - `make dist` creates `../PGXN-VERSION.zip` -- Always creates git tag matching version +- Creates a git tag matching version, unless that tag already exists and points at HEAD (in which case tagging is skipped) - Uses `git archive` to package - Validates repo is clean before tagging @@ -219,7 +226,7 @@ When tests fail, examine the diff output carefully. The actual test output in `t pgxntool can generate pg_tle (Trusted Language Extensions) registration SQL for deploying extensions in AWS RDS/Aurora without filesystem access. -**Usage:** `make pgtle` or `make pgtle PGTLE_VERSION=1.5.0+` +**Usage:** `make pgtle` or `make pgtle PGXNTOOL_PGTLE_VERSION=1.5.0+` **Output:** `pg_tle/{version_range}/{extension}.sql` @@ -273,11 +280,11 @@ Generated files depend on: ## Scripts -- **setup.sh** - Initializes pgxntool in a new extension project (copies templates, creates directories) -- **build_meta.sh** - Strips empty fields from META.in.json to create META.json -- **meta.mk.sh** - Parses META.json via JSON.sh and generates meta.mk with Make variables -- **JSON.sh** - Third-party bash JSON parser (MIT licensed) -- **safesed** - Utility for safe sed operations +Each script at the top level of this repository (pgxntool/) begins with a shebang followed by a short +header comment describing its purpose within the first ~10 lines — read that header +rather than relying on a hand-maintained list here, which would just go stale the same +way this section itself once did. Exception: trivial third-party or one-line utilities +may lack such a header. ## Related Repositories diff --git a/pgxntool/HISTORY.asc b/pgxntool/HISTORY.asc index 231809f..bdd5121 100644 --- a/pgxntool/HISTORY.asc +++ b/pgxntool/HISTORY.asc @@ -1,3 +1,30 @@ +2.3.0 +----- +== Rename `PGTLE_VERSION` to `PGXNTOOL_PGTLE_VERSION` +`make pgtle`'s version-limiting variable was named `PGTLE_VERSION`, which +make auto-imports from an identically-named environment variable. That name +is also a natural choice for a CI job's "which pg_tle to test against" env +var -- when one was set, `make pgtle`/`make run-pgtle` silently misbehaved +instead of erroring. Renamed to `PGXNTOOL_PGTLE_VERSION` to avoid the +collision. If you invoke `make pgtle PGTLE_VERSION=...` directly, update it +to `PGXNTOOL_PGTLE_VERSION`. + +== Fix `DATA` wildcard skipping historical full-install version scripts +`base.mk`'s `DATA` wildcard (`sql/*--*--*.sql`) only matched upgrade scripts +(two `--` separators), silently skipping historical full-install scripts +like `sql/ext--0.9.6.sql` (one `--`) even though these are meant to be +committed to git. `make install` never placed them in the extension +directory, so `CREATE EXTENSION ext VERSION 'x.y.z'` failed for any version +older than current. Widened to `sql/*--*.sql`, which matches both. + +== Fix `make test` always exiting 0, even when every test fails +`.IGNORE: installcheck` made `make` treat `installcheck` as always successful +regardless of `pg_regress`'s exit status, and `test`'s recipe only `cat`ed +`regression.diffs` for visibility without checking it. `make test` now exits +non-zero when `regression.diffs` is non-empty, after printing it. + +Issues fixed in this release: #35, #49, #78, #79, #48 + 2.2.0 ----- == Add `check-stale-expected` to catch orphaned test/expected files diff --git a/pgxntool/README.asc b/pgxntool/README.asc index a799c7c..45c4b59 100644 --- a/pgxntool/README.asc +++ b/pgxntool/README.asc @@ -51,6 +51,8 @@ Whether `test-build` runs is controlled by the `PGXNTOOL_ENABLE_TEST_BUILD` vari NOTE: `test` intentionally does *not* depend on `clean` — that caused problems with incremental/watch-based builds. If your tests need a clean build to pass, that's a sign of a missing dependency elsewhere rather than something to fix by adding `clean` back. +NOTE: `test` exits non-zero (after printing `regression.diffs`) if any test fails. Previously it always exited 0 regardless of test results, silently masking failures from CI and other automation that relies on the exit code. + NOTE: While you can still run `make installcheck` or any other valid PGXS make target directly, it's recommended to use `make test` when using pgxntool. The `test` target ensures proper test isolation and correct dependency installation. === test-build @@ -270,7 +272,7 @@ WARNING: If your project has a `.gitattributes` file, `make dist`/`make dist-onl `make forcedist` is a shortcut for `forcetag dist`: it force-recreates the tag (see `forcetag` above) before rebuilding the distribution .zip. === pgxntool-sync -This rule will pull down the latest released version of PGXNtool via `git subtree pull` and then reconcile the files `setup.sh` copied into your project (`.gitignore`, `test/deps.sql`) with a 3-way merge. +This rule will pull down the latest released version of PGXNtool via `git subtree pull` and then reconcile the files `setup.sh` copied into your project (`.gitignore`, `test/deps.sql`) with a 3-way merge (it also verifies the `test/pgxntool` symlink, recreating it if missing). NOTE: Your repository must be clean (no modified files) in order to run this. Running this command will produce a git commit of the merge. @@ -308,10 +310,10 @@ Generates pg_tle (Trusted Language Extensions) registration SQL files for deploy `make pgtle` generates SQL files in `pg_tle/` subdirectories organized by pg_tle version ranges. For version range details, see `pgtle_versions.md`. -Set `PGTLE_VERSION` on the command line to limit generation to the single version range that value falls into, instead of every known range: +Set `PGXNTOOL_PGTLE_VERSION` on the command line to limit generation to the single version range that value falls into, instead of every known range: ---- -make pgtle PGTLE_VERSION=1.5.0 +make pgtle PGXNTOOL_PGTLE_VERSION=1.5.0 ---- === check-pgtle @@ -457,6 +459,21 @@ If you need to support multiple versions of your extension: The version file for the current version (specified in the `.control` file's `default_version`) will be automatically regenerated when you run `make`, but other version files you create manually will be preserved. +=== Gotchas + +==== Generated SQL Files Can Be Missed on a Clean Build + +`DATA` (the PGXS variable controlling what `make install` actually installs) is seeded by pgxntool with `$(wildcard sql/*--*.sql)`, so PGXS installs whatever version-specific and upgrade files exist under `sql/` when `DATA` is evaluated. + +If your extension *generates* its versioned SQL from another source (e.g. a `.sql.in` template expanded by a custom rule) and gitignores the generated `.sql`, a clean `make install` (or `make test`, `make all install` in one invocation) can silently omit files that were only just generated during that same run. This isn't a pattern-matching problem — the glob is correct — it's a GNU Make quirk: `wildcard` caches a directory's contents internally for the life of one `make` invocation, and that cache isn't invalidated just because a recipe created new files earlier in the same run. So even though `DATA` is a recursively-expanded variable (re-evaluated each time it's referenced), re-referencing it later in the same invocation can still see the stale, pre-generation listing. + +Workarounds: + +- Run generation and install as separate `make` invocations (e.g. `make && make install`) so the second invocation's parse-time wildcard scan sees files the first invocation created. +- List the generated files in `DATA` explicitly, keyed off stable names derived from your `.sql.in` sources (known at parse time) rather than relying on `wildcard` to discover the generated `.sql` output. + +Tracked as https://github.com/Postgres-Extensions/pgxntool/issues/44[issue #44] — pgxntool doesn't yet have first-class support for generated versioned SQL; the ideas under discussion there (e.g. a dedicated generated-output directory pgxntool builds before installing) are a bigger design change than a one-line fix. + == Document Handling PGXNtool supports generation and installation of document files. There are several variables and rules that control this behavior. @@ -665,9 +682,9 @@ make dist PGXN_REMOTE=upstream Default: auto-detected, the first of `asciidoctor` or `asciidoc` found on `PATH`. Path to the Asciidoc processor used to build `.html` files from `$(ASCIIDOC_EXTS)` source files. Override if the processor you want isn't first on `PATH`, or isn't on `PATH` at all. See <<_document_handling>>. -=== PGTLE_VERSION +=== PGXNTOOL_PGTLE_VERSION -Default: unset (generates every known pg_tle version range). Set on the command line to limit `make pgtle` to the single version range this value falls into. See <<_pgtle>>. +Default: unset (generates every known pg_tle version range). Set on the command line to limit `make pgtle` to the single version range this value falls into. Not named `PGTLE_VERSION`: make auto-imports same-named environment variables, and that name collided silently with CI jobs that set a `PGTLE_VERSION` env var for an unrelated purpose (which pg_tle to test against). See <<_pgtle>>. === PG_CONFIG diff --git a/pgxntool/README.html b/pgxntool/README.html index 36d3cf5..ba71c2a 100644 --- a/pgxntool/README.html +++ b/pgxntool/README.html @@ -475,6 +475,7 @@

PGXNtool

  • 5.5. Alternative: Ignoring All Version Files
  • 5.6. Distribution Inclusion
  • 5.7. Multiple Versions
  • +
  • 5.8. Gotchas
  • 6. Document Handling @@ -499,7 +500,7 @@

    PGXNtool