public inbox for [email protected]  

help / color / mirror / Atom feed

[PATCH] Move tablespace cleanup out of pg_regress.
24+ messages / 3 participants
[nested] [flat]

* [PATCH 1/2] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)

Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make, which is ugly. In addition to that, pg_regress does
the clean up twice at once. This patch moves the cleanup code out of
pg_regress into vcregress.pl, which is more sutable place.
---
 src/test/regress/pg_regress.c | 22 ----------------------
 src/tools/msvc/vcregress.pl   | 14 ++++++++++++++
 2 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
 
 	snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
 
-#ifdef WIN32
-
-	/*
-	 * On Windows only, clean out the test tablespace dir, or create it if it
-	 * doesn't exist.  On other platforms we expect the Makefile to take care
-	 * of that.  (We don't migrate that functionality in here because it'd be
-	 * harder to cope with platform-specific issues such as SELinux.)
-	 *
-	 * XXX it would be better if pg_regress.c had nothing at all to do with
-	 * testtablespace, and this were handled by a .BAT file or similar on
-	 * Windows.  See pgsql-hackers discussion of 2008-01-18.
-	 */
-	if (directory_exists(testtablespace))
-		if (!rmtree(testtablespace, true))
-		{
-			fprintf(stderr, _("\n%s: could not remove test tablespace \"%s\"\n"),
-					progname, testtablespace);
-			exit(2);
-		}
-	make_directory(testtablespace);
-#endif
-
 	/* finally loop on each file and do the replacement */
 	for (name = names; *name; name++)
 	{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f95f7a5c7a..74d37a31de 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -104,6 +104,7 @@ exit 0;
 sub installcheck_internal
 {
 	my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+
 	my @args = (
 		"../../../$Config/pg_regress/pg_regress",
 		"--dlpath=.",
@@ -114,6 +115,7 @@ sub installcheck_internal
 		"--no-locale");
 	push(@args, $maxconn) if $maxconn;
 	push(@args, @EXTRA_REGRESS_OPTS);
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -143,6 +145,7 @@ sub check
 		"--temp-instance=./tmp_check");
 	push(@args, $maxconn)     if $maxconn;
 	push(@args, $temp_config) if $temp_config;
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -178,6 +181,7 @@ sub ecpgcheck
 sub isolationcheck
 {
 	chdir "../isolation";
+	CleanupTablespaceDirectory();
 	copy("../../../$Config/isolationtester/isolationtester.exe",
 		"../../../$Config/pg_isolation_regress");
 	my @args = (
@@ -382,6 +386,7 @@ sub plcheck
 			"$topdir/$Config/pg_regress/pg_regress",
 			"--bindir=$topdir/$Config/psql",
 			"--dbname=pl_regression", @lang_args, @tests);
+		CleanupTablespaceDirectory();
 		system(@args);
 		my $status = $? >> 8;
 		exit $status if $status;
@@ -439,6 +444,7 @@ sub subdircheck
 
 	print "============================================================\n";
 	print "Checking $module\n";
+	CleanupTablespaceDirectory();
 	my @args = (
 		"$topdir/$Config/pg_regress/pg_regress",
 		"--bindir=${topdir}/${Config}/psql",
@@ -741,6 +747,14 @@ sub InstallTemp
 	return;
 }
 
+sub CleanupTablespaceDirectory
+{
+	my $tablespace = 'testtablespace';
+
+	rmtree($tablespace) if (-e $tablespace);
+	mkdir($tablespace);
+}
+
 sub usage
 {
 	print STDERR
-- 
2.18.2


----Next_Part(Mon_May_11_17_13_54_2020_626)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="0002-Don-t-setup-tablespace-directory-while-testing-pg_up.patch"



^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* [PATCH] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)

Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make.  That is not only ugly also leads to do the clean up
twice at once.  Let's move the cleanup code out of pg_regress into
vcregress.pl, where is more sutable place.

This also fixes a bug that the test for pg_upgrade mistakenly cleans
up the tablespace directory of default tmp-install location, instead
of its own installation directoty.
---
 src/test/regress/GNUmakefile  |  6 ++++--
 src/test/regress/pg_regress.c | 22 ----------------------
 src/tools/msvc/vcregress.pl   | 25 +++++++++++++++++++++++++
 3 files changed, 29 insertions(+), 24 deletions(-)

diff --git a/src/test/regress/GNUmakefile b/src/test/regress/GNUmakefile
index 1a3164065f..815d87904b 100644
--- a/src/test/regress/GNUmakefile
+++ b/src/test/regress/GNUmakefile
@@ -118,8 +118,10 @@ submake-contrib-spi: | submake-libpgport submake-generated-headers
 
 .PHONY: tablespace-setup
 tablespace-setup:
-	rm -rf ./testtablespace
-	mkdir ./testtablespace
+# extract --outputdir optsion from EXTRA_REGRESS_OPTS
+	$(eval dir := $(shell perl -e 'use Getopt::Long qw(GetOptionsFromString Configure); Configure("pass_through", "gnu_getopt"); ($$r, $$x) = GetOptionsFromString($$ENV{"EXTRA_REGRESS_OPTS"}, "outputdir=s" => \$$dir); print ((defined $$dir ? $$dir : ".") . "/testtablespace");'))
+	rm -rf $(dir)
+	mkdir $(dir)
 
 
 ##
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
 
 	snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
 
-#ifdef WIN32
-
-	/*
-	 * On Windows only, clean out the test tablespace dir, or create it if it
-	 * doesn't exist.  On other platforms we expect the Makefile to take care
-	 * of that.  (We don't migrate that functionality in here because it'd be
-	 * harder to cope with platform-specific issues such as SELinux.)
-	 *
-	 * XXX it would be better if pg_regress.c had nothing at all to do with
-	 * testtablespace, and this were handled by a .BAT file or similar on
-	 * Windows.  See pgsql-hackers discussion of 2008-01-18.
-	 */
-	if (directory_exists(testtablespace))
-		if (!rmtree(testtablespace, true))
-		{
-			fprintf(stderr, _("\n%s: could not remove test tablespace \"%s\"\n"),
-					progname, testtablespace);
-			exit(2);
-		}
-	make_directory(testtablespace);
-#endif
-
 	/* finally loop on each file and do the replacement */
 	for (name = names; *name; name++)
 	{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index 0a98f6e37d..4bada14092 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -114,6 +114,13 @@ sub installcheck_internal
 		"--no-locale");
 	push(@args, $maxconn) if $maxconn;
 	push(@args, @EXTRA_REGRESS_OPTS);
+
+	my $outputdir;
+	foreach (@EXTRA_REGRESS_OPTS)
+	{
+		$outputdir = $1 if (/^ *--outputdir *= *(.+) *$/);
+	}
+	CleanupTablespaceDirectory($outputdir);
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -143,6 +150,7 @@ sub check
 		"--temp-instance=./tmp_check");
 	push(@args, $maxconn)     if $maxconn;
 	push(@args, $temp_config) if $temp_config;
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -169,6 +177,7 @@ sub ecpgcheck
 		"--no-locale",
 		"--temp-instance=./tmp_chk");
 	push(@args, $maxconn) if $maxconn;
+	CleanupTablespaceDirectory();
 	system(@args);
 	$status = $? >> 8;
 	exit $status if $status;
@@ -186,6 +195,7 @@ sub isolationcheck
 		"--inputdir=.",
 		"--schedule=./isolation_schedule");
 	push(@args, $maxconn) if $maxconn;
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -382,6 +392,7 @@ sub plcheck
 			"$topdir/$Config/pg_regress/pg_regress",
 			"--bindir=$topdir/$Config/psql",
 			"--dbname=pl_regression", @lang_args, @tests);
+		CleanupTablespaceDirectory();
 		system(@args);
 		my $status = $? >> 8;
 		exit $status if $status;
@@ -444,6 +455,7 @@ sub subdircheck
 		"--bindir=${topdir}/${Config}/psql",
 		"--dbname=contrib_regression", @opts, @tests);
 	print join(' ', @args), "\n";
+	CleanupTablespaceDirectory();
 	system(@args);
 	chdir "..";
 	return;
@@ -739,6 +751,19 @@ sub InstallTemp
 	return;
 }
 
+sub CleanupTablespaceDirectory
+{
+	my ($testdir) = @_;
+
+	$testdir = cwd() if (! defined $testdir);
+
+	# don't bother checking trailing directory separator in $testdir
+	my $tablespace = "$testdir/testtablespace";
+
+	rmtree($tablespace) if (-d $tablespace);
+	mkdir($tablespace);
+}
+
 sub usage
 {
 	print STDERR
-- 
2.18.2


----Next_Part(Fri_May_15_17_25_08_2020_691)----





^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* [PATCH 1/2] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)

Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make, which is ugly. In addition to that, pg_regress does
the clean up twice at once. This patch moves the cleanup code out of
pg_regress into vcregress.pl, which is more sutable place.
---
 src/test/regress/pg_regress.c | 22 ----------------------
 src/tools/msvc/vcregress.pl   | 14 ++++++++++++++
 2 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
 
 	snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
 
