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(at)leadboat(dot)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