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