On Sat, Apr 11, 2015 at 4:35 AM, Peter Eisentraut <pete...@gmx.net> wrote: > On 3/9/15 2:51 AM, Michael Paquier wrote: >> On Sun, Mar 8, 2015 at 10:46 PM, Michael Paquier >> <michael.paqu...@gmail.com> wrote: >>> Speaking of which, attached is a patch rewritten in-line with those >>> comments, simplifying a bit the whole at the same time. Note this >>> patch changes ecpgcheck as it should be patched, but as ecpgcheck test >>> is broken even on HEAD, I'll raise a separate thread about that with a >>> proper fix (for example bowerbird skips this test). >> >> Correction: HEAD is fine, but previous patch was broken because it >> tried to use --top-builddir. Also for ecpgcheck it looks that tricking >> PATH is needed to avoid dll loading errors related to libecpg.dll and >> libecpg_compat.dll. Updated patch is attached. > > To clarify: Are you of the opinion that your patch (on top of my base > patch) is sufficient to make all of this work on Windows?
Things will work. I just tested again each test target with the patch attached while wrapping again my mind on this stuff (actually found one bug in plcheck where --bindir was not correct, and removed tmp_install in upgradecheck). Now, what this patch does is enforcing the temporary install for each *check target of vcregress.pl. This has the disadvantage of making the benefits of MAKELEVEL=0 seen for build methods using the Makefiles go away for MSVC, but it minimizes the use of ENV variables which is a good thing to me by setting --bindir to a value as much as possible (ecpgcheck being an exception), and it makes the set of tests more consistent with each other in the way they run. Another thing to know that this patch changes is that vcregress does not rely anymore on the contents of Release/$projects, aka the build structure of MS stuff, but just fetches binaries from the temporary installation. That's more consistent with Makefile builds, now perhaps some people have a different opinion. What we could add later on a new target, allcheck, that would kick all the tests at once and installs the temporary installation just once. It would be straight-forward to write a patch, but this is a separate discussion as installcheck would need to be skipped. isolationcheck should as well be changed to be more self-dependent as even of HEAD it needs to have an instance of PG running to work. Regards, -- Michael
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl index bd3dd2c..d9ff7ec 100644 --- a/src/tools/msvc/vcregress.pl +++ b/src/tools/msvc/vcregress.pl @@ -16,6 +16,7 @@ my $startdir = getcwd(); chdir "../../.." if (-d "../../../src/tools/msvc"); my $topdir = getcwd(); +my $tmp_installdir = "$topdir/tmp_install"; require 'src/tools/msvc/config_default.pl'; require 'src/tools/msvc/config.pl' if (-f 'src/tools/msvc/config.pl'); @@ -94,7 +95,7 @@ sub installcheck my @args = ( "../../../$Config/pg_regress/pg_regress", "--dlpath=.", - "--psqldir=../../../$Config/psql", + "--bindir=../../../$Config/psql", "--schedule=${schedule}_schedule", "--encoding=SQL_ASCII", "--no-locale"); @@ -106,15 +107,19 @@ sub installcheck sub check { + chdir $startdir; + + InstallTemp(); + chdir "${topdir}/src/test/regress"; + my @args = ( - "../../../$Config/pg_regress/pg_regress", + "${tmp_installdir}/bin/pg_regress", "--dlpath=.", - "--psqldir=../../../$Config/psql", + "--bindir=${tmp_installdir}/bin", "--schedule=${schedule}_schedule", "--encoding=SQL_ASCII", "--no-locale", - "--temp-install=./tmp_check", - "--top-builddir=\"$topdir\""); + "--temp-instance=./tmp_check"); push(@args, $maxconn) if $maxconn; push(@args, $temp_config) if $temp_config; system(@args); @@ -128,18 +133,20 @@ sub ecpgcheck system("msbuild ecpg_regression.proj /p:config=$Config"); my $status = $? >> 8; exit $status if $status; + InstallTemp(); chdir "$topdir/src/interfaces/ecpg/test"; + + $ENV{PATH} = "${tmp_installdir}/bin;${tmp_installdir}/lib;$ENV{PATH}"; $schedule = "ecpg"; my @args = ( - "../../../../$Config/pg_regress_ecpg/pg_regress_ecpg", - "--psqldir=../../../$Config/psql", + "${tmp_installdir}/bin/pg_regress_ecpg", + "--bindir=", "--dbname=regress1,connectdb", "--create-role=connectuser,connectdb", "--schedule=${schedule}_schedule", "--encoding=SQL_ASCII", "--no-locale", - "--temp-install=./tmp_chk", - "--top-builddir=\"$topdir\""); + "--temp-instance=./tmp_chk"); push(@args, $maxconn) if $maxconn; system(@args); $status = $? >> 8; @@ -148,12 +155,14 @@ sub ecpgcheck sub isolationcheck { - chdir "../isolation"; - copy("../../../$Config/isolationtester/isolationtester.exe", - "../../../$Config/pg_isolation_regress"); + chdir $startdir; + + InstallTemp(); + chdir "${topdir}/src/test/isolation"; + my @args = ( - "../../../$Config/pg_isolation_regress/pg_isolation_regress", - "--psqldir=../../../$Config/psql", + "${tmp_installdir}/bin/pg_isolation_regress", + "--bindir=${tmp_installdir}/bin", "--inputdir=.", "--schedule=./isolation_schedule"); push(@args, $maxconn) if $maxconn; @@ -164,7 +173,10 @@ sub isolationcheck sub plcheck { - chdir "../../pl"; + chdir $startdir; + + InstallTemp(); + chdir "${topdir}/src/pl"; foreach my $pl (glob("*")) { @@ -201,8 +213,8 @@ sub plcheck "============================================================\n"; print "Checking $lang\n"; my @args = ( - "../../../$Config/pg_regress/pg_regress", - "--psqldir=../../../$Config/psql", + "${tmp_installdir}/bin/pg_regress", + "--bindir=${tmp_installdir}/bin", "--dbname=pl_regression", @lang_args, @tests); system(@args); my $status = $? >> 8; @@ -236,8 +248,8 @@ sub contribcheck my @tests = fetchTests(); my @opts = fetchRegressOpts(); my @args = ( - "../../$Config/pg_regress/pg_regress", - "--psqldir=../../$Config/psql", + "${tmp_installdir}/bin/pg_regress", + "--bindir=${tmp_installdir}/bin", "--dbname=contrib_regression", @opts, @tests); system(@args); my $status = $? >> 8; @@ -251,8 +263,8 @@ sub contribcheck sub standard_initdb { return ( - system('initdb', '-N') == 0 and system( - "$topdir/$Config/pg_regress/pg_regress", '--config-auth', + system("${tmp_installdir}/bin/initdb", '-N') == 0 and system( + "${tmp_installdir}/bin/pg_regress", '--config-auth', $ENV{PGDATA}) == 0); } @@ -271,14 +283,13 @@ sub upgradecheck $ENV{PGPORT} ||= 50432; my $tmp_root = "$topdir/contrib/pg_upgrade/tmp_check"; (mkdir $tmp_root || die $!) unless -d $tmp_root; - my $tmp_install = "$tmp_root/install"; - print "Setting up temp install\n\n"; - Install($tmp_install, "all", $config); + + InstallTemp(); # Install does a chdir, so change back after that chdir $cwd; my ($bindir, $libdir, $oldsrc, $newsrc) = - ("$tmp_install/bin", "$tmp_install/lib", $topdir, $topdir); + ("$tmp_installdir/bin", "$tmp_installdir/lib", $topdir, $topdir); $ENV{PATH} = "$bindir;$ENV{PATH}"; my $data = "$tmp_root/data"; $ENV{PGDATA} = "$data.old"; @@ -413,6 +424,12 @@ sub GetTests return ""; } +sub InstallTemp +{ + print "Setting up temp install\n\n"; + Install("$tmp_installdir", "all", $config); +} + sub usage { print STDERR
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers