Re: PGXS "check" target forcing an install ? - Mailing list pgsql-hackers

From Noah Misch
Subject Re: PGXS "check" target forcing an install ?
Date
Msg-id 20150924202159.GA3900560@tornado.leadboat.com
Whole thread Raw
In response to Re: PGXS "check" target forcing an install ?  (Robert Haas <robertmhaas@gmail.com>)
Responses Re: PGXS "check" target forcing an install ?  (Robert Haas <robertmhaas@gmail.com>)
List 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.paquier@gmail.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_installinstall || exit; done)
 
>  endif
> +endif

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



pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: Rename withCheckOptions to insertedCheckClauses
Next
From: Tom Lane
Date:
Subject: Re: Rename withCheckOptions to insertedCheckClauses