Re: pg_regress cleans up tablespace twice.

Michael Paquier <michael@paquier.xyz>

From: Michael Paquier <michael@paquier.xyz>
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: tgl@sss.pgh.pa.us, pgsql-hackers@lists.postgresql.org
Date: 2020-05-15T02:58:55Z
Lists: pgsql-hackers
On Mon, May 11, 2020 at 05:13:54PM +0900, Kyotaro Horiguchi wrote:
> Tablespace directory cleanup is not done for all testing
> targets. Actually it is not done for the tools under bin/ except
> pg_upgrade.

Let's first take one problem at a time, as I can see that your patch
0002 is modifying a portion of what you added in 0001, and so let's
try to remove this WIN32 stuff from pg_regress.c.

+sub CleanupTablespaceDirectory
+{
+   my $tablespace = 'testtablespace';
+
+   rmtree($tablespace) if (-e $tablespace);
+   mkdir($tablespace);
+}
This check should use "-d" and not "-e" as it would be true for a file
as well.  Also, in pg_regress.c, we remove the existing tablespace
test directory in --outputdir, which is "." by default but it can be a
custom one.  Shouldn't you do the same logic in this new routine?  So
we should have an optional argument for the output directory that
defaults to `pwd` if not defined, no?  This means passing down the
argument only for upgradecheck() in vcregress.pl.

 sub isolationcheck
 {
 	chdir "../isolation";
+	CleanupTablespaceDirectory();
 	copy("../../../$Config/isolationtester/isolationtester.exe",
 		"../../../$Config/pg_isolation_regress");
 	my @args = (
[...]
 	print "============================================================\n";
 	print "Checking $module\n";
+	CleanupTablespaceDirectory();
 	my @args = (
 		"$topdir/$Config/pg_regress/pg_regress",
 		"--bindir=${topdir}/${Config}/psql",
I would put that just before the system() calls for consistency with
the rest.
--
Michael

Commits

  1. Revert "Remove reset of testtablespace from pg_regress on Windows"

  2. Remove reset of testtablespace from pg_regress on Windows

  3. Lower privilege level of programs calling regression_main