From: Alexander Lakhin <exclusion@gmail.com>
Subject: Re: make installcheck-world in a clean environment
To: Thomas Munro <thomas.munro@gmail.com>
Cc: David Steele <david@pgmasters.net>, Michael Paquier
 <michael@paquier.xyz>, Andres Freund <andres@anarazel.de>,
 Dmitry Dolgov <9erthalion6@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>,
 Robert Haas <robertmhaas@gmail.com>,
 PostgreSQL-development <pgsql-hackers@postgresql.org>
References: <24834.1525712835@sss.pgh.pa.us>
 <20180912072049.GL25160@paquier.xyz>
 <171724bb-0436-3665-6738-cbd42bdf9783@gmail.com>
 <8351.1542331145@sss.pgh.pa.us>
 <6f7a6818-78d8-48ee-1a0f-e4ad30615af3@gmail.com>
 <CA+q6zcWE_tWhwZibGU+1ktDjHZR__Bxe5a+aybqwr9ummkAbBQ@mail.gmail.com>
 <502e87c5-da7d-fc75-6597-94dffc387fd9@gmail.com>
 <dabc52ea-14b6-3109-26b1-e83a9dc62cc0@gmail.com>
 <20190204021103.GR1881@paquier.xyz>
 <20190214194029.kggud627dmi67gou@alap3.anarazel.de>
 <20190218040743.GK15532@paquier.xyz>
 <44c101dc-f763-f47f-1ac1-6a0709455033@pgmasters.net>
 <2a4cf0d7-dd67-560a-ac62-cbc127662e95@gmail.com>
 <CA+hUKGLTYCDStwrLuH0O-CeVqEQJ=otzia2EVfScXtatOxUzxg@mail.gmail.com>
Message-ID: <20fd7da2-cc15-b538-d3b9-896aad695d23@gmail.com>
Date: Fri, 5 Jul 2019 15:41:08 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.7.2
MIME-Version: 1.0
In-Reply-To: 
 <CA+hUKGLTYCDStwrLuH0O-CeVqEQJ=otzia2EVfScXtatOxUzxg@mail.gmail.com>
Content-Type: multipart/mixed;
 boundary="------------DD6B753B7614C94516FB8462"
Content-Language: en-US
Precedence: bulk

This is a multi-part message in MIME format.
--------------DD6B753B7614C94516FB8462
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit

Hello Thomas,

01.07.2019 13:47, Thomas Munro wrote:
>
> A new CF is here and this is in "Needs Review".  Would you like to
> provide a rebased patch, or should it really be withdrawn?
The rebased patch is attached, but I still can't find anyone interested
in reviewing it.
So let's withdraw it.
With hope for better days,
Alexander


--------------DD6B753B7614C94516FB8462
Content-Type: text/x-patch;
 name="make-installcheck-v6.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="make-installcheck-v6.patch"

diff --git a/doc/src/sgml/regress.sgml b/doc/src/sgml/regress.sgml
index 7b68213266..bc34d1a80d 100644
--- a/doc/src/sgml/regress.sgml
+++ b/doc/src/sgml/regress.sgml
@@ -111,7 +111,12 @@ make installcheck-parallel
    default port number, unless directed otherwise by <envar>PGHOST</envar> and
    <envar>PGPORT</envar> environment variables.  The tests will be run in a
    database named <literal>regression</literal>; any existing database by this name
-   will be dropped.
+   will be dropped.  The tests in the installcheck mode will use the executables,
+   libraries, and headers, which are already present in the installation tree,
+   not their copies in the source tree (if any).  This mode is suitable for
+   testing distribution packages of PostgreSQL installed on user systems as
+   it allows to check the installation in its entirety without rebuilding
+   any parts of it.
   </para>
 
   <para>
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index 321af38b0c..dd27d669c0 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -244,14 +244,16 @@ PG_SYSROOT = @PG_SYSROOT@
 
 override CPPFLAGS := $(ICU_CFLAGS) $(CPPFLAGS)
 
