pg-tle-test: extend to the 0.9.6 -> current update path via DESTDIR sandboxing

.github/workflows/ci.yml

@@ -20,7 +20,8 @@
 #                              job here, installing two full PostgreSQL
 #                              majors and running the real pg_upgrade
 #                              binary per leg).
-#   pg-tle-test             -- pg_tle DEPLOYMENT: fresh install registered
+#   pg-tle-test             -- pg_tle DEPLOYMENT: fresh install AND the
+#                              0.9.6 -> current update path, both registered
 #                              through AWS pg_tle's database-backed catalog
 #                              instead of a filesystem .control file.
 #
@@ -364,15 +365,18 @@ jobs:
           bin/compare_fresh_vs_update "" 0.9.6 count_nulls_upgrade_none
           bin/compare_fresh_vs_update Quoted 0.9.6 count_nulls_upgrade_quoted
 
-  # Fresh-install smoke test only, deliberately - NOT extended to the
-  # update path. pgxntool 2.3.0's fix for installcheck's ordering bug
+  # Covers both a fresh install AND the 0.9.6 -> current update path, both
+  # purely via pg_tle. pgxntool 2.3.0's fix for installcheck's ordering bug
   # (Postgres-Extensions/pgxntool#83) made `installcheck` (and so `make
   # test`) unconditionally depend on `install`, which writes a real
   # .control file to disk - defeating the entire point of proving a pg_tle
-  # deployment never touches the filesystem. There's currently no way to
-  # invoke the real pgTAP suite without that happening first; filed as
-  # Postgres-Extensions/pgxntool#90. Revisit extending this job to the
-  # update path once that's resolved.
+  # deployment never touches the filesystem. There's still no upstream fix
+  # for that (Postgres-Extensions/pgxntool#90, open) that would let
+  # bin/test_existing's real pgTAP suite run without it, so the update-path
+  # steps below use TEST_EXISTING_DEPLOY=pgtle (see bin/test_existing),
+  # which instead sandboxes `make test`'s install step behind a scratch
+  # DESTDIR - harmless here since a pg_tle-deployed database never needs
+  # those files.
   pg-tle-test:
     needs: [changes]
     if: needs.changes.outputs.docs_only != 'true'
@@ -494,6 +498,44 @@ jobs:
           fi
       - name: Verify no stray extension control files after the fresh-install smoke test
         run: bin/assert_fs_clean verify ${{ matrix.pg }} /tmp/control_baseline.txt pg_tle.control
+      - name: Install count_nulls at 0.9.6, purely via pg_tle (update-path prep)
+        # A SECOND, separate scratch database, created after the template1
+        # registration above so it inherits both registrations too (same
+        # reasoning as count_nulls_smoke). prepare-old creates the database,
+        # CREATE EXTENSIONs at 0.9.6 (pure SQL - resolves through pg_tle's
+        # catalog, no `make install` call, which would defeat the whole
+        # point), then plants + proves the dependency guard so a stray
+        # CASCADE drop anywhere below can't silently turn the eventual
+        # existing-mode run into a fresh install instead.
+        run: |
+          test ! -e /usr/share/postgresql/${{ matrix.pg }}/extension/count_nulls.control
+          bin/test_existing prepare-old count_nulls_pgtle_update "" 0.9.6
+      - name: Verify no stray extension control files after installing 0.9.6 via pg_tle
+        run: bin/assert_fs_clean verify ${{ matrix.pg }} /tmp/control_baseline.txt pg_tle.control
+      - name: Update 0.9.6 -> current, purely via pg_tle
+        # Pure SQL (ALTER EXTENSION ... UPDATE), no filesystem write either.
+        run: bin/test_existing update count_nulls_pgtle_update
+      - name: Verify no stray extension control files after the pg_tle update
+        run: bin/assert_fs_clean verify ${{ matrix.pg }} /tmp/control_baseline.txt pg_tle.control
+      - name: Run the real pgTAP suite against the pg_tle-updated database (existing mode)
+        # run-suite re-proves the dependency guard, drops it, then runs the
+        # FULL suite via --use-existing against the real pg_tle-deployed +
+        # updated database - the same suite/expected-output as every other
+        # leg (see test/README.md). TEST_EXISTING_DEPLOY=pgtle makes
+        # run-suite sandbox `make test`'s otherwise-unavoidable `install`
+        # step behind a scratch DESTDIR instead of writing to the real
+        # extension directory (see bin/test_existing's TEST_EXISTING_DEPLOY
+        # comment), and makes test/install/load.sql's existing-mode
+        # assertion cross-check pgtle.available_extensions() instead of
+        # pg_available_extensions (which never sees pg_tle registrations -
+        # see the Makefile's TEST_EXISTING_DEPLOY comment).
+        run: TEST_EXISTING_DEPLOY=pgtle bin/test_existing run-suite count_nulls_pgtle_update ""
+      - name: Verify no stray extension control files after the pgTAP suite
+        # THE step that actually proves the DESTDIR sandboxing worked: the
+        # real extension directory must still be clean after `make test`
+        # ran (with its otherwise-unavoidable `install` step) sandboxed
+        # behind a scratch DESTDIR.
+        run: bin/assert_fs_clean verify ${{ matrix.pg }} /tmp/control_baseline.txt pg_tle.control
 
   # A single stable check name for use as a required status check in branch
   # protection rules. Matrix jobs produce check names like

