From: Noah Misch <noah@leadboat.com>
To: Michael Paquier <michael.paquier@gmail.com>
Cc: PostgreSQL mailing lists <pgsql-hackers@postgresql.org>
Cc: peter_e@gmx.net, Heikki Linnakangas <hlinnakangas@vmware.com>
Subject: Re: Supporting TAP tests with MSVC and Windows
Date: Fri, 3 Apr 2015 02:22:30 -0400
Message-ID: <20150403062230.GA1153652@tornado.leadboat.com> (raw)
In-Reply-To: <CAB7nPqTQwphkDfZP07w7yBnbFNDhW_JBAMyCFAkarE2VWg8irQ@mail.gmail.com>
References: <CAB7nPqTQwphkDfZP07w7yBnbFNDhW_JBAMyCFAkarE2VWg8irQ@mail.gmail.com>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-hackers>
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.
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.
> 7) TAP tests use sometimes top_builddir directly. I think that's ugly,
> and if there are no objections I think that we should change it to use
> a dedicated environment variable like TESTTOP or similar.
I sense nothing ugly about a top_builddir reference, but see below.
> --- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
> +++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
> open HBA, ">>$tempdir/pgdata/pg_hba.conf";
> -print HBA "local replication all trust\n";
> +# Windows builds do not support local connections
> +if ($Config{osname} ne "MSWin32")
> +{
> + print HBA "local replication all trust\n";
> +}
> print HBA "host replication all 127.0.0.1/32 trust\n";
> print HBA "host replication all ::1/128 trust\n";
> close HBA;
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.
> --- 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.
That's not a complete review, just some highlights.
Thanks,
nm
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: noah@leadboat.com, michael.paquier@gmail.com, hlinnakangas@vmware.com
Subject: Re: Supporting TAP tests with MSVC and Windows
In-Reply-To: <20150403062230.GA1153652@tornado.leadboat.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox