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