From 6966d3c736fbefc4b44aa1a977f14594d8058908 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Sat, 1 Aug 2026 16:47:25 -0500 Subject: [PATCH 1/2] Add PGXNTOOL_ENABLE_FS_INSTALL and PGXNTOOL_ENABLE_PGXN_INSTALL `test`/`verify-results`/`installcheck`/`test-build` always filesystem-installed the extension via PGXS's `install`, with no way to disable it. That defeated "existing mode" testing, where the extension under test was deployed some other way (e.g. a pg_tle registration, or a real pg_upgrade) -- the whole point is to prove that other deployment path works, not to filesystem-install as a side effect. `PGXNTOOL_ENABLE_FS_INSTALL` (default `yes`) now gates `install` out of all four. `installcheck` also unconditionally auto-installed the pgtap dependency via `pgxn install pgtap --sudo` -- itself a filesystem-install side effect. `PGXNTOOL_ENABLE_PGXN_INSTALL` independently gates that (defaulting to follow `PGXNTOOL_ENABLE_FS_INSTALL`, but overridable on its own). Documented both in README.asc's make-targets and environment-variable reference sections. Fixes #55. Fixes #90. Related changes in pgxntool-test: - Added BATS coverage in `test/standard/make-test.bats`: default/override behavior for both variables, structural proof (`make -p -n`) that `install`/`pgtap` prerequisites are genuinely removed, and end-to-end proof that `make test PGXNTOOL_ENABLE_FS_INSTALL=no` succeeds against an already-installed tree but fails against a genuinely uninstalled one. Co-Authored-By: Claude --- HISTORY.asc | 20 ++++++++++++ README.asc | 10 ++++++ README.html | 49 +++++++++++++++++++++++------ base.mk | 88 ++++++++++++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 156 insertions(+), 11 deletions(-) diff --git a/HISTORY.asc b/HISTORY.asc index bdd5121..7e50038 100644 --- a/HISTORY.asc +++ b/HISTORY.asc @@ -1,3 +1,23 @@ +STABLE +------ +== 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. + +Issues fixed in this release: #55, #90 + 2.3.0 ----- == Rename `PGTLE_VERSION` to `PGXNTOOL_PGTLE_VERSION` diff --git a/README.asc b/README.asc index 45c4b59..3a3e19a 100644 --- a/README.asc +++ b/README.asc @@ -55,6 +55,8 @@ NOTE: `test` exits non-zero (after printing `regression.diffs`) if any test fail 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. +Whether `test`/`installcheck`/`verify-results` filesystem-install the extension first is controlled by `PGXNTOOL_ENABLE_FS_INSTALL`, and whether `installcheck` auto-installs the pgtap dependency via `pgxn install` is controlled by `PGXNTOOL_ENABLE_PGXN_INSTALL` — see <<_pgxntool_enable_fs_install>> and <<_pgxntool_enable_pgxn_install>> for "existing mode"/pg_tle-style testing, where the extension (and/or pgtap) was deployed some other way and a filesystem install would defeat the point of the test. + === test-build Validates that extension SQL files are syntactically correct before running the full test suite. This feature runs SQL files from `test/build/` through `pg_regress`, providing better error messages than `CREATE EXTENSION` failures when there are syntax errors in your extension code. @@ -718,6 +720,14 @@ Default: auto-detected -- `yes` if `test/build/*.sql` files exist, `no` otherwis Default: auto-detected -- `yes` if `test/install/*.sql` files exist, `no` otherwise. Enables or disables the <<_testinstall,test/install>> schedule-based setup feature. Same explicit-override semantics as `PGXNTOOL_ENABLE_TEST_BUILD`. +=== PGXNTOOL_ENABLE_FS_INSTALL * + +Default: `yes`. Controls whether `install` is a prerequisite of `test`/`verify-results`, and whether `installcheck` depends on `install`. Set to `no` for "existing mode" testing, where the extension under test was deployed some other way (e.g. registered via pg_tle instead of the filesystem, or installed by a binary pg_upgrade) -- with this set, `make test`/`make installcheck`/`make verify-results` run against whatever is already installed instead of filesystem-installing first. + +=== PGXNTOOL_ENABLE_PGXN_INSTALL * + +Default: follows `PGXNTOOL_ENABLE_FS_INSTALL` (so it's off automatically whenever filesystem install is off), but can be set independently. Controls whether `installcheck` auto-installs the pgtap dependency via `pgxn install pgtap --sudo` when it isn't already filesystem-installed. Set to `no` to make this a complete no-op, e.g. when pgtap is already provided some other way and you don't want a `sudo pgxn install` invoked as a side effect of testing. + === PGXNTOOL_ENABLE_VERIFY_RESULTS * Default: `yes`. Enables or disables the <<_verify_results_safeguard,verify-results safeguard>> that blocks `make results` when tests are failing. Setting it to empty on the command line (`make PGXNTOOL_ENABLE_VERIFY_RESULTS= results`) also disables it. diff --git a/README.html b/README.html index ba71c2a..db633b5 100644 --- a/README.html +++ b/README.html @@ -507,10 +507,12 @@

