agora inbox for [email protected]  

help / color / mirror / Atom feed

[PATCH 1/2] Move tablespace cleanup out of pg_regress.
22+ messages / 2 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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ messages in thread

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

* Re: Add bump memory context type and use it for tuplesorts
@ 2023-07-26 00:11 Nathan Bossart <[email protected]>
  0 siblings, 0 replies; 22+ messages in thread

From: Nathan Bossart @ 2023-07-26 00:11 UTC (permalink / raw)
  To: David Rowley <[email protected]>; +Cc: PostgreSQL Developers <[email protected]>

On Tue, Jun 27, 2023 at 09:19:26PM +1200, David Rowley wrote:
> Because of all of what is mentioned above about the current state of
> tuplesort, there does not really seem to be much need to have chunk
> headers in memory we allocate for tuples at all.  Not having these
> saves us a further 8 bytes per tuple.
> 
> In the attached patch, I've added a bump memory allocator which
> allocates chunks without and chunk header.  This means the chunks
> cannot be pfree'd or realloc'd.  That seems fine for the use case for
> storing tuples in tuplesort. I've coded bump.c in such a way that when
> built with MEMORY_CONTEXT_CHECKING, we *do* have chunk headers.  That
> should allow us to pick up any bugs that are introduced by any code
> which accidentally tries to pfree a bump.c chunk.

This is a neat idea.

> In terms of performance of tuplesort, there's a small (~5-6%)
> performance gain. Not as much as I'd hoped, but I'm also doing a bit
> of other work on tuplesort to make it more efficient in terms of CPU,
> so I suspect the cache efficiency improvements might be more
> pronounced after those.

Nice.

> One thing that might need more thought is that we're running a bit low
> on  MemoryContextMethodIDs. I had to use an empty slot that has a bit
> pattern like glibc malloc'd chunks sized 128kB.  Maybe it's worth
> freeing up a bit from the block offset in MemoryChunk. This is
> currently 30 bits allowing 1GB offset, but these offsets are always
> MAXALIGNED, so we could free up a couple of bits since those 2
> lowest-order bits will always be 0 anyway.

I think it'd be okay to steal those bits.  AFAICT it'd complicate the
macros in memutils_memorychunk.h a bit, but that doesn't seem like such a
terrible price to pay to allow us to keep avoiding the glibc bit patterns.

> +	if (base->sortopt & TUPLESORT_ALLOWBOUNDED)
> +		tuplen = GetMemoryChunkSpace(tuple);
> +	else
> +		tuplen = MAXALIGN(tuple->t_len);

nitpick: I see this repeated in a few places, and I wonder if it might
deserve a comment.

I haven't had a chance to try out your benchmark, but I'm hoping to do so
in the near future.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com





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

end of thread, other threads:[~2023-07-26 00:11 UTC | newest]

Thread overview: 22+ 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 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] 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]>
2023-07-26 00:11 Re: Add bump memory context type and use it for tuplesorts Nathan Bossart <[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