Re: set TESTDIR from perl rather than Makefile - Mailing list pgsql-hackers

On 2/24/22 20:17, Justin Pryzby wrote:
> On Mon, Feb 21, 2022 at 07:00:54AM -0500, Andrew Dunstan wrote:
>> On 2/19/22 18:53, Justin Pryzby wrote:
>>> On Sat, Feb 19, 2022 at 05:41:49PM -0600, Justin Pryzby wrote:
>>>> I rebased and fixed the check-guc script to work, made it work with vpath
>>>> builds, and cleaned it up some.
>>> I also meant to also attach it.
>> This is going to break a bunch of stuff as written.
>>
>> First, it's not doing the same thing. The current system sets TESTDIR to
>> be the parent of the directory that holds the test. e.g. for
>> src/bin/pg_ctl/t/001_start_stop.pl it's src/bin/pg_ctl in the build
>> tree, not the 't' subdirectory. This patch apparently sets it to the 't'
>> subdirectory. That will break anything that goes looking for log files
>> in the current location, like the buildfarm client, and possibly some CI
>> setups as well.
> Yes, thanks.
>
> I changed the patch to use ENV{CURDIR} || dirname(dirname($0)).  If I'm not
> wrong, that seems to be doing the right thing.
>
>> Also, unless I'm mistaken it appears to to the wrong thing for vpath
>> builds:
>>
>>     my $test_dir = File::Spec->rel2abs(dirname($0));
>>
>> is completely wrong for vpaths, since that will place it in the source
>> tree, not the build tree.
>>
>> Last, and for no explained reason that I can see, the patch undoes
>> commit f4ce6c4d3a, but only for msvc builds. Even if that's justified it
>> appears to have no relevance to this patch.
> Andres' idea is that perl should set TESTDIR and PATH.  Here I commented out
> PATH, and had the improbable issue that nothing seemed to be breaking,
> including the pipeline test under msvc.  It'd be helpful to know what
> configuration that breaks so I can test that it's broken and then test that
> it's fixed when set from within perl...


I'm fairly sure what's broken here is the MSVC install procedure, which
is massively overeager. We should fix that rather than take it for granted.


>
> I got busy here, and may not be able to progress this for awhile.



You have fixed the vpath issue. But the changes in vcregress.pl look
wrong to me.


-    $ENV{TESTDIR} = "$dir";


If we set PG_SUBDIR in the Makefile shouldn't we set it here too? Yes it
probably doesn't matter as our MSVC build system doesn't support vpath
builds, but it's good to maintain as much equivalence between the two as
possible. (Supporting vpath builds for MSVC might be a nice
student/intern project)


     my $module = basename $dir;
-    # add the module build dir as the second element in the PATH
-    $ENV{PATH} =~ s!;!;$topdir/$Config/$module;!;


See above comment re msvc install. If we're not reverting f4ce6c4d3a in
the Makefile we shouldn't be reverting here either.


+    # $ENV{VCREGRESS_MODE} = $Config;


Why is this commented out line here?


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com