Re: 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.

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

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.

Updated, rebased patch.

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" 2>&1
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.

> # 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.

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.

Peter Eisentraut <peter_e(at)gmx(dot)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

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).

- {"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.
--
Michael

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.

On Sun, Mar 8, 2015 at 10:22 PM, Peter Eisentraut <peter_e(at)gmx(dot)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

On Sun, Mar 8, 2015 at 10:46 PM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)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

On 3/9/15 2:51 AM, Michael Paquier wrote:
> On Sun, Mar 8, 2015 at 10:46 PM, Michael Paquier
> <michael(dot)paquier(at)gmail(dot)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?

On Sat, Apr 11, 2015 at 4:35 AM, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:
> On 3/9/15 2:51 AM, Michael Paquier wrote:
>> On Sun, Mar 8, 2015 at 10:46 PM, Michael Paquier
>> <michael(dot)paquier(at)gmail(dot)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

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

On Thu, Aug 14, 2014 at 10:45 PM, Peter Eisentraut <peter_e(at)gmx(dot)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

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 "all"s 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.

On Sat, Apr 25, 2015 at 7:59 AM, Peter Eisentraut <peter_e(at)gmx(dot)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 "all"s 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

On Sat, Apr 25, 2015 at 7:23 AM, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
wrote:

> On Sat, Apr 25, 2015 at 7:59 AM, Peter Eisentraut <peter_e(at)gmx(dot)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 "all"s 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
>

On Tue, Apr 28, 2015 at 1:46 AM, Jeff Janes <jeff(dot)janes(at)gmail(dot)com> wrote:
> On Sat, Apr 25, 2015 at 7:23 AM, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
> wrote:
>>
>> On Sat, Apr 25, 2015 at 7:59 AM, Peter Eisentraut <peter_e(at)gmx(dot)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 "all"s 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?

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.
--
Michael

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.

Michael Paquier <michael(dot)paquier(at)gmail(dot)com> writes:
> On Tue, Apr 28, 2015 at 1:46 AM, Jeff Janes <jeff(dot)janes(at)gmail(dot)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