Makefile

@@ -83,6 +83,43 @@ $(error TEST_LOAD_SOURCE must be 'fresh', 'update' or 'existing', got '$(TEST_LO
 endif
 export PGOPTIONS := $(PGOPTIONS) -c count_nulls.test_load_mode=$(TEST_LOAD_SOURCE)
 
+# TEST_EXISTING_DEPLOY: in 'existing' mode (TEST_LOAD_SOURCE=existing), how
+# was the extension actually deployed onto the cluster before this run
+# started? Unlike TEST_LOAD_SOURCE/TEST_SCHEMA above, this does not select or
+# change any install behavior - it only tells test/install/load.sql's
+# existing-mode assertion where to cross-check "what does the cluster
+# consider count_nulls's current version" against the actually-installed
+# extversion:
+#   - filesystem (default): a real .control file is on disk (a real `make
+#     install`, or a pg_upgrade'd cluster carrying one over) - cross-check
+#     against pg_available_extensions.default_version, which reads .control
+#     files directly off disk.
+#   - pgtle: count_nulls was registered purely through pg_tle's
+#     database-backed catalog (see the pg-tle-test CI job), never touching
+#     the filesystem. pg_available_extensions does NOT see pg_tle
+#     registrations at all - it only ever reads .control files off disk - so
+#     its default_version comes back NULL for a pg_tle-only extension even
+#     though a version-less CREATE EXTENSION resolves correctly through
+#     pg_tle. pg_tle ships its own separate, non-integrated analog instead:
+#     pgtle.available_extensions(), a C function whose own doc comment in
+#     pg_tle's tleextension.c says "The system view pg_available_extensions
+#     provides a user interface to this SRF" - i.e. pg_tle's SRF is modeled
+#     on pg_available_extensions, but pg_tle never hooks or populates the
+#     real view itself. In this mode, cross-check against pg_tle's SRF
+#     instead.
+#
+# Deliberately the SAME env var name bin/test_existing already reads (to
+# decide whether to sandbox `make test`'s install step via a scratch
+# DESTDIR) - it's already exported to any `make` invocation bin/test_existing
+# spawns as a child process (and explicitly passed through on run_suite's
+# `make test` command line too), so no extra plumbing is needed to get it
+# here.
+TEST_EXISTING_DEPLOY ?= filesystem
+ifeq ($(filter $(TEST_EXISTING_DEPLOY),filesystem pgtle),)
+$(error TEST_EXISTING_DEPLOY must be 'filesystem' or 'pgtle', got '$(TEST_EXISTING_DEPLOY)')
+endif
+export PGOPTIONS := $(PGOPTIONS) -c count_nulls.test_existing_deploy=$(TEST_EXISTING_DEPLOY)
+
 # Convenience wrapper: `make test-update` == `make test TEST_LOAD_SOURCE=update`.
 # Must recurse (a fresh $(MAKE)) rather than depend on `test`, so the
 # parse-time TEST_LOAD_SOURCE conditional above re-evaluates with update set.

bin/test_existing

@@ -15,9 +15,14 @@
 #   prepare-old (install + plant guard) -> [real pg_upgrade binary, in CI] ->
 #   update (ALTER EXTENSION UPDATE) -> run-suite (assert + run existing-mode)
 #
-# so it lives here once instead of being duplicated as inline YAML. Not
-# CI-only: a developer can run any subcommand locally against a scratch
-# database. Modeled on Postgres-Extensions/cat_tools's bin/test_existing.
+# so it lives here once instead of being duplicated as inline YAML. The
+# pg-tle-test CI job's update-path leg reuses the exact same three
+# subcommands unmodified (a pg_tle-registered database needs no real
+# pg_upgrade or filesystem install between prepare-old and update - both are
+# pure SQL) - see TEST_EXISTING_DEPLOY below for the one piece of plumbing
+# that job needed. Not CI-only: a developer can run any subcommand locally
+# against a scratch database. Modeled on Postgres-Extensions/cat_tools's
+# bin/test_existing.
 # Two differences from that script: count_nulls ships no
 # SELECT-*-over-catalog views, so it has no known pg_upgrade-unsafe old
 # version to bridge past before running pg_upgrade; and its own suite has a
@@ -47,6 +52,37 @@
 # actively PROVE that (see bin/test_existing.sql/assert_guard.sql): if the
 # drop unexpectedly succeeds, this script fails CI rather than silently
 # passing.
+#
+# TEST_EXISTING_DEPLOY (optional environment variable, read by run_suite):
+#   unset/filesystem (default): count_nulls was deployed the normal way (a
+#     real `make install`). run_suite calls `make test` exactly as before.
+#   pgtle: count_nulls was deployed purely through pg_tle's database-backed
+#     catalog (see the pg-tle-test CI job), never `make install`ed onto the
+#     filesystem. pgxntool's `test` target unconditionally depends on
+#     `install` (base.mk: `TEST_DEPS += install installcheck`, and
+#     `installcheck: install` - the pgxntool#83/2.3.0 ordering fix), so
+#     naively calling `make test` here would silently write a real
+#     count_nulls.control to disk - which would then shadow the pg_tle
+#     registration on any FUTURE version-less CREATE EXTENSION in the same
+#     cluster, defeating the entire point of proving a pg_tle-only
+#     deployment, without ever raising an error.
+#
+#     In this mode run_suite instead passes DESTDIR=<scratch dir> to `make
+#     test`: PGXS's own `install` target prefixes every path with
+#     $(DESTDIR), so it still "runs" (satisfying Make's dependency graph)
+#     but writes harmlessly to a throwaway directory instead of the real
+#     extension directory - fine here specifically because the pg_tle-
+#     deployed database under test never needs those files (it's already
+#     live via pg_tle's own catalog). One more prerequisite needs help:
+#     `installcheck: pgtap` has a DESTDIR-aware prerequisite check
+#     ($(DESTDIR)$(datadir)/extension/pgtap.control) but a DESTDIR-blind
+#     recipe (`pgxn install pgtap --sudo`) - against an empty scratch
+#     DESTDIR, Make would consider pgtap "missing" and re-run that recipe
+#     for real (network + sudo), even though pgtap is already genuinely
+#     installed from an earlier CI step. seed_pgtap_stub() pre-seeds a
+#     zero-byte stub at that exact scratch-prefixed path so Make considers
+#     the prerequisite already satisfied and never invokes the real recipe.
+#     See Postgres-Extensions/pgxntool#90 (open, no upstream fix yet).
 set -euo pipefail
 
 # Run from the repository root (where `make` works and test paths resolve),
@@ -149,6 +185,30 @@ create_extension_in_schema() {
   psql_do "$db" -c "${sql}CREATE EXTENSION count_nulls VERSION '$version'"
 }
 
+# ---------------------------------------------------------------------------
+# pg_tle DESTDIR sandbox helpers (see TEST_EXISTING_DEPLOY in the file header)
+# ---------------------------------------------------------------------------
+
+# Seed a stub pgtap.control at the exact DESTDIR-prefixed path pgxntool's
+# `pgtap` target's prerequisite check looks for
+# ($(DESTDIR)$(datadir)/extension/pgtap.control), so Make considers that
+# prerequisite already satisfied and never invokes the real recipe (`pgxn
+# install pgtap --sudo`) against a scratch DESTDIR that can never contain a
+# real install. Existence is all Make checks here - pgtap has no other
+# prerequisites to compare timestamps against - so a zero-byte file is
+# enough.
+seed_pgtap_stub() {
+  local destdir=$1 datadir control_path
+  datadir=$(make -s print-datadir 2>/dev/null | sed -n 's/.*set to "\(.*\)"$/\1/p')
+  if [ -z "$datadir" ]; then
+    echo "FAIL: could not resolve datadir (make -s print-datadir)" >&2
+    exit 1
+  fi
+  control_path="${destdir}${datadir}/extension/pgtap.control"
+  mkdir -p "$(dirname "$control_path")"
+  : > "$control_path"
+}
+
 # ---------------------------------------------------------------------------
 # Subcommand implementations
 # ---------------------------------------------------------------------------
@@ -173,16 +233,44 @@ prepare_old() {
 # then runs the suite via --use-existing so pg_regress does NOT drop/recreate
 # the database.
 run_suite() {
-  local db=$1 schema=$2
+  local db=$1 schema=$2 deploy=${TEST_EXISTING_DEPLOY:-filesystem}
+  case "$deploy" in
+    filesystem|pgtle) ;;
+    *)
+      echo "FAIL: TEST_EXISTING_DEPLOY must be 'filesystem' or 'pgtle', got '$deploy'" >&2
+      exit 1
+      ;;
+  esac
+
   assert_version "$db" current
   assert_drop_blocked "$db"
   drop_guard "$db"
+
+  # See TEST_EXISTING_DEPLOY in the file header. Only 'pgtle' changes
+  # anything here; a scratch DESTDIR is created and baked into a trap so it
+  # is cleaned up regardless of how this function/script exits (a RETURN
+  # trap would NOT fire under `set -e` if `make test` below reports a
+  # regression - EXIT does, for any exit reason).
+  local destdir_opt=()
+  if [ "$deploy" = pgtle ]; then
+    local scratch
+    scratch=$(mktemp -d)
+    trap "rm -rf '$scratch'" EXIT
+    seed_pgtap_stub "$scratch"
+    destdir_opt=(DESTDIR="$scratch")
+  fi
+
   # In existing mode pg_regress runs against $db via --use-existing and must
   # NOT create/drop its own database. `make test` (not just `make
   # verify-results`) is a real gate as of pgxntool 2.3.0 - it now exits
   # non-zero on regression failures instead of always exiting 0 regardless
   # of pg_regress's result (see this repo's pgxntool 2.3.0 bump).
-  make test TEST_LOAD_SOURCE=existing TEST_SCHEMA="$schema" CONTRIB_TESTDB="$db" EXTRA_REGRESS_OPTS=--use-existing
+  # TEST_EXISTING_DEPLOY is passed through explicitly (not left to
+  # environment inheritance) so test/install/load.sql's existing-mode
+  # assertion (via the count_nulls.test_existing_deploy GUC - see the
+  # Makefile) picks the right source regardless of how this function is
+  # invoked.
+  make test TEST_LOAD_SOURCE=existing TEST_SCHEMA="$schema" CONTRIB_TESTDB="$db" EXTRA_REGRESS_OPTS=--use-existing TEST_EXISTING_DEPLOY="$deploy" "${destdir_opt[@]}"
 }
 
 usage() {

test/install/load.sql

@@ -67,20 +67,52 @@ SELECT :'count_nulls_test_load_mode' = 'existing' AS count_nulls_existing_mode
 \if :count_nulls_existing_mode
 /*
  * Already installed by something external to this pg_regress invocation
- * (a real pg_upgrade run - see the pg-upgrade-test CI job). Only assert
- * it's present and at the current version; do NOT drop/create/update it -
- * the whole point of this mode is testing the REAL migrated objects.
+ * (a real pg_upgrade run, or a pg_tle registration - see the
+ * pg-upgrade-test / pg-tle-test CI jobs). Only assert it's present and at
+ * the current version; do NOT drop/create/update it - the whole point of
+ * this mode is testing the REAL migrated/deployed objects.
+ *
+ * The "current version" half of that assertion (v_default below) has two
+ * sources depending on count_nulls.test_existing_deploy (see the
+ * TEST_EXISTING_DEPLOY comment in the Makefile):
+ *   - filesystem (default): pg_available_extensions.default_version, read
+ *     straight from a real .control file on disk.
+ *   - pgtle: count_nulls was registered purely through pg_tle's
+ *     database-backed catalog, never touching the filesystem.
+ *     pg_available_extensions does NOT see pg_tle registrations at all - it
+ *     only ever reads .control files off disk - so it comes back NULL here
+ *     even though CREATE EXTENSION correctly resolves the default version
+ *     through pg_tle. pg_tle ships its own separate, non-integrated analog
+ *     for this: pgtle.available_extensions() (see pg_tle's tleextension.c,
+ *     which documents pg_available_extensions as merely modeled on this
+ *     SRF, not backed by it). Use that instead when running under pg_tle,
+ *     rather than weakening the check for the filesystem case.
  */
 DO $$
 DECLARE
   v_installed text := (SELECT extversion FROM pg_extension WHERE extname = 'count_nulls');
-  v_default   text := (SELECT default_version FROM pg_available_extensions WHERE name = 'count_nulls');
+  v_deploy    text := current_setting('count_nulls.test_existing_deploy');
+  v_default   text;
 BEGIN
   IF v_installed IS NULL THEN
     RAISE EXCEPTION 'count_nulls.test_load_mode=existing but count_nulls is not installed';
   END IF;
+
+  IF v_deploy = 'pgtle' THEN
+    SELECT default_version INTO v_default
+      FROM pgtle.available_extensions() WHERE name = 'count_nulls';
+  ELSIF v_deploy = 'filesystem' THEN
+    SELECT default_version INTO v_default
+      FROM pg_available_extensions WHERE name = 'count_nulls';
+  ELSE
+    RAISE EXCEPTION
+      'count_nulls.test_existing_deploy must be ''filesystem'' or ''pgtle'', got ''%'''
+      , v_deploy
+    ;
+  END IF;
+
   IF v_installed IS DISTINCT FROM v_default THEN
-    RAISE EXCEPTION 'count_nulls installed at % but default_version is %', v_installed, v_default;
+    RAISE EXCEPTION 'count_nulls installed at % but default_version (deploy=%) is %', v_installed, v_deploy, v_default;
   END IF;
 END
 $$;