On Thu, Jan 13, 2022 at 10:55:27AM -0800, Andres Freund wrote:
> I'll probably apply that part and 0002 separately.

I've hacked on it a bit more now..

Question: are data-checksums tested at all ?  The only thing I can find is that
some buildfarm members *might* exercise it during installcheck.

I added pg_regress --initdb-opts since that seems to be a deficiency.

-- 
Justin
>From 10121f588d0b3cab67ee810a718511879d6f24a8 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Sun, 9 Jan 2022 18:25:02 -0600
Subject: [PATCH 1/7] vcregress/ci: test modules/contrib with NO_INSTALLCHECK=1

---
 .cirrus.yml                                |  4 +-
 contrib/pg_stat_statements/Makefile        |  2 +-
 contrib/test_decoding/Makefile             |  2 +-
 src/test/modules/snapshot_too_old/Makefile |  2 +-
 src/test/modules/worker_spi/Makefile       |  2 +-
 src/tools/msvc/vcregress.pl                | 46 +++++++++++++++++++---
 6 files changed, 47 insertions(+), 11 deletions(-)

diff --git a/.cirrus.yml b/.cirrus.yml
index 19b3737fa11..02ea7e67189 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -398,9 +398,9 @@ task:
   test_isolation_script:
     - perl src/tools/msvc/vcregress.pl isolationcheck
   test_modules_script:
-    - perl src/tools/msvc/vcregress.pl modulescheck
+    - perl src/tools/msvc/vcregress.pl modulescheck install
   test_contrib_script:
-    - perl src/tools/msvc/vcregress.pl contribcheck
+    - perl src/tools/msvc/vcregress.pl contribcheck install
   stop_script:
     - tmp_install\bin\pg_ctl.exe stop -D tmp_check/db -l tmp_check/postmaster.log
   test_ssl_script:
diff --git a/contrib/pg_stat_statements/Makefile b/contrib/pg_stat_statements/Makefile
index 7fabd96f38d..d732e1ade73 100644
--- a/contrib/pg_stat_statements/Makefile
+++ b/contrib/pg_stat_statements/Makefile
@@ -15,7 +15,7 @@ PGFILEDESC = "pg_stat_statements - execution statistics of SQL statements"
 
 LDFLAGS_SL += $(filter -lm, $(LIBS))
 
-REGRESS_OPTS = --temp-config $(top_srcdir)/contrib/pg_stat_statements/pg_stat_statements.conf
+REGRESS_OPTS = --temp-config=$(top_srcdir)/contrib/pg_stat_statements/pg_stat_statements.conf
 REGRESS = pg_stat_statements oldextversions
 # Disabled because these tests require "shared_preload_libraries=pg_stat_statements",
 # which typical installcheck users do not have (e.g. buildfarm clients).
diff --git a/contrib/test_decoding/Makefile b/contrib/test_decoding/Makefile
index 9a31e0b8795..14fd847ba7f 100644
--- a/contrib/test_decoding/Makefile
+++ b/contrib/test_decoding/Makefile
@@ -10,7 +10,7 @@ ISOLATION = mxact delayed_startup ondisk_startup concurrent_ddl_dml \
 	oldest_xmin snapshot_transfer subxact_without_top concurrent_stream \
 	twophase_snapshot
 
-REGRESS_OPTS = --temp-config $(top_srcdir)/contrib/test_decoding/logical.conf
+REGRESS_OPTS = --temp-config=$(top_srcdir)/contrib/test_decoding/logical.conf
 ISOLATION_OPTS = --temp-config $(top_srcdir)/contrib/test_decoding/logical.conf
 
 # Disabled because these tests require "wal_level=logical", which
diff --git a/src/test/modules/snapshot_too_old/Makefile b/src/test/modules/snapshot_too_old/Makefile
index dfb4537f63c..752a0039fdc 100644
--- a/src/test/modules/snapshot_too_old/Makefile
+++ b/src/test/modules/snapshot_too_old/Makefile
@@ -5,7 +5,7 @@
 EXTRA_CLEAN = $(pg_regress_clean_files)
 
 ISOLATION = sto_using_cursor sto_using_select sto_using_hash_index
-ISOLATION_OPTS = --temp-config $(top_srcdir)/src/test/modules/snapshot_too_old/sto.conf
+ISOLATION_OPTS = --temp-config=$(top_srcdir)/src/test/modules/snapshot_too_old/sto.conf
 
 # Disabled because these tests require "old_snapshot_threshold" >= 0, which
 # typical installcheck users do not have (e.g. buildfarm clients).
