Extract versioned-SQL generation into sql.mk; install generated upgrade scripts (fixes #28)#32
Open
jnasbyupgrade wants to merge 1 commit into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
2136cb5 to
cb4c207
Compare
cb4c207 to
077c091
Compare
…de scripts (fixes Postgres-Extensions#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 (included after pgxntool/base.mk so it can use EXTENSION_VERSION_FILES, PG_CONFIG, MAJORVER, datadir, ...). sql.mk documents the GNU Make two-phase (parse vs. recipe) hazard at the root of Postgres-Extensions#28: 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 (Postgres-Extensions#28). 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 <noreply@anthropic.com>
077c091 to
f56c8bb
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this is
This PR is a pure build-system refactor: it extracts the versioned-SQL generation machinery out of the
Makefileinto a new, documentedsql.mk, and fixes #28 so generated upgrade scripts land inDATAreliably on a clean build.What moved into
sql.mk(new)Included after
include pgxntool/base.mk,sql.mknow owns:B,versioned_in/versioned_out,upgrade_scripts_outhistorical_installs— the committedcat_tools--0.1.*install list, derived (glob the committed install-shaped.sql, subtract update scripts and everything generated from.sql.in) instead of the old hardcoded listDATAhandling, including the Clean-build 'make test' fails on all PG>=12: generated upgrade scripts excluded from DATA (parse-time wildcard) #28 fix (DATA += $(upgrade_scripts_out)thenDATA := $(sort $(DATA)))all/installcheck/EXTRA_CLEANLT95/LT93,_apply_version_seds), the$B/%.sqlpattern rule, and theEXTENSION_VERSION_FILESrulesThe
Makefilenow justinclude sql.mk(with a pointer comment) and keeps its non-SQL-gen content (the upgrade-testTEST_BUILD_DIRblock,old_version/clean_old_version).Why #28 happens (documented in sql.mk's header)
GNU Make is two-phase. We generate the
.sqlwe install; the generated files are gitignored and absent on a clean tree when the makefile is parsed. Any parse-time$(wildcard)over them — including base.mk'sDATAseed$(wildcard sql/*--*--*.sql)— silently comes up short, and Make's cached directory listing means it never notices them later. So on a fresh build the generated update scripts never make it intoDATA,make installskips them, and a laterALTER EXTENSION cat_tools UPDATEfails with "no update path". The fix: name-derive the lists from the.sql.insources, feed the generated names intoDATAexplicitly, and$(sort). Rule of thumb: glob what git tracks, name-derive what we generate.Clean-tree #28 proof
On a truly clean tree (
make clean, no generated0.2.*.sqlpresent):0.2.0--0.2.1,0.2.0--0.2.2,0.2.1--0.2.2upgrade scripts appear inDATA.DATA, and a cleanmake installcopies all of them into the extension directory.make print-DATAis byte-identical on a clean tree vs. a fully-built tree (parse-stable), andhistorical_installsresolves to exactly the committedcat_tools--0.1.0/0.1.3/0.1.4/0.1.5.sql.