On Sun, Mar 8, 2015 at 10:22 PM, Peter Eisentraut <pete...@gmx.net> wrote:
> On 2/24/15 3:06 AM, Michael Paquier wrote:
>> On Sun, Feb 15, 2015 at 11:01 AM, Peter Eisentraut wrote:
>>> Here is an updated patch.
>>
>> Nice patch. This is going to save a lot of resources.
>>
>> An update of vcregress.pl is necessary. This visibly just consists in
>> updating the options that have been renamed in pg_regress (don't mind
>> testing any code sent out).
>
> Well, that turns out to be more complicated than initially thought.
> Apparently, the msvc has a bit of a different idea of what check and
> installcheck do with respect to temporary installs. For instance,
> vcregress installcheck does not use psql from the installation but from
> the build tree. vcregress check uses psql from the build tree but other
> binaries (initdb, pg_ctl) from the temporary installation. It is hard
> for me to straighten this out by just looking at the code. Attached is
> a patch that shows the idea, but I can't easily take it further than that.
Urg. Yes for installcheck I guess that we cannot do much but simply
use the psql from the tree, but on the contrary for all the other
targets we can use the temporary installation as $topdir/tmp_install.
Regarding the patch you sent, IMO it is not a good idea to kick
install with system() as this can fail as an unrecognized command
runnable. And the command that should be used is not "vcregress
install $path" but simply "vcregress install". Hence I think that
calling simply Install() makes more sense. Also, I think that it would
be better to not enforce PATH before kicking the commands.
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).
On my side, with this patch, installcheck, check, plcheck,
upgradecheck work properly and all of them use the common
installation. It would be more adapted to add checks on the existence
of $tmp_installdir/bin though in InstallTemp instead of kicking an
installation all the time. But that's simple enough to change.
Regards,
--
Michael
From 2b4551a7bc411fc03703f2641b655faf76f2c679 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@otacoo.com>
Date: Sun, 8 Mar 2015 22:39:10 -0700
Subject: [PATCH] Make vcregress use common installation path for all tests
installcheck is let as-is as it depends on psql being present in PATH.
---
src/tools/msvc/vcregress.pl | 66 +++++++++++++++++++++++++++++----------------
1 file changed, 43 insertions(+), 23 deletions(-)
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index bd3dd2c..aa7fbaa 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);
@@ -125,20 +130,24 @@ sub check
sub ecpgcheck
{
chdir $startdir;
+
system("msbuild ecpg_regression.proj /p:config=$Config");
my $status = $? >> 8;
exit $status if $status;
+
+ InstallTemp();
chdir "$topdir/src/interfaces/ecpg/test";
+
$schedule = "ecpg";
my @args = (
- "../../../../$Config/pg_regress_ecpg/pg_regress_ecpg",
- "--psqldir=../../../$Config/psql",
+ "${tmp_installdir}/bin/pg_regress_ecpg",
+ "--bindir=${tmp_installdir}/bin",
"--dbname=regress1,connectdb",
"--create-role=connectuser,connectdb",
"--schedule=${schedule}_schedule",
"--encoding=SQL_ASCII",
"--no-locale",
- "--temp-install=./tmp_chk",
+ "--temp-instance=./tmp_chk",
"--top-builddir=\"$topdir\"");
push(@args, $maxconn) if $maxconn;
system(@args);
@@ -148,12 +157,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 +175,10 @@ sub isolationcheck
sub plcheck
{
- chdir "../../pl";
+ chdir $startdir;
+
+ InstallTemp();
+ chdir "${topdir}/src/pl";
foreach my $pl (glob("*"))
{
@@ -201,8 +215,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/psql",
"--dbname=pl_regression", @lang_args, @tests);
system(@args);
my $status = $? >> 8;
@@ -236,8 +250,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 +265,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);
}
@@ -272,13 +286,13 @@ sub upgradecheck
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 +427,12 @@ sub GetTests
return "";
}
+sub InstallTemp
+{
+ print "Setting up temp install\n\n";
+ Install("$tmp_installdir", "all", $config);
+}
+
sub usage
{
print STDERR
--
1.9.2.msysgit.0
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
