From: Andrew Dunstan <andrew.dunstan@2ndquadrant.com>
Subject: Re: pgsql: Remove reset of testtablespace from pg_regress on Windows
To: Michael Paquier <michael@paquier.xyz>
Cc: pgsql-committers@lists.postgresql.org
References: <E1jljXu-0007fp-Ct@gemulon.postgresql.org>
 <6d9eee97-54c8-e14a-48f7-3194e712f54f@2ndQuadrant.com>
 <20200710010202.GD11153@paquier.xyz>
Autocrypt: addr=andrew.dunstan@2ndQuadrant.com; prefer-encrypt=mutual;
 keydata=
 mQENBE7KWFkBCAClridxur2AIc7eW2AR7izbfp3EnNefie2HbLF0izW5Ik5UjX2HBXBx4syI
 gY6b0ugohXrr274+baoAlvSbq6cAoQuEVrk5IZFzt20b1Xkx65FwGSEj526yiKLocqkJceSq
 Xr9xcA5SGY+FZv441chh5SU92v4q6z+6LPpoHOh97ptAVXZYNTtU0LevyvD5lja0TzbvJm6C
 eFXitJfnm1pLEr0DGJCR/iUOl/N62Kh4855zZC7NHIjQHPOvV5Stz/l5ilDhvGVk+xkXFPys
 SjZoUr1rXhYLpiyi5sR0X9FHXT0KnGuz1F5ERO7ZTLSSQ6fJwPj6gOk9K+vvoKvoeql5ABEB
 AAG0L0FuZHJldyBEdW5zdGFuIDxhbmRyZXcuZHVuc3RhbkAybmRRdWFkcmFudC5jb20+iQFY
 BBMBAgBCAhsDBgsJCAcDAgYVCAIJCgsEFgIDAQIeAQIXgBYhBOQ+WEYd/Hy/RGkVpZn6f8tZ
 /DuBAhkBBQJd1pSYBQkSzqMsAAoJEJn6f8tZ/DuBJmEIAIVFBIC3qGqh+YTPVTkfowwxpHz0
 tZSPjZ6GS/5y62kgoZrZOVXw+Rg2ceuMM3a+In/RRjTx4bembgYqRKgzXlDfiNBJeq5ioVuN
 H1faDKE2lm5a6QkRQ2JU6DKf60fqSJGP5LisCUr1V0wz6EnlO31ArrKtXCNB+yk0KDAS6o6I
 Ql4pkXpr+vq4iilwJmiJOqTBam0LmQE7ulYCCYU8Z6MkpiSl+dO7QSramcGyxauQRI3a7gcq
 dpo8Vrv3gFTQDeMYxk7cqZp08pIVH5XR0+LgBz4X84f59rA/VAKlC/r6CyEQk59rvDLxCMyP
 2buoj51Z25N3MVghRDGcsU6mT7e5AQ0ETspYWQEIANGc4zQULOxhbqO2dyD51YhqCNRmm9oK
 Waqf+wmW4tpDe/VVcxAnNizd4LWCHfzpb5cHAtGkOPePMfzWVf6nvdF7d3eglbtf59+zG7O7
 llV0xSSoFiieQBsrGvqDInXYX/4mRRXMtyhM353/tixC9RWLs1oofyYmCPPXXY7h9R7en3B8
 BoVrRFcdzlIY/NFNhFGW/9dkEiGjgna2Rk6e15kln4ZvFBWUg23p93w/pqXcxY6+k/8TEk+C
 4R+M6w7o2PLGOjdZ+kPiUcw5H85zf/yZJwQXzisXaNduwWB6Vads9YC9dj6kPR1c4VGRqAaY
 L++LAEOqrlvm2TvqQqZRtnEAEQEAAYkBJQQYAQIADwIbDAUCXdaUtAUJEs6jVQAKCRCZ+n/L
 Wfw7gQi5B/9sHbmRloZNCWWwZGm+O9iG/JGQxS0WWa4GaZqzqSTWOft2/gO0Bgag89r1djq4
 PFgSZtzHvKQpqYz/7HkPrWh1jpcS83i/uYY1uyHXUUtpdEmn4buNObEtD0YNaJkGdhASsECq
 6akw+u7WJYmAHs/SpR6ok6HqLw7JHDVc2h5dViLfTk0qrgDa7sq/LVMoJje5Ct/Ku3YaWAV6
 YSmzoPP5BmPVFqP11OMQkCqurIu9vTLXKFWX623vCmqRDHJ6e5GCDHbH8USU+4wJBfPymZkd
 cA7NGm/mmWeJiWBFIDyjzYxrjku8ty9TZJVsaRXnS+OFQzXz9Eu2wTpKdHij46uY
Message-ID: <47e25dce-8dcb-004d-255b-1999ef8b0851@2ndQuadrant.com>
Date: Thu, 9 Jul 2020 22:21:57 -0400
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.6.0
MIME-Version: 1.0
In-Reply-To: <20200710010202.GD11153@paquier.xyz>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit
Content-Language: en-US
Precedence: bulk


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:
> 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?


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