pgsql: Remove reset of testtablespace from pg_regress on Windows
Remove reset of testtablespace from pg_regress on Windows
testtablespace is an extra path used as tablespace location in the main
regression test suite, computed from --outputdir as defined by the
caller of pg_regress (current directory if undefined).
This special handling was introduced as of f10589e to be specific to
MSVC, as we let pg_regress' Makefile handle this cleanup in other
environments. This moves the cleanup to the MSVC script running
regression tests instead where needed: check, installcheck and
upgradecheck. I have also checked this patch on MSVC with repeated runs
of each target.
Author: Kyotaro Horiguchi, Michael Paquier
Discussion: /messages/by-id/20200219.142519.437573253063431435.horikyota.ntt@gmail.com
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/2b2a070d98b2f2c7ecc031e582cfefa400316ce3
Modified Files
--------------
src/test/regress/pg_regress.c | 22 ----------------------
src/tools/msvc/vcregress.pl | 17 +++++++++++++++--
2 files changed, 15 insertions(+), 24 deletions(-)
On 6/17/20 9:40 PM, Michael Paquier wrote:
Remove reset of testtablespace from pg_regress on Windows
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.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
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:
/messages/by-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.patchtext/x-diff; charset=us-asciiDownload+21-15
On 7/9/20 9:02 PM, Michael Paquier wrote:
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:
/messages/by-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?
I certainly agree we should document more clearly why it's there, to
help forestall anyone else who comes along and thinks it would just be
neater to remove it. so +1.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Thu, Jul 09, 2020 at 10:21:57PM -0400, Andrew Dunstan wrote:
I certainly agree we should document more clearly why it's there, to
help forestall anyone else who comes along and thinks it would just be
neater to remove it. so +1.
Okay. Done, then.
--
Michael