jrgemignani commented on code in PR #2364:
URL: https://github.com/apache/age/pull/2364#discussion_r3029289912
##########
Makefile:
##########
@@ -157,15 +226,78 @@ src/backend/parser/cypher_parser.bc:
src/backend/parser/cypher_gram.c src/includ
src/backend/parser/cypher_keywords.o: src/backend/parser/cypher_gram.c
src/include/parser/cypher_gram_def.h
src/backend/parser/cypher_keywords.bc: src/backend/parser/cypher_gram.c
src/include/parser/cypher_gram_def.h
-# Strip PASSEDBYVALUE on 32-bit (SIZEOF_DATUM=4) for graphid pass-by-reference
+# Build the default install SQL (age--<VER>.sql) from the INITIAL version-bump
+# commit in git history. This means CREATE EXTENSION age installs the "day-one
+# release" SQL — the state of sql/ at the moment the version was bumped in
+# age.control. All other regression tests run against this SQL + the current
+# HEAD .so, implicitly validating that the .so is backward-compatible with the
+# initial release SQL.
+#
+# The current HEAD SQL goes into the synthetic next version (age--<NEXT>.sql)
+# which is only reachable via ALTER EXTENSION UPDATE in the upgrade test.
+ifneq ($(AGE_VER_COMMIT),)
+$(age_sql): age.control
+ @echo "Building initial-release install SQL: $@ from commit
$(AGE_VER_COMMIT)"
+ @for f in $$(git show $(AGE_VER_COMMIT):sql/sql_files 2>/dev/null); do \
+ git show $(AGE_VER_COMMIT):sql/$${f}.sql 2>/dev/null; \
+ done > $@
+ifeq ($(SIZEOF_DATUM),4)
+ @echo "32-bit build: removing PASSEDBYVALUE from graphid type"
+ @sed 's/^ PASSEDBYVALUE,$$/ -- PASSEDBYVALUE removed for 32-bit (see
Makefile)/' $@ > [email protected] && mv [email protected] $@
+endif
+else
+# Fallback: no git history (tarball build) — use current HEAD SQL fragments
$(age_sql): $(SQLS)
@cat $(SQLS) > $@
ifeq ($(SIZEOF_DATUM),4)
@echo "32-bit build: removing PASSEDBYVALUE from graphid type"
@sed 's/^ PASSEDBYVALUE,$$/ -- PASSEDBYVALUE removed for 32-bit (see
Makefile)/' $@ > [email protected] && mv [email protected] $@
@grep -q 'PASSEDBYVALUE removed for 32-bit' $@ || { echo "Error:
PASSEDBYVALUE replacement failed in $@"; exit 1; }
endif
+endif
+
+# Build synthetic "next" version install SQL from current HEAD's sql/sql_files.
+# This represents what the extension SQL looks like in the PR being tested.
+ifneq ($(AGE_HAS_UPGRADE_TEST),)
+$(age_next_sql): $(SQLS)
+ @echo "Building synthetic next version install SQL: $@
($(AGE_NEXT_VER))"
+ @cat $(SQLS) > $@
+ifeq ($(SIZEOF_DATUM),4)
+ @sed 's/^ PASSEDBYVALUE,$$/ -- PASSEDBYVALUE removed for 32-bit (see
Makefile)/' $@ > [email protected] && mv [email protected] $@
+endif
+
+# Stamp upgrade template as upgrade from current to synthetic next version
+$(age_upgrade_test_sql): $(AGE_UPGRADE_TEMPLATE)
+ @echo "Stamping upgrade template: $< -> $@"
+ @sed -e "s/1\.X\.0/$(AGE_NEXT_VER)/g" -e "s/y\.y\.y/$(AGE_NEXT_VER)/g"
$< > $@
+endif
src/backend/parser/ag_scanner.c: FLEX_NO_BACKUP=yes
+# --- Upgrade test file lifecycle during installcheck ---
+#
+# Problem: The upgrade test needs age--<NEXT>.sql and age--<CURR>--<NEXT>.sql
+# in the PG extension directory for ALTER EXTENSION UPDATE to find them, but
+# we must not leave them installed permanently (they would confuse users).
+#
+# Solution: A Make prerequisite installs them before pg_regress runs, and the
+# test SQL removes them at the end via \! (shell escape in psql). A generated
+# cleanup script (regress/age_upgrade_cleanup.sh) contains the exact absolute
+# paths so the removal works regardless of the working directory. EXTRA_CLEAN
+# also removes them on "make clean" as a safety net.
+#
+# This adds a prerequisite to "installcheck" but does NOT override the PGXS
+# recipe, so there are no Makefile warnings.
+SHAREDIR = $(shell $(PG_CONFIG) --sharedir)
+
installcheck: export LC_COLLATE=C
+ifneq ($(AGE_HAS_UPGRADE_TEST),)
+.PHONY: _install_upgrade_test_files
+_install_upgrade_test_files: $(age_next_sql) $(age_upgrade_test_sql) ##
Build, install synthetic files, generate cleanup script
+ @echo "Installing upgrade test files to $(SHAREDIR)/extension/"
+ @$(INSTALL_DATA) $(age_next_sql) $(age_upgrade_test_sql)
'$(SHAREDIR)/extension/'
+ @printf '#!/bin/sh\nrm -f "$(SHAREDIR)/extension/$(age_next_sql)"
"$(SHAREDIR)/extension/$(age_upgrade_test_sql)"\n' >
$(ag_regress_dir)/age_upgrade_cleanup.sh
+ @chmod +x $(ag_regress_dir)/age_upgrade_cleanup.sh
+
Review Comment:
AI response: The synthetic files are already handled by: (1) EXTRA_CLEAN on
make clean, (2) in-SQL cleanup via the \! shell escape at test end, and (3) the
_install_upgrade_test_files prerequisite overwrites stale files on the next
run. Adding a double-colon installcheck:: rule conflicts with PGXS's
single-colon definition. Not worth the complexity for a self-resolving edge
case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
