Re: [COMMITTERS] pgsql: Default monitoring roles - Mailing list pgsql-hackers

From Tom Lane
Subject Re: [COMMITTERS] pgsql: Default monitoring roles
Date
Msg-id 7955.1490905221@sss.pgh.pa.us
Whole thread Raw
In response to Re: [COMMITTERS] pgsql: Default monitoring roles  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
I wrote:
> Simon Riggs <simon@2ndquadrant.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

diff --git a/src/makefiles/pgxs.mk b/src/makefiles/pgxs.mk
index c27004e..c156806 100644
*** a/src/makefiles/pgxs.mk
--- b/src/makefiles/pgxs.mk
*************** check:
*** 282,291 ****
  else
  check: submake $(REGRESS_PREP)
      $(pg_regress_check) $(REGRESS_OPTS) $(REGRESS)

  temp-install: EXTRA_INSTALL+=$(subdir)
  endif
- endif # REGRESS


  # STANDARD RULES
--- 282,293 ----
  else
  check: submake $(REGRESS_PREP)
      $(pg_regress_check) $(REGRESS_OPTS) $(REGRESS)
+ endif
+ endif # REGRESS

+ ifndef PGXS
  temp-install: EXTRA_INSTALL+=$(subdir)
  endif


  # STANDARD RULES

pgsql-hackers by date:

Previous
From: Pavel Stehule
Date:
Subject: Re: New CORRESPONDING clause design
Next
From: Stephen Frost
Date:
Subject: Re: [PATCH] Reduce src/test/recovery verbosity