Re: pg_regress cleans up tablespace twice.

Kyotaro Horiguchi <horikyota.ntt@gmail.com>

From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
To: michael@paquier.xyz
Cc: tgl@sss.pgh.pa.us, pgsql-hackers@lists.postgresql.org
Date: 2020-05-11T08:13:54Z
Lists: pgsql-hackers

Attachments

At Fri, 21 Feb 2020 17:05:07 +0900 (JST), Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote in 
> At Thu, 20 Feb 2020 14:23:22 +0900, Michael Paquier <michael@paquier.xyz> wrote in 
> > Removing this code from pg_regress.c makes also sense to me.  Now, the
> > patch breaks "vcregress installcheck" as this is missing to patch
> > installcheck_internal() for the tablespace path creation.  I would
> > also recommend using a full path for the directory location to avoid
> > any potential issues if this code is refactored or moved around, the
> > patch now relying on the current path used.
> 
> Hmm. Right. I confused database directory and tablespace
> directory. Tablespace directory should be provided by the test script,
> even though the database directory is preexisting in installcheck. To
> avoid useless future failure, I would do that that for all
> subcommands, as regress/GNUmakefile does.

Tablespace directory cleanup is not done for all testing
targets. Actually it is not done for the tools under bin/ except
pg_upgrade.

On the other hand, it was done every by pg_regress run for Windows
build.  So I made vcregress.pl do the same with that. Spcecifically to
set up tablespace always before pg_regress is executed.

There is a place where --outputdir is specified for pg_regress,
pg_upgrade/test.sh. It is explained as the follows.

# Send installcheck outputs to a private directory.  This avoids conflict when
# check-world runs pg_upgrade check concurrently with src/test/regress check.
# To retrieve interesting files after a run, use pattern tmp_check/*/*.diffs.
outputdir="$temp_root/regress"
EXTRA_REGRESS_OPTS="$EXTRA_REGRESS_OPTS --outputdir=$outputdir"

Where the $temp_root is $(TOP)/src/bin/pg_upgrade/tmp_check/regress.

Thus the current regress/GNUMakefile does break this consideration and
the current vc_regress (of Windows build) does the right thing in the
light of the comment.  Don't we need to avoid cleaning up
"$(TOP)/src/test/regress/tablesapce" in that case? (the second patch
attached)


regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

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