Re: Teach pg_upgrade test to honor NO_TEMP_INSTALL

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

In response to

Responses

Browse pgsql-hackers by date

  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