From 5c0fee0be2f64439149a37bc4bc4f44b1fcdcda8 Mon Sep 17 00:00:00 2001
From: Noah Misch <noah@leadboat.com>
Date: Tue, 28 May 2019 12:59:00 -0700
Subject: [PATCH] In the pg_upgrade test suite, don't write to
 src/test/regress.

When this suite runs installcheck, redirect file creations from
src/test/regress to src/bin/pg_upgrade/tmp_check/regress.  This closes a
race condition in "make -j check-world".  If the pg_upgrade suite wrote
to a given src/test/regress/results file in parallel with the regular
src/test/regress invocation writing it, a test failed spuriously.  Even
without parallelism, in "make -k check-world", the suite finishing
second overwrote the other's regression.diffs.  This revealed test
"largeobject" assuming @abs_builddir@ is getcwd(), so fix that, too.

Buildfarm client REL_10, released fifty-four days ago, supports saving
regression.diffs from its new location.  When an older client reports a
pg_upgradeCheck failure, it will no longer include regression.diffs.
Back-patch to 9.5, where pg_upgrade moved to src/bin.

Reviewed (in earlier versions) by Andrew Dunstan.

Discussion: https://postgr.es/m/20181224034411.GA3224776@rfd.leadboat.com
---
 src/bin/pg_upgrade/test.sh                   | 11 ++++++++++
 src/test/regress/input/largeobject.source    |  6 +++---
 src/test/regress/output/largeobject.source   |  6 +++---
 src/test/regress/output/largeobject_1.source |  6 +++---
 src/tools/msvc/vcregress.pl                  | 21 +++++++++++++++++---
 5 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/src/bin/pg_upgrade/test.sh b/src/bin/pg_upgrade/test.sh
index 82db9849608..8a8b826e9d9 100644
--- a/src/bin/pg_upgrade/test.sh
+++ b/src/bin/pg_upgrade/test.sh
@@ -107,6 +107,17 @@ BASE_PGDATA=$temp_root/data
 PGDATA="$BASE_PGDATA.old"
 export PGDATA
 
+# Send installcheck outputs to a private directory.  This avoids conflict when
+# check-world runs pg_upgrade check concurrently with src/test/regress check.
+# To retrieve interesting files after a run, use pattern tmp_check/*/*.diffs.
+outputdir="$temp_root/regress"
+EXTRA_REGRESS_OPTS="$EXTRA_REGRESS_OPTS --outputdir=$outputdir"
+export EXTRA_REGRESS_OPTS
+mkdir "$outputdir"
+mkdir "$outputdir"/sql
+mkdir "$outputdir"/expected
+mkdir "$outputdir"/testtablespace
+
 logdir=`pwd`/log
 rm -rf "$logdir"
 mkdir "$logdir"
diff --git a/src/test/regress/input/largeobject.source b/src/test/regress/input/largeobject.source
index 50c13eb4f3c..9aa7b16193d 100644
--- a/src/test/regress/input/largeobject.source
+++ b/src/test/regress/input/largeobject.source
@@ -203,12 +203,12 @@ END;
 
 SELECT lo_export(loid, '@abs_builddir@/results/lotest.txt') FROM lotest_stash_values;
 
-\lo_import 'results/lotest.txt'
+\lo_import '@abs_builddir@/results/lotest.txt'
 
 \set newloid :LASTOID
 
 -- just make sure \lo_export does not barf
-\lo_export :newloid 'results/lotest2.txt'
+\lo_export :newloid '@abs_builddir@/results/lotest2.txt'
 
 -- This is a hack to test that export/import are reversible
 -- This uses knowledge about the inner workings of large object mechanism
@@ -223,7 +223,7 @@ TRUNCATE lotest_stash_values;
 
 \lo_unlink :newloid
 
-\lo_import 'results/lotest.txt'
+\lo_import '@abs_builddir@/results/lotest.txt'
 
 \set newloid_1 :LASTOID
 
diff --git a/src/test/regress/output/largeobject.source b/src/test/regress/output/largeobject.source
index afdcb4b7dd0..969a7c7f40f 100644
--- a/src/test/regress/output/largeobject.source
+++ b/src/test/regress/output/largeobject.source
@@ -385,10 +385,10 @@ SELECT lo_export(loid, '@abs_builddir@/results/lotest.txt') FROM lotest_stash_va
          1
 (1 row)
 