-ifdef PGXS
+# For PGXS and in the installcheck mode (when we use the installed assets)
+# we need to target already installed headers and libraries
+ifneq (,$(PGXS)$(USE_INSTALLED_ASSETS))
 override CPPFLAGS := -I$(includedir_server) -I$(includedir_internal) $(CPPFLAGS)
-else # not PGXS
+else # not PGXS and not USE_INSTALLED_ASSETS
 override CPPFLAGS := -I$(top_srcdir)/src/include $(CPPFLAGS)
 ifdef VPATH
 override CPPFLAGS := -I$(top_builddir)/src/include $(CPPFLAGS)
 endif
-endif # not PGXS
+endif # not PGXS and USE_INSTALLED_ASSETS
 
 CC = @CC@
 GCC = @GCC@
@@ -306,7 +308,7 @@ with_gnu_ld = @with_gnu_ld@
 # "LDFLAGS := something" anywhere, ditto for LDFLAGS_INTERNAL.
 # These initial assignments must be "=" type, and elsewhere we must only do
 # "LDFLAGS += something" or "LDFLAGS_INTERNAL += something".
-ifdef PGXS
+ifneq (,$(PGXS)$(USE_INSTALLED_ASSETS))
   LDFLAGS_INTERNAL = -L$(libdir)
 else
   LDFLAGS_INTERNAL = -L$(top_builddir)/src/port -L$(top_builddir)/src/common
@@ -379,6 +381,9 @@ endif
 # install tree just once in any recursive "make check".  The additional test
 # on abs_top_builddir prevents doing anything foolish to the root directory.
 
+# In the installcheck mode we should use what is already installed
+# in the DESTDIR (namely, pg_regress).
+
 check: temp-install
 
 .PHONY: temp-install
@@ -430,7 +435,7 @@ ifeq ($(enable_tap_tests),yes)
 define prove_installcheck
 rm -rf '$(CURDIR)'/tmp_check
 $(MKDIR_P) '$(CURDIR)'/tmp_check
