Actually run test/extension_upgrade.sh (#396)

docs/testing.md

@@ -167,6 +167,51 @@ rather than per-PR because of its cost, and not because it is optional. A claim
 in the documentation with only a suite that nobody runs behind it is how coverage
 becomes stale without notice.
 
+## Extension upgrade
+
+`PGC_RUN_UPGRADE=1` also runs `test/extension_upgrade.sh`, once, against the first
+major in the list. It covers a different upgrade from the one above: not a new
+PostgreSQL major, but a new build of pgColumnar on the same one.
+
+Every function the extension installs records the name of a C symbol. When a build
+changes those names, the recorded names stop resolving, and the extension is inert
+until `ALTER EXTENSION pgcolumnar UPDATE` runs. A build cannot see that, and neither can a suite that
+creates the extension from scratch. That is why it needs its own gate.
+
+The suite installs the previous release and creates a columnar table with rows in it.
+It then installs the tree under test over the top and runs the update. Reads, writes,
+table creation and a maintenance call must all still work.
+
+It also asserts that every C function owned by the extension has a link name inside the
+`pgcolumnar` namespace. That catches the next rename as well as the last one.
+
+It builds the old version, so it needs to be told where to find it. Given a git ref
+it builds from a throwaway clone, which needs a checkout with tags:
+
+```sh
+git fetch --tags
+test/extension_upgrade.sh /usr/local/pg18/bin/pg_config v1.0-alpha
+```
+
+The container loop copies the tree without `.git`, so the ref form cannot work there.
+Pass a directory holding the old source instead:
+
+```sh
+test/extension_upgrade.sh /usr/local/pg18/bin/pg_config /root/pgcolumnar-1.0-alpha
+```
+
+Through the runner, give it the same directory by environment:
+
+```sh
+PGC_RUN_UPGRADE=1 PGC_UPGRADE_OLD_SRC=/root/pgcolumnar-1.0-alpha \
+    test/run_all_versions.sh
+```
+
+Without it, in a tree that has no `.git`, the suite reports **SKIP** and the matrix
+records `SKIP`. It does not report a pass, because a gate that goes green having run
+nothing is the defect this suite exists to catch. It does not report a failure either. Being unable to
+obtain an old build is a property of the environment, not of the code under test.
+
 ## make installcheck
 
 The conventional entry point for a PostgreSQL extension:

test/extension_upgrade.sh

@@ -17,15 +17,27 @@
 # second gate beside the matrix, run explicitly.
 #
 # Usage:
-#   test/extension_upgrade.sh [PG_CONFIG] [OLD_REF]
+#   test/extension_upgrade.sh [PG_CONFIG] [OLD_REF_OR_DIR]
 #
-# OLD_REF defaults to the previous release, and may be any ref that still has the old
-# link names. The old build is made in a throwaway clone, so the working tree is never
-# checked out from under the caller.
+# PGC_UPGRADE_OLD_SRC is the same thing by environment, which is how run_all_versions.sh
+# supplies it in an environment with no .git. An explicit second argument wins over it.
+#
+# The second argument is either a git ref or a path to an already-checked-out source
+# tree. A ref defaults to the previous release and is built in a throwaway clone, so the
+# working tree is never checked out from under the caller.
+#
+# The directory form exists because the container dev loop copies the tree WITHOUT .git
+# (see docs/testing.md), so the ref form cannot work there. Point it at a second copy of
+# the old source instead:
+#
+#   test/extension_upgrade.sh /usr/local/pg18a/bin/pg_config /root/pgcolumnar-1.0-alpha
+#
+# Requires a real checkout with tags when the ref form is used. That is a precondition,
+# not a bug, and it is reported as one.
 set -uo pipefail
 
 PG_CONFIG=${1:-pg_config}
-OLD_REF=${2:-v1.0-alpha}
+OLD_SRC=${2:-${PGC_UPGRADE_OLD_SRC:-v1.0-alpha}}
 SRCDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)
 
 # The cluster binds a port, so it must come from the band portlib.sh carves BELOW the
