From bbb42ecdae9f710b94cf5bd7ff06a0b21d888131 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Wed, 15 Jul 2026 16:43:56 -0500 Subject: [PATCH] Extract versioned-SQL generation into sql.mk; install generated upgrade scripts (fixes #28) The machinery that generates our versioned SQL (the cat_tools--X.Y.Z.sql install scripts and cat_tools--A.B.C--X.Y.Z.sql update scripts) from .sql.in sources, and the DATA list that installs it, is moved out of the Makefile into a new, documented sql.mk. sql.mk now OWNS `include pgxntool/base.mk` (at its top, so it can use EXTENSION_VERSION_FILES, PG_CONFIG, MAJORVER, datadir, ...); the Makefile just does `include sql.mk`. base.mk has no include guard, so it must be included exactly once -- see https://github.com/Postgres-Extensions/pgxntool/issues/50. sql.mk documents the GNU Make two-phase (parse vs. recipe) hazard at the root of the bug: we GENERATE the .sql we install, the generated files are gitignored and absent on a clean tree at parse time, and any parse-time $(wildcard) over them (including base.mk's DATA seed $(wildcard sql/*--*--*.sql)) silently comes up short -- the cached directory listing means it never notices them later either. The fix is to name-derive the install/update lists from the .sql.in SOURCES (which DO exist at parse time), feed the generated names into DATA explicitly, and $(sort) to dedup against base.mk's own wildcard. This makes the generated upgrade scripts land in DATA reliably on a clean build, so `make install` copies them and a later `ALTER EXTENSION cat_tools UPDATE` finds its path. The historical cat_tools--0.1.* install list is no longer hardcoded: it is derived by globbing the committed install-shaped .sql and subtracting the update scripts and everything we generate from .sql.in. Globbing COMMITTED files at parse time is safe; the rule of thumb is glob what git tracks, name-derive what we generate. Co-Authored-By: Claude Opus 4.8 --- Makefile | 98 ++++---------------------------------- sql.mk | 143 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 153 insertions(+), 88 deletions(-) create mode 100644 sql.mk diff --git a/Makefile b/Makefile index 2a551ee..b7774e5 100644 --- a/Makefile +++ b/Makefile @@ -1,92 +1,14 @@ -B = sql - testdeps: $(wildcard test/*.sql test/helpers/*.sql) # Be careful not to include directories in this -include pgxntool/base.mk - -LT95 = $(call test, $(MAJORVER), -lt, 95) -LT93 = $(call test, $(MAJORVER), -lt, 93) - -$B: - @mkdir -p $@ - -versioned_in = $(wildcard sql/*--*.sql.in) -versioned_out = $(subst sql/,$B/,$(subst .sql.in,.sql,$(versioned_in))) -# Upgrade scripts (*--*--*.sql) are already added by base.mk via $(wildcard sql/*--*--*.sql). -upgrade_scripts_out = $(subst sql/,$B/,$(subst .sql.in,.sql,$(wildcard sql/*--*--*.sql.in))) - -# Pre-built historical install scripts (no .sql.in source available) -DATA += sql/cat_tools--0.1.0.sql sql/cat_tools--0.1.3.sql sql/cat_tools--0.1.4.sql sql/cat_tools--0.1.5.sql -# Generated historical install scripts (built from .sql.in source). -# Exclude EXTENSION_VERSION_FILES (managed by control.mk) and upgrade scripts -# ($(upgrade_scripts_out), already handled by base.mk) to avoid duplicates. -DATA += $(filter-out $(EXTENSION_VERSION_FILES) $(upgrade_scripts_out), $(versioned_out)) - -all: $B/cat_tools.sql $(versioned_out) -installcheck: $B/cat_tools.sql $(versioned_out) -EXTRA_CLEAN += $B/cat_tools.sql $(filter-out $(EXTENSION_VERSION_FILES), $(versioned_out)) -# Also clean the generated .sql.in for the current version -EXTRA_CLEAN += $(EXTENSION_VERSION_FILES:.sql=.sql.in) - -# Temporary ugly hack for 9.x — remove these two blocks when 9.x support is dropped. -# $@ is deferred via = and expands to the target name at recipe time. -ifeq ($(LT95),yes) -_sql_sed_95 = pgxntool/safesed $@.tmp -E -e 's/(.*)-- SED: REQUIRES 9\.5!/-- Requires 9.5: \1/' -else -_sql_sed_95 = pgxntool/safesed $@.tmp -E -e 's/(.*)-- SED: PRIOR TO 9\.5!/-- Not used prior to 9.5: \1/' -endif -ifeq ($(LT93),yes) -_sql_sed_93 = pgxntool/safesed $@.tmp -E -e 's/(.*)-- SED: REQUIRES 9\.3!/-- Requires 9.3: \1/' -else -_sql_sed_93 = pgxntool/safesed $@.tmp -E -e 's/(.*)-- SED: PRIOR TO 9\.3!/-- Not used prior to 9.3: \1/' -endif - -# Apply all version-conditional SED markers to $@.tmp. -# 9.x handled by the above variables (temporary hack, to be removed with 9.x support). -# 10+ handled generically via awk: REQUIRES N → commented if MAJORVER < N*10; -# PRIOR TO N → commented if MAJORVER >= N*10. -# IMPORTANT: Use only POSIX awk features here (no gawk extensions like gensub(), -# 3-arg match(), etc.) — awk availability and compatibility across platforms is -# the whole reason this approach was chosen over sed. -define _apply_version_seds - $(_sql_sed_95) - $(_sql_sed_93) - awk -v mv=$(MAJORVER) '\ - /-- SED: REQUIRES [1-9][0-9]+!/ {t=$$0; sub(/.*REQUIRES /,"",t); sub(/!.*/,"",t); if(mv=t*10) $$0="-- Not used prior to "t": "$$0}\ - {print}' $@.tmp > $@.tmp2 && mv $@.tmp2 $@.tmp -endef - -# TODO: refactor the version stuff into a function -# -# This initially creates $@.tmp before moving it into place atomically. That's -# important to make the use of .PRECIOUS safe, which is necessary for -# watch-make not to freak out. -# -# Actually, that doesn't even fix it. TODO: Figure out why this breaks watch-make. -# -# Make sure not to insert blank lines here; everything needs to be part of the cat_tools.sql recipe! -#.PRECIOUS: $B/cat_tools.sql -$B/%.sql: sql/%.sql.in pgxntool/safesed - (echo @generated@ && cat $< && echo @generated@) | sed -e 's#@generated@#-- GENERATED FILE! DO NOT EDIT! See $<#' > $@.tmp - $(_apply_version_seds) - mv $@.tmp $@ - -# Generate the current version's .sql.in by copying the base source. -# This intermediate file is then processed by the pattern rule above to produce -# the final .sql with version-conditional SED substitutions applied. -# (EXTENSION_VERSION_FILES is just sql/cat_tools--.sql) -$(EXTENSION_VERSION_FILES:.sql=.sql.in): sql/cat_tools.sql.in cat_tools.control - cp $< $@ - -# Override the control.mk rule that builds EXTENSION_VERSION_FILES directly from -# cat_tools.sql (bypassing SED processing). Instead, build from the .sql.in above -# so that version-conditional substitutions (-- SED: REQUIRES X!) are applied. -# Note: GNU Make will emit "overriding recipe" for this target — that is expected. -$(EXTENSION_VERSION_FILES): $(EXTENSION_VERSION_FILES:.sql=.sql.in) pgxntool/safesed - (echo @generated@ && cat $< && echo @generated@) | sed -e 's#@generated@#-- GENERATED FILE! DO NOT EDIT! See $<#' > $@.tmp - $(_apply_version_seds) - mv $@.tmp $@ +# Versioned SQL is generated from .sql.in at build time. That generation and the +# DATA list that installs it live in sql.mk, which also owns +# `include pgxntool/base.mk` (base.mk has no include guard, so it must be +# included exactly once; sql.mk includes it at its top). sql.mk documents the +# GNU Make two-phase (parse vs. recipe) hazards involved (e.g. +# https://github.com/Postgres-Extensions/cat_tools/issues/28) and relies on +# base.mk/control.mk/PGXS vars (EXTENSION_VERSION_FILES, PG_CONFIG, MAJORVER, +# datadir, ...). +include sql.mk # Support for upgrade test # @@ -109,7 +31,7 @@ set-test-upgrade: $(TEST_BUILD_DIR) echo 'TEST_LOAD_SOURCE = upgrade' > $(TEST_BUILD_DIR)/dep.mk -$(TEST_BUILD_DIR)/active.sql: $(TEST_BUILD_DIR)/dep.mk $(TEST_BUILD_DIR)/$(TEST_LOAD_SOURCE).sql +$(TEST_BUILD_DIR)/active.sql: $(TEST_BUILD_DIR)/dep.mk $(TEST_BUILD_DIR)/$(TEST_LOAD_SOURCE).sql ln -sf $(TEST_LOAD_SOURCE).sql $@ $(TEST_BUILD_DIR)/upgrade.sql: test/load_upgrade.sql $(TEST_BUILD_DIR) old_version diff --git a/sql.mk b/sql.mk new file mode 100644 index 0000000..e2f673d --- /dev/null +++ b/sql.mk @@ -0,0 +1,143 @@ +# +# sql.mk -- versioned SQL generation and everything that depends on it. +# +# Owns `include pgxntool/base.mk` (which pulls in control.mk and, at its end, +# PGXS), included at the top of this file before the DATA/rules below. The +# Makefile includes THIS file (once) and does NOT include base.mk itself: +# base.mk has no include guard, so a second include causes "overriding recipe" +# errors (upstream: https://github.com/Postgres-Extensions/pgxntool/issues/50). +# The code below uses vars from those includes: EXTENSION_VERSION_FILES +# (control.mk); PG_CONFIG / MAJORVER / $(call test,...) (base.mk); datadir (PGXS). +# +# ============================================================================ +# GNU Make is TWO-PHASE, and we GENERATE the .sql we install -- beware. +# ============================================================================ +# Phase 1 (parse) expands every $(wildcard ...) and CACHES directory listings +# before any recipe runs. Phase 2 (recipes) then generates our versioned SQL +# (sql/cat_tools--X.Y.Z.sql install scripts and sql/cat_tools--A.B.C--X.Y.Z.sql +# update scripts) from .sql.in sources. Those generated .sql are gitignored (see +# sql/.gitignore) and absent on a clean tree at parse time, so any parse-time +# $(wildcard) over them comes up short -- and the cached listing means Make never +# notices them later either. +# +# https://github.com/Postgres-Extensions/cat_tools/issues/28 is the canonical +# symptom: base.mk seeds DATA with $(wildcard sql/*--*--*.sql), a phase-1 glob +# that misses our generated update scripts on a clean tree, so `make install` +# skips them and a later `ALTER EXTENSION cat_tools UPDATE` fails with "no update +# path". +# +# The rule this file follows: glob what git TRACKS; name-DERIVE what we generate. +# We build the install/update lists from the .sql.in SOURCE names (present at +# parse time), not from the generated .sql, then add those names to DATA and +# $(sort) to dedup against base.mk's own wildcard. Such lists are parse-stable: +# identical on a clean tree and a built tree. +# ============================================================================ + +# See header: base.mk must be included exactly once, before the DATA/rules below. +include pgxntool/base.mk + +LT95 = $(call test, $(MAJORVER), -lt, 95) +LT93 = $(call test, $(MAJORVER), -lt, 93) + +# +# Parse-stable lists derived from the .sql.in SOURCES (see header). versioned_in +# globs the committed .sql.in; _out renames each to the .sql we generate. +# +versioned_in = $(wildcard sql/*--*.sql.in) +versioned_out = $(subst .sql.in,.sql,$(versioned_in)) +# base.mk also seeds the update scripts via $(wildcard sql/*--*--*.sql), but that +# glob misses the generated ones on a clean tree (see header), so add by name. +upgrade_scripts_out = $(subst .sql.in,.sql,$(wildcard sql/*--*--*.sql.in)) + +# +# Historical install scripts committed directly as .sql with no .sql.in (the +# frozen cat_tools--0.1.* set; see sql/.gitignore). Globbing committed files is +# safe. Derive rather than hardcode: take every install-shaped .sql, then drop +# the update scripts and everything we generate from .sql.in. Parse-stable (the +# subtracted names are name-derived), and picks up new historical files auto. +historical_installs = $(filter-out $(versioned_out) $(wildcard sql/*--*--*.sql), $(wildcard sql/*--*.sql)) + +# +# DATA -- the INSTALL manifest: what `make install` copies into the extension +# dir. Installed != committed. The versioned .sql (install and update scripts) +# are BUILT from the committed .sql.in at build time and installed, but are +# gitignored, not tracked -- only the .sql.in are. The historical +# cat_tools--0.1.*.sql are the exception: committed directly (no .sql.in source). +# base.mk only seeds DATA; we append here. +# +DATA += $(historical_installs) +# Generated install scripts, minus the current-version file (EXTENSION_VERSION_ +# FILES, from control.mk) and the update scripts. +DATA += $(filter-out $(EXTENSION_VERSION_FILES) $(upgrade_scripts_out), $(versioned_out)) +# Generated update scripts: base.mk's parse-time wildcard misses these on a clean +# tree (see header), so add by name. +DATA += $(upgrade_scripts_out) +# Dedup against base.mk's wildcard on trees where the generated .sql already +# exist (also gives a stable order). +DATA := $(sort $(DATA)) + +all: sql/cat_tools.sql $(versioned_out) +installcheck: sql/cat_tools.sql $(versioned_out) +EXTRA_CLEAN += sql/cat_tools.sql $(versioned_out) + +# +# Versioned-SQL generation from .sql.in +# +# 9.x version-conditional markers (dotted, e.g. -- SED: REQUIRES/PRIOR TO 9.5!) +# are handled by these two safesed blocks; 10+ (two-digit) markers by the awk in +# _apply_version_seds. The two sets are disjoint -- the awk's [1-9][0-9]+ can't +# match a dotted "9.5" -- and the sources still use the 9.x markers, so both are +# needed. Remove these blocks only once the sources drop all 9.x markers. +# +# FUTURE CLEANUP: once support for the relevant old PG versions is dropped, a +# .sql.in whose only version-conditional content targets those versions can be +# frozen to a raw committed .sql (dropping its .sql.in and thus its share of this +# processing chain). Precedent: the historical cat_tools--0.1.*.sql are already +# committed raw, with no .sql.in source. +# +# $@ is deferred (via =) and expands to the target name at recipe time. +ifeq ($(LT95),yes) +_sql_sed_95 = pgxntool/safesed $@.tmp -E -e 's/(.*)-- SED: REQUIRES 9\.5!/-- Requires 9.5: \1/' +else +_sql_sed_95 = pgxntool/safesed $@.tmp -E -e 's/(.*)-- SED: PRIOR TO 9\.5!/-- Not used prior to 9.5: \1/' +endif +ifeq ($(LT93),yes) +_sql_sed_93 = pgxntool/safesed $@.tmp -E -e 's/(.*)-- SED: REQUIRES 9\.3!/-- Requires 9.3: \1/' +else +_sql_sed_93 = pgxntool/safesed $@.tmp -E -e 's/(.*)-- SED: PRIOR TO 9\.3!/-- Not used prior to 9.3: \1/' +endif + +# Apply all version-conditional SED markers to $@.tmp: 9.x via the safesed vars +# above; 10+ generically via awk (REQUIRES N -> commented if MAJORVER < N*10; +# PRIOR TO N -> commented if MAJORVER >= N*10). POSIX awk only (no gawk +# extensions like gensub() / 3-arg match()) -- portability is why this uses awk. +define _apply_version_seds + $(_sql_sed_95) + $(_sql_sed_93) + awk -v mv=$(MAJORVER) '\ + /-- SED: REQUIRES [1-9][0-9]+!/ {t=$$0; sub(/.*REQUIRES /,"",t); sub(/!.*/,"",t); if(mv=t*10) $$0="-- Not used prior to "t": "$$0}\ + {print}' $@.tmp > $@.tmp2 && mv $@.tmp2 $@.tmp +endef + +# The recipe builds $@.tmp then moves it into place atomically. +# TODO: refactor the version handling into a function. +sql/%.sql: sql/%.sql.in pgxntool/safesed + (echo @generated@ && cat $< && echo @generated@) | sed -e 's#@generated@#-- GENERATED FILE! DO NOT EDIT! See $<#' > $@.tmp + $(_apply_version_seds) + mv $@.tmp $@ + +# Make the current version's .sql.in by copying the base source; the pattern +# rule above then turns it into the final .sql with SED substitutions applied. +# (EXTENSION_VERSION_FILES is just sql/cat_tools--.sql) +$(EXTENSION_VERSION_FILES:.sql=.sql.in): sql/cat_tools.sql.in cat_tools.control + cp $< $@ + +# Override control.mk's rule (which builds EXTENSION_VERSION_FILES straight from +# cat_tools.sql, skipping SED) so we build from the .sql.in above instead, with +# version-conditional substitutions applied. GNU Make's "overriding recipe" +# warning for this target is expected. +$(EXTENSION_VERSION_FILES): $(EXTENSION_VERSION_FILES:.sql=.sql.in) pgxntool/safesed + (echo @generated@ && cat $< && echo @generated@) | sed -e 's#@generated@#-- GENERATED FILE! DO NOT EDIT! See $<#' > $@.tmp + $(_apply_version_seds) + mv $@.tmp $@