On Tue, Jun 09, 2020 at 10:28:32AM +0200, Daniel Gustafsson wrote:
>> On 9 Jun 2020, at 07:55, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> FWIW I agree with the extended cleanup you proposed but I also agree that it
> doesn't seem worth to backpatch.
Thanks.
>> That will create the output directory even if indir doesn't exist, which
>> seems likely to lead to bogus directories hanging around in places where
>> we don't need them --- plus it seems rather ugly to drop this into the
>> middle of the sequence concerned with indir. So shouldn't the new code
>> be after the exit for indir not existing?
>
> Makes sense, +1 for moving it after indir processing.
Makes sense. Done this way in the updated version attached.
> Looking at the diff, I noticed this hunk:
>
> --- a/src/tools/msvc/vcregress.pl
> +++ b/src/tools/msvc/vcregress.pl
> @@ -571,8 +571,6 @@ sub upgradecheck
> my $outputdir = "$tmp_root/regress";
> my @EXTRA_REGRESS_OPTS = ("--outputdir=$outputdir");
> mkdir "$outputdir" || die $!;
> - mkdir "$outputdir/sql" || die $!;
> - mkdir "$outputdir/expected" || die $!;
> mkdir "$outputdir/testtablespace" || die $!;
>
> On Windows we don't want testtablespace to exist whilst on non-Windows we do.
> I wonder if this is a logical copy-paste from test.sh which could be removed as
> well?
Yes, we could remove this part in vcregress.pl. And as you visibly
noted the cleanup of testtablespace is taken care of in
src/test/regress/GNUmakefile for the non-Windows case, while for MSVC
we do this work in pg_regress.c. I have left this path creation
intentionally actually because there is a patch to improve this area
in the works and we have to check this area of the code so leaving it
behind makes grepping for testtablespace harder to forget (did not get
back to it yet, but I am planning to do so):
https://www.postgresql.org/message-id/20200219.142519.437573253063431435.horikyota.ntt@gmail.com
--
Michael