PGXNtool

  • 8.7. PGXNTOOL_VERIFY_RESULTS_MODE
  • 8.8. PGXNTOOL_ENABLE_TEST_BUILD *
  • 8.9. PGXNTOOL_ENABLE_TEST_INSTALL *
    • -
  • 8.10. PGXNTOOL_ENABLE_VERIFY_RESULTS *
    • -
  • 8.11. PGXNTOOL_ENABLE_CHECK_STALE_EXPECTED *
    • -
  • 8.12. PGXNTOOL_CHECK_EXPECTED_FILE_TYPES *
    • -
  • 8.13. PGXNTOOL_NO_PGXS_INCLUDE
    • +
  • 8.10. PGXNTOOL_ENABLE_FS_INSTALL *
    • +
  • 8.11. PGXNTOOL_ENABLE_PGXN_INSTALL *
    • +
  • 8.12. PGXNTOOL_ENABLE_VERIFY_RESULTS *
    • +
  • 8.13. PGXNTOOL_ENABLE_CHECK_STALE_EXPECTED *
    • +
  • 8.14. PGXNTOOL_CHECK_EXPECTED_FILE_TYPES *
    • +
  • 8.15. PGXNTOOL_NO_PGXS_INCLUDE
  • 9. Copyright
    • @@ -635,11 +637,26 @@

    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.
    +
    +

    Whether test/installcheck/verify-results filesystem-install the extension first is controlled by PGXNTOOL_ENABLE_FS_INSTALL, and whether installcheck auto-installs the pgtap dependency via pgxn install is controlled by PGXNTOOL_ENABLE_PGXN_INSTALL — see PGXNTOOL_ENABLE_FS_INSTALL * and PGXNTOOL_ENABLE_PGXN_INSTALL * for "existing mode"/pg_tle-style testing, where the extension (and/or pgtap) was deployed some other way and a filesystem install would defeat the point of the test.

    +

    4.3. test-build

    @@ -1071,7 +1088,7 @@

    4.9. 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).

    @@ -1941,13 +1958,25 @@

    -

    8.10. PGXNTOOL_ENABLE_VERIFY_RESULTS *

    +

    8.10. PGXNTOOL_ENABLE_FS_INSTALL *

    +
    +

    Default: yes. Controls whether install is a prerequisite of test/verify-results, and whether installcheck depends on install. Set to no for "existing mode" testing, where the extension under test was deployed some other way (e.g. registered via pg_tle instead of the filesystem, or installed by a binary pg_upgrade) — with this set, make test/make installcheck/make verify-results run against whatever is already installed instead of filesystem-installing first.

    +
    + +
    +

    8.11. PGXNTOOL_ENABLE_PGXN_INSTALL *

    +
    +

    Default: follows PGXNTOOL_ENABLE_FS_INSTALL (so it’s off automatically whenever filesystem install is off), but can be set independently. Controls whether installcheck auto-installs the pgtap dependency via pgxn install pgtap --sudo when it isn’t already filesystem-installed. Set to no to make this a complete no-op, e.g. when pgtap is already provided some other way and you don’t want a sudo pgxn install invoked as a side effect of testing.

    +
    +
    +
    +

    8.12. PGXNTOOL_ENABLE_VERIFY_RESULTS *

    Default: yes. Enables or disables the verify-results safeguard that blocks make results when tests are failing. Setting it to empty on the command line (make PGXNTOOL_ENABLE_VERIFY_RESULTS= results) also disables it.

    -

    8.11. PGXNTOOL_ENABLE_CHECK_STALE_EXPECTED *

    +

    8.13. PGXNTOOL_ENABLE_CHECK_STALE_EXPECTED *

    Default: yes. Enables or disables the check-stale-expected safeguard, which fails make test if test/expected/ (or test/build/expected/) contains a .out file with no corresponding .sql file — catching a stale file left behind after a test was renamed or removed. Set to no to make the check a complete no-op (it’s dropped from TEST_DEPS entirely).

    @@ -1956,13 +1985,13 @@

    -

    8.12. PGXNTOOL_CHECK_EXPECTED_FILE_TYPES *

    +

    8.14. PGXNTOOL_CHECK_EXPECTED_FILE_TYPES *

    Default: yes. Sub-check of check-stale-expected, independent of PGXNTOOL_ENABLE_CHECK_STALE_EXPECTED: fails (with a distinct error message and exit code from the orphaned-.out check) if test/expected/ (or test/build/expected/) contains any file that isn’t *.out. Set to no to disable just this sub-check while leaving the orphaned-.out check active.

    -

    8.13. PGXNTOOL_NO_PGXS_INCLUDE

    +

    8.15. PGXNTOOL_NO_PGXS_INCLUDE

    Default: unset (PGXS is included normally). Skips including PGXS ($(PGXS)) entirely. This is only for advanced scenarios where you need to manage the PGXS include yourself; most projects should never set this.

    @@ -1983,7 +2012,7 @@

    diff --git a/base.mk b/base.mk index 00dab50..fb147c3 100644 --- a/base.mk +++ b/base.mk @@ -121,6 +121,65 @@ else endif endif +# ------------------------------------------------------------------------------ +# install/installcheck: Filesystem-install the extension before testing +# ------------------------------------------------------------------------------ +# Purpose: `test`/`verify-results` normally filesystem-install the extension +# (PGXS's `install`) before running pg_regress against it. That's +# wrong for "existing mode" testing, where the extension under test +# was deployed some other way (e.g. registered via pg_tle instead of +# the filesystem, or installed by a binary pg_upgrade) -- the whole +# point of that kind of test is to prove the other deployment path +# works, and a silent filesystem install as a side effect defeats it. +# +# Variable: PGXNTOOL_ENABLE_FS_INSTALL +# - Can be set manually in Makefile or command line +# - Allowed values: "yes" or "no" (case-insensitive) +# - Default: "yes" (enabled by default for all pgxntool projects) +# - Set to "no" to drop `install` from TEST_DEPS and stop `installcheck` +# from depending on `install`, so `make test`/`make installcheck`/ +# `make verify-results` run against whatever is already installed +# instead of filesystem-installing first +# +# Implementation: See TEST_DEPS assembly and the `installcheck: install` +# edge below (search for "PGXNTOOL_ENABLE_FS_INSTALL" in this file) +# +ifdef PGXNTOOL_ENABLE_FS_INSTALL + override PGXNTOOL_ENABLE_FS_INSTALL := $(call pgxntool_validate_yesno,$(PGXNTOOL_ENABLE_FS_INSTALL),PGXNTOOL_ENABLE_FS_INSTALL) +else + PGXNTOOL_ENABLE_FS_INSTALL = yes +endif + +# ------------------------------------------------------------------------------ +# pgtap: Auto-install the pgtap dependency via `pgxn install --sudo` +# ------------------------------------------------------------------------------ +# Purpose: `installcheck` depends on pgtap being filesystem-installed, and +# auto-installs it via `pgxn install pgtap --sudo` when it isn't +# found already. That's itself a filesystem-install side effect -- +# the same problem PGXNTOOL_ENABLE_FS_INSTALL above solves for the +# extension under test -- so it needs its own way to disable, and +# it makes no sense to leave it on when filesystem install is +# otherwise turned off. +# +# Variable: PGXNTOOL_ENABLE_PGXN_INSTALL +# - Can be set manually in Makefile or command line +# - Allowed values: "yes" or "no" (case-insensitive) +# - Default: follows PGXNTOOL_ENABLE_FS_INSTALL (off automatically +# whenever filesystem install is off), but can be set independently -- +# e.g. to keep PGXNTOOL_ENABLE_FS_INSTALL=yes for your own extension +# while still skipping the pgxn auto-install of pgtap because it's +# already provided some other way +# - Set to "no" to make `pgtap` (and the `installcheck: pgtap` edge) a +# complete no-op: pg_regress runs assuming pgtap is already available +# +# Implementation: See pgtap target definition (search for "pgtap:" in this file) +# +ifdef PGXNTOOL_ENABLE_PGXN_INSTALL + override PGXNTOOL_ENABLE_PGXN_INSTALL := $(call pgxntool_validate_yesno,$(PGXNTOOL_ENABLE_PGXN_INSTALL),PGXNTOOL_ENABLE_PGXN_INSTALL) +else + PGXNTOOL_ENABLE_PGXN_INSTALL = $(PGXNTOOL_ENABLE_FS_INSTALL) +endif + # ------------------------------------------------------------------------------ # test/install: Run setup files before all tests in the same pg_regress session # ------------------------------------------------------------------------------ @@ -323,7 +382,13 @@ installcheck: $(TEST_RESULT_FILES) $(TEST_SQL_FILES) | $(TESTDIR)/sql/ $(TESTDIR # unordered prerequisites, so nothing stops installcheck's own prerequisite # chain from running before install. An explicit edge here, same as # check-stale-expected's, is the only ordering guarantee Make actually gives. +# +# Gated behind PGXNTOOL_ENABLE_FS_INSTALL (see its definition above): when +# disabled, `installcheck` must run against whatever is already installed +# (e.g. via pg_tle) instead of forcing a filesystem install first. +ifeq ($(PGXNTOOL_ENABLE_FS_INSTALL),yes) installcheck: install +endif # # TEST SUPPORT @@ -371,7 +436,13 @@ endif ifeq ($(PGXNTOOL_ENABLE_TEST_BUILD),yes) TEST_DEPS += test-build endif -TEST_DEPS += install installcheck +# install is gated behind PGXNTOOL_ENABLE_FS_INSTALL (see its definition +# above): when disabled, `test`/`verify-results` run against whatever is +# already installed instead of forcing a filesystem install first. +ifeq ($(PGXNTOOL_ENABLE_FS_INSTALL),yes) +TEST_DEPS += install +endif +TEST_DEPS += installcheck test: $(TEST_DEPS) @if [ -r $(TESTOUT)/regression.diffs ]; then cat $(TESTOUT)/regression.diffs; exit 1; fi @@ -500,7 +571,17 @@ ifeq ($(PGXNTOOL_ENABLE_TEST_BUILD),yes) TEST_BUILD_SQL_DIR = $(TESTDIR)/build/sql TEST_BUILD_REGRESS = $(sort $(notdir $(basename $(TEST_BUILD_SQL_FILES)))) .PHONY: test-build +# Gated behind PGXNTOOL_ENABLE_FS_INSTALL (see its definition above): without +# this, test-build would force a real filesystem install even when the rest +# of test/installcheck was told not to, defeating the point of disabling it. +# The prerequisite is added via its own separate rule line (no recipe of its +# own) rather than wrapping the ifeq/endif around the recipe-bearing line +# below -- a recipe must immediately follow its own target line, and an +# intervening `endif` would orphan it when the condition is false. +ifeq ($(PGXNTOOL_ENABLE_FS_INSTALL),yes) test-build: install +endif +test-build: @$(PGXNTOOL_DIR)/run-test-build.sh $(TESTDIR) $(MAKE) -C . REGRESS="$(TEST_BUILD_REGRESS)" REGRESS_OPTS="--inputdir=$(TESTDIR)/build --outputdir=$(TESTDIR)/build" installcheck @if [ -r $(TESTDIR)/build/regression.diffs ]; then \ @@ -679,11 +760,16 @@ endif # the META handling stuff is it's own makefile. # .PHONY: pgtap +# Gated behind PGXNTOOL_ENABLE_PGXN_INSTALL (see its definition above): when +# disabled, pgtap is a no-op and pg_regress runs assuming pgtap is already +# available some other way. +ifeq ($(PGXNTOOL_ENABLE_PGXN_INSTALL),yes) installcheck: pgtap pgtap: $(DESTDIR)$(datadir)/extension/pgtap.control $(DESTDIR)$(datadir)/extension/pgtap.control: pgxn install pgtap --sudo +endif endif # fndef PGXNTOOL_NO_PGXS_INCLUDE From 7d66042844e1c82eb9cd19a7549460903e1a4cd3 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Sat, 1 Aug 2026 17:38:46 -0500 Subject: [PATCH 2/2] Explain why pgtap's .PHONY sits outside its ifeq block testdeps' own `testdeps: pgtap` prerequisite is unconditional, so without an unconditional .PHONY declaration for pgtap, disabling PGXNTOOL_ENABLE_PGXN_INSTALL would leave `pgtap` completely undefined and fail with "No rule to make target 'pgtap'". Document that instead of leaving it looking like an inconsistency with check-stale-expected's own (intentionally different) .PHONY placement. Co-Authored-By: Claude --- base.mk | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/base.mk b/base.mk index fb147c3..b19314b 100644 --- a/base.mk +++ b/base.mk @@ -759,6 +759,15 @@ endif # $(DESTDIR)$(datadir) aren't being expanded. This can probably change after # the META handling stuff is it's own makefile. # +# +# This declaration is deliberately OUTSIDE the ifeq below (unlike e.g. +# check-stale-expected's own .PHONY, which lives inside its ifeq): testdeps' +# own `testdeps: pgtap` prerequisite (see testdeps' definition) is +# unconditional, so pgtap must always resolve to *some* rule. Without this +# unconditional .PHONY, disabling the block below would leave `pgtap` +# completely undefined, and testdeps would fail with "No rule to make +# target 'pgtap'". An empty phony rule (no prerequisites, no recipe) is +# exactly the harmless no-op that's needed in that case. .PHONY: pgtap # Gated behind PGXNTOOL_ENABLE_PGXN_INSTALL (see its definition above): when # disabled, pgtap is a no-op and pg_regress runs assuming pgtap is already