@@ -54,19 +66,64 @@ trap cleanup EXIT
 runpg () { runuser -u postgres -- "$@"; }
 q () { runpg "$BINDIR/psql" -h /tmp -p "$PORT" -d extupg -X -Atc "$1" 2>&1; }
 
-echo "== extension_upgrade: PG$PGMAJ, old ref $OLD_REF"
+echo "== extension_upgrade: PG$PGMAJ, old $OLD_SRC"
 
 # ---- 1. build and install the old extension, from a throwaway clone ------------------
-# A missing ref must fail, not skip. This gate is invoked deliberately, and a skip that
-# exits 0 would let it go inert the moment someone clones without tags. That is the same
-# shape as the bug it exists to catch: everything green, nothing checked.
-if ! git -C "$SRCDIR" rev-parse --verify -q "$OLD_REF^{commit}" >/dev/null 2>&1; then
-	echo "  FAIL  $OLD_REF is not present. Fetch tags, or pass an explicit ref:"
-	echo "        git fetch --tags && test/extension_upgrade.sh $PG_CONFIG <ref>"
-	exit 1
+# Skip and fail are different answers, and the difference is who asked for what.
+#
+#   named an old source that cannot be honoured -> FAIL, the caller asked for a thing
+#   named nothing, and the tree cannot supply one -> SKIP, the environment cannot
+#
+# A silent skip that exits 0 is the bug this suite exists to catch, so the skip is loud,
+# reports SKIP rather than PASS, and exits 2 so the runner can tell the two apart.
+EXPLICIT=0
+{ [ "$#" -ge 2 ] || [ -n "${PGC_UPGRADE_OLD_SRC:-}" ]; } && EXPLICIT=1
+
+if [ "$EXPLICIT" = 0 ] && [ ! -d "$SRCDIR/.git" ]; then
+	echo "  SKIP  $SRCDIR is not a git checkout, so the default ref $OLD_SRC cannot be built."
+	echo "        The container loop copies the tree without .git. Supply the old source:"
+	echo "        PGC_UPGRADE_OLD_SRC=/path/to/old/source, or pass it as the second argument."
+	echo "== extension_upgrade: SKIP"
+	exit 2
+fi
+
+# A value that looks like a path but is not there is a wrong path, not a ref. Falling
+# through to the ref branch would answer a question about directories with an error about
+# git checkouts, which is the kind of message that costs someone an afternoon.
+case "$OLD_SRC" in
+	*/*)
+		if [ ! -d "$OLD_SRC" ]; then
+			echo "  FAIL  $OLD_SRC looks like a path and is not a directory."
+			echo "        Give a checked-out source tree of the previous release."
+			exit 1
+		fi
+		;;
+esac
+
+# Directory form: a path to an already-checked-out old tree needs no git at all.
+if [ -d "$OLD_SRC" ]; then
+	[ -f "$OLD_SRC/Makefile" ] || { echo "  FAIL  $OLD_SRC has no Makefile"; exit 1; }
+	cp -a "$OLD_SRC" "$TMP/old" || { echo "FATAL: could not copy $OLD_SRC"; exit 1; }
+	echo "  old source: directory $OLD_SRC"
+else
+	# Only reachable when an old source was named explicitly, so this is a failure and
+	# not a skip: the caller asked for something this tree cannot provide.
+	if [ ! -d "$SRCDIR/.git" ]; then
+		echo "  FAIL  $SRCDIR is not a git checkout, so the ref $OLD_SRC cannot be built."
+		echo "        The container loop copies the tree without .git. Pass a directory:"
+		echo "        test/extension_upgrade.sh $PG_CONFIG /path/to/old/source"
+		exit 1
+	fi
+	if ! git -C "$SRCDIR" rev-parse --verify -q "$OLD_SRC^{commit}" >/dev/null 2>&1; then
+		echo "  FAIL  $OLD_SRC is not present. Fetch tags, or pass an explicit ref or dir:"
+		echo "        git fetch --tags && test/extension_upgrade.sh $PG_CONFIG <ref>"
+		exit 1
+	fi
+	git clone -q --shared "$SRCDIR" "$TMP/old" || { echo "FATAL: clone failed"; exit 1; }
+	git -C "$TMP/old" checkout -q --detach "$OLD_SRC" \
+		|| { echo "FATAL: checkout $OLD_SRC failed"; exit 1; }
+	echo "  old source: ref $OLD_SRC"
 fi
-git clone -q --shared "$SRCDIR" "$TMP/old" || { echo "FATAL: clone failed"; exit 1; }
-git -C "$TMP/old" checkout -q --detach "$OLD_REF" || { echo "FATAL: checkout $OLD_REF failed"; exit 1; }
 make -C "$TMP/old" PG_CONFIG="$PG_CONFIG" clean >/dev/null 2>&1
 make -C "$TMP/old" PG_CONFIG="$PG_CONFIG" -j"$(nproc)" >"$TMP/build_old.log" 2>&1 \
 	|| { echo "FAIL  old build"; tail -20 "$TMP/build_old.log"; exit 1; }

test/run_all_versions.sh

@@ -528,6 +528,52 @@ if [ "${PGC_RUN_UPGRADE:-0}" = 1 ]; then
 		SUMMARY+=("FAIL   upgrade (no runnable pair)")
 		overall=1
 	fi
+
+	# Extension upgrade, on the same opt-in switch and for the same reason.
+	# test/extension_upgrade.sh had pg_upgrade's exemption from the registration
+	# check without pg_upgrade's invocation, so nothing ran it (#396). A guard
+	# nobody runs is the failure mode #257 existed to close, and it matters more
+	# here: the break it catches is invisible until a user upgrades.
+	#
+	# One major is enough, so this runs once against the first config rather than
+	# per pair. It builds the previous release from a throwaway clone, so it needs
+	# a checkout with tags.
+	_ex="${CONFIGS[0]}"
+	if [ -x "$_ex" ]; then
+		_exmaj="$("$_ex" --version | sed -E 's/^[^0-9]*([0-9]+).*/\1/')"
+		_exlog="$(mktemp "/tmp/pgcolumnar-extupgrade-${_exmaj}.XXXXXX.log")"
+		# The suite builds a previous release, so it has to be told where to find one.
+		# PGC_UPGRADE_OLD_SRC carries a directory through, which is the only form that
+		# works where the tree has no .git, and the documented container loop is exactly
+		# that. Without it the suite falls back to a ref and cannot build one there.
+		bash "$SRCDIR/test/extension_upgrade.sh" "$_ex" \
+			${PGC_UPGRADE_OLD_SRC:+"$PGC_UPGRADE_OLD_SRC"} >"$_exlog" 2>&1
+		_exrc=$?
+		# Exit 2 is "the environment could not supply an old source", which is not a
+		# product failure. It is reported as SKIP and not as PASS, because a gate that
+		# reports green having run nothing is the defect this suite was written for.
+		if [ "$_exrc" = 0 ]; then
+			echo "  PASS  extension_upgrade PG$_exmaj"
+			SUMMARY+=("PASS   extension_upgrade PG$_exmaj")
+			rm -f "$_exlog"
+		elif [ "$_exrc" = 2 ]; then
+			echo "  SKIP  extension_upgrade PG$_exmaj"
+			grep -E '^\s*(SKIP|  )' "$_exlog" | sed 's/^/      /'
+			SUMMARY+=("SKIP   extension_upgrade PG$_exmaj")
+			rm -f "$_exlog"
+		else
+			echo "  FAIL  extension_upgrade PG$_exmaj"
+			grep -E '^\s*FAIL' "$_exlog" | sed 's/^/      >> /'
+			tail -30 "$_exlog" | sed 's/^/      /'
+			echo "      full log: $_exlog"
+			SUMMARY+=("FAIL   extension_upgrade PG$_exmaj")
+			overall=1
+		fi
+	else
+		echo "  FAIL  PGC_RUN_UPGRADE=1 but no runnable pg_config for extension_upgrade"
+		SUMMARY+=("FAIL   extension_upgrade (no runnable pg_config)")
+		overall=1
+	fi
 fi
 
 echo