test-build doesn't gate installcheck: full suite runs before build sanity check

[jnasbyupgrade]

Repo: pgxntool

Problem

test-build exists to sanity-check that the extension actually builds/installs
cleanly (better error messages than a CREATE EXTENSION failure buried inside
the full test run). The point isn't just to save time — there's no value in
running test/install and test/sql against a build that's already known to
be broken; their results would be meaningless. So a test-build failure
should prevent the main suite from running at all.

But in the default configuration nothing enforces that: installcheck (and
therefore the full test suite, including test/install) runs before
test-build ever executes.

Root cause

In base.mk, TEST_DEPS is built up in this order:

TEST_DEPS = testdeps
TEST_DEPS += check-stale-expected   # (default: enabled)
TEST_DEPS += test-build             # (if test/build/*.sql present)
TEST_DEPS += install installcheck

check-stale-expected has an explicit dependency edge on installcheck
(check-stale-expected: installcheck), and it's listed before test-build
in TEST_DEPS. Since GNU Make (serially) builds a target's prerequisites in
listed order, resolving check-stale-expected pulls in install/installcheck
first — before test-build is ever reached.

Reproduced with a minimal Makefile mirroring the same target names/edges:

$ make -f order-test.mk test
RUN testdeps
RUN install
RUN installcheck
RUN check-stale-expected
RUN test-build
test recipe

install/installcheck run before test-build, in the default config
(PGXNTOOL_ENABLE_CHECK_STALE_EXPECTED=yes, which is the default).

This is the same class of ordering hazard the file already calls out
elsewhere — e.g. the comment on installcheck: install: "TEST_DEPS lists
install/installcheck as independent, unordered prerequisites, so nothing
stops installcheck's own prerequisite chain from running before install. An
explicit edge here ... is the only ordering guarantee Make actually gives."
The same reasoning applies to test-build vs installcheck, but no explicit
edge currently enforces it.

Separately (not the main issue, but related): installcheck is marked
.IGNORE:, and test/test-build both detect failure post-hoc by checking
for regression.diffs after the fact rather than stopping immediately. So
even with correct ordering, a test-build failure wouldn't currently produce
an early, hard stop — it would need to actually block the dependency chain
(not just run first).

Suggested fix

Add an explicit ordering edge so test-build (when enabled) must complete
successfully before installcheck starts, mirroring the existing
installcheck: install / check-stale-expected: installcheck pattern, e.g.:

ifeq ($(PGXNTOOL_ENABLE_TEST_BUILD),yes)
installcheck: test-build
endif

placed so it takes effect regardless of TEST_DEPS ordering, and confirm
test-build's own failure (regression.diffs present) actually aborts make
before installcheck runs, not just before its recipe prints "test recipe".

[jnasbyupgrade]

Heads up: the suggested fix here (installcheck: test-build) is not safe as written — it causes infinite recursion.

test-build's own recipe (further down in base.mk) recursively invokes a fresh $(MAKE) ... installcheck to run its own separate pg_regress pass over test/build/*.sql (with REGRESS_OPTS pointed at $(TESTDIR)/build). If installcheck itself now depends on test-build, that recursive installcheck call re-triggers test-build again, which recurses into installcheck again, forever.

Reproduced this directly (had to kill it manually, reached make[3000+] before doing so).

The fix needs to specifically exempt test-build's own recursive installcheck invocation from the new edge — one way: detect that specific recursive call (e.g. by checking whether REGRESS_OPTS is already pointed at the build-test directory, which only happens inside test-build's own recipe) and skip the installcheck: test-build dependency in that case. Verified this approach empirically: a genuinely broken test/build/*.sql file now correctly aborts make test before the main suite runs (both serially and under -j4), and a clean run passes fully in the corrected order, with no recursion.

Worth folding into whatever the actual upstream fix ends up being — the direction (test-build gating installcheck) is right, but the straightforward edge alone will recurse infinitely given how test-build's recipe is currently structured.