Re: PGXS "check" target forcing an install ?

From: Noah Misch <noah(at)leadboat(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Cc: PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PGXS "check" target forcing an install ?
Date: 2015-09-24 20:21:59
Message-ID: 20150924202159.GA3900560@tornado.leadboat.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Jun 26, 2015 at 09:09:15AM -0400, Robert Haas wrote:
> On Tue, Jun 23, 2015 at 1:31 AM, Michael Paquier <michael(dot)paquier(at)gmail(dot)com> wrote:
> >> I tracked the dangerous -rf to come from Makefile.global and it's empty
> >> string being due to abs_top_builddir not being define in my own Makefile.
> >> But beside that, which I can probably fix, it doesn't sound correct
> >> that a "check" rule insists in finding an "install" rule.
> >
> > Oops, this is a regression, and a dangerous one indeed. This is caused
> > by dcae5fac.
> >
> > One fix is to use NO_TEMP_INSTALL=yes in Makefile.global in the
> > context of PGXS, like in the patch attached, this variable needing to
> > be set before Makefile.global is loaded.

This seems reasonable in concept, though the patch's addition is off-topic in
a section marked "# Support for VPATH builds". However, ...

> Gulp. I certainly agree that emitting rm -rf /tmp_install is a scary
> thing for a PostgreSQL Makefile to be doing. Fortunately, people
> aren't likely to have a directory under / by that name, and maybe not
> permissions on it even if they did, but all the same it's not good. I
> propose trying to guard against that a bit more explicitly, as in the
> attached.

... agreed.

> --- a/src/Makefile.global.in
> +++ b/src/Makefile.global.in
> @@ -304,12 +304,14 @@ check: temp-install
> .PHONY: temp-install
> temp-install:
> ifndef NO_TEMP_INSTALL
> +ifneq ($(abs_top_builddir),)
> ifeq ($(MAKELEVEL),0)
> rm -rf '$(abs_top_builddir)'/tmp_install
> $(MAKE) -C '$(top_builddir)' DESTDIR='$(abs_top_builddir)'/tmp_install install
> endif
> $(if $(EXTRA_INSTALL),for extra in $(EXTRA_INSTALL); do $(MAKE) -C '$(top_builddir)'/$$extra DESTDIR='$(abs_top_builddir)'/tmp_install install || exit; done)
> endif
> +endif

With this in place, there's no need for the NO_TEMP_INSTALL change.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2015-09-24 20:25:03 Re: Rename withCheckOptions to insertedCheckClauses
Previous Message Tom Lane 2015-09-24 20:08:00 Re: Rename withCheckOptions to insertedCheckClauses