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

Reply via email to