diff --git a/src/test/modules/worker_spi/Makefile b/src/test/modules/worker_spi/Makefile
index cbf9b2e37fd..d9f7d9bab6d 100644
--- a/src/test/modules/worker_spi/Makefile
+++ b/src/test/modules/worker_spi/Makefile
@@ -9,7 +9,7 @@ PGFILEDESC = "worker_spi - background worker example"
 REGRESS = worker_spi
 
 # enable our module in shared_preload_libraries for dynamic bgworkers
-REGRESS_OPTS = --temp-config $(top_srcdir)/src/test/modules/worker_spi/dynamic.conf
+REGRESS_OPTS = --temp-config=$(top_srcdir)/src/test/modules/worker_spi/dynamic.conf
 
 # Disable installcheck to ensure we cover dynamic bgworkers.
 NO_INSTALLCHECK = 1
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index 7575acdfdf5..b24ea11aa4a 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -443,6 +443,7 @@ sub plcheck
 sub subdircheck
 {
 	my $module = shift;
+	my $installcheck = shift || 1;
 
 	if (   !-d "$module/sql"
 		|| !-d "$module/expected"
@@ -452,7 +453,7 @@ sub subdircheck
 	}
 
 	chdir $module;
-	my @tests = fetchTests();
+	my @tests = fetchTests($installcheck);
 
 	# Leave if no tests are listed in the module.
 	if (scalar @tests == 0)
@@ -462,6 +463,7 @@ sub subdircheck
 	}
 
 	my @opts = fetchRegressOpts();
+	push @opts, "--temp-instance=tmp_check" if $installcheck == -1;
 
 	# Special processing for python transform modules, see their respective
 	# Makefiles for more details regarding Python-version specific
@@ -489,7 +491,7 @@ sub subdircheck
 	print "Checking $module\n";
 	my @args = (
 		"$topdir/$Config/pg_regress/pg_regress",
-		"--bindir=${topdir}/${Config}/psql",
+		"--bindir=$tmp_installdir/bin",
 		"--dbname=contrib_regression", @opts, @tests);
 	print join(' ', @args), "\n";
 	system(@args);
@@ -499,6 +501,8 @@ sub subdircheck
 
 sub contribcheck
 {
+	my $mode = shift || '';
+
 	chdir "../../../contrib";
 	my $mstat = 0;
 	foreach my $module (glob("*"))
@@ -516,12 +520,25 @@ sub contribcheck
 		my $status = $? >> 8;
 		$mstat ||= $status;
 	}
+
+	# As above, but creates new DB instance for each module.  For CI.
+	if ($mode eq "install")
+	{
+		foreach my $module (glob("*"))
+		{
+			subdircheck("$module", -1);
+			$mstat ||= $? >> 8;
+		}
+	}
+
 	exit $mstat if $mstat;
 	return;
 }
 
 sub modulescheck
 {
+	my $mode = shift || '';
+
 	chdir "../../../src/test/modules";
 	my $mstat = 0;
 	foreach my $module (glob("*"))
@@ -530,6 +547,17 @@ sub modulescheck
 		my $status = $? >> 8;
 		$mstat ||= $status;
 	}
+
+	# As above, but creates new DB instance for each module.  For CI.
+	if ($mode eq "install")
+	{
+		foreach my $module (glob("*"))
+		{
+			subdircheck("$module", -1);
+			$mstat ||= $? >> 8;
+		}
+	}
+
 	exit $mstat if $mstat;
 	return;
 }
@@ -700,6 +728,7 @@ sub fetchRegressOpts
 		# option starting with "--".
 		@opts = grep { !/\$\(/ && /^--/ }
 		  map { (my $x = $_) =~ s/\Q$(top_builddir)\E/\"$topdir\"/; $x; }
+		  map { (my $x = $_) =~ s/\Q$(top_srcdir)\E/\"$topdir\"/; $x; }
 		  split(/\s+/, $1);
 	}
 	if ($m =~ /^\s*ENCODING\s*=\s*(\S+)/m)
@@ -726,14 +755,19 @@ sub fetchTests
 	my $m = <$handle>;
 	close($handle);
 	my $t = "";
+	my $installcheck = shift || 1;
 
 	$m =~ s{\\\r?\n}{}g;
 
-	# A module specifying NO_INSTALLCHECK does not support installcheck,
-	# so bypass its run by returning an empty set of tests.
 	if ($m =~ /^\s*NO_INSTALLCHECK\s*=\s*\S+/m)
 	{
-		return ();
+		# Skip modules marked installcheck unless running installcheck tests.
+		return () if $installcheck == 1;
+	}
+	else
+	{
+		# Skip modules not marked installcheck if running installcheck tests.
+		return () if $installcheck == -1;
 	}
 
 	if ($m =~ /^REGRESS\s*=\s*(.*)$/gm)
@@ -799,6 +833,8 @@ sub usage
 	  "\nOptions for <arg>: (used by check and installcheck)\n",
 	  "  serial         serial mode\n",
 	  "  parallel       parallel mode\n",
+	  "\nOptions for <arg>: (used by contribcheck and modulescheck)\n",
+	  "  install        also run tests which require a new instance\n",
 	  "\nOption for <arg>: for taptest\n",
 	  "  TEST_DIR       (required) directory where tests reside\n";
 	exit(1);
-- 
2.17.1

>From b67cd05895c8fa42e13e6743db36412a68292607 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Sun, 9 Jan 2022 22:54:32 -0600
Subject: [PATCH 2/7] CI: run initdb with --no-sync for windows

---
 .cirrus.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.cirrus.yml b/.cirrus.yml
index 02ea7e67189..a0d733bb594 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -390,7 +390,7 @@ task:
     - perl src/tools/msvc/vcregress.pl check parallel
   startcreate_script:
     # paths to binaries need backslashes
-    - tmp_install\bin\pg_ctl.exe initdb -D tmp_check/db -l tmp_check/initdb.log
+    - tmp_install\bin\pg_ctl.exe initdb -D tmp_check/db -l tmp_check/initdb.log --options=--no-sync
     - echo include '%TEMP_CONFIG%' >> tmp_check/db/postgresql.conf
     - tmp_install\bin\pg_ctl.exe start -D tmp_check/db -l tmp_check/postmaster.log
   test_pl_script:
-- 
2.17.1

>From 420fa0a66d85c61a2922907f8a321104f3ebab00 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Sun, 9 Jan 2022 23:05:18 -0600
Subject: [PATCH 3/7] vcregress: style

---
 src/tools/msvc/vcregress.pl | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index b24ea11aa4a..181727e8c81 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -566,7 +566,6 @@ sub recoverycheck
 {
 	InstallTemp();
 
-	my $mstat  = 0;
 	my $dir    = "$topdir/src/test/recovery";
 	my $status = tap_check($dir);
 	exit $status if $status;
@@ -746,7 +745,6 @@ sub fetchRegressOpts
 # list is returned if the module does not need to run anything.
 sub fetchTests
 {
-
 	my $handle;
 	open($handle, '<', "GNUmakefile")
 	  || open($handle, '<', "Makefile")
-- 
2.17.1

>From 885becd19f630a69ab1de44cefcdda21ca8146d6 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Tue, 11 Jan 2022 01:30:37 -0600
Subject: [PATCH 4/7] cirrus/linux: script test.log..

For consistency, and because otherwise errors can be lost (?) or hard to find.
---
 .cirrus.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.cirrus.yml b/.cirrus.yml
index a0d733bb594..77f5cbc3ae9 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -209,7 +209,7 @@ task:
   test_world_script: |
     su postgres <<-EOF
       ulimit -c unlimited # default is 0
-      make -s ${CHECK} ${CHECKFLAGS} -j${TEST_JOBS}
+      script --command "make -s ${CHECK} ${CHECKFLAGS} -j${TEST_JOBS}" test.log
     EOF
 
   on_failure:
-- 
2.17.1

>From a4e477f1dee7221f9c8e7422f654c04360a8a278 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Wed, 12 Jan 2022 15:44:37 -0600
Subject: [PATCH 5/7] cirrus/mac: uname -a and export at end of sysinfo

---
 .cirrus.yml | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/.cirrus.yml b/.cirrus.yml
index 77f5cbc3ae9..89e5980cbd1 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -237,8 +237,9 @@ task:
 
   sysinfo_script: |
     id
-    export
+    uname -a
     ulimit -a -H && ulimit -a -S
+    export
 
   setup_cores_script:
     - mkdir ${HOME}/cores
-- 
2.17.1

>From 9e9d9377f1abc9136b8abd82c77471d25c1f4be5 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Wed, 12 Jan 2022 15:42:33 -0600
Subject: [PATCH 6/7] cirrus: factor out common configure options

---
 .cirrus.yml | 49 +++++++++++++++++++++----------------------------
 1 file changed, 21 insertions(+), 28 deletions(-)

diff --git a/.cirrus.yml b/.cirrus.yml
index 89e5980cbd1..d215619860e 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -26,6 +26,16 @@ env:
   TEMP_CONFIG: ${CIRRUS_WORKING_DIR}/src/tools/ci/pg_ci_base.conf
   PG_TEST_EXTRA: kerberos ldap ssl
 
+COMMON_CONFIG_LIBS: &COMMON_CONFIG_LIBS >-
+  --with-gssapi
+  --with-icu
+  --with-ldap
+  --with-libxml
+  --with-libxslt
+  --with-lz4
+  --with-perl
+  --with-python
+  --with-ssl=openssl
 
 # What files to preserve in case tests fail
 on_failure: &on_failure
@@ -51,6 +61,7 @@ task:
     TEST_JOBS: 3
 
     CCACHE_DIR: /tmp/ccache_dir
+    COMMON_CONFIG_LIBS: *COMMON_CONFIG_LIBS
 
   only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*freebsd.*'
 
@@ -90,17 +101,9 @@ task:
       ./configure \
         --enable-cassert --enable-debug --enable-tap-tests \
         --enable-nls \
+        ${COMMON_CONFIG_LIBS} \
         \
-        --with-gssapi \
-        --with-icu \
-        --with-ldap \
-        --with-libxml \
-        --with-libxslt \
-        --with-lz4 \
         --with-pam \
-        --with-perl \
-        --with-python \
-        --with-ssl=openssl \
         --with-tcl --with-tclconfig=/usr/local/lib/tcl8.6/ \
         --with-uuid=bsd \
         \
@@ -129,18 +132,9 @@ task:
 # configure feature flags, shared between the task running the linux tests and
 # the CompilerWarnings task
 LINUX_CONFIGURE_FEATURES: &LINUX_CONFIGURE_FEATURES >-
-  --with-gssapi
-  --with-icu
-  --with-ldap
-  --with-libxml
-  --with-libxslt
   --with-llvm
-  --with-lz4
   --with-pam
-  --with-perl
-  --with-python
   --with-selinux
-  --with-ssl=openssl
   --with-systemd
   --with-tcl --with-tclconfig=/usr/lib/tcl8.6/
   --with-uuid=ossp
@@ -157,6 +151,7 @@ task:
     CCACHE_DIR: /tmp/ccache_dir
     DEBUGINFOD_URLS: "https://debuginfod.debian.net";
 
+    COMMON_CONFIG_LIBS: *COMMON_CONFIG_LIBS
     LINUX_CONFIGURE_FEATURES: *LINUX_CONFIGURE_FEATURES
 
   only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*linux.*'
@@ -183,7 +178,7 @@ task:
     mkdir -p ${CCACHE_DIR}
     chown -R postgres:postgres ${CCACHE_DIR}
     echo '* - memlock 134217728' > /etc/security/limits.d/postgres.conf
-    su postgres -c "ulimit -l -H && ulimit -l -S"
+    su postgres -c "ulimit -l -H && ulimit -l -S" # XXX
   setup_cores_script: |
     mkdir -m 770 /tmp/cores
     chown root:postgres /tmp/cores
@@ -195,6 +190,7 @@ task:
         --enable-cassert --enable-debug --enable-tap-tests \
         --enable-nls \
         \
+        ${COMMON_CONFIG_LIBS} \
         ${LINUX_CONFIGURE_FEATURES} \
         \
         CC="ccache gcc" \
@@ -229,6 +225,7 @@ task:
     CCACHE_DIR: ${HOME}/ccache
     HOMEBREW_CACHE: ${HOME}/homebrew-cache
     PERL5LIB: ${HOME}/perl5/lib/perl5
+    COMMON_CONFIG_LIBS: *COMMON_CONFIG_LIBS
 
   only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*(macos|darwin|osx).*'
 
@@ -294,16 +291,8 @@ task:
       --enable-cassert --enable-debug --enable-tap-tests \
       --enable-nls \
       \
+      ${COMMON_CONFIG_LIBS} \
       --with-bonjour \
-      --with-gssapi \
-      --with-icu \
-      --with-ldap \
-      --with-libxml \
-      --with-libxslt \
-      --with-lz4 \
-      --with-perl \
-      --with-python \
-      --with-ssl=openssl \
       --with-tcl --with-tclconfig=${brewpath}/opt/tcl-tk/lib/ \
       --with-uuid=e2fs \
       \
@@ -444,6 +433,7 @@ task:
     CCACHE_DIR: "/tmp/ccache_dir"
 
     LINUX_CONFIGURE_FEATURES: *LINUX_CONFIGURE_FEATURES
+    COMMON_CONFIG_LIBS: *COMMON_CONFIG_LIBS
 
   # task that did not run, count as a success, so we need to recheck Linux'
   # condition here ...
@@ -481,6 +471,7 @@ task:
       time ./configure \
         --cache gcc.cache \
         --enable-dtrace \
+        ${COMMON_CONFIG_LIBS} \
         ${LINUX_CONFIGURE_FEATURES} \
         CC="ccache gcc" CXX="ccache g++" CLANG="ccache clang"
       make -s -j${BUILD_JOBS} clean
@@ -492,6 +483,7 @@ task:
       time ./configure \
         --cache gcc.cache \
         --enable-cassert \
+        ${COMMON_CONFIG_LIBS} \
         ${LINUX_CONFIGURE_FEATURES} \
         CC="ccache gcc" CXX="ccache g++" CLANG="ccache clang"
       make -s -j${BUILD_JOBS} clean
@@ -514,6 +506,7 @@ task:
         --cache clang.cache \
         --enable-cassert \
         --enable-dtrace \
+        ${COMMON_CONFIG_LIBS} \
         ${LINUX_CONFIGURE_FEATURES} \
         CC="ccache clang" CXX="ccache clang++" CLANG="ccache clang"
       make -s -j${BUILD_JOBS} clean
-- 
2.17.1

>From d9bed052b7334ab36ae63cd2db22aa2f6447878e Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Thu, 13 Jan 2022 12:53:56 -0600
Subject: [PATCH 7/7] wip: pg_regress --initdb-opts

---
 src/test/regress/pg_regress.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index cc311dba4c5..33ac2860282 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -95,6 +95,7 @@ static char *dlpath = PKGLIBDIR;
 static char *user = NULL;
 static _stringlist *extraroles = NULL;
 static char *config_auth_datadir = NULL;
+static char *initdb_opts = NULL;
 
 /* internal variables */
 static const char *progname;
@@ -2011,6 +2012,7 @@ help(void)
 	printf(_("      --encoding=ENCODING       use ENCODING as the encoding\n"));
 	printf(_("  -h, --help                    show this help, then exit\n"));
 	printf(_("      --inputdir=DIR            take input files from DIR (default \".\")\n"));
+	printf(_("      --initdb-opts=OPTS        options to pass to initdb\n"));
 	printf(_("      --launcher=CMD            use CMD as launcher of psql\n"));
 	printf(_("      --load-extension=EXT      load the named extension before running the\n"));
 	printf(_("                                tests; can appear multiple times\n"));
@@ -2074,6 +2076,7 @@ regression_main(int argc, char *argv[],
 		{"config-auth", required_argument, NULL, 24},
 		{"max-concurrent-tests", required_argument, NULL, 25},
 		{"make-testtablespace-dir", no_argument, NULL, 26},
+		{"initdb-opts", required_argument, NULL, 27},
 		{NULL, 0, NULL, 0}
 	};
 
@@ -2207,6 +2210,9 @@ regression_main(int argc, char *argv[],
 			case 26:
 				make_testtablespace_dir = true;
 				break;
+			case 27:
+				initdb_opts = pg_strdup(optarg);
+				break;
 			default:
 				/* getopt_long already emitted a complaint */
 				fprintf(stderr, _("\nTry \"%s -h\" for more information.\n"),
@@ -2296,12 +2302,13 @@ regression_main(int argc, char *argv[],
 		/* initdb */
 		header(_("initializing database system"));
 		snprintf(buf, sizeof(buf),
-				 "\"%s%sinitdb\" -D \"%s/data\" --no-clean --no-sync%s%s > \"%s/log/initdb.log\" 2>&1",
+				 "\"%s%sinitdb\" -D \"%s/data\" --no-clean --no-sync%s%s %s > \"%s/log/initdb.log\" 2>&1",
 				 bindir ? bindir : "",
 				 bindir ? "/" : "",
 				 temp_instance,
 				 debug ? " --debug" : "",
 				 nolocale ? " --no-locale" : "",
+				 initdb_opts ? initdb_opts : "",
 				 outputdir);
 		if (system(buf))
 		{
-- 
2.17.1

Reply via email to