-cd $(srcdir) && TESTDIR='$(CURDIR)' PATH="$(bindir):$$PATH" PGPORT='6$(DEF_PGPORT)' top_builddir='$(CURDIR)/$(top_builddir)' PG_REGRESS='$(CURDIR)/$(top_builddir)/src/test/regress/pg_regress' REGRESS_SHLIB='$(abs_top_builddir)/src/test/regress/regress$(DLSUFFIX)' $(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) $(if $(PROVE_TESTS),$(PROVE_TESTS),t/*.pl)
+cd $(srcdir) && TESTDIR='$(CURDIR)' PATH="$(bindir):$$PATH" PGPORT='6$(DEF_PGPORT)' top_builddir='$(CURDIR)/$(top_builddir)' PG_REGRESS='$(DESTDIR)$(pgxsdir)/src/test/regress/pg_regress' REGRESS_SHLIB='$(DESTDIR)$(pgxsdir)/src/test/regress/regress$(DLSUFFIX)' $(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) $(if $(PROVE_TESTS),$(PROVE_TESTS),t/*.pl)
 endef
 
 define prove_check
@@ -574,6 +579,8 @@ endif
 #
 # Commonly used submake targets
 
+# build these libraries only when we are not going to use the installed ones
+ifndef USE_INSTALLED_ASSETS
 submake-libpq: | submake-generated-headers
 	$(MAKE) -C $(libpq_builddir) all
 
@@ -585,6 +592,7 @@ submake-libpgfeutils: | submake-generated-headers
 	$(MAKE) -C $(top_builddir)/src/port all
 	$(MAKE) -C $(top_builddir)/src/common all
 	$(MAKE) -C $(top_builddir)/src/fe_utils all
+endif
 
 .PHONY: submake-libpq submake-libpgport submake-libpgfeutils
 
@@ -636,7 +644,7 @@ pg_regress_check = \
     $(TEMP_CONF) \
     $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS)
 pg_regress_installcheck = \
-    $(top_builddir)/src/test/regress/pg_regress \
+   '$(DESTDIR)$(pgxsdir)/src/test/regress/pg_regress' \
     --inputdir=$(srcdir) \
     --bindir='$(bindir)' \
     $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS)
diff --git a/src/bin/pg_upgrade/test.sh b/src/bin/pg_upgrade/test.sh
index 78820247b3..1a595e50dc 100644
--- a/src/bin/pg_upgrade/test.sh
+++ b/src/bin/pg_upgrade/test.sh
@@ -163,7 +163,7 @@ createdb "regression$dbname1" || createdb_status=$?
 createdb "regression$dbname2" || createdb_status=$?
 createdb "regression$dbname3" || createdb_status=$?
 
-if "$MAKE" -C "$oldsrc" installcheck-parallel; then
+if "$MAKE" -C "$oldsrc" installcheck-parallel DESTDIR="$temp_install"; then
 	oldpgversion=`psql -X -A -t -d regression -c "SHOW server_version_num"`
 
 	# before dumping, get rid of objects not existing in later versions
diff --git a/src/interfaces/ecpg/test/Makefile b/src/interfaces/ecpg/test/Makefile
index be53b7b94d..a5e932f438 100644
--- a/src/interfaces/ecpg/test/Makefile
+++ b/src/interfaces/ecpg/test/Makefile
@@ -86,7 +86,11 @@ check: all
 checktcp: all | temp-install
 	$(with_temp_install) ./pg_regress $(REGRESS_OPTS) --temp-instance=./tmp_check $(TEMP_CONF) --bindir= $(pg_regress_locale_flags) $(THREAD) --schedule=$(srcdir)/ecpg_schedule --host=localhost sql/twophase connect/test1
 
-installcheck: all
+.PHONY: all-installcheck
+all-installcheck:
+	USE_INSTALLED_ASSETS=1 $(MAKE) all
+
+installcheck: all-installcheck
 	./pg_regress $(REGRESS_OPTS) --bindir='$(bindir)' $(pg_regress_locale_flags) $(THREAD) --schedule=$(srcdir)/ecpg_schedule
 
 # Versions of the check tests that include the twophase commit test.
diff --git a/src/interfaces/ecpg/test/Makefile.regress b/src/interfaces/ecpg/test/Makefile.regress
index b0647cd2c5..dbdef3b612 100644
--- a/src/interfaces/ecpg/test/Makefile.regress
+++ b/src/interfaces/ecpg/test/Makefile.regress
@@ -1,19 +1,29 @@
 # This file is included into the Makefiles of subdirectories of ecpg/test/,
 # so the file references have one more level of .. than you might expect.
 
+ifdef USE_INSTALLED_ASSETS
+# Standard way to invoke the installed ecpg preprocessor
+ECPG = '$(DESTDIR)$(bindir)/ecpg' --regression -I'$(DESTDIR)$(includedir)' \
+	-I'$(DESTDIR)$(pkgincludedir)/informix/esql'
+# Use installed headers and libs
+override CPPFLAGS := -I'$(DESTDIR)$(includedir)' \
+	-I'$(DESTDIR)$(pkgincludedir)/informix/esql' $(CPPFLAGS)
+LDFLAGS_INTERNAL += -lecpg -lpgtypes $(libpq)
+else
+# Standard way to invoke the ecpg preprocessor
+ECPG = ../../preproc/ecpg --regression -I$(srcdir)/../../include -I$(srcdir)
+
+ECPG_DEP = ../../preproc/ecpg$(X)
 override CPPFLAGS := -I../../include -I$(top_srcdir)/src/interfaces/ecpg/include \
 	-I$(libpq_srcdir) $(CPPFLAGS)
-override CFLAGS += $(PTHREAD_CFLAGS)
-
 LDFLAGS_INTERNAL += -L../../ecpglib -lecpg -L../../pgtypeslib -lpgtypes $(libpq)
+endif
 
+override CFLAGS += $(PTHREAD_CFLAGS)
 override LIBS += $(PTHREAD_LIBS)
 
-# Standard way to invoke the ecpg preprocessor
-ECPG = ../../preproc/ecpg --regression -I$(srcdir)/../../include -I$(srcdir)
-
 # Files that most or all ecpg preprocessor test outputs depend on
-ECPG_TEST_DEPENDENCIES = ../../preproc/ecpg$(X) \
+ECPG_TEST_DEPENDENCIES = $(ECPG_DEP) \
 	$(srcdir)/../regression.h \
 	$(srcdir)/../printf_hack.h \
 	$(srcdir)/../../include/sqlca.h \
diff --git a/src/makefiles/pgxs.mk b/src/makefiles/pgxs.mk
index 271e7eaba8..f14f942a90 100644
--- a/src/makefiles/pgxs.mk
+++ b/src/makefiles/pgxs.mk
@@ -415,7 +415,7 @@ endif
 # Standard rules to run regression tests including multiple test suites.
 # Runs against an installed postmaster.
 ifndef NO_INSTALLCHECK
-installcheck: submake $(REGRESS_PREP)
+installcheck: $(REGRESS_PREP)
 ifdef REGRESS
 	$(pg_regress_installcheck) $(REGRESS_OPTS) $(REGRESS)
 endif
diff --git a/src/pl/plperl/GNUmakefile b/src/pl/plperl/GNUmakefile
index 9b1c514101..da605f8abd 100644
--- a/src/pl/plperl/GNUmakefile
+++ b/src/pl/plperl/GNUmakefile
@@ -113,7 +113,7 @@ uninstall-data:
 check: submake
 	$(pg_regress_check) $(REGRESS_OPTS) $(REGRESS)
 
-installcheck: submake
+installcheck:
 	$(pg_regress_installcheck) $(REGRESS_OPTS) $(REGRESS)
 
 .PHONY: submake
diff --git a/src/pl/plpgsql/src/Makefile b/src/pl/plpgsql/src/Makefile
index af9f6fb209..10460f1529 100644
--- a/src/pl/plpgsql/src/Makefile
+++ b/src/pl/plpgsql/src/Makefile
@@ -90,7 +90,7 @@ pl_unreserved_kwlist_d.h: pl_unreserved_kwlist.h $(GEN_KEYWORDLIST_DEPS)
 check: submake
 	$(pg_regress_check) $(REGRESS_OPTS) $(REGRESS)
 
-installcheck: submake
+installcheck:
 	$(pg_regress_installcheck) $(REGRESS_OPTS) $(REGRESS)
 
 .PHONY: submake
diff --git a/src/pl/plpython/Makefile b/src/pl/plpython/Makefile
index 667a74469e..839f57d8e3 100644
--- a/src/pl/plpython/Makefile
+++ b/src/pl/plpython/Makefile
@@ -142,7 +142,7 @@ include $(srcdir)/regress-python3-mangle.mk
 check: submake-pg-regress
 	$(pg_regress_check) $(REGRESS_OPTS) $(REGRESS)
 
-installcheck: submake-pg-regress
+installcheck:
 	$(pg_regress_installcheck) $(REGRESS_OPTS) $(REGRESS)
 
 
diff --git a/src/pl/tcl/Makefile b/src/pl/tcl/Makefile
index 8ee4c3ff15..d51d37f609 100644
--- a/src/pl/tcl/Makefile
+++ b/src/pl/tcl/Makefile
@@ -82,7 +82,7 @@ uninstall-data:
 check: submake
 	$(pg_regress_check) $(REGRESS_OPTS) $(REGRESS)
 
-installcheck: submake
+installcheck:
 	$(pg_regress_installcheck) $(REGRESS_OPTS) $(REGRESS)
 
 .PHONY: submake
diff --git a/src/test/isolation/Makefile b/src/test/isolation/Makefile
index c3c8280ea2..4c3a937c2f 100644
--- a/src/test/isolation/Makefile
+++ b/src/test/isolation/Makefile
@@ -15,6 +15,9 @@ OBJS =  specparse.o isolationtester.o $(WIN32RES)
 
 all: isolationtester$(X) pg_isolation_regress$(X)
 
+all-installcheck:
+	USE_INSTALLED_ASSETS=1 $(MAKE) all
+
 # Though we don't install these binaries, build them during installation
 # (including temp-install).  Otherwise, "make -j check-world" and "make -j
 # installcheck-world" would spawn multiple, concurrent builds in this
@@ -49,7 +52,7 @@ clean distclean:
 maintainer-clean: distclean
 	rm -f specparse.c specscanner.c
 
-installcheck: all
+installcheck: all-installcheck
 	$(pg_isolation_regress_installcheck) --schedule=$(srcdir)/isolation_schedule
 
 check: all
@@ -59,7 +62,7 @@ check: all
 # It only makes sense to run these if set up to use prepared transactions,
 # via TEMP_CONFIG for the check case, or via the postgresql.conf for the
 # installcheck case.
-installcheck-prepared-txns: all temp-install
+installcheck-prepared-txns: all-installcheck temp-install
 	$(pg_isolation_regress_installcheck) --schedule=$(srcdir)/isolation_schedule prepared-transactions
 
 check-prepared-txns: all temp-install
diff --git a/src/test/regress/GNUmakefile b/src/test/regress/GNUmakefile
index a24cfd4e01..0a3df3b8b2 100644
--- a/src/test/regress/GNUmakefile
+++ b/src/test/regress/GNUmakefile
@@ -34,11 +34,17 @@ EXTRADEFS = '-DHOST_TUPLE="$(host_tuple)"' \
 
 # Build regression test driver
 
+ifndef USE_INSTALLED_ASSETS
 all: pg_regress$(X)
+endif
 
 pg_regress$(X): pg_regress.o pg_regress_main.o $(WIN32RES) | submake-libpgport
 	$(CC) $(CFLAGS) $^ $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@
 
+.PHONY: all-installcheck
+all-installcheck:
+	USE_INSTALLED_ASSETS=1 $(MAKE) all
+
 # dependencies ensure that path changes propagate
 pg_regress.o: pg_regress.c $(top_builddir)/src/port/pg_config_paths.h
 pg_regress.o: override CPPFLAGS += -I$(top_builddir)/src/port $(EXTRADEFS)
@@ -134,16 +140,16 @@ check: all tablespace-setup
 check-tests: all tablespace-setup | temp-install
 	$(pg_regress_check) $(REGRESS_OPTS) $(MAXCONNOPT) $(TESTS) $(EXTRA_TESTS)
 
-installcheck: all tablespace-setup
+installcheck: all-installcheck tablespace-setup
 	$(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/serial_schedule $(EXTRA_TESTS)
 
-installcheck-parallel: all tablespace-setup
+installcheck-parallel: all-installcheck tablespace-setup
 	$(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/parallel_schedule $(MAXCONNOPT) $(EXTRA_TESTS)
 
-installcheck-tests: all tablespace-setup
+installcheck-tests: all-installcheck tablespace-setup
 	$(pg_regress_installcheck) $(REGRESS_OPTS) $(TESTS) $(EXTRA_TESTS)
 
-standbycheck: all
+standbycheck: all-installcheck
 	$(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/standby_schedule --use-existing
 
 # old interfaces follow...
@@ -152,7 +158,7 @@ runcheck: check
 runtest: installcheck
 runtest-parallel: installcheck-parallel
 
-bigtest: all tablespace-setup
+bigtest: all-installcheck tablespace-setup
 	$(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/serial_schedule numeric_big
 
 bigcheck: all tablespace-setup | temp-install


--------------DD6B753B7614C94516FB8462--