-#ifdef WIN32
-
-	/*
-	 * On Windows only, clean out the test tablespace dir, or create it if it
-	 * doesn't exist.  On other platforms we expect the Makefile to take care
-	 * of that.  (We don't migrate that functionality in here because it'd be
-	 * harder to cope with platform-specific issues such as SELinux.)
-	 *
-	 * XXX it would be better if pg_regress.c had nothing at all to do with
-	 * testtablespace, and this were handled by a .BAT file or similar on
-	 * Windows.  See pgsql-hackers discussion of 2008-01-18.
-	 */
-	if (directory_exists(testtablespace))
-		if (!rmtree(testtablespace, true))
-		{
-			fprintf(stderr, _("\n%s: could not remove test tablespace \"%s\"\n"),
-					progname, testtablespace);
-			exit(2);
-		}
-	make_directory(testtablespace);
-#endif
-
 	/* finally loop on each file and do the replacement */
 	for (name = names; *name; name++)
 	{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f95f7a5c7a..74d37a31de 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -104,6 +104,7 @@ exit 0;
 sub installcheck_internal
 {
 	my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+
 	my @args = (
 		"../../../$Config/pg_regress/pg_regress",
 		"--dlpath=.",
@@ -114,6 +115,7 @@ sub installcheck_internal
 		"--no-locale");
 	push(@args, $maxconn) if $maxconn;
 	push(@args, @EXTRA_REGRESS_OPTS);
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -143,6 +145,7 @@ sub check
 		"--temp-instance=./tmp_check");
 	push(@args, $maxconn)     if $maxconn;
 	push(@args, $temp_config) if $temp_config;
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -178,6 +181,7 @@ sub ecpgcheck
 sub isolationcheck
 {
 	chdir "../isolation";
+	CleanupTablespaceDirectory();
 	copy("../../../$Config/isolationtester/isolationtester.exe",
 		"../../../$Config/pg_isolation_regress");
 	my @args = (
@@ -382,6 +386,7 @@ sub plcheck
 			"$topdir/$Config/pg_regress/pg_regress",
 			"--bindir=$topdir/$Config/psql",
 			"--dbname=pl_regression", @lang_args, @tests);
+		CleanupTablespaceDirectory();
 		system(@args);
 		my $status = $? >> 8;
 		exit $status if $status;
@@ -439,6 +444,7 @@ sub subdircheck
 
 	print "============================================================\n";
 	print "Checking $module\n";
+	CleanupTablespaceDirectory();
 	my @args = (
 		"$topdir/$Config/pg_regress/pg_regress",
 		"--bindir=${topdir}/${Config}/psql",
@@ -741,6 +747,14 @@ sub InstallTemp
 	return;
 }
 
+sub CleanupTablespaceDirectory
+{
+	my $tablespace = 'testtablespace';
+
+	rmtree($tablespace) if (-e $tablespace);
+	mkdir($tablespace);
+}
+
 sub usage
 {
 	print STDERR
-- 
2.18.2


----Next_Part(Mon_May_11_17_13_54_2020_626)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="0002-Don-t-setup-tablespace-directory-while-testing-pg_up.patch"



^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* [PATCH 1/2] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)

Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make, which is ugly. In addition to that, pg_regress does
the clean up twice at once. This patch moves the cleanup code out of
pg_regress into vcregress.pl, which is more sutable place.
---
 src/test/regress/pg_regress.c | 22 ----------------------
 src/tools/msvc/vcregress.pl   | 14 ++++++++++++++
 2 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
 
 	snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
 
-#ifdef WIN32
-
-	/*
-	 * On Windows only, clean out the test tablespace dir, or create it if it
-	 * doesn't exist.  On other platforms we expect the Makefile to take care
-	 * of that.  (We don't migrate that functionality in here because it'd be
-	 * harder to cope with platform-specific issues such as SELinux.)
-	 *
-	 * XXX it would be better if pg_regress.c had nothing at all to do with
-	 * testtablespace, and this were handled by a .BAT file or similar on
-	 * Windows.  See pgsql-hackers discussion of 2008-01-18.
-	 */
-	if (directory_exists(testtablespace))
-		if (!rmtree(testtablespace, true))
-		{
-			fprintf(stderr, _("\n%s: could not remove test tablespace \"%s\"\n"),
-					progname, testtablespace);
-			exit(2);
-		}
-	make_directory(testtablespace);
-#endif
-
 	/* finally loop on each file and do the replacement */
 	for (name = names; *name; name++)
 	{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f95f7a5c7a..74d37a31de 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -104,6 +104,7 @@ exit 0;
 sub installcheck_internal
 {
 	my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+
 	my @args = (
 		"../../../$Config/pg_regress/pg_regress",
 		"--dlpath=.",
@@ -114,6 +115,7 @@ sub installcheck_internal
 		"--no-locale");
 	push(@args, $maxconn) if $maxconn;
 	push(@args, @EXTRA_REGRESS_OPTS);
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -143,6 +145,7 @@ sub check
 		"--temp-instance=./tmp_check");
 	push(@args, $maxconn)     if $maxconn;
 	push(@args, $temp_config) if $temp_config;
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -178,6 +181,7 @@ sub ecpgcheck
 sub isolationcheck
 {
 	chdir "../isolation";
+	CleanupTablespaceDirectory();
 	copy("../../../$Config/isolationtester/isolationtester.exe",
 		"../../../$Config/pg_isolation_regress");
 	my @args = (
@@ -382,6 +386,7 @@ sub plcheck
 			"$topdir/$Config/pg_regress/pg_regress",
 			"--bindir=$topdir/$Config/psql",
 			"--dbname=pl_regression", @lang_args, @tests);
+		CleanupTablespaceDirectory();
 		system(@args);
 		my $status = $? >> 8;
 		exit $status if $status;
@@ -439,6 +444,7 @@ sub subdircheck
 
 	print "============================================================\n";
 	print "Checking $module\n";
+	CleanupTablespaceDirectory();
 	my @args = (
 		"$topdir/$Config/pg_regress/pg_regress",
 		"--bindir=${topdir}/${Config}/psql",
@@ -741,6 +747,14 @@ sub InstallTemp
 	return;
 }
 
+sub CleanupTablespaceDirectory
+{
+	my $tablespace = 'testtablespace';
+
+	rmtree($tablespace) if (-e $tablespace);
+	mkdir($tablespace);
+}
+
 sub usage
 {
 	print STDERR
-- 
2.18.2


----Next_Part(Mon_May_11_17_13_54_2020_626)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="0002-Don-t-setup-tablespace-directory-while-testing-pg_up.patch"



^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* [PATCH 1/2] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)

Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make, which is ugly. In addition to that, pg_regress does
the clean up twice at once. This patch moves the cleanup code out of
pg_regress into vcregress.pl, which is more sutable place.
---
 src/test/regress/pg_regress.c | 22 ----------------------
 src/tools/msvc/vcregress.pl   | 14 ++++++++++++++
 2 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
 
 	snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
 
-#ifdef WIN32
-
-	/*
-	 * On Windows only, clean out the test tablespace dir, or create it if it
-	 * doesn't exist.  On other platforms we expect the Makefile to take care
-	 * of that.  (We don't migrate that functionality in here because it'd be
-	 * harder to cope with platform-specific issues such as SELinux.)
-	 *
-	 * XXX it would be better if pg_regress.c had nothing at all to do with
-	 * testtablespace, and this were handled by a .BAT file or similar on
-	 * Windows.  See pgsql-hackers discussion of 2008-01-18.
-	 */
-	if (directory_exists(testtablespace))
-		if (!rmtree(testtablespace, true))
-		{
-			fprintf(stderr, _("\n%s: could not remove test tablespace \"%s\"\n"),
-					progname, testtablespace);
-			exit(2);
-		}
-	make_directory(testtablespace);
-#endif
-
 	/* finally loop on each file and do the replacement */
 	for (name = names; *name; name++)
 	{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f95f7a5c7a..74d37a31de 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -104,6 +104,7 @@ exit 0;
 sub installcheck_internal
 {
 	my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+
 	my @args = (
 		"../../../$Config/pg_regress/pg_regress",
 		"--dlpath=.",
@@ -114,6 +115,7 @@ sub installcheck_internal
 		"--no-locale");
 	push(@args, $maxconn) if $maxconn;
 	push(@args, @EXTRA_REGRESS_OPTS);
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -143,6 +145,7 @@ sub check
 		"--temp-instance=./tmp_check");
 	push(@args, $maxconn)     if $maxconn;
 	push(@args, $temp_config) if $temp_config;
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -178,6 +181,7 @@ sub ecpgcheck
 sub isolationcheck
 {
 	chdir "../isolation";
+	CleanupTablespaceDirectory();
 	copy("../../../$Config/isolationtester/isolationtester.exe",
 		"../../../$Config/pg_isolation_regress");
 	my @args = (
@@ -382,6 +386,7 @@ sub plcheck
 			"$topdir/$Config/pg_regress/pg_regress",
 			"--bindir=$topdir/$Config/psql",
 			"--dbname=pl_regression", @lang_args, @tests);
+		CleanupTablespaceDirectory();
 		system(@args);
 		my $status = $? >> 8;
 		exit $status if $status;
@@ -439,6 +444,7 @@ sub subdircheck
 
 	print "============================================================\n";
 	print "Checking $module\n";
+	CleanupTablespaceDirectory();
 	my @args = (
 		"$topdir/$Config/pg_regress/pg_regress",
 		"--bindir=${topdir}/${Config}/psql",
@@ -741,6 +747,14 @@ sub InstallTemp
 	return;
 }
 
+sub CleanupTablespaceDirectory
+{
+	my $tablespace = 'testtablespace';
+
+	rmtree($tablespace) if (-e $tablespace);
+	mkdir($tablespace);
+}
+
 sub usage
 {
 	print STDERR
-- 
2.18.2


----Next_Part(Mon_May_11_17_13_54_2020_626)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="0002-Don-t-setup-tablespace-directory-while-testing-pg_up.patch"



^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* [PATCH 1/2] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)

Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make, which is ugly. In addition to that, pg_regress does
the clean up twice at once. This patch moves the cleanup code out of
pg_regress into vcregress.pl, which is more sutable place.
---
 src/test/regress/pg_regress.c | 22 ----------------------
 src/tools/msvc/vcregress.pl   | 14 ++++++++++++++
 2 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
 
 	snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
 
-#ifdef WIN32
-
-	/*
-	 * On Windows only, clean out the test tablespace dir, or create it if it
-	 * doesn't exist.  On other platforms we expect the Makefile to take care
-	 * of that.  (We don't migrate that functionality in here because it'd be
-	 * harder to cope with platform-specific issues such as SELinux.)
-	 *
-	 * XXX it would be better if pg_regress.c had nothing at all to do with
-	 * testtablespace, and this were handled by a .BAT file or similar on
-	 * Windows.  See pgsql-hackers discussion of 2008-01-18.
-	 */
-	if (directory_exists(testtablespace))
-		if (!rmtree(testtablespace, true))
-		{
-			fprintf(stderr, _("\n%s: could not remove test tablespace \"%s\"\n"),
-					progname, testtablespace);
-			exit(2);
-		}
-	make_directory(testtablespace);
-#endif
-
 	/* finally loop on each file and do the replacement */
 	for (name = names; *name; name++)
 	{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f95f7a5c7a..74d37a31de 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -104,6 +104,7 @@ exit 0;
 sub installcheck_internal
 {
 	my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+
 	my @args = (
 		"../../../$Config/pg_regress/pg_regress",
 		"--dlpath=.",
@@ -114,6 +115,7 @@ sub installcheck_internal
 		"--no-locale");
 	push(@args, $maxconn) if $maxconn;
 	push(@args, @EXTRA_REGRESS_OPTS);
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -143,6 +145,7 @@ sub check
 		"--temp-instance=./tmp_check");
 	push(@args, $maxconn)     if $maxconn;
 	push(@args, $temp_config) if $temp_config;
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -178,6 +181,7 @@ sub ecpgcheck
 sub isolationcheck
 {
 	chdir "../isolation";
+	CleanupTablespaceDirectory();
 	copy("../../../$Config/isolationtester/isolationtester.exe",
 		"../../../$Config/pg_isolation_regress");
 	my @args = (
@@ -382,6 +386,7 @@ sub plcheck
 			"$topdir/$Config/pg_regress/pg_regress",
 			"--bindir=$topdir/$Config/psql",
 			"--dbname=pl_regression", @lang_args, @tests);
+		CleanupTablespaceDirectory();
 		system(@args);
 		my $status = $? >> 8;
 		exit $status if $status;
@@ -439,6 +444,7 @@ sub subdircheck
 
 	print "============================================================\n";
 	print "Checking $module\n";
+	CleanupTablespaceDirectory();
 	my @args = (
 		"$topdir/$Config/pg_regress/pg_regress",
 		"--bindir=${topdir}/${Config}/psql",
@@ -741,6 +747,14 @@ sub InstallTemp
 	return;
 }
 
+sub CleanupTablespaceDirectory
+{
+	my $tablespace = 'testtablespace';
+
+	rmtree($tablespace) if (-e $tablespace);
+	mkdir($tablespace);
+}
+
 sub usage
 {
 	print STDERR
-- 
2.18.2


----Next_Part(Mon_May_11_17_13_54_2020_626)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="0002-Don-t-setup-tablespace-directory-while-testing-pg_up.patch"



^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* [PATCH 1/2] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)

Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make, which is ugly. In addition to that, pg_regress does
the clean up twice at once. This patch moves the cleanup code out of
pg_regress into vcregress.pl, which is more sutable place.
---
 src/test/regress/pg_regress.c | 22 ----------------------
 src/tools/msvc/vcregress.pl   | 14 ++++++++++++++
 2 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
 
 	snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
 
-#ifdef WIN32
-
-	/*
-	 * On Windows only, clean out the test tablespace dir, or create it if it
-	 * doesn't exist.  On other platforms we expect the Makefile to take care
-	 * of that.  (We don't migrate that functionality in here because it'd be
-	 * harder to cope with platform-specific issues such as SELinux.)
-	 *
-	 * XXX it would be better if pg_regress.c had nothing at all to do with
-	 * testtablespace, and this were handled by a .BAT file or similar on
-	 * Windows.  See pgsql-hackers discussion of 2008-01-18.
-	 */
-	if (directory_exists(testtablespace))
-		if (!rmtree(testtablespace, true))
-		{
-			fprintf(stderr, _("\n%s: could not remove test tablespace \"%s\"\n"),
-					progname, testtablespace);
-			exit(2);
-		}
-	make_directory(testtablespace);
-#endif
-
 	/* finally loop on each file and do the replacement */
 	for (name = names; *name; name++)
 	{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f95f7a5c7a..74d37a31de 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -104,6 +104,7 @@ exit 0;
 sub installcheck_internal
 {
 	my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+
 	my @args = (
 		"../../../$Config/pg_regress/pg_regress",
 		"--dlpath=.",
@@ -114,6 +115,7 @@ sub installcheck_internal
 		"--no-locale");
 	push(@args, $maxconn) if $maxconn;
 	push(@args, @EXTRA_REGRESS_OPTS);
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -143,6 +145,7 @@ sub check
 		"--temp-instance=./tmp_check");
 	push(@args, $maxconn)     if $maxconn;
 	push(@args, $temp_config) if $temp_config;
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -178,6 +181,7 @@ sub ecpgcheck
 sub isolationcheck
 {
 	chdir "../isolation";
+	CleanupTablespaceDirectory();
 	copy("../../../$Config/isolationtester/isolationtester.exe",
 		"../../../$Config/pg_isolation_regress");
 	my @args = (
@@ -382,6 +386,7 @@ sub plcheck
 			"$topdir/$Config/pg_regress/pg_regress",
 			"--bindir=$topdir/$Config/psql",
 			"--dbname=pl_regression", @lang_args, @tests);
+		CleanupTablespaceDirectory();
 		system(@args);
 		my $status = $? >> 8;
 		exit $status if $status;
@@ -439,6 +444,7 @@ sub subdircheck
 
 	print "============================================================\n";
 	print "Checking $module\n";
+	CleanupTablespaceDirectory();
 	my @args = (
 		"$topdir/$Config/pg_regress/pg_regress",
 		"--bindir=${topdir}/${Config}/psql",
@@ -741,6 +747,14 @@ sub InstallTemp
 	return;
 }
 
+sub CleanupTablespaceDirectory
+{
+	my $tablespace = 'testtablespace';
+
+	rmtree($tablespace) if (-e $tablespace);
+	mkdir($tablespace);
+}
+
 sub usage
 {
 	print STDERR
-- 
2.18.2


----Next_Part(Mon_May_11_17_13_54_2020_626)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="0002-Don-t-setup-tablespace-directory-while-testing-pg_up.patch"



^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* [PATCH 1/2] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)

Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make, which is ugly. In addition to that, pg_regress does
the clean up twice at once. This patch moves the cleanup code out of
pg_regress into vcregress.pl, which is more sutable place.
---
 src/test/regress/pg_regress.c | 22 ----------------------
 src/tools/msvc/vcregress.pl   | 14 ++++++++++++++
 2 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
 
 	snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
 
-#ifdef WIN32
-
-	/*
-	 * On Windows only, clean out the test tablespace dir, or create it if it
-	 * doesn't exist.  On other platforms we expect the Makefile to take care
-	 * of that.  (We don't migrate that functionality in here because it'd be
-	 * harder to cope with platform-specific issues such as SELinux.)
-	 *
-	 * XXX it would be better if pg_regress.c had nothing at all to do with
-	 * testtablespace, and this were handled by a .BAT file or similar on
-	 * Windows.  See pgsql-hackers discussion of 2008-01-18.
-	 */
-	if (directory_exists(testtablespace))
-		if (!rmtree(testtablespace, true))
-		{
-			fprintf(stderr, _("\n%s: could not remove test tablespace \"%s\"\n"),
-					progname, testtablespace);
-			exit(2);
-		}
-	make_directory(testtablespace);
-#endif
-
 	/* finally loop on each file and do the replacement */
 	for (name = names; *name; name++)
 	{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f95f7a5c7a..74d37a31de 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -104,6 +104,7 @@ exit 0;
 sub installcheck_internal
 {
 	my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+
 	my @args = (
 		"../../../$Config/pg_regress/pg_regress",
 		"--dlpath=.",
@@ -114,6 +115,7 @@ sub installcheck_internal
 		"--no-locale");
 	push(@args, $maxconn) if $maxconn;
 	push(@args, @EXTRA_REGRESS_OPTS);
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -143,6 +145,7 @@ sub check
 		"--temp-instance=./tmp_check");
 	push(@args, $maxconn)     if $maxconn;
 	push(@args, $temp_config) if $temp_config;
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -178,6 +181,7 @@ sub ecpgcheck
 sub isolationcheck
 {
 	chdir "../isolation";
+	CleanupTablespaceDirectory();
 	copy("../../../$Config/isolationtester/isolationtester.exe",
 		"../../../$Config/pg_isolation_regress");
 	my @args = (
@@ -382,6 +386,7 @@ sub plcheck
 			"$topdir/$Config/pg_regress/pg_regress",
 			"--bindir=$topdir/$Config/psql",
 			"--dbname=pl_regression", @lang_args, @tests);
+		CleanupTablespaceDirectory();
 		system(@args);
 		my $status = $? >> 8;
 		exit $status if $status;
@@ -439,6 +444,7 @@ sub subdircheck
 
 	print "============================================================\n";
 	print "Checking $module\n";
+	CleanupTablespaceDirectory();
 	my @args = (
 		"$topdir/$Config/pg_regress/pg_regress",
 		"--bindir=${topdir}/${Config}/psql",
@@ -741,6 +747,14 @@ sub InstallTemp
 	return;
 }
 
+sub CleanupTablespaceDirectory
+{
+	my $tablespace = 'testtablespace';
+
+	rmtree($tablespace) if (-e $tablespace);
+	mkdir($tablespace);
+}
+
 sub usage
 {
 	print STDERR
-- 
2.18.2


----Next_Part(Mon_May_11_17_13_54_2020_626)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="0002-Don-t-setup-tablespace-directory-while-testing-pg_up.patch"



^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* [PATCH 1/2] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)

Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make, which is ugly. In addition to that, pg_regress does
the clean up twice at once. This patch moves the cleanup code out of
pg_regress into vcregress.pl, which is more sutable place.
---
 src/test/regress/pg_regress.c | 22 ----------------------
 src/tools/msvc/vcregress.pl   | 14 ++++++++++++++
 2 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
 
 	snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
 
-#ifdef WIN32
-
-	/*
-	 * On Windows only, clean out the test tablespace dir, or create it if it
-	 * doesn't exist.  On other platforms we expect the Makefile to take care
-	 * of that.  (We don't migrate that functionality in here because it'd be
-	 * harder to cope with platform-specific issues such as SELinux.)
-	 *
-	 * XXX it would be better if pg_regress.c had nothing at all to do with
-	 * testtablespace, and this were handled by a .BAT file or similar on
-	 * Windows.  See pgsql-hackers discussion of 2008-01-18.
-	 */
-	if (directory_exists(testtablespace))
-		if (!rmtree(testtablespace, true))
-		{
-			fprintf(stderr, _("\n%s: could not remove test tablespace \"%s\"\n"),
-					progname, testtablespace);
-			exit(2);
-		}
-	make_directory(testtablespace);
-#endif
-
 	/* finally loop on each file and do the replacement */
 	for (name = names; *name; name++)
 	{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f95f7a5c7a..74d37a31de 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -104,6 +104,7 @@ exit 0;
 sub installcheck_internal
 {
 	my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+
 	my @args = (
 		"../../../$Config/pg_regress/pg_regress",
 		"--dlpath=.",
@@ -114,6 +115,7 @@ sub installcheck_internal
 		"--no-locale");
 	push(@args, $maxconn) if $maxconn;
 	push(@args, @EXTRA_REGRESS_OPTS);
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -143,6 +145,7 @@ sub check
 		"--temp-instance=./tmp_check");
 	push(@args, $maxconn)     if $maxconn;
 	push(@args, $temp_config) if $temp_config;
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -178,6 +181,7 @@ sub ecpgcheck
 sub isolationcheck
 {
 	chdir "../isolation";
+	CleanupTablespaceDirectory();
 	copy("../../../$Config/isolationtester/isolationtester.exe",
 		"../../../$Config/pg_isolation_regress");
 	my @args = (
@@ -382,6 +386,7 @@ sub plcheck
 			"$topdir/$Config/pg_regress/pg_regress",
 			"--bindir=$topdir/$Config/psql",
 			"--dbname=pl_regression", @lang_args, @tests);
+		CleanupTablespaceDirectory();
 		system(@args);
 		my $status = $? >> 8;
 		exit $status if $status;
@@ -439,6 +444,7 @@ sub subdircheck
 
 	print "============================================================\n";
 	print "Checking $module\n";
+	CleanupTablespaceDirectory();
 	my @args = (
 		"$topdir/$Config/pg_regress/pg_regress",
 		"--bindir=${topdir}/${Config}/psql",
@@ -741,6 +747,14 @@ sub InstallTemp
 	return;
 }
 
+sub CleanupTablespaceDirectory
+{
+	my $tablespace = 'testtablespace';
+
+	rmtree($tablespace) if (-e $tablespace);
+	mkdir($tablespace);
+}
+
 sub usage
 {
 	print STDERR
-- 
2.18.2


----Next_Part(Mon_May_11_17_13_54_2020_626)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="0002-Don-t-setup-tablespace-directory-while-testing-pg_up.patch"



^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* [PATCH 1/2] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)

Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make, which is ugly. In addition to that, pg_regress does
the clean up twice at once. This patch moves the cleanup code out of
pg_regress into vcregress.pl, which is more sutable place.
---
 src/test/regress/pg_regress.c | 22 ----------------------
 src/tools/msvc/vcregress.pl   | 14 ++++++++++++++
 2 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
 
 	snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
 
-#ifdef WIN32
-
-	/*
-	 * On Windows only, clean out the test tablespace dir, or create it if it
-	 * doesn't exist.  On other platforms we expect the Makefile to take care
-	 * of that.  (We don't migrate that functionality in here because it'd be
-	 * harder to cope with platform-specific issues such as SELinux.)
-	 *
-	 * XXX it would be better if pg_regress.c had nothing at all to do with
-	 * testtablespace, and this were handled by a .BAT file or similar on
-	 * Windows.  See pgsql-hackers discussion of 2008-01-18.
-	 */
-	if (directory_exists(testtablespace))
-		if (!rmtree(testtablespace, true))
-		{
-			fprintf(stderr, _("\n%s: could not remove test tablespace \"%s\"\n"),
-					progname, testtablespace);
-			exit(2);
-		}
-	make_directory(testtablespace);
-#endif
-
 	/* finally loop on each file and do the replacement */
 	for (name = names; *name; name++)
 	{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f95f7a5c7a..74d37a31de 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -104,6 +104,7 @@ exit 0;
 sub installcheck_internal
 {
 	my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+
 	my @args = (
 		"../../../$Config/pg_regress/pg_regress",
 		"--dlpath=.",
@@ -114,6 +115,7 @@ sub installcheck_internal
 		"--no-locale");
 	push(@args, $maxconn) if $maxconn;
 	push(@args, @EXTRA_REGRESS_OPTS);
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -143,6 +145,7 @@ sub check
 		"--temp-instance=./tmp_check");
 	push(@args, $maxconn)     if $maxconn;
 	push(@args, $temp_config) if $temp_config;
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -178,6 +181,7 @@ sub ecpgcheck
 sub isolationcheck
 {
 	chdir "../isolation";
+	CleanupTablespaceDirectory();
 	copy("../../../$Config/isolationtester/isolationtester.exe",
 		"../../../$Config/pg_isolation_regress");
 	my @args = (
@@ -382,6 +386,7 @@ sub plcheck
 			"$topdir/$Config/pg_regress/pg_regress",
 			"--bindir=$topdir/$Config/psql",
 			"--dbname=pl_regression", @lang_args, @tests);
+		CleanupTablespaceDirectory();
 		system(@args);
 		my $status = $? >> 8;
 		exit $status if $status;
@@ -439,6 +444,7 @@ sub subdircheck
 
 	print "============================================================\n";
 	print "Checking $module\n";
+	CleanupTablespaceDirectory();
 	my @args = (
 		"$topdir/$Config/pg_regress/pg_regress",
 		"--bindir=${topdir}/${Config}/psql",
@@ -741,6 +747,14 @@ sub InstallTemp
 	return;
 }
 
+sub CleanupTablespaceDirectory
+{
+	my $tablespace = 'testtablespace';
+
+	rmtree($tablespace) if (-e $tablespace);
+	mkdir($tablespace);
+}
+
 sub usage
 {
 	print STDERR
-- 
2.18.2


----Next_Part(Mon_May_11_17_13_54_2020_626)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="0002-Don-t-setup-tablespace-directory-while-testing-pg_up.patch"



^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* [PATCH 1/2] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)

Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make, which is ugly. In addition to that, pg_regress does
the clean up twice at once. This patch moves the cleanup code out of
pg_regress into vcregress.pl, which is more sutable place.
---
 src/test/regress/pg_regress.c | 22 ----------------------
 src/tools/msvc/vcregress.pl   | 14 ++++++++++++++
 2 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
 
 	snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
 
-#ifdef WIN32
-
-	/*
-	 * On Windows only, clean out the test tablespace dir, or create it if it
-	 * doesn't exist.  On other platforms we expect the Makefile to take care
-	 * of that.  (We don't migrate that functionality in here because it'd be
-	 * harder to cope with platform-specific issues such as SELinux.)
-	 *
-	 * XXX it would be better if pg_regress.c had nothing at all to do with
-	 * testtablespace, and this were handled by a .BAT file or similar on
-	 * Windows.  See pgsql-hackers discussion of 2008-01-18.
-	 */
-	if (directory_exists(testtablespace))
-		if (!rmtree(testtablespace, true))
-		{
-			fprintf(stderr, _("\n%s: could not remove test tablespace \"%s\"\n"),
-					progname, testtablespace);
-			exit(2);
-		}
-	make_directory(testtablespace);
-#endif
-
 	/* finally loop on each file and do the replacement */
 	for (name = names; *name; name++)
 	{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f95f7a5c7a..74d37a31de 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -104,6 +104,7 @@ exit 0;
 sub installcheck_internal
 {
 	my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+
 	my @args = (
 		"../../../$Config/pg_regress/pg_regress",
 		"--dlpath=.",
@@ -114,6 +115,7 @@ sub installcheck_internal
 		"--no-locale");
 	push(@args, $maxconn) if $maxconn;
 	push(@args, @EXTRA_REGRESS_OPTS);
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -143,6 +145,7 @@ sub check
 		"--temp-instance=./tmp_check");
 	push(@args, $maxconn)     if $maxconn;
 	push(@args, $temp_config) if $temp_config;
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -178,6 +181,7 @@ sub ecpgcheck
 sub isolationcheck
 {
 	chdir "../isolation";
+	CleanupTablespaceDirectory();
 	copy("../../../$Config/isolationtester/isolationtester.exe",
 		"../../../$Config/pg_isolation_regress");
 	my @args = (
@@ -382,6 +386,7 @@ sub plcheck
 			"$topdir/$Config/pg_regress/pg_regress",
 			"--bindir=$topdir/$Config/psql",
 			"--dbname=pl_regression", @lang_args, @tests);
+		CleanupTablespaceDirectory();
 		system(@args);
 		my $status = $? >> 8;
 		exit $status if $status;
@@ -439,6 +444,7 @@ sub subdircheck
 
 	print "============================================================\n";
 	print "Checking $module\n";
+	CleanupTablespaceDirectory();
 	my @args = (
 		"$topdir/$Config/pg_regress/pg_regress",
 		"--bindir=${topdir}/${Config}/psql",
@@ -741,6 +747,14 @@ sub InstallTemp
 	return;
 }
 
+sub CleanupTablespaceDirectory
+{
+	my $tablespace = 'testtablespace';
+
+	rmtree($tablespace) if (-e $tablespace);
+	mkdir($tablespace);
+}
+
 sub usage
 {
 	print STDERR
-- 
2.18.2


----Next_Part(Mon_May_11_17_13_54_2020_626)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="0002-Don-t-setup-tablespace-directory-while-testing-pg_up.patch"



^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* [PATCH 1/2] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)

Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make, which is ugly. In addition to that, pg_regress does
the clean up twice at once. This patch moves the cleanup code out of
pg_regress into vcregress.pl, which is more sutable place.
---
 src/test/regress/pg_regress.c | 22 ----------------------
 src/tools/msvc/vcregress.pl   | 14 ++++++++++++++
 2 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
 
 	snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
 
-#ifdef WIN32
-
-	/*
-	 * On Windows only, clean out the test tablespace dir, or create it if it
-	 * doesn't exist.  On other platforms we expect the Makefile to take care
-	 * of that.  (We don't migrate that functionality in here because it'd be
-	 * harder to cope with platform-specific issues such as SELinux.)
-	 *
-	 * XXX it would be better if pg_regress.c had nothing at all to do with
-	 * testtablespace, and this were handled by a .BAT file or similar on
-	 * Windows.  See pgsql-hackers discussion of 2008-01-18.
-	 */
-	if (directory_exists(testtablespace))
-		if (!rmtree(testtablespace, true))
-		{
-			fprintf(stderr, _("\n%s: could not remove test tablespace \"%s\"\n"),
-					progname, testtablespace);
-			exit(2);
-		}
-	make_directory(testtablespace);
-#endif
-
 	/* finally loop on each file and do the replacement */
 	for (name = names; *name; name++)
 	{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f95f7a5c7a..74d37a31de 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -104,6 +104,7 @@ exit 0;
 sub installcheck_internal
 {
 	my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+
 	my @args = (
 		"../../../$Config/pg_regress/pg_regress",
 		"--dlpath=.",
@@ -114,6 +115,7 @@ sub installcheck_internal
 		"--no-locale");
 	push(@args, $maxconn) if $maxconn;
 	push(@args, @EXTRA_REGRESS_OPTS);
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -143,6 +145,7 @@ sub check
 		"--temp-instance=./tmp_check");
 	push(@args, $maxconn)     if $maxconn;
 	push(@args, $temp_config) if $temp_config;
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -178,6 +181,7 @@ sub ecpgcheck
 sub isolationcheck
 {
 	chdir "../isolation";
+	CleanupTablespaceDirectory();
 	copy("../../../$Config/isolationtester/isolationtester.exe",
 		"../../../$Config/pg_isolation_regress");
 	my @args = (
@@ -382,6 +386,7 @@ sub plcheck
 			"$topdir/$Config/pg_regress/pg_regress",
 			"--bindir=$topdir/$Config/psql",
 			"--dbname=pl_regression", @lang_args, @tests);
+		CleanupTablespaceDirectory();
 		system(@args);
 		my $status = $? >> 8;
 		exit $status if $status;
@@ -439,6 +444,7 @@ sub subdircheck
 
 	print "============================================================\n";
 	print "Checking $module\n";
+	CleanupTablespaceDirectory();
 	my @args = (
 		"$topdir/$Config/pg_regress/pg_regress",
 		"--bindir=${topdir}/${Config}/psql",
@@ -741,6 +747,14 @@ sub InstallTemp
 	return;
 }
 
+sub CleanupTablespaceDirectory
+{
+	my $tablespace = 'testtablespace';
+
+	rmtree($tablespace) if (-e $tablespace);
+	mkdir($tablespace);
+}
+
 sub usage
 {
 	print STDERR
-- 
2.18.2


----Next_Part(Mon_May_11_17_13_54_2020_626)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="0002-Don-t-setup-tablespace-directory-while-testing-pg_up.patch"



^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* [PATCH 1/2] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)

Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make, which is ugly. In addition to that, pg_regress does
the clean up twice at once. This patch moves the cleanup code out of
pg_regress into vcregress.pl, which is more sutable place.
---
 src/test/regress/pg_regress.c | 22 ----------------------
 src/tools/msvc/vcregress.pl   | 14 ++++++++++++++
 2 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
 
 	snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
 
-#ifdef WIN32
-
-	/*
-	 * On Windows only, clean out the test tablespace dir, or create it if it
-	 * doesn't exist.  On other platforms we expect the Makefile to take care
-	 * of that.  (We don't migrate that functionality in here because it'd be
-	 * harder to cope with platform-specific issues such as SELinux.)
-	 *
-	 * XXX it would be better if pg_regress.c had nothing at all to do with
-	 * testtablespace, and this were handled by a .BAT file or similar on
-	 * Windows.  See pgsql-hackers discussion of 2008-01-18.
-	 */
-	if (directory_exists(testtablespace))
-		if (!rmtree(testtablespace, true))
-		{
-			fprintf(stderr, _("\n%s: could not remove test tablespace \"%s\"\n"),
-					progname, testtablespace);
-			exit(2);
-		}
-	make_directory(testtablespace);
-#endif
-
 	/* finally loop on each file and do the replacement */
 	for (name = names; *name; name++)
 	{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f95f7a5c7a..74d37a31de 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -104,6 +104,7 @@ exit 0;
 sub installcheck_internal
 {
 	my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+
 	my @args = (
 		"../../../$Config/pg_regress/pg_regress",
 		"--dlpath=.",
@@ -114,6 +115,7 @@ sub installcheck_internal
 		"--no-locale");
 	push(@args, $maxconn) if $maxconn;
 	push(@args, @EXTRA_REGRESS_OPTS);
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -143,6 +145,7 @@ sub check
 		"--temp-instance=./tmp_check");
 	push(@args, $maxconn)     if $maxconn;
 	push(@args, $temp_config) if $temp_config;
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -178,6 +181,7 @@ sub ecpgcheck
 sub isolationcheck
 {
 	chdir "../isolation";
+	CleanupTablespaceDirectory();
 	copy("../../../$Config/isolationtester/isolationtester.exe",
 		"../../../$Config/pg_isolation_regress");
 	my @args = (
@@ -382,6 +386,7 @@ sub plcheck
 			"$topdir/$Config/pg_regress/pg_regress",
 			"--bindir=$topdir/$Config/psql",
 			"--dbname=pl_regression", @lang_args, @tests);
+		CleanupTablespaceDirectory();
 		system(@args);
 		my $status = $? >> 8;
 		exit $status if $status;
@@ -439,6 +444,7 @@ sub subdircheck
 
 	print "============================================================\n";
 	print "Checking $module\n";
+	CleanupTablespaceDirectory();
 	my @args = (
 		"$topdir/$Config/pg_regress/pg_regress",
 		"--bindir=${topdir}/${Config}/psql",
@@ -741,6 +747,14 @@ sub InstallTemp
 	return;
 }
 
+sub CleanupTablespaceDirectory
+{
+	my $tablespace = 'testtablespace';
+
+	rmtree($tablespace) if (-e $tablespace);
+	mkdir($tablespace);
+}
+
 sub usage
 {
 	print STDERR
-- 
2.18.2


----Next_Part(Mon_May_11_17_13_54_2020_626)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="0002-Don-t-setup-tablespace-directory-while-testing-pg_up.patch"



^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* [PATCH 1/2] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)

Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make, which is ugly. In addition to that, pg_regress does
the clean up twice at once. This patch moves the cleanup code out of
pg_regress into vcregress.pl, which is more sutable place.
---
 src/test/regress/pg_regress.c | 22 ----------------------
 src/tools/msvc/vcregress.pl   | 14 ++++++++++++++
 2 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
 
 	snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
 
-#ifdef WIN32
-
-	/*
-	 * On Windows only, clean out the test tablespace dir, or create it if it
-	 * doesn't exist.  On other platforms we expect the Makefile to take care
-	 * of that.  (We don't migrate that functionality in here because it'd be
-	 * harder to cope with platform-specific issues such as SELinux.)
-	 *
-	 * XXX it would be better if pg_regress.c had nothing at all to do with
-	 * testtablespace, and this were handled by a .BAT file or similar on
-	 * Windows.  See pgsql-hackers discussion of 2008-01-18.
-	 */
-	if (directory_exists(testtablespace))
-		if (!rmtree(testtablespace, true))
-		{
-			fprintf(stderr, _("\n%s: could not remove test tablespace \"%s\"\n"),
-					progname, testtablespace);
-			exit(2);
-		}
-	make_directory(testtablespace);
-#endif
-
 	/* finally loop on each file and do the replacement */
 	for (name = names; *name; name++)
 	{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f95f7a5c7a..74d37a31de 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -104,6 +104,7 @@ exit 0;
 sub installcheck_internal
 {
 	my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+
 	my @args = (
 		"../../../$Config/pg_regress/pg_regress",
 		"--dlpath=.",
@@ -114,6 +115,7 @@ sub installcheck_internal
 		"--no-locale");
 	push(@args, $maxconn) if $maxconn;
 	push(@args, @EXTRA_REGRESS_OPTS);
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -143,6 +145,7 @@ sub check
 		"--temp-instance=./tmp_check");
 	push(@args, $maxconn)     if $maxconn;
 	push(@args, $temp_config) if $temp_config;
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -178,6 +181,7 @@ sub ecpgcheck
 sub isolationcheck
 {
 	chdir "../isolation";
+	CleanupTablespaceDirectory();
 	copy("../../../$Config/isolationtester/isolationtester.exe",
 		"../../../$Config/pg_isolation_regress");
 	my @args = (
@@ -382,6 +386,7 @@ sub plcheck
 			"$topdir/$Config/pg_regress/pg_regress",
 			"--bindir=$topdir/$Config/psql",
 			"--dbname=pl_regression", @lang_args, @tests);
+		CleanupTablespaceDirectory();
 		system(@args);
 		my $status = $? >> 8;
 		exit $status if $status;
@@ -439,6 +444,7 @@ sub subdircheck
 
 	print "============================================================\n";
 	print "Checking $module\n";
+	CleanupTablespaceDirectory();
 	my @args = (
 		"$topdir/$Config/pg_regress/pg_regress",
 		"--bindir=${topdir}/${Config}/psql",
@@ -741,6 +747,14 @@ sub InstallTemp
 	return;
 }
 
+sub CleanupTablespaceDirectory
+{
+	my $tablespace = 'testtablespace';
+
+	rmtree($tablespace) if (-e $tablespace);
+	mkdir($tablespace);
+}
+
 sub usage
 {
 	print STDERR
-- 
2.18.2


----Next_Part(Mon_May_11_17_13_54_2020_626)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="0002-Don-t-setup-tablespace-directory-while-testing-pg_up.patch"



^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* [PATCH 1/2] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)

Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make, which is ugly. In addition to that, pg_regress does
the clean up twice at once. This patch moves the cleanup code out of
pg_regress into vcregress.pl, which is more sutable place.
---
 src/test/regress/pg_regress.c | 22 ----------------------
 src/tools/msvc/vcregress.pl   | 14 ++++++++++++++
 2 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
 
 	snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
 
-#ifdef WIN32
-
-	/*
-	 * On Windows only, clean out the test tablespace dir, or create it if it
-	 * doesn't exist.  On other platforms we expect the Makefile to take care
-	 * of that.  (We don't migrate that functionality in here because it'd be
-	 * harder to cope with platform-specific issues such as SELinux.)
-	 *
-	 * XXX it would be better if pg_regress.c had nothing at all to do with
-	 * testtablespace, and this were handled by a .BAT file or similar on
-	 * Windows.  See pgsql-hackers discussion of 2008-01-18.
-	 */
-	if (directory_exists(testtablespace))
-		if (!rmtree(testtablespace, true))
-		{
-			fprintf(stderr, _("\n%s: could not remove test tablespace \"%s\"\n"),
-					progname, testtablespace);
-			exit(2);
-		}
-	make_directory(testtablespace);
-#endif
-
 	/* finally loop on each file and do the replacement */
 	for (name = names; *name; name++)
 	{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f95f7a5c7a..74d37a31de 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -104,6 +104,7 @@ exit 0;
 sub installcheck_internal
 {
 	my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+
 	my @args = (
 		"../../../$Config/pg_regress/pg_regress",
 		"--dlpath=.",
@@ -114,6 +115,7 @@ sub installcheck_internal
 		"--no-locale");
 	push(@args, $maxconn) if $maxconn;
 	push(@args, @EXTRA_REGRESS_OPTS);
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -143,6 +145,7 @@ sub check
 		"--temp-instance=./tmp_check");
 	push(@args, $maxconn)     if $maxconn;
 	push(@args, $temp_config) if $temp_config;
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -178,6 +181,7 @@ sub ecpgcheck
 sub isolationcheck
 {
 	chdir "../isolation";
+	CleanupTablespaceDirectory();
 	copy("../../../$Config/isolationtester/isolationtester.exe",
 		"../../../$Config/pg_isolation_regress");
 	my @args = (
@@ -382,6 +386,7 @@ sub plcheck
 			"$topdir/$Config/pg_regress/pg_regress",
 			"--bindir=$topdir/$Config/psql",
 			"--dbname=pl_regression", @lang_args, @tests);
+		CleanupTablespaceDirectory();
 		system(@args);
 		my $status = $? >> 8;
 		exit $status if $status;
@@ -439,6 +444,7 @@ sub subdircheck
 
 	print "============================================================\n";
 	print "Checking $module\n";
+	CleanupTablespaceDirectory();
 	my @args = (
 		"$topdir/$Config/pg_regress/pg_regress",
 		"--bindir=${topdir}/${Config}/psql",
@@ -741,6 +747,14 @@ sub InstallTemp
 	return;
 }
 
+sub CleanupTablespaceDirectory
+{
+	my $tablespace = 'testtablespace';
+
+	rmtree($tablespace) if (-e $tablespace);
+	mkdir($tablespace);
+}
+
 sub usage
 {
 	print STDERR
-- 
2.18.2


----Next_Part(Mon_May_11_17_13_54_2020_626)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="0002-Don-t-setup-tablespace-directory-while-testing-pg_up.patch"



^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* [PATCH 1/2] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)

Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make, which is ugly. In addition to that, pg_regress does
the clean up twice at once. This patch moves the cleanup code out of
pg_regress into vcregress.pl, which is more sutable place.
---
 src/test/regress/pg_regress.c | 22 ----------------------
 src/tools/msvc/vcregress.pl   | 14 ++++++++++++++
 2 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
 
 	snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
 
-#ifdef WIN32
-
-	/*
-	 * On Windows only, clean out the test tablespace dir, or create it if it
-	 * doesn't exist.  On other platforms we expect the Makefile to take care
-	 * of that.  (We don't migrate that functionality in here because it'd be
-	 * harder to cope with platform-specific issues such as SELinux.)
-	 *
-	 * XXX it would be better if pg_regress.c had nothing at all to do with
-	 * testtablespace, and this were handled by a .BAT file or similar on
-	 * Windows.  See pgsql-hackers discussion of 2008-01-18.
-	 */
-	if (directory_exists(testtablespace))
-		if (!rmtree(testtablespace, true))
-		{
-			fprintf(stderr, _("\n%s: could not remove test tablespace \"%s\"\n"),
-					progname, testtablespace);
-			exit(2);
-		}
-	make_directory(testtablespace);
-#endif
-
 	/* finally loop on each file and do the replacement */
 	for (name = names; *name; name++)
 	{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f95f7a5c7a..74d37a31de 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -104,6 +104,7 @@ exit 0;
 sub installcheck_internal
 {
 	my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+
 	my @args = (
 		"../../../$Config/pg_regress/pg_regress",
 		"--dlpath=.",
@@ -114,6 +115,7 @@ sub installcheck_internal
 		"--no-locale");
 	push(@args, $maxconn) if $maxconn;
 	push(@args, @EXTRA_REGRESS_OPTS);
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -143,6 +145,7 @@ sub check
 		"--temp-instance=./tmp_check");
 	push(@args, $maxconn)     if $maxconn;
 	push(@args, $temp_config) if $temp_config;
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -178,6 +181,7 @@ sub ecpgcheck
 sub isolationcheck
 {
 	chdir "../isolation";
+	CleanupTablespaceDirectory();
 	copy("../../../$Config/isolationtester/isolationtester.exe",
 		"../../../$Config/pg_isolation_regress");
 	my @args = (
@@ -382,6 +386,7 @@ sub plcheck
 			"$topdir/$Config/pg_regress/pg_regress",
 			"--bindir=$topdir/$Config/psql",
 			"--dbname=pl_regression", @lang_args, @tests);
+		CleanupTablespaceDirectory();
 		system(@args);
 		my $status = $? >> 8;
 		exit $status if $status;
@@ -439,6 +444,7 @@ sub subdircheck
 
 	print "============================================================\n";
 	print "Checking $module\n";
+	CleanupTablespaceDirectory();
 	my @args = (
 		"$topdir/$Config/pg_regress/pg_regress",
 		"--bindir=${topdir}/${Config}/psql",
@@ -741,6 +747,14 @@ sub InstallTemp
 	return;
 }
 
+sub CleanupTablespaceDirectory
+{
+	my $tablespace = 'testtablespace';
+
+	rmtree($tablespace) if (-e $tablespace);
+	mkdir($tablespace);
+}
+
 sub usage
 {
 	print STDERR
-- 
2.18.2


----Next_Part(Mon_May_11_17_13_54_2020_626)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="0002-Don-t-setup-tablespace-directory-while-testing-pg_up.patch"



^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* [PATCH 1/2] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)

Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make, which is ugly. In addition to that, pg_regress does
the clean up twice at once. This patch moves the cleanup code out of
pg_regress into vcregress.pl, which is more sutable place.
---
 src/test/regress/pg_regress.c | 22 ----------------------
 src/tools/msvc/vcregress.pl   | 14 ++++++++++++++
 2 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
 
 	snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
 
-#ifdef WIN32
-
-	/*
-	 * On Windows only, clean out the test tablespace dir, or create it if it
-	 * doesn't exist.  On other platforms we expect the Makefile to take care
-	 * of that.  (We don't migrate that functionality in here because it'd be
-	 * harder to cope with platform-specific issues such as SELinux.)
-	 *
-	 * XXX it would be better if pg_regress.c had nothing at all to do with
-	 * testtablespace, and this were handled by a .BAT file or similar on
-	 * Windows.  See pgsql-hackers discussion of 2008-01-18.
-	 */
-	if (directory_exists(testtablespace))
-		if (!rmtree(testtablespace, true))
-		{
-			fprintf(stderr, _("\n%s: could not remove test tablespace \"%s\"\n"),
-					progname, testtablespace);
-			exit(2);
-		}
-	make_directory(testtablespace);
-#endif
-
 	/* finally loop on each file and do the replacement */
 	for (name = names; *name; name++)
 	{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f95f7a5c7a..74d37a31de 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -104,6 +104,7 @@ exit 0;
 sub installcheck_internal
 {
 	my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+
 	my @args = (
 		"../../../$Config/pg_regress/pg_regress",
 		"--dlpath=.",
@@ -114,6 +115,7 @@ sub installcheck_internal
 		"--no-locale");
 	push(@args, $maxconn) if $maxconn;
 	push(@args, @EXTRA_REGRESS_OPTS);
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -143,6 +145,7 @@ sub check
 		"--temp-instance=./tmp_check");
 	push(@args, $maxconn)     if $maxconn;
 	push(@args, $temp_config) if $temp_config;
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -178,6 +181,7 @@ sub ecpgcheck
 sub isolationcheck
 {
 	chdir "../isolation";
+	CleanupTablespaceDirectory();
 	copy("../../../$Config/isolationtester/isolationtester.exe",
 		"../../../$Config/pg_isolation_regress");
 	my @args = (
@@ -382,6 +386,7 @@ sub plcheck
 			"$topdir/$Config/pg_regress/pg_regress",
 			"--bindir=$topdir/$Config/psql",
 			"--dbname=pl_regression", @lang_args, @tests);
+		CleanupTablespaceDirectory();
 		system(@args);
 		my $status = $? >> 8;
 		exit $status if $status;
@@ -439,6 +444,7 @@ sub subdircheck
 
 	print "============================================================\n";
 	print "Checking $module\n";
+	CleanupTablespaceDirectory();
 	my @args = (
 		"$topdir/$Config/pg_regress/pg_regress",
 		"--bindir=${topdir}/${Config}/psql",
@@ -741,6 +747,14 @@ sub InstallTemp
 	return;
 }
 
+sub CleanupTablespaceDirectory
+{
+	my $tablespace = 'testtablespace';
+
+	rmtree($tablespace) if (-e $tablespace);
+	mkdir($tablespace);
+}
+
 sub usage
 {
 	print STDERR
-- 
2.18.2


----Next_Part(Mon_May_11_17_13_54_2020_626)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="0002-Don-t-setup-tablespace-directory-while-testing-pg_up.patch"



^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* [PATCH 1/2] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)

Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make, which is ugly. In addition to that, pg_regress does
the clean up twice at once. This patch moves the cleanup code out of
pg_regress into vcregress.pl, which is more sutable place.
---
 src/test/regress/pg_regress.c | 22 ----------------------
 src/tools/msvc/vcregress.pl   | 14 ++++++++++++++
 2 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
 
 	snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
 
-#ifdef WIN32
-
-	/*
-	 * On Windows only, clean out the test tablespace dir, or create it if it
-	 * doesn't exist.  On other platforms we expect the Makefile to take care
-	 * of that.  (We don't migrate that functionality in here because it'd be
-	 * harder to cope with platform-specific issues such as SELinux.)
-	 *
-	 * XXX it would be better if pg_regress.c had nothing at all to do with
-	 * testtablespace, and this were handled by a .BAT file or similar on
-	 * Windows.  See pgsql-hackers discussion of 2008-01-18.
-	 */
-	if (directory_exists(testtablespace))
-		if (!rmtree(testtablespace, true))
-		{
-			fprintf(stderr, _("\n%s: could not remove test tablespace \"%s\"\n"),
-					progname, testtablespace);
-			exit(2);
-		}
-	make_directory(testtablespace);
-#endif
-
 	/* finally loop on each file and do the replacement */
 	for (name = names; *name; name++)
 	{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f95f7a5c7a..74d37a31de 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -104,6 +104,7 @@ exit 0;
 sub installcheck_internal
 {
 	my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+
 	my @args = (
 		"../../../$Config/pg_regress/pg_regress",
 		"--dlpath=.",
@@ -114,6 +115,7 @@ sub installcheck_internal
 		"--no-locale");
 	push(@args, $maxconn) if $maxconn;
 	push(@args, @EXTRA_REGRESS_OPTS);
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -143,6 +145,7 @@ sub check
 		"--temp-instance=./tmp_check");
 	push(@args, $maxconn)     if $maxconn;
 	push(@args, $temp_config) if $temp_config;
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -178,6 +181,7 @@ sub ecpgcheck
 sub isolationcheck
 {
 	chdir "../isolation";
+	CleanupTablespaceDirectory();
 	copy("../../../$Config/isolationtester/isolationtester.exe",
 		"../../../$Config/pg_isolation_regress");
 	my @args = (
@@ -382,6 +386,7 @@ sub plcheck
 			"$topdir/$Config/pg_regress/pg_regress",
 			"--bindir=$topdir/$Config/psql",
 			"--dbname=pl_regression", @lang_args, @tests);
+		CleanupTablespaceDirectory();
 		system(@args);
 		my $status = $? >> 8;
 		exit $status if $status;
@@ -439,6 +444,7 @@ sub subdircheck
 
 	print "============================================================\n";
 	print "Checking $module\n";
+	CleanupTablespaceDirectory();
 	my @args = (
 		"$topdir/$Config/pg_regress/pg_regress",
 		"--bindir=${topdir}/${Config}/psql",
@@ -741,6 +747,14 @@ sub InstallTemp
 	return;
 }
 
+sub CleanupTablespaceDirectory
+{
+	my $tablespace = 'testtablespace';
+
+	rmtree($tablespace) if (-e $tablespace);
+	mkdir($tablespace);
+}
+
 sub usage
 {
 	print STDERR
-- 
2.18.2


----Next_Part(Mon_May_11_17_13_54_2020_626)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="0002-Don-t-setup-tablespace-directory-while-testing-pg_up.patch"



^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* [PATCH 1/2] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)

Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make, which is ugly. In addition to that, pg_regress does
the clean up twice at once. This patch moves the cleanup code out of
pg_regress into vcregress.pl, which is more sutable place.
---
 src/test/regress/pg_regress.c | 22 ----------------------
 src/tools/msvc/vcregress.pl   | 14 ++++++++++++++
 2 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
 
 	snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
 
-#ifdef WIN32
-
-	/*
-	 * On Windows only, clean out the test tablespace dir, or create it if it
-	 * doesn't exist.  On other platforms we expect the Makefile to take care
-	 * of that.  (We don't migrate that functionality in here because it'd be
-	 * harder to cope with platform-specific issues such as SELinux.)
-	 *
-	 * XXX it would be better if pg_regress.c had nothing at all to do with
-	 * testtablespace, and this were handled by a .BAT file or similar on
-	 * Windows.  See pgsql-hackers discussion of 2008-01-18.
-	 */
-	if (directory_exists(testtablespace))
-		if (!rmtree(testtablespace, true))
-		{
-			fprintf(stderr, _("\n%s: could not remove test tablespace \"%s\"\n"),
-					progname, testtablespace);
-			exit(2);
-		}
-	make_directory(testtablespace);
-#endif
-
 	/* finally loop on each file and do the replacement */
 	for (name = names; *name; name++)
 	{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f95f7a5c7a..74d37a31de 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -104,6 +104,7 @@ exit 0;
 sub installcheck_internal
 {
 	my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+
 	my @args = (
 		"../../../$Config/pg_regress/pg_regress",
 		"--dlpath=.",
@@ -114,6 +115,7 @@ sub installcheck_internal
 		"--no-locale");
 	push(@args, $maxconn) if $maxconn;
 	push(@args, @EXTRA_REGRESS_OPTS);
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -143,6 +145,7 @@ sub check
 		"--temp-instance=./tmp_check");
 	push(@args, $maxconn)     if $maxconn;
 	push(@args, $temp_config) if $temp_config;
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -178,6 +181,7 @@ sub ecpgcheck
 sub isolationcheck
 {
 	chdir "../isolation";
+	CleanupTablespaceDirectory();
 	copy("../../../$Config/isolationtester/isolationtester.exe",
 		"../../../$Config/pg_isolation_regress");
 	my @args = (
@@ -382,6 +386,7 @@ sub plcheck
 			"$topdir/$Config/pg_regress/pg_regress",
 			"--bindir=$topdir/$Config/psql",
 			"--dbname=pl_regression", @lang_args, @tests);
+		CleanupTablespaceDirectory();
 		system(@args);
 		my $status = $? >> 8;
 		exit $status if $status;
@@ -439,6 +444,7 @@ sub subdircheck
 
 	print "============================================================\n";
 	print "Checking $module\n";
+	CleanupTablespaceDirectory();
 	my @args = (
 		"$topdir/$Config/pg_regress/pg_regress",
 		"--bindir=${topdir}/${Config}/psql",
@@ -741,6 +747,14 @@ sub InstallTemp
 	return;
 }
 
+sub CleanupTablespaceDirectory
+{
+	my $tablespace = 'testtablespace';
+
+	rmtree($tablespace) if (-e $tablespace);
+	mkdir($tablespace);
+}
+
 sub usage
 {
 	print STDERR
-- 
2.18.2


----Next_Part(Mon_May_11_17_13_54_2020_626)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="0002-Don-t-setup-tablespace-directory-while-testing-pg_up.patch"



^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* [PATCH 1/2] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)

Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make, which is ugly. In addition to that, pg_regress does
the clean up twice at once. This patch moves the cleanup code out of
pg_regress into vcregress.pl, which is more sutable place.
---
 src/test/regress/pg_regress.c | 22 ----------------------
 src/tools/msvc/vcregress.pl   | 14 ++++++++++++++
 2 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
 
 	snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
 
-#ifdef WIN32
-
-	/*
-	 * On Windows only, clean out the test tablespace dir, or create it if it
-	 * doesn't exist.  On other platforms we expect the Makefile to take care
-	 * of that.  (We don't migrate that functionality in here because it'd be
-	 * harder to cope with platform-specific issues such as SELinux.)
-	 *
-	 * XXX it would be better if pg_regress.c had nothing at all to do with
-	 * testtablespace, and this were handled by a .BAT file or similar on
-	 * Windows.  See pgsql-hackers discussion of 2008-01-18.
-	 */
-	if (directory_exists(testtablespace))
-		if (!rmtree(testtablespace, true))
-		{
-			fprintf(stderr, _("\n%s: could not remove test tablespace \"%s\"\n"),
-					progname, testtablespace);
-			exit(2);
-		}
-	make_directory(testtablespace);
-#endif
-
 	/* finally loop on each file and do the replacement */
 	for (name = names; *name; name++)
 	{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f95f7a5c7a..74d37a31de 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -104,6 +104,7 @@ exit 0;
 sub installcheck_internal
 {
 	my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+
 	my @args = (
 		"../../../$Config/pg_regress/pg_regress",
 		"--dlpath=.",
@@ -114,6 +115,7 @@ sub installcheck_internal
 		"--no-locale");
 	push(@args, $maxconn) if $maxconn;
 	push(@args, @EXTRA_REGRESS_OPTS);
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -143,6 +145,7 @@ sub check
 		"--temp-instance=./tmp_check");
 	push(@args, $maxconn)     if $maxconn;
 	push(@args, $temp_config) if $temp_config;
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -178,6 +181,7 @@ sub ecpgcheck
 sub isolationcheck
 {
 	chdir "../isolation";
+	CleanupTablespaceDirectory();
 	copy("../../../$Config/isolationtester/isolationtester.exe",
 		"../../../$Config/pg_isolation_regress");
 	my @args = (
@@ -382,6 +386,7 @@ sub plcheck
 			"$topdir/$Config/pg_regress/pg_regress",
 			"--bindir=$topdir/$Config/psql",
 			"--dbname=pl_regression", @lang_args, @tests);
+		CleanupTablespaceDirectory();
 		system(@args);
 		my $status = $? >> 8;
 		exit $status if $status;
@@ -439,6 +444,7 @@ sub subdircheck
 
 	print "============================================================\n";
 	print "Checking $module\n";
+	CleanupTablespaceDirectory();
 	my @args = (
 		"$topdir/$Config/pg_regress/pg_regress",
 		"--bindir=${topdir}/${Config}/psql",
@@ -741,6 +747,14 @@ sub InstallTemp
 	return;
 }
 
+sub CleanupTablespaceDirectory
+{
+	my $tablespace = 'testtablespace';
+
+	rmtree($tablespace) if (-e $tablespace);
+	mkdir($tablespace);
+}
+
 sub usage
 {
 	print STDERR
-- 
2.18.2


----Next_Part(Mon_May_11_17_13_54_2020_626)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="0002-Don-t-setup-tablespace-directory-while-testing-pg_up.patch"



^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* [PATCH 1/2] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)

Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make, which is ugly. In addition to that, pg_regress does
the clean up twice at once. This patch moves the cleanup code out of
pg_regress into vcregress.pl, which is more sutable place.
---
 src/test/regress/pg_regress.c | 22 ----------------------
 src/tools/msvc/vcregress.pl   | 14 ++++++++++++++
 2 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
 
 	snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
 
-#ifdef WIN32
-
-	/*
-	 * On Windows only, clean out the test tablespace dir, or create it if it
-	 * doesn't exist.  On other platforms we expect the Makefile to take care
-	 * of that.  (We don't migrate that functionality in here because it'd be
-	 * harder to cope with platform-specific issues such as SELinux.)
-	 *
-	 * XXX it would be better if pg_regress.c had nothing at all to do with
-	 * testtablespace, and this were handled by a .BAT file or similar on
-	 * Windows.  See pgsql-hackers discussion of 2008-01-18.
-	 */
-	if (directory_exists(testtablespace))
-		if (!rmtree(testtablespace, true))
-		{
-			fprintf(stderr, _("\n%s: could not remove test tablespace \"%s\"\n"),
-					progname, testtablespace);
-			exit(2);
-		}
-	make_directory(testtablespace);
-#endif
-
 	/* finally loop on each file and do the replacement */
 	for (name = names; *name; name++)
 	{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f95f7a5c7a..74d37a31de 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -104,6 +104,7 @@ exit 0;
 sub installcheck_internal
 {
 	my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+
 	my @args = (
 		"../../../$Config/pg_regress/pg_regress",
 		"--dlpath=.",
@@ -114,6 +115,7 @@ sub installcheck_internal
 		"--no-locale");
 	push(@args, $maxconn) if $maxconn;
 	push(@args, @EXTRA_REGRESS_OPTS);
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -143,6 +145,7 @@ sub check
 		"--temp-instance=./tmp_check");
 	push(@args, $maxconn)     if $maxconn;
 	push(@args, $temp_config) if $temp_config;
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -178,6 +181,7 @@ sub ecpgcheck
 sub isolationcheck
 {
 	chdir "../isolation";
+	CleanupTablespaceDirectory();
 	copy("../../../$Config/isolationtester/isolationtester.exe",
 		"../../../$Config/pg_isolation_regress");
 	my @args = (
@@ -382,6 +386,7 @@ sub plcheck
 			"$topdir/$Config/pg_regress/pg_regress",
 			"--bindir=$topdir/$Config/psql",
 			"--dbname=pl_regression", @lang_args, @tests);
+		CleanupTablespaceDirectory();
 		system(@args);
 		my $status = $? >> 8;
 		exit $status if $status;
@@ -439,6 +444,7 @@ sub subdircheck
 
 	print "============================================================\n";
 	print "Checking $module\n";
+	CleanupTablespaceDirectory();
 	my @args = (
 		"$topdir/$Config/pg_regress/pg_regress",
 		"--bindir=${topdir}/${Config}/psql",
@@ -741,6 +747,14 @@ sub InstallTemp
 	return;
 }
 
+sub CleanupTablespaceDirectory
+{
+	my $tablespace = 'testtablespace';
+
+	rmtree($tablespace) if (-e $tablespace);
+	mkdir($tablespace);
+}
+
 sub usage
 {
 	print STDERR
-- 
2.18.2


----Next_Part(Mon_May_11_17_13_54_2020_626)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="0002-Don-t-setup-tablespace-directory-while-testing-pg_up.patch"



^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: Grant read-only access to exactly one database amongst many
@ 2024-02-05 00:54 David G. Johnston <[email protected]>
  2024-02-05 08:51 ` Re: Grant read-only access to exactly one database amongst many Graham Leggett <[email protected]>
  0 siblings, 1 reply; 24+ messages in thread

From: David G. Johnston @ 2024-02-05 00:54 UTC (permalink / raw)
  To: Graham Leggett <[email protected]>; +Cc: [email protected]

On Sun, Feb 4, 2024 at 5:04 PM Graham Leggett <[email protected]> wrote:

> Hi all,
>
> I have a postgresql 15 instance with two databases in it, and I have a
> need to grant read-only access to one of those databases to a given user.
>
> To do this I created a dedicated role for readonly access to the database
> db1:
>
> CREATE ROLE "dv_read_db1"
> GRANT CONNECT ON DATABASE db1 TO dv_read_db1
>

This grant is basically pointless since by default all roles can connect
everywhere via the PUBLIC pseudo-role.  You need to revoke that grant, or
even alter it being given out by default.



> Trouble is, I can create tables in db1 which is write access.


Since in v15 PUBLIC also gets CREATE on the public schema.

I can also connect to db2 (bad),


See my comment regarding the pointless grant in a default setup.

and I can enumerate the tables in db2 (bad),
>

Connect privilege grants reading all catalog data by design.


> I appears the mechanism I am using above has insecure side effects.
>

It has, from your expectation, insecure defaults which you never changed.
We changed public schema in v16 but the ease-of-use database connecting
remains.

David J.


^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: Grant read-only access to exactly one database amongst many
  2024-02-05 00:54 Re: Grant read-only access to exactly one database amongst many David G. Johnston <[email protected]>
@ 2024-02-05 08:51 ` Graham Leggett <[email protected]>
  2024-02-05 14:59   ` Re: Grant read-only access to exactly one database amongst many David G. Johnston <[email protected]>
  0 siblings, 1 reply; 24+ messages in thread

From: Graham Leggett @ 2024-02-05 08:51 UTC (permalink / raw)
  To: David G. Johnston <[email protected]>; +Cc: [email protected]

On 05 Feb 2024, at 00:54, David G. Johnston <[email protected]> wrote:

> I have a postgresql 15 instance with two databases in it, and I have a need to grant read-only access to one of those databases to a given user.
>> 
>> To do this I created a dedicated role for readonly access to the database db1:
>> 
>> CREATE ROLE "dv_read_db1"
>> GRANT CONNECT ON DATABASE db1 TO dv_read_db1
> 
> This grant is basically pointless since by default all roles can connect everywhere via the PUBLIC pseudo-role.  You need to revoke that grant, or even alter it being given out by default.

More on this point at the end…

>> Trouble is, I can create tables in db1 which is write access.
> 
> Since in v15 PUBLIC also gets CREATE on the public schema.

…ouch…

>> I can also connect to db2 (bad),
> 
> See my comment regarding the pointless grant in a default setup.
> 
>> and I can enumerate the tables in db2 (bad),
> 
> Connect privilege grants reading all catalog data by design.
> 
>> 
>> I appears the mechanism I am using above has insecure side effects.
> 
> It has, from your expectation, insecure defaults which you never changed.  We changed public schema in v16 but the ease-of-use database connecting remains.

It looks like changing these defaults is likely to be difficult, which is why I posted here.

I want to optionally allow user minfrin to access both databases by doing this:

CREATE USER minfrin LOGIN;
GRANT dv_read_db1 TO minfrin;
GRANT dv_read_db2 TO minfrin;

If I am understanding you correctly to prevent dv_read_db1 from connecting to db2, I need to actively revoke access to db2. Also, to prevent dv_read_db2 from connecting to db1, I need to actively revoke access to db1.

Would the two grants above dv_read_db1 and dv_read_db2 not cause the unintended side effect of revoking access to each other, resulting in no access being allowed at all?

Also, how do you handle the race condition between the time a database db3 is created, and the the time all readonly users have their access revoked to db3?

Regards,
Graham
—



^ permalink  raw  reply  [nested|flat] 24+ messages in thread

* Re: Grant read-only access to exactly one database amongst many
  2024-02-05 00:54 Re: Grant read-only access to exactly one database amongst many David G. Johnston <[email protected]>
  2024-02-05 08:51 ` Re: Grant read-only access to exactly one database amongst many Graham Leggett <[email protected]>
@ 2024-02-05 14:59   ` David G. Johnston <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: David G. Johnston @ 2024-02-05 14:59 UTC (permalink / raw)
  To: Graham Leggett <[email protected]>; +Cc: [email protected] <[email protected]>

On Monday, February 5, 2024, Graham Leggett <[email protected]> wrote:

>
> Also, how do you handle the race condition between the time a database db3
> is created, and the the time all readonly users have their access revoked
> to db3?
>
>
You alter the default privileges for the system so PUBLIC does not get
connect privileges on newly created databases.

David J.

p.s. this mailing list is for discussing patches, discussions Lon how to
use the system belong on the -general list.


^ permalink  raw  reply  [nested|flat] 24+ messages in thread

end of thread, other threads:[~2024-02-05 14:59 UTC | newest]

Thread overview: 24+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-04-30 05:06 [PATCH 1/2] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2020-04-30 05:06 [PATCH] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2020-04-30 05:06 [PATCH 1/2] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2020-04-30 05:06 [PATCH 1/2] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2020-04-30 05:06 [PATCH 1/2] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2020-04-30 05:06 [PATCH 1/2] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2020-04-30 05:06 [PATCH 1/2] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2020-04-30 05:06 [PATCH 1/2] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2020-04-30 05:06 [PATCH 1/2] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2020-04-30 05:06 [PATCH 1/2] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2020-04-30 05:06 [PATCH 1/2] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2020-04-30 05:06 [PATCH 1/2] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2020-04-30 05:06 [PATCH 1/2] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2020-04-30 05:06 [PATCH 1/2] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2020-04-30 05:06 [PATCH 1/2] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2020-04-30 05:06 [PATCH 1/2] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2020-04-30 05:06 [PATCH 1/2] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2020-04-30 05:06 [PATCH 1/2] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2020-04-30 05:06 [PATCH 1/2] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2020-04-30 05:06 [PATCH 1/2] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2020-04-30 05:06 [PATCH 1/2] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2024-02-05 00:54 Re: Grant read-only access to exactly one database amongst many David G. Johnston <[email protected]>
2024-02-05 08:51 ` Re: Grant read-only access to exactly one database amongst many Graham Leggett <[email protected]>
2024-02-05 14:59   ` Re: Grant read-only access to exactly one database amongst many David G. Johnston <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox