Hi,
When building with meson, TEMP_CONFIG is supported for TAP tests, but doesn't
do anything for regress/isolation.
The reason for that is that meson's (and ninja's) architecture is to separate
"build setup" from the "build/test/whatever" stage, moving dynamism (and more
costly operations) to the "setup" phase.
In this case the implication is that the command line for the test isn't
re-computed dynamically. But pg_regress doesn't look at TEMP_CONFIG, it just
has a --temp-config=... parameter, that src/Makefile.global.in dynamically
adds if TEMP_CONFIG is set.
In contrast to that, TEMP_CONFIG support for tap tests is implemented in
Cluster.pm, and thus works transparently.
My inclination is to move TEMP_CONFIG support from the Makefile to
pg_regress.c. That way it's consistent across the build tools and isn't
duplicated. pg_regress already looks at a bunch of temporary variables
(e.g. PG_REGRESS_SOCK_DIR, PG_TEST_USE_UNIX_SOCKETS), so this isn't really
breaking new ground.
It can be implemented differently, e.g. by adding the parameter dynamically in
the wrapper around pg_regress, but I don't see an advantage in that.
Patch attached.
Greetings,
Andres Freund
>From 22d931aa350698e63e61c449d76b9ac2e3ee37b0 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Sat, 18 Feb 2023 12:20:09 -0800
Subject: [PATCH v1 1/2] Move TEMP_CONFIG into pg_regress.c
That way it works for meson as well, and gets rid of a bit of duplicated
handling of TEMP_CONFIG.
---
src/interfaces/ecpg/test/Makefile | 6 +++---
src/test/regress/pg_regress.c | 8 ++++++++
src/Makefile.global.in | 8 --------
src/tools/msvc/vcregress.pl | 5 -----
4 files changed, 11 insertions(+), 16 deletions(-)
diff --git a/src/interfaces/ecpg/test/Makefile b/src/interfaces/ecpg/test/Makefile
index d7a7d1d1ca5..ce4b7237b66 100644
--- a/src/interfaces/ecpg/test/Makefile
+++ b/src/interfaces/ecpg/test/Makefile
@@ -71,11 +71,11 @@ REGRESS_OPTS = --expecteddir=$(srcdir) \
$(EXTRA_REGRESS_OPTS)
check: all
- $(with_temp_install) ./pg_regress $(REGRESS_OPTS) --temp-instance=./tmp_check $(TEMP_CONF) --bindir= $(pg_regress_locale_flags) $(THREAD) --schedule=$(srcdir)/ecpg_schedule sql/twophase
+ $(with_temp_install) ./pg_regress $(REGRESS_OPTS) --temp-instance=./tmp_check --bindir= $(pg_regress_locale_flags) $(THREAD) --schedule=$(srcdir)/ecpg_schedule sql/twophase
# Connect to the server using TCP, and add a TCP-specific test.
checktcp: all | temp-install
- $(with_temp_install) ./pg_regress $(REGRESS_OPTS) --temp-instance=./tmp_check $(TEMP_CONF) --bindir= $(pg_regress_locale_flags) $(THREAD) --schedule=$(srcdir)/ecpg_schedule --host=localhost sql/twophase connect/test1
+ $(with_temp_install) ./pg_regress $(REGRESS_OPTS) --temp-instance=./tmp_check --bindir= $(pg_regress_locale_flags) $(THREAD) --schedule=$(srcdir)/ecpg_schedule --host=localhost sql/twophase connect/test1
installcheck: all
./pg_regress $(REGRESS_OPTS) --bindir='$(bindir)' $(pg_regress_locale_flags) $(THREAD) --schedule=$(srcdir)/ecpg_schedule
@@ -89,4 +89,4 @@ installcheck-prepared-txns: all
./pg_regress $(REGRESS_OPTS) --bindir='$(bindir)' $(pg_regress_locale_flags) $(THREAD) --schedule=$(srcdir)/ecpg_schedule sql/twophase
check-prepared-txns: all | temp-install
- $(with_temp_install) ./pg_regress $(REGRESS_OPTS) --temp-instance=./tmp_check $(TEMP_CONF) --bindir= $(pg_regress_locale_flags) $(THREAD) --schedule=$(srcdir)/ecpg_schedule sql/twophase
+ $(with_temp_install) ./pg_regress $(REGRESS_OPTS) --temp-instance=./tmp_check --bindir= $(pg_regress_locale_flags) $(THREAD) --schedule=$(srcdir)/ecpg_schedule sql/twophase
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 6cd5998b9d7..759bf00d981 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -2032,6 +2032,14 @@ regression_main(int argc, char *argv[],
if (!use_unix_sockets)
hostname = "localhost";
+ /*
+ * If the TEMP_CONFIG environment variable is set, use it. Give higher
+ * precedence to an explicitly specified --temp-config by adding it later
+ * to the list, as later config assignments override earlier ones.
+ */
+ if (getenv("TEMP_CONFIG"))
+ add_stringlist_item(&temp_configs, getenv("TEMP_CONFIG"));
+
/*
* We call the initialization function here because that way we can set
* default parameters and let them be overwritten by the commandline.
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index fb3e197fc06..24656a1c74c 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -655,12 +655,6 @@ ifdef NO_LOCALE
NOLOCALE += --no-locale
endif
-# file with extra config for temp build
-TEMP_CONF =
-ifdef TEMP_CONFIG
-TEMP_CONF += --temp-config=$(TEMP_CONFIG)
-endif
-
pg_regress_locale_flags = $(if $(ENCODING),--encoding=$(ENCODING)) $(NOLOCALE)
pg_regress_clean_files = results/ regression.diffs regression.out tmp_check/ tmp_check_iso/ log/ output_iso/
@@ -671,7 +665,6 @@ pg_regress_check = \
--temp-instance=./tmp_check \
--inputdir=$(srcdir) \
--bindir= \
- $(TEMP_CONF) \
$(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS)
pg_regress_installcheck = \
echo "+++ regress install-check in $(subdir) +++" && \
@@ -687,7 +680,6 @@ pg_isolation_regress_check = \
--temp-instance=./tmp_check_iso \
--inputdir=$(srcdir) --outputdir=output_iso \
--bindir= \
- $(TEMP_CONF) \
$(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS)
pg_isolation_regress_installcheck = \
echo "+++ isolation install-check in $(subdir) +++" && \
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index 9d2eee89f31..776585bea90 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -92,10 +92,6 @@ my $maxconn = "";
$maxconn = "--max-connections=$ENV{MAX_CONNECTIONS}"
if $ENV{MAX_CONNECTIONS};
-my $temp_config = "";
-$temp_config = "--temp-config=\"$ENV{TEMP_CONFIG}\""
- if $ENV{TEMP_CONFIG};
-
chdir "src/test/regress";
my %command = (
@@ -204,7 +200,6 @@ sub check
"--no-locale",
"--temp-instance=./tmp_check");
push(@args, $maxconn) if $maxconn;
- push(@args, $temp_config) if $temp_config;
system(@args);
my $status = $? >> 8;
exit $status if $status;
--
2.38.0
