From 29e2ae45bfbab02f81029851273bcdd112b2ddfe Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Wed, 7 Oct 2026 16:29:09 -0500 Subject: [PATCH 1/3] Error when PGXNTOOL_ENABLE_TEST_INSTALL=yes but test/install/ is empty Setting `PGXNTOOL_ENABLE_TEST_INSTALL=yes` is documented to error when `test/install/` has no SQL files, the same way `PGXNTOOL_ENABLE_TEST_BUILD=yes` does for `test/build/`. Instead the schedule was generated with no entries and `make test` silently ran no install files. The `test/install/schedule` recipe now fails with `ERROR: no .sql files found in test/install/` before writing the schedule. The check lives in the recipe rather than at parse time, so targets that never run the tests (`clean`, `dist`, ...) keep working with the variable set. Related changes in pgxntool-test: - Test that `make test/install/schedule PGXNTOOL_ENABLE_TEST_INSTALL=yes` fails on an empty `test/install/` and writes no schedule, while `make -n dist` and `make clean` still succeed Co-Authored-By: Claude Opus 5.5 --- HISTORY.asc | 8 ++++++++ base.mk | 10 +++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/HISTORY.asc b/HISTORY.asc index 2e99b76..0638009 100644 --- a/HISTORY.asc +++ b/HISTORY.asc @@ -88,6 +88,14 @@ 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. +== Fix `PGXNTOOL_ENABLE_TEST_INSTALL=yes` not erroring on an empty `test/install/` +Explicitly setting `PGXNTOOL_ENABLE_TEST_INSTALL=yes` is documented to +error when `test/install/` has no SQL files, the same as +`PGXNTOOL_ENABLE_TEST_BUILD=yes` does for `test/build/`. Instead it +generated an empty schedule and ran no install files. `make test` now fails +with `no .sql files found in test/install/`. Targets that don't run the +tests, such as `clean` and `dist`, are unaffected. + Issues fixed in this release: #20, #21, #55, #87, #90, #97, #108, #111, #115, #119 2.3.0 diff --git a/base.mk b/base.mk index 8802012..2b5ff37 100644 --- a/base.mk +++ b/base.mk @@ -403,9 +403,17 @@ EXTRA_CLEAN += $(_PGXNTOOL_INSTALL_SCHEDULE) # Add install schedule; REGRESS stays as-is (regular tests run after schedule) REGRESS_OPTS += --schedule=$(_PGXNTOOL_INSTALL_SCHEDULE) -# Always regenerate schedule file to catch added/removed files +# Always regenerate schedule file to catch added/removed files. +# +# Erroring here, rather than at parse time, means only the test path fails +# when PGXNTOOL_ENABLE_TEST_INSTALL=yes but test/install/ has no SQL files; +# clean, dist, etc. keep working. .PHONY: $(_PGXNTOOL_INSTALL_SCHEDULE) $(_PGXNTOOL_INSTALL_SCHEDULE): + @if [ -z "$(strip $(TEST_INSTALL_SQL_FILES))" ]; then \ + echo "ERROR: no .sql files found in $(TESTDIR)/install/ (PGXNTOOL_ENABLE_TEST_INSTALL=yes)" >&2; \ + exit 1; \ + fi @echo "# Auto-generated - DO NOT EDIT" > $@ @for f in $(notdir $(basename $(TEST_INSTALL_SQL_FILES))); do \ echo "test: ../install/$$f" >> $@; \ From bd5aa7d59f7ea0fd18549bcf66929dad61a4b43d Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Wed, 7 Oct 2026 17:07:01 -0500 Subject: [PATCH 2/3] HISTORY: drop test/install empty-dir entry Co-Authored-By: Claude Sonnet 5.5 --- HISTORY.asc | 8 -------- 1 file changed, 8 deletions(-) diff --git a/HISTORY.asc b/HISTORY.asc index a99985a..d5e9985 100644 --- a/HISTORY.asc +++ b/HISTORY.asc @@ -95,14 +95,6 @@ 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. -== Fix `PGXNTOOL_ENABLE_TEST_INSTALL=yes` not erroring on an empty `test/install/` -Explicitly setting `PGXNTOOL_ENABLE_TEST_INSTALL=yes` is documented to -error when `test/install/` has no SQL files, the same as -`PGXNTOOL_ENABLE_TEST_BUILD=yes` does for `test/build/`. Instead it -generated an empty schedule and ran no install files. `make test` now fails -with `no .sql files found in test/install/`. Targets that don't run the -tests, such as `clean` and `dist`, are unaffected. - Issues fixed in this release: #20, #21, #55, #87, #90, #97, #108, #111, #115, #119 2.3.0 From 2e8f4d082dbf3c51fb28e807eb5092beda8e024e Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Wed, 7 Oct 2026 17:16:36 -0500 Subject: [PATCH 3/3] Error at parse time when PGXNTOOL_ENABLE_TEST_INSTALL=yes with no test/install SQL files Co-Authored-By: Claude Sonnet 5.5 --- base.mk | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/base.mk b/base.mk index 1e5ee4e..ab746dd 100644 --- a/base.mk +++ b/base.mk @@ -248,6 +248,9 @@ ifdef PGXNTOOL_ENABLE_TEST_INSTALL # override needed so command-line values (make VAR=YES) are normalized, not silently ignored. # := needed for immediate evaluation of the function call (avoids infinite recursion with =). override PGXNTOOL_ENABLE_TEST_INSTALL := $(call pgxntool_validate_yesno,$(PGXNTOOL_ENABLE_TEST_INSTALL),PGXNTOOL_ENABLE_TEST_INSTALL) + ifeq ($(PGXNTOOL_ENABLE_TEST_INSTALL)$(strip $(TEST_INSTALL_SQL_FILES)),yes) + $(error no .sql files found in $(TESTDIR)/install/ (PGXNTOOL_ENABLE_TEST_INSTALL=yes)) + endif else # Auto-detect: enable if test/install/ directory has SQL files ifneq ($(strip $(TEST_INSTALL_SQL_FILES)),) @@ -403,17 +406,9 @@ EXTRA_CLEAN += $(_PGXNTOOL_INSTALL_SCHEDULE) # Add install schedule; REGRESS stays as-is (regular tests run after schedule) REGRESS_OPTS += --schedule=$(_PGXNTOOL_INSTALL_SCHEDULE) -# Always regenerate schedule file to catch added/removed files. -# -# Erroring here, rather than at parse time, means only the test path fails -# when PGXNTOOL_ENABLE_TEST_INSTALL=yes but test/install/ has no SQL files; -# clean, dist, etc. keep working. +# Always regenerate schedule file to catch added/removed files .PHONY: $(_PGXNTOOL_INSTALL_SCHEDULE) $(_PGXNTOOL_INSTALL_SCHEDULE): - @if [ -z "$(strip $(TEST_INSTALL_SQL_FILES))" ]; then \ - echo "ERROR: no .sql files found in $(TESTDIR)/install/ (PGXNTOOL_ENABLE_TEST_INSTALL=yes)" >&2; \ - exit 1; \ - fi @echo "# Auto-generated - DO NOT EDIT" > $@ @for f in $(notdir $(basename $(TEST_INSTALL_SQL_FILES))); do \ echo "test: ../install/$$f" >> $@; \