Re: [HACKERS] improving speed of make check-world
On 4/28/15 9:09 AM, Michael Paquier wrote: I guess by redirecting it into the log file you indicated, but is that a good idea to redirect stderr? I am sure that Peter did that on purpose, both approaches having advantages and disadvantages. Personally I don't mind looking at the install log file in tmp_install to see the state of the installation, but it is true that this change is a bit disturbing regarding the fact that everything was directly outputted to stderr and stdout for many years. Not really. Before, pg_regress put the output of its internal make install run into a log file. Now make is just replicating that behavior. I would agree that that seems kind of obsolete now, because it's really just another sub-make. So if no one disagrees, I'd just remove the log file redirection in temp-install. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] improving speed of make check-world
Michael Paquier michael.paqu...@gmail.com writes: On Tue, Apr 28, 2015 at 1:46 AM, Jeff Janes jeff.ja...@gmail.com wrote: This change fixed the problem for me. It also made this age-old compiler warning go away: In file included from gram.y:14515: scan.c: In function 'yy_try_NUL_trans': scan.c:10307: warning: unused variable 'yyg' I guess by redirecting it into the log file you indicated, but is that a good idea to redirect stderr? I am sure that Peter did that on purpose, both approaches having advantages and disadvantages. Personally I don't mind looking at the install log file in tmp_install to see the state of the installation, but it is true that this change is a bit disturbing regarding the fact that everything was directly outputted to stderr and stdout for many years. Hm. I don't really like the idea that make all behaves differently if invoked by make check than if invoked directly. Hiding warnings from you is not very good, and hiding errors would be even more annoying. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] improving speed of make check-world
On Sat, Apr 25, 2015 at 7:23 AM, Michael Paquier michael.paqu...@gmail.com wrote: On Sat, Apr 25, 2015 at 7:59 AM, Peter Eisentraut pete...@gmx.net wrote: On 4/23/15 1:22 PM, Jeff Janes wrote: Something about this commit (dcae5faccab64776376d354d) broke make check in parallel conditions when started from a clean directory. It fails with a different error each time, one example: make -j4 check /dev/null In file included from gram.y:14515: scan.c: In function 'yy_try_NUL_trans': scan.c:10307: warning: unused variable 'yyg' /usr/bin/ld: tab-complete.o: No such file: No such file or directory collect2: ld returned 1 exit status make[3]: *** [psql] Error 1 make[2]: *** [all-psql-recurse] Error 2 make[2]: *** Waiting for unfinished jobs make[1]: *** [all-bin-recurse] Error 2 make: *** [all-src-recurse] Error 2 make: *** Waiting for unfinished jobs I think the problem is that check depends on all, but now also depends on temp-install, which in turn runs install and all. With a sufficient amount of parallelism, you end up running two alls on top of each other. It seems this can be fixed by removing the check: all dependency. Try removing that in the top-level GNUmakefile.in and see if the problem goes away. For completeness, we should then also remove it in the other makefiles. Yep. I spent some time yesterday and today poking at that, but I clearly missed that dependency.. Attached is a patch fixing the problem. I tested check and check-world with some parallel jobs and both worked. In the case of check, the amount of logs is very reduced because all the install process is done by temp-install which redirects everything into tmp_install/log/install.log. This change fixed the problem for me. It also made this age-old compiler warning go away: In file included from gram.y:14515: scan.c: In function 'yy_try_NUL_trans': scan.c:10307: warning: unused variable 'yyg' I guess by redirecting it into the log file you indicated, but is that a good idea to redirect stderr? Cheers, Jeff -- Michael
Re: [HACKERS] improving speed of make check-world
On Sat, Apr 25, 2015 at 7:59 AM, Peter Eisentraut pete...@gmx.net wrote: On 4/23/15 1:22 PM, Jeff Janes wrote: Something about this commit (dcae5faccab64776376d354d) broke make check in parallel conditions when started from a clean directory. It fails with a different error each time, one example: make -j4 check /dev/null In file included from gram.y:14515: scan.c: In function 'yy_try_NUL_trans': scan.c:10307: warning: unused variable 'yyg' /usr/bin/ld: tab-complete.o: No such file: No such file or directory collect2: ld returned 1 exit status make[3]: *** [psql] Error 1 make[2]: *** [all-psql-recurse] Error 2 make[2]: *** Waiting for unfinished jobs make[1]: *** [all-bin-recurse] Error 2 make: *** [all-src-recurse] Error 2 make: *** Waiting for unfinished jobs I think the problem is that check depends on all, but now also depends on temp-install, which in turn runs install and all. With a sufficient amount of parallelism, you end up running two alls on top of each other. It seems this can be fixed by removing the check: all dependency. Try removing that in the top-level GNUmakefile.in and see if the problem goes away. For completeness, we should then also remove it in the other makefiles. Yep. I spent some time yesterday and today poking at that, but I clearly missed that dependency.. Attached is a patch fixing the problem. I tested check and check-world with some parallel jobs and both worked. In the case of check, the amount of logs is very reduced because all the install process is done by temp-install which redirects everything into tmp_install/log/install.log. -- Michael diff --git a/GNUmakefile.in b/GNUmakefile.in index 361897a..42aeaa6 100644 --- a/GNUmakefile.in +++ b/GNUmakefile.in @@ -62,7 +62,7 @@ distclean maintainer-clean: # Garbage from autoconf: @rm -rf autom4te.cache/ -check check-tests: all +check check-tests: check check-tests installcheck installcheck-parallel installcheck-tests: $(MAKE) -C src/test/regress $@ diff --git a/contrib/test_decoding/Makefile b/contrib/test_decoding/Makefile index 613e9c3..e47ebab 100644 --- a/contrib/test_decoding/Makefile +++ b/contrib/test_decoding/Makefile @@ -39,7 +39,7 @@ submake-test_decoding: REGRESSCHECKS=ddl rewrite toast permissions decoding_in_xact decoding_into_rel binary prepared -regresscheck: all | submake-regress submake-test_decoding temp-install +regresscheck: | submake-regress submake-test_decoding temp-install $(MKDIR_P) regression_output $(pg_regress_check) \ --temp-config $(top_srcdir)/contrib/test_decoding/logical.conf \ @@ -53,7 +53,7 @@ regresscheck-install-force: | submake-regress submake-test_decoding temp-install ISOLATIONCHECKS=mxact delayed_startup ondisk_startup concurrent_ddl_dml -isolationcheck: all | submake-isolation submake-test_decoding temp-install +isolationcheck: | submake-isolation submake-test_decoding temp-install $(MKDIR_P) isolation_output $(pg_isolation_regress_check) \ --temp-config $(top_srcdir)/contrib/test_decoding/logical.conf \ diff --git a/src/bin/initdb/Makefile b/src/bin/initdb/Makefile index fc809a0..d479788 100644 --- a/src/bin/initdb/Makefile +++ b/src/bin/initdb/Makefile @@ -58,7 +58,7 @@ clean distclean maintainer-clean: # ensure that changes in datadir propagate into object file initdb.o: initdb.c $(top_builddir)/src/Makefile.global -check: all +check: $(prove_check) installcheck: diff --git a/src/bin/pg_basebackup/Makefile b/src/bin/pg_basebackup/Makefile index 58f8b66..0d8421a 100644 --- a/src/bin/pg_basebackup/Makefile +++ b/src/bin/pg_basebackup/Makefile @@ -50,7 +50,7 @@ clean distclean maintainer-clean: $(OBJS) rm -rf tmp_check -check: all +check: $(prove_check) installcheck: diff --git a/src/bin/pg_config/Makefile b/src/bin/pg_config/Makefile index 71ce236..dbc9899 100644 --- a/src/bin/pg_config/Makefile +++ b/src/bin/pg_config/Makefile @@ -49,7 +49,7 @@ clean distclean maintainer-clean: rm -f pg_config$(X) $(OBJS) rm -rf tmp_check -check: all +check: $(prove_check) installcheck: diff --git a/src/bin/pg_controldata/Makefile b/src/bin/pg_controldata/Makefile index f7a4010..fd7399b 100644 --- a/src/bin/pg_controldata/Makefile +++ b/src/bin/pg_controldata/Makefile @@ -35,7 +35,7 @@ clean distclean maintainer-clean: rm -f pg_controldata$(X) $(OBJS) rm -rf tmp_check -check: all +check: $(prove_check) installcheck: diff --git a/src/bin/pg_ctl/Makefile b/src/bin/pg_ctl/Makefile index 525e1d4..37eb482 100644 --- a/src/bin/pg_ctl/Makefile +++ b/src/bin/pg_ctl/Makefile @@ -38,7 +38,7 @@ clean distclean maintainer-clean: rm -f pg_ctl$(X) $(OBJS) rm -rf tmp_check -check: all +check: $(prove_check) installcheck: diff --git a/src/bin/scripts/Makefile b/src/bin/scripts/Makefile index 8b6f54c..c831716 100644 --- a/src/bin/scripts/Makefile +++ b/src/bin/scripts/Makefile @@ -69,7 +69,7 @@ clean distclean maintainer-clean: rm -f dumputils.c print.c mbprint.c kwlookup.c
Re: [HACKERS] improving speed of make check-world
On 4/23/15 1:22 PM, Jeff Janes wrote: Something about this commit (dcae5faccab64776376d354d) broke make check in parallel conditions when started from a clean directory. It fails with a different error each time, one example: make -j4 check /dev/null In file included from gram.y:14515: scan.c: In function 'yy_try_NUL_trans': scan.c:10307: warning: unused variable 'yyg' /usr/bin/ld: tab-complete.o: No such file: No such file or directory collect2: ld returned 1 exit status make[3]: *** [psql] Error 1 make[2]: *** [all-psql-recurse] Error 2 make[2]: *** Waiting for unfinished jobs make[1]: *** [all-bin-recurse] Error 2 make: *** [all-src-recurse] Error 2 make: *** Waiting for unfinished jobs I think the problem is that check depends on all, but now also depends on temp-install, which in turn runs install and all. With a sufficient amount of parallelism, you end up running two alls on top of each other. It seems this can be fixed by removing the check: all dependency. Try removing that in the top-level GNUmakefile.in and see if the problem goes away. For completeness, we should then also remove it in the other makefiles. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] improving speed of make check-world
On Thu, Aug 14, 2014 at 10:45 PM, Peter Eisentraut pete...@gmx.net wrote: make check-world creates a temporary installation in every subdirectory it runs a test in, which is stupid: it's very slow and uses a lot of disk space. It's enough to do this once per run. That is the essence of what I have implemented. It cuts the time for make check-world in half or less, and it saves gigabytes of disk space. Something about this commit (dcae5faccab64776376d354d) broke make check in parallel conditions when started from a clean directory. It fails with a different error each time, one example: make -j4 check /dev/null In file included from gram.y:14515: scan.c: In function 'yy_try_NUL_trans': scan.c:10307: warning: unused variable 'yyg' /usr/bin/ld: tab-complete.o: No such file: No such file or directory collect2: ld returned 1 exit status make[3]: *** [psql] Error 1 make[2]: *** [all-psql-recurse] Error 2 make[2]: *** Waiting for unfinished jobs make[1]: *** [all-bin-recurse] Error 2 make: *** [all-src-recurse] Error 2 make: *** Waiting for unfinished jobs If I rerun it without cleaning the tree, is usually passes the second time. Or I can just separate the make and the check like make -j4 /dev/null make check /dev/null but I've grown accustomed to being able to combine them since this problem was first fixed a couple years ago (in a commit I can't seem to find) I have: GNU Make 4.0 Built for x86_64-unknown-linux-gnu I was using ccache, but I still get the problem without using it. Cheers, Jeff
Re: [HACKERS] improving speed of make check-world
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
Re: [HACKERS] improving speed of make check-world
On Sat, Apr 11, 2015 at 8:48 PM, Michael Paquier wrote: 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 A trick that we could use here is to store a timestamp when running up to completion build and the temporary installation in vcregress, and skip the temporary installation if timestamp of vcregress' temporary installation is newer than the one of build. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] improving speed of make check-world
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.
Bonus track: pg_regress.c is missing the description of --bindir in help().
--
Michael
From 5b729058335cf4a77e22de25bce4c90fa6d686c8 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, 42 insertions(+), 24 deletions(-)
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index bd3dd2c..c7ce5aa 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/psql,
--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(
+
Re: [HACKERS] improving speed of make check-world
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
Re: [HACKERS] improving speed of make check-world
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.
- {top-builddir, required_argument, NULL, 11},
+ {datadir, required_argument, NULL, 12},
In pg_regress.c datadir is a new option but it is used nowhere, so it
could be as well removed.
Yeah, that's an oversight that is easily corrected.
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index bd3dd2c..1e09c74 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -94,7 +94,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,39 +106,56 @@ sub installcheck
sub check
{
+ my $cwd = getcwd();
+
+ system($0, 'install.pl', $cwd/tmp_install);
+ my $status = $? 8;
+ exit $status if $status;
+
+ $ENV{PATH} = $cwd/tmp_install/bin;$cwd/tmp_install/lib;$ENV{PATH};
+
my @args = (
../../../$Config/pg_regress/pg_regress,
--dlpath=.,
- --psqldir=../../../$Config/psql,
+ --bindir=,
--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);
- my $status = $? 8;
+ $status = $? 8;
exit $status if $status;
}
sub ecpgcheck
{
chdir $startdir;
+
system(msbuild ecpg_regression.proj /p:config=$Config);
my $status = $? 8;
exit $status if $status;
+
+ my $cwd = getcwd();
+
+ system($0, 'install.pl', $cwd/tmp_install);
+ $status = $? 8;
+ exit $status if $status;
+
+ $ENV{PATH} = $cwd/tmp_install/bin;$cwd/tmp_install/lib;$ENV{PATH};
+
chdir $topdir/src/interfaces/ecpg/test;
$schedule = ecpg;
my @args = (
../../../../$Config/pg_regress_ecpg/pg_regress_ecpg,
- --psqldir=../../../$Config/psql,
+ --bindir=,
--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);
@@ -153,7 +170,7 @@ sub isolationcheck
../../../$Config/pg_isolation_regress);
my @args = (
../../../$Config/pg_isolation_regress/pg_isolation_regress,
- --psqldir=../../../$Config/psql,
+ --bindir=../../../$Config/psql,
--inputdir=.,
--schedule=./isolation_schedule);
push(@args, $maxconn) if $maxconn;
@@ -202,7 +219,7 @@ sub plcheck
print Checking $lang\n;
my @args = (
../../../$Config/pg_regress/pg_regress,
- --psqldir=../../../$Config/psql,
+ --bindir=../../../$Config/psql,
--dbname=pl_regression, @lang_args, @tests);
system(@args);
my $status = $? 8;
@@ -237,7 +254,7 @@ sub contribcheck
my @opts = fetchRegressOpts();
my @args = (
../../$Config/pg_regress/pg_regress,
- --psqldir=../../$Config/psql,
+ --bindir=../../$Config/psql,
--dbname=contrib_regression, @opts, @tests);
system(@args);
my $status = $? 8;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] improving speed of make check-world
On 8/31/14 5:36 AM, Fabien COELHO wrote: Running make -j2 check-world does not work because initdb is not found by pg_regress. but make -j1 check-world does work fine. It seems that some dependencies might be missing and there is a race condition between temporary install and running some checks?? Maybe it is not expected to work anyway? See below suggestions to make it work. Here is an updated patch that fixes this problem. The previous problem was simply a case were the make rules were not parallel-safe. For recursive make, we (through magic) set up targets like this: check: check-subdir1-check check-subdir2-check And with my old patch we added check: temp-install So the aggregate prerequisites were in effect something like check: check-subdir1-check check-subdir2-check temp-install And so there was nothing stopping a parallel make to descend into the subdirectories before the temp install was set up. What we need is additional prerequisites like check-subdir1-check check-subdir2-check: temp-install I have hacked this directly into the $(recurse) function, which is ugly. This could possibly be improved somehow, but the effect would be the same in any case. With this, I can now run things like make -C src/pl check -j3 make -C src/bin check -j8 A full make check-world -j$X is still prone to fail because some test suites can't run in parallel with others, but that's a separate problem to fix. Note: We have in the meantime added logic to pg_regress to clean up the temporary installation after the run. This changes that because pg_regress is no longer responsible for the temporary installation. pg_regress still cleans up the temporary data directory, so you still get quite a bit of space savings. But the temporary installation is not cleaned up. But since we would now only use a single temporary installation, the disk space usage still stays in the same order of magnitude. diff --git a/.gitignore b/.gitignore index 715f817..77feb4c 100644 --- a/.gitignore +++ b/.gitignore @@ -35,3 +35,4 @@ lib*.pc /pgsql.sln.cache /Debug/ /Release/ +/tmp_install/ diff --git a/GNUmakefile.in b/GNUmakefile.in index 69e0824..361897a 100644 --- a/GNUmakefile.in +++ b/GNUmakefile.in @@ -47,6 +47,7 @@ $(call recurse,distprep,doc src config contrib) # it's not built by default $(call recurse,clean,doc contrib src config) clean: + rm -rf tmp_install/ # Garbage from autoconf: @rm -rf autom4te.cache/ diff --git a/contrib/earthdistance/Makefile b/contrib/earthdistance/Makefile index 93dcbe3..cde1ae6 100644 --- a/contrib/earthdistance/Makefile +++ b/contrib/earthdistance/Makefile @@ -7,7 +7,7 @@ DATA = earthdistance--1.0.sql earthdistance--unpackaged--1.0.sql PGFILEDESC = earthdistance - calculate distances on the surface of the Earth REGRESS = earthdistance -REGRESS_OPTS = --extra-install=contrib/cube +EXTRA_INSTALL = contrib/cube LDFLAGS_SL += $(filter -lm, $(LIBS)) diff --git a/contrib/pg_upgrade/test.sh b/contrib/pg_upgrade/test.sh index 75b6357..3012ed0 100644 --- a/contrib/pg_upgrade/test.sh +++ b/contrib/pg_upgrade/test.sh @@ -87,7 +87,7 @@ if [ $1 = '--install' ]; then # use psql from the proper installation directory, which might # be outdated or missing. But don't override anything else that's # already in EXTRA_REGRESS_OPTS. - EXTRA_REGRESS_OPTS=$EXTRA_REGRESS_OPTS --psqldir='$bindir' + EXTRA_REGRESS_OPTS=$EXTRA_REGRESS_OPTS --bindir='$bindir' export EXTRA_REGRESS_OPTS fi diff --git a/contrib/test_decoding/Makefile b/contrib/test_decoding/Makefile index 438be44..613e9c3 100644 --- a/contrib/test_decoding/Makefile +++ b/contrib/test_decoding/Makefile @@ -39,35 +39,33 @@ submake-test_decoding: REGRESSCHECKS=ddl rewrite toast permissions decoding_in_xact decoding_into_rel binary prepared -regresscheck: all | submake-regress submake-test_decoding +regresscheck: all | submake-regress submake-test_decoding temp-install $(MKDIR_P) regression_output $(pg_regress_check) \ --temp-config $(top_srcdir)/contrib/test_decoding/logical.conf \ - --temp-install=./tmp_check \ - --extra-install=contrib/test_decoding \ + --temp-instance=./tmp_check \ --outputdir=./regression_output \ $(REGRESSCHECKS) -regresscheck-install-force: | submake-regress submake-test_decoding +regresscheck-install-force: | submake-regress submake-test_decoding temp-install $(pg_regress_installcheck) \ - --extra-install=contrib/test_decoding \ $(REGRESSCHECKS) ISOLATIONCHECKS=mxact delayed_startup ondisk_startup concurrent_ddl_dml -isolationcheck: all | submake-isolation submake-test_decoding +isolationcheck: all | submake-isolation submake-test_decoding temp-install $(MKDIR_P) isolation_output $(pg_isolation_regress_check) \ --temp-config $(top_srcdir)/contrib/test_decoding/logical.conf \ - --extra-install=contrib/test_decoding \ --outputdir=./isolation_output \ $(ISOLATIONCHECKS)
Re: [HACKERS] improving speed of make check-world
Peter Eisentraut pete...@gmx.net writes: On 8/31/14 5:36 AM, Fabien COELHO wrote: Running make -j2 check-world does not work because initdb is not found by pg_regress. but make -j1 check-world does work fine. It seems that some dependencies might be missing and there is a race condition between temporary install and running some checks?? Maybe it is not expected to work anyway? See below suggestions to make it work. Here is an updated patch that fixes this problem. Doesn't the Windows side of the house still depend on that functionality you removed from pg_regress? Perhaps that's not a big deal to fix, but it seems like a commit-blocker from here. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] improving speed of make check-world
Hello Peter, Here is a review: The version 2 of the patch applies cleanly on current head. The ability to generate and reuse a temporary installation for different tests looks quite useful, thus putting install out of pg_regress and in make seems reasonnable. However I'm wondering whether there could be some use case of pg_regress where doing the install might be useful nevertheless, say for manually doing things on the command line, in which case keeping the feature, even if not used by default, could be nice? About changes: I think that more comments would be welcome, eg with_temp_install. There is not a single documentation change. Should there be some? Hmmm... I have not found much documentation about pg_regress... I have tested make check, check-world, as well as make check in contrib sub-directories. It seems to work fine in sequential mode. Running make -j2 check-world does not work because initdb is not found by pg_regress. but make -j1 check-world does work fine. It seems that some dependencies might be missing and there is a race condition between temporary install and running some checks?? Maybe it is not expected to work anyway? See below suggestions to make it work. However in this case the error message passed by pg_regress contains an error: pg_regress: initdb failed Examine /home/fabien/DEV/GIT/postgresql/contrib/btree_gin/log/initdb.log for the reason. Command was: initdb -D /home/fabien/DEV/GIT/postgresql/contrib/btree_gin/./tmp_check/data --noclean --nosync /home/fabien/DEV/GIT/postgresql/contrib/btree_gin/./tmp_check/log/initdb.log 21 make[2]: *** [check] Error 2 The error messages point to non existing log file (./tmp_check is missing), the temp_instance variable should be used in the error message as well as in the commands. Maybe other error messages have the same issues. I do not like much the MAKELEVEL hack in a phony target. When running in parallel, several makes may have the same level anyway, not sure what would happen... Maybe it is the origin of the race condition which makes initdb not to be found above. I would suggest to try to rely on dependences, maybe something like the following could work to ensure that an installation is done once and only once per make invocation, whatever the underlying parallelism levels, as well as a possibility to reuse the previous installation. # must be defined somewhere common to all makefiles ifndef MAKE_NONCE # define a nanosecond timestamp export MAKE_NONCE := $(shell date +%s.%N) endif # actual new tmp installation .tmp_install: $(RM) ./.tmp_install.* $(RM) -r ./tmp_install # create tmp installation... touch $@ # tmp installation for the nonce .tmp_install.$(MAKE_NONCE): .tmp_install touch $@ # generate a new tmp installation by default # make USE_TMP_INSTALL=1 ... reuses a previous installation if available ifdef USE_TMP_INSTALL TMP_INSTALL = .tmp_install else TMP_INSTALL = .tmp_install.$(MAKE_NONCE) endif # USE_TMP_INSTALL .PHONY: main-temp-install main-temp-install: $(TMP_INSTALL) .PHONY: extra-temp-install extra-temp-install: main-temp-install # do EXTRA_INSTALL MSVC build is not yet adjusted for this. Looking at vcregress.pl, this should be fairly straightforward. No idea about that point. -- Fabien. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] improving speed of make check-world
# actual new tmp installation .tmp_install: $(RM) ./.tmp_install.* $(RM) -r ./tmp_install # create tmp installation... touch $@ # tmp installation for the nonce .tmp_install.$(MAKE_NONCE): .tmp_install touch $@ Oops, I got it wrong, the install would not be reexecuted the second time. Maybe someting more like: ifdef USE_ONE_INSTALL TMP_INSTALL = .tmp_install.once else TMP_INSTALL = .tmp_install.$(MAKE_NONCE) endif $(TMP_INSTALL): $(RM) -r ./tmp_install .tmp_install.* # do installation... touch $@ So that the file target is different each time it is run. Hopefully. -- Fabien. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] improving speed of make check-world
Updated, rebased patch. diff --git a/.gitignore b/.gitignore index 681af08..823d3ac 100644 --- a/.gitignore +++ b/.gitignore @@ -34,3 +34,4 @@ lib*.pc /pgsql.sln.cache /Debug/ /Release/ +/tmp_install/ diff --git a/GNUmakefile.in b/GNUmakefile.in index 69e0824..5667943 100644 --- a/GNUmakefile.in +++ b/GNUmakefile.in @@ -47,6 +47,7 @@ $(call recurse,distprep,doc src config contrib) # it's not built by default $(call recurse,clean,doc contrib src config) clean: + rm -rf tmp_install/ # Garbage from autoconf: @rm -rf autom4te.cache/ @@ -61,6 +62,8 @@ distclean maintainer-clean: # Garbage from autoconf: @rm -rf autom4te.cache/ +check-world: temp-install + check check-tests: all check check-tests installcheck installcheck-parallel installcheck-tests: diff --git a/contrib/earthdistance/Makefile b/contrib/earthdistance/Makefile index 93dcbe3..cde1ae6 100644 --- a/contrib/earthdistance/Makefile +++ b/contrib/earthdistance/Makefile @@ -7,7 +7,7 @@ DATA = earthdistance--1.0.sql earthdistance--unpackaged--1.0.sql PGFILEDESC = earthdistance - calculate distances on the surface of the Earth REGRESS = earthdistance -REGRESS_OPTS = --extra-install=contrib/cube +EXTRA_INSTALL = contrib/cube LDFLAGS_SL += $(filter -lm, $(LIBS)) diff --git a/contrib/pg_upgrade/test.sh b/contrib/pg_upgrade/test.sh index 7bbd2c7..7d493d9 100644 --- a/contrib/pg_upgrade/test.sh +++ b/contrib/pg_upgrade/test.sh @@ -80,7 +80,7 @@ if [ $1 = '--install' ]; then # use psql from the proper installation directory, which might # be outdated or missing. But don't override anything else that's # already in EXTRA_REGRESS_OPTS. - EXTRA_REGRESS_OPTS=$EXTRA_REGRESS_OPTS --psqldir='$bindir' + EXTRA_REGRESS_OPTS=$EXTRA_REGRESS_OPTS --bindir='$bindir' export EXTRA_REGRESS_OPTS fi diff --git a/contrib/test_decoding/Makefile b/contrib/test_decoding/Makefile index d7f32c3..6210ddb 100644 --- a/contrib/test_decoding/Makefile +++ b/contrib/test_decoding/Makefile @@ -39,35 +39,33 @@ submake-test_decoding: REGRESSCHECKS=ddl rewrite toast permissions decoding_in_xact binary prepared -regresscheck: all | submake-regress submake-test_decoding +regresscheck: all | submake-regress submake-test_decoding temp-install $(MKDIR_P) regression_output $(pg_regress_check) \ --temp-config $(top_srcdir)/contrib/test_decoding/logical.conf \ - --temp-install=./tmp_check \ - --extra-install=contrib/test_decoding \ + --temp-instance=./tmp_check \ --outputdir=./regression_output \ $(REGRESSCHECKS) -regresscheck-install-force: | submake-regress submake-test_decoding +regresscheck-install-force: | submake-regress submake-test_decoding temp-install $(pg_regress_installcheck) \ - --extra-install=contrib/test_decoding \ $(REGRESSCHECKS) ISOLATIONCHECKS=mxact delayed_startup concurrent_ddl_dml -isolationcheck: all | submake-isolation submake-test_decoding +isolationcheck: all | submake-isolation submake-test_decoding temp-install $(MKDIR_P) isolation_output $(pg_isolation_regress_check) \ --temp-config $(top_srcdir)/contrib/test_decoding/logical.conf \ - --extra-install=contrib/test_decoding \ --outputdir=./isolation_output \ $(ISOLATIONCHECKS) -isolationcheck-install-force: all | submake-isolation submake-test_decoding +isolationcheck-install-force: all | submake-isolation submake-test_decoding temp-install $(pg_isolation_regress_installcheck) \ - --extra-install=contrib/test_decoding \ $(ISOLATIONCHECKS) PHONY: submake-test_decoding submake-regress check \ regresscheck regresscheck-install-force \ isolationcheck isolationcheck-install-force + +temp-install: EXTRA_INSTALL=contrib/test_decoding diff --git a/src/Makefile.global.in b/src/Makefile.global.in index 0ffc1e8..3238c5c 100644 --- a/src/Makefile.global.in +++ b/src/Makefile.global.in @@ -41,6 +41,7 @@ MAJORVERSION = @PG_MAJORVERSION@ # Support for VPATH builds vpath_build = @vpath_build@ +abs_top_builddir = @abs_top_builddir@ abs_top_srcdir = @abs_top_srcdir@ ifneq ($(vpath_build),yes) @@ -296,6 +297,17 @@ BZIP2 = bzip2 # Testing +check: all temp-install + +.PHONY: temp-install +temp-install: +ifeq ($(MAKELEVEL),0) + rm -rf $(abs_top_builddir)/tmp_install + $(MKDIR_P) $(abs_top_builddir)/tmp_install/log + $(MAKE) -C $(top_builddir) DESTDIR=$(abs_top_builddir)/tmp_install install $(abs_top_builddir)/tmp_install/log/install.log 21 +endif + for extra in $(EXTRA_INSTALL); do $(MAKE) -C $(top_builddir)/$$extra DESTDIR=$(abs_top_builddir)/tmp_install install $(abs_top_builddir)/tmp_install/log/install.log 21 || exit; done + PROVE = @PROVE@ PG_PROVE_FLAGS = --ext='.pl' -I $(top_srcdir)/src/test/perl/ PROVE_FLAGS = --verbose @@ -310,14 +322,16 @@ define ld_library_path_var $(if $(filter $(PORTNAME),darwin),DYLD_LIBRARY_PATH,$(if $(filter $(PORTNAME),aix),LIBPATH,LD_LIBRARY_PATH)) endef +define with_temp_install +PATH=$(abs_top_builddir)/tmp_install$(bindir):$$PATH $(call
Re: [HACKERS] improving speed of make check-world
On 08/15/2014 08:45 AM, Peter Eisentraut wrote: make check-world creates a temporary installation in every subdirectory it runs a test in, which is stupid: it's very slow and uses a lot of disk space. It's enough to do this once per run. That is the essence of what I have implemented. It cuts the time for make check-world in half or less, and it saves gigabytes of disk space. Nice! The idea is that we only maintain one temporary installation under the top-level directory. By looking at the variable MAKELEVEL, we can tell whether we are the top-level make invocation. If so, make a temp installation. If not, we know that the upper layers have already done it and we can just point to the existing temp installation. I do this by ripping out the temp installation logic from pg_regress and implementing it directly in the makefiles. This is much simpler and has additional potential benefits: The pg_regress temp install mode is actually a combination of two functionalities: temp install plus temp instance. Until now, you could only get the two together, but the temp instance functionality is actually quite useful by itself. It opens up the possibility of implementing make check for external pgxs modules, for example. Also, you could now run the temp installation step using parallel make, making it even faster. This was previously disabled because the make flags would have to pass through pg_regress. It still won't quite work to run make check-world -j8, say, because we can't actually run the tests in parallel (yet), but something like make -C src/test/regress check -j8 should work. To that end, I have renamed the pg_regress --temp-install option to --temp-instance. Since --temp-install is only used inside the source tree, this shouldn't cause any compatibility problems. Yeah, that all makes a lot of sense. The new EXTRA_INSTALL makefile variable ought to be documented in extend.sgml, where we list REGRESS_OPTS and others. - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] improving speed of make check-world
On 8/25/14 1:32 PM, Heikki Linnakangas wrote: The new EXTRA_INSTALL makefile variable ought to be documented in extend.sgml, where we list REGRESS_OPTS and others. But EXTRA_INSTALL is only of use inside the main source tree, not by extensions. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] improving speed of make check-world
make check-world creates a temporary installation in every subdirectory it runs a test in, which is stupid: it's very slow and uses a lot of disk space. It's enough to do this once per run. That is the essence of what I have implemented. It cuts the time for make check-world in half or less, and it saves gigabytes of disk space. The idea is that we only maintain one temporary installation under the top-level directory. By looking at the variable MAKELEVEL, we can tell whether we are the top-level make invocation. If so, make a temp installation. If not, we know that the upper layers have already done it and we can just point to the existing temp installation. I do this by ripping out the temp installation logic from pg_regress and implementing it directly in the makefiles. This is much simpler and has additional potential benefits: The pg_regress temp install mode is actually a combination of two functionalities: temp install plus temp instance. Until now, you could only get the two together, but the temp instance functionality is actually quite useful by itself. It opens up the possibility of implementing make check for external pgxs modules, for example. Also, you could now run the temp installation step using parallel make, making it even faster. This was previously disabled because the make flags would have to pass through pg_regress. It still won't quite work to run make check-world -j8, say, because we can't actually run the tests in parallel (yet), but something like make -C src/test/regress check -j8 should work. To that end, I have renamed the pg_regress --temp-install option to --temp-instance. Since --temp-install is only used inside the source tree, this shouldn't cause any compatibility problems. MSVC build is not yet adjusted for this. Looking at vcregress.pl, this should be fairly straightforward. diff --git a/.gitignore b/.gitignore index 681af08..823d3ac 100644 --- a/.gitignore +++ b/.gitignore @@ -34,3 +34,4 @@ lib*.pc /pgsql.sln.cache /Debug/ /Release/ +/tmp_install/ diff --git a/GNUmakefile.in b/GNUmakefile.in index 69e0824..5667943 100644 --- a/GNUmakefile.in +++ b/GNUmakefile.in @@ -47,6 +47,7 @@ $(call recurse,distprep,doc src config contrib) # it's not built by default $(call recurse,clean,doc contrib src config) clean: + rm -rf tmp_install/ # Garbage from autoconf: @rm -rf autom4te.cache/ @@ -61,6 +62,8 @@ distclean maintainer-clean: # Garbage from autoconf: @rm -rf autom4te.cache/ +check-world: temp-install + check check-tests: all check check-tests installcheck installcheck-parallel installcheck-tests: diff --git a/contrib/earthdistance/Makefile b/contrib/earthdistance/Makefile index 93dcbe3..cde1ae6 100644 --- a/contrib/earthdistance/Makefile +++ b/contrib/earthdistance/Makefile @@ -7,7 +7,7 @@ DATA = earthdistance--1.0.sql earthdistance--unpackaged--1.0.sql PGFILEDESC = earthdistance - calculate distances on the surface of the Earth REGRESS = earthdistance -REGRESS_OPTS = --extra-install=contrib/cube +EXTRA_INSTALL = contrib/cube LDFLAGS_SL += $(filter -lm, $(LIBS)) diff --git a/contrib/pg_upgrade/test.sh b/contrib/pg_upgrade/test.sh index 7bbd2c7..7d493d9 100644 --- a/contrib/pg_upgrade/test.sh +++ b/contrib/pg_upgrade/test.sh @@ -80,7 +80,7 @@ if [ $1 = '--install' ]; then # use psql from the proper installation directory, which might # be outdated or missing. But don't override anything else that's # already in EXTRA_REGRESS_OPTS. - EXTRA_REGRESS_OPTS=$EXTRA_REGRESS_OPTS --psqldir='$bindir' + EXTRA_REGRESS_OPTS=$EXTRA_REGRESS_OPTS --bindir='$bindir' export EXTRA_REGRESS_OPTS fi diff --git a/contrib/test_decoding/Makefile b/contrib/test_decoding/Makefile index d7f32c3..6210ddb 100644 --- a/contrib/test_decoding/Makefile +++ b/contrib/test_decoding/Makefile @@ -39,35 +39,33 @@ submake-test_decoding: REGRESSCHECKS=ddl rewrite toast permissions decoding_in_xact binary prepared -regresscheck: all | submake-regress submake-test_decoding +regresscheck: all | submake-regress submake-test_decoding temp-install $(MKDIR_P) regression_output $(pg_regress_check) \ --temp-config $(top_srcdir)/contrib/test_decoding/logical.conf \ - --temp-install=./tmp_check \ - --extra-install=contrib/test_decoding \ + --temp-instance=./tmp_check \ --outputdir=./regression_output \ $(REGRESSCHECKS) -regresscheck-install-force: | submake-regress submake-test_decoding +regresscheck-install-force: | submake-regress submake-test_decoding temp-install $(pg_regress_installcheck) \ - --extra-install=contrib/test_decoding \ $(REGRESSCHECKS) ISOLATIONCHECKS=mxact delayed_startup concurrent_ddl_dml -isolationcheck: all | submake-isolation submake-test_decoding +isolationcheck: all | submake-isolation submake-test_decoding temp-install $(MKDIR_P) isolation_output $(pg_isolation_regress_check) \ --temp-config $(top_srcdir)/contrib/test_decoding/logical.conf \ -
