Re: [COMMITTERS] pgsql: Default monitoring roles

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Simon Riggs <simon(at)2ndquadrant(dot)com>
Cc: Erik Rijkers <er(at)xs4all(dot)nl>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: [COMMITTERS] pgsql: Default monitoring roles
Date: 2017-03-30 20:20:21
Message-ID: 7955.1490905221@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

I wrote:
> Simon Riggs <simon(at)2ndquadrant(dot)com> writes:
>> Weird. make check-world just skipped that directory. I guess for Dave also.

> If you just did check-world, it probably didn't build contrib modules that
> don't have tests, because the "check" target wouldn't do anything without
> tests to run.

I wondered why this is --- wouldn't "check" depend on "all", so that
things get built, even if there's no tests to run?

The answer is, no it doesn't. "check" depends on "temp-install", and
then temp-install indirectly depends on "all". So when you say
"make check-world", the reason that any code gets built at all is that
we're trying to install all the executables into the temporary
installation. However, the basic temp-install target only installs
(and therefore only forces building of) the core code. In a contrib
subdirectory, what is supposed to happen is that this line in
pgxs.mk causes that contrib subdirectory to also get built/installed:

temp-install: EXTRA_INSTALL+=$(subdir)

But if you look around a bit further, you discover that that line is
inside an "ifdef REGRESS" block --- therefore, unless the module
defines at least one REGRESS-style test, it's not built by "make check".

I thought that the attached patch would be a narrow fix for this,
just moving that dependency out of the "ifdef REGRESS" block.
However, it seems to send "make" into some infinite loop, at
least with make 3.81 which I'm using here. Not sure why.

I also experimented with just changing "check: temp-install"
in Makefile.global to be "check: all temp-install", and that
seemed to work, but since I don't understand why it wasn't
like that already, I'm a bit afraid of that solution.

regards, tom lane

Attachment Content-Type Size
make-check-fix-FAILS.patch text/x-diff 594 bytes

In response to

Browse pgsql-committers by date

  From Date Subject
Next Message Tom Lane 2017-03-30 21:13:52 pgsql: Fix broken markup.
Previous Message Dave Page 2017-03-30 19:36:39 Re: pgsql: Default monitoring roles

Browse pgsql-hackers by date

  From Date Subject
Next Message Stephen Frost 2017-03-30 20:29:19 Re: [PATCH] Reduce src/test/recovery verbosity
Previous Message Paul Jungwirth 2017-03-30 20:19:52 Re: Postgres Permissions Article