From: Michael Paquier <michael@paquier.xyz>
To: Andrew Dunstan <andrew.dunstan@2ndquadrant.com>
Cc: pgsql-committers@lists.postgresql.org
Subject: Re: pgsql: Remove reset of testtablespace from pg_regress on Windows
Date: Fri, 10 Jul 2020 10:02:02 +0900
Message-ID: <20200710010202.GD11153@paquier.xyz> (raw)
In-Reply-To: <6d9eee97-54c8-e14a-48f7-3194e712f54f@2ndQuadrant.com>
References: <E1jljXu-0007fp-Ct@gemulon.postgresql.org>
<6d9eee97-54c8-e14a-48f7-3194e712f54f@2ndQuadrant.com>
On Thu, Jul 09, 2020 at 07:28:02PM -0400, Andrew Dunstan wrote:
> This patch has carefully removed the ability to run the regression tests
> as a Windows administrative user, as I just discovered. This was the
> whole point of commit ce5d3424d6.
>
> I assume the testing referred to above was not as a privileged user. I
> think this should be reverted.
Thanks Andrew. This was discussed on the original thread and what I
wanted to do a rvert if you look at its newest history:
https://www.postgresql.org/message-id/20200623014036.GF50978@paquier.xyz
And then, the thread just stalled.. So I was not sure if something
was actually wanted or not.
Now, I don't think that just a simple revert is the best answer we can
provide. Just look at this comment in pg_regress.c that does not give
a hint that we actually should not remove this code:
- * 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.
So instead I would like to propose the attached, reworking this
comment as follows (basically a revert, except for this comment):
+ /*
+ * On Windows only, clean out the test tablespace dir, or create it if it
+ * doesn't exist so as it is possible to run the regression tests as a
+ * Windows administrative user account with the restricted token obtained
+ * when starting pg_regress. On other platforms we expect the Makefile
+ * to take care of that.
+ */
What do you think?
--
Michael
Attachments:regress-win32-tbspace.patch (text/x-diff, 2.3K) downloaddiff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.cindex c8d190d248..8bfc336ec2 100644--- a/src/test/regress/pg_regress.c+++ b/src/test/regress/pg_regress.c@@ -494,6 +494,25 @@ 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 so as it is possible to run the regression tests as a+ * Windows administrative user account with the restricted token obtained+ * when starting pg_regress. On other platforms we expect the Makefile+ * to take care of that.+ */+ 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.plindex d6763ad4ac..3365ee578c 100644--- a/src/tools/msvc/vcregress.pl+++ b/src/tools/msvc/vcregress.pl@@ -123,8 +123,6 @@ sub installcheck_internal
sub installcheck
{
my $schedule = shift || 'serial';
-- CleanupTablespaceDirectory();
installcheck_internal($schedule);
return;
}
@@ -145,7 +143,6 @@ 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;
@@ -573,8 +570,8 @@ sub upgradecheck
$ENV{PGDATA} = "$data.old";
my $outputdir = "$tmp_root/regress";
my @EXTRA_REGRESS_OPTS = ("--outputdir=$outputdir");
- mkdir "$outputdir" || die $!;- CleanupTablespaceDirectory($outputdir);+ mkdir "$outputdir" || die $!;+ mkdir "$outputdir/testtablespace" || die $!;
my $logdir = "$topdir/src/bin/pg_upgrade/log";
rmtree($logdir);
@@ -740,16 +737,6 @@ sub InstallTemp
return;
}
-sub CleanupTablespaceDirectory-{- my $testdir = shift || getcwd();-- my $testtablespace = "$testdir/testtablespace";-- rmtree($testtablespace) if (-d $testtablespace);- mkdir($testtablespace);-}-
sub usage
{
print STDERR
signature.asc (application/pgp-signature, 832B) download
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-committers@postgresql.org
Cc: michael@paquier.xyz, andrew.dunstan@2ndquadrant.com, pgsql-committers@lists.postgresql.org
Subject: Re: pgsql: Remove reset of testtablespace from pg_regress on Windows
In-Reply-To: <20200710010202.GD11153@paquier.xyz>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox