Re: Supporting TAP tests with MSVC and Windows

Thanks for your input, Noah.

On Fri, Apr 3, 2015 at 3:22 PM, Noah Misch <noah@leadboat.com> wrote:
> Each Windows patch should cover all Windows build systems; patches that fix a
> problem in the src/tools/msvc build while leaving it broken in the GNU make
> build are a bad trend.  In this case, I expect you'll need few additional
> changes to cover both.

Yes I am planning to do more tests with MinGW as well, once I got the
patch in a more advanced state in May/June. For Cygwin, I am not much
familiar with it yet, but I am sure I'll come up with something.

> On Thu, Apr 02, 2015 at 06:30:02PM +0900, Michael Paquier wrote:
>> 2) tapcheck does not check if IPC::Run is installed or not. Should we
>> add a check in the code for that? If yes, should we bypass the test or
>> fail?
>
> The src/tools/msvc build system officially requires ActivePerl, and I seem to
> recall ActivePerl installs IPC::Run by default.  A check is optional.

I recall installing ActivePerl for my own box some time ago. It is
5.16, so now it is outdated, but the package manager does not contain
IPC::Run and I had to install it by hand.

> Test suites that run as part of "make check-world", including all src/bin TAP
> suites, _must not_ enable trust authentication on Windows.  To do so would
> reintroduce CVE-2014-0067.  (The standard alternative is to call "pg_regress
> --config-auth", which switches a data directory to SSPI authentication.)
> Suites not in check-world, though not obligated to follow that rule, will have
> a brighter future if they do so anyway.

OK. I'll rework this part.

>> --- a/src/test/perl/TestLib.pm
>> +++ b/src/test/perl/TestLib.pm
>
>> @@ -73,9 +74,19 @@ sub tempdir_short
>>  sub standard_initdb
>>  {
>>       my $pgdata = shift;
>> -     system_or_bail("initdb -D '$pgdata' -A trust -N >/dev/null");
>> -     system_or_bail("$ENV{top_builddir}/src/test/regress/pg_regress",
>> -                                '--config-auth', $pgdata);
>> +
>> +     if ($Config{osname} eq "MSWin32")
>> +     {
>> +             system_or_bail("initdb -D $pgdata -A trust -N > NUL");
>> +             system_or_bail("$ENV{TESTREGRESS}",
>> +                                        '--config-auth', $pgdata);
>> +     }
>> +     else
>> +     {
>> +             system_or_bail("initdb -D '$pgdata' -A trust -N >/dev/null");
>> +             system_or_bail("$ENV{top_builddir}/src/test/regress/pg_regress",
>> +                                        '--config-auth', $pgdata);
>> +     }
>>  }
>
> Make all build systems set TESTREGRESS, and use it unconditionally.

Yeah, that would be better.

> That's not a complete review, just some highlights.

Thanks again.
-- 
Michael