From: | Andres Freund <andres(at)anarazel(dot)de> |
---|---|
To: | Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Teach pg_upgrade test to honor NO_TEMP_INSTALL |
Date: | 2019-05-21 19:19:18 |
Message-ID: | 20190521191918.z7kwnrlj45mk2k67@alap3.anarazel.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
On 2019-05-21 14:48:27 -0400, Andrew Dunstan wrote:
> On 5/20/19 9:58 PM, Andres Freund wrote:
> > Hi Andrew,
> >
> > On 2019-03-30 16:42:16 -0400, Andrew Dunstan wrote:
> >> On some machines (*cough* Mingw *cough*) installs are very slow. We've
> >> ameliorated this by allowing temp installs to be reused, but the
> >> pg_upgrade Makefile never got the message. Here's a patch that does
> >> that. I'd like to backpatch it, at least to 9.5 where we switched the
> >> pg_upgrade location. The risk seems appropriately low and it only
> >> affects our test regime.
> > I'm confused as to why this was done as a purely optional path, rather
> > than just ripping out the pg_upgrade specific install?
> >
> > See also discussion around https://www.postgresql.org/message-id/21766.1558397960%40sss.pgh.pa.us
> >
>
> By specifying NO_TEMP_INSTALL you are in effect certifying that there is
> already a suitable temp install available. But that might well not be
> the case.
But all that takes is adding a dependency to temp-install in
src/bin/pg_upgrade/Makefile's check target? Like many other regression
test? And the temp-install rule already honors NO_TEMP_INSTALL:
temp-install: | submake-generated-headers
ifndef NO_TEMP_INSTALL
ifneq ($(abs_top_builddir),)
ifeq ($(MAKELEVEL),0)
rm -rf '$(abs_top_builddir)'/tmp_install
$(MKDIR_P) '$(abs_top_builddir)'/tmp_install/log
$(MAKE) -C '$(top_builddir)' DESTDIR='$(abs_top_builddir)'/tmp_install install >'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1
$(MAKE) -j1 $(if $(CHECKPREP_TOP),-C $(CHECKPREP_TOP),) checkprep >>'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1
endif
endif
endif
I'm not saying that you shouldn't have added NO_TEMP_INSTALL support or
something, I'm confused as to why the support for custom installations
inside test.sh was retained.
Roughly like in the attached?
Greetings,
Andres Freund
Attachment | Content-Type | Size |
---|---|---|
noinstall.diff | text/x-diff | 2.8 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Fujii Masao | 2019-05-21 19:32:38 | Re: VACUUM fails to parse 0 and 1 as boolean value |
Previous Message | Bruce Momjian | 2019-05-21 19:11:57 | Re: PG 12 draft release notes |