-\lo_import 'results/lotest.txt'
+\lo_import '@abs_builddir@/results/lotest.txt'
 \set newloid :LASTOID
 -- just make sure \lo_export does not barf
-\lo_export :newloid 'results/lotest2.txt'
+\lo_export :newloid '@abs_builddir@/results/lotest2.txt'
 -- This is a hack to test that export/import are reversible
 -- This uses knowledge about the inner workings of large object mechanism
 -- which should not be used outside it.  This makes it a HACK
@@ -407,7 +407,7 @@ SELECT lo_unlink(loid) FROM lotest_stash_values;
 
 TRUNCATE lotest_stash_values;
 \lo_unlink :newloid
-\lo_import 'results/lotest.txt'
+\lo_import '@abs_builddir@/results/lotest.txt'
 \set newloid_1 :LASTOID
 SELECT lo_from_bytea(0, lo_get(:newloid_1)) AS newloid_2
 \gset
diff --git a/src/test/regress/output/largeobject_1.source b/src/test/regress/output/largeobject_1.source
index 86aa3c00322..bb636b6fe62 100644
--- a/src/test/regress/output/largeobject_1.source
+++ b/src/test/regress/output/largeobject_1.source
@@ -385,10 +385,10 @@ SELECT lo_export(loid, '@abs_builddir@/results/lotest.txt') FROM lotest_stash_va
          1
 (1 row)
 
-\lo_import 'results/lotest.txt'
+\lo_import '@abs_builddir@/results/lotest.txt'
 \set newloid :LASTOID
 -- just make sure \lo_export does not barf
-\lo_export :newloid 'results/lotest2.txt'
+\lo_export :newloid '@abs_builddir@/results/lotest2.txt'
 -- This is a hack to test that export/import are reversible
 -- This uses knowledge about the inner workings of large object mechanism
 -- which should not be used outside it.  This makes it a HACK
@@ -407,7 +407,7 @@ SELECT lo_unlink(loid) FROM lotest_stash_values;
 
 TRUNCATE lotest_stash_values;
 \lo_unlink :newloid
-\lo_import 'results/lotest.txt'
+\lo_import '@abs_builddir@/results/lotest.txt'
 \set newloid_1 :LASTOID
 SELECT lo_from_bytea(0, lo_get(:newloid_1)) AS newloid_2
 \gset
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index 7b388d2365b..699873cd029 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -97,9 +97,9 @@ exit 0;
 
 ########################################################################
 
-sub installcheck
+sub installcheck_internal
 {
-	my $schedule = shift || 'serial';
+	my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
 	my @args = (
 		"../../../$Config/pg_regress/pg_regress",
 		"--dlpath=.",
@@ -108,11 +108,19 @@ sub installcheck
 		"--encoding=SQL_ASCII",
 		"--no-locale");
 	push(@args, $maxconn) if $maxconn;
+	push(@args, @EXTRA_REGRESS_OPTS);
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
 }
 
+sub installcheck
+{
+	my $schedule = shift || 'serial';
+	installcheck_internal($schedule);
+	return;
+}
+
 sub check
 {
 	my $schedule = shift || 'parallel';
@@ -501,6 +509,13 @@ sub upgradecheck
 	$ENV{PATH} = "$bindir;$ENV{PATH}";
 	my $data = "$tmp_root/data";
 	$ENV{PGDATA} = "$data.old";
+	my $outputdir          = "$tmp_root/regress";
+	my @EXTRA_REGRESS_OPTS = ("--outputdir=$outputdir");
+	mkdir "$outputdir"                || die $!;
+	mkdir "$outputdir/sql"            || die $!;
+	mkdir "$outputdir/expected"       || die $!;
+	mkdir "$outputdir/testtablespace" || die $!;
+
 	my $logdir = "$topdir/src/bin/pg_upgrade/log";
 	rmtree($logdir);
 	mkdir $logdir || die $!;
@@ -516,7 +531,7 @@ sub upgradecheck
 	generate_db('',       91, 127, '');
 
 	print "\nSetting up data for upgrading\n\n";
-	installcheck();
+	installcheck_internal('serial', @EXTRA_REGRESS_OPTS);
 
 	# now we can chdir into the source dir
 	chdir "$topdir/src/bin/pg_upgrade";
-- 
GitLab