pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
improving speed of make check-world
22+ messages / 6 participants
[nested] [flat]

* improving speed of make check-world
@ 2014-08-15 05:45 Peter Eisentraut <peter_e@gmx.net>
  2014-08-25 17:32 ` Re: improving speed of make check-world Heikki Linnakangas <hlinnakangas@vmware.com>
  2014-08-31 09:36 ` Re: improving speed of make check-world Fabien COELHO <coelho@cri.ensmp.fr>
  2015-04-23 17:22 ` Re: improving speed of make check-world Jeff Janes <jeff.janes@gmail.com>
  0 siblings, 3 replies; 22+ messages in thread

From: Peter Eisentraut @ 2014-08-15 05:45 UTC (permalink / raw)
  To: pgsql-hackers

make check-world creates a temporary installation in every subdirectory
it runs a test in, which is stupid: it's very slow and uses a lot of
disk space.  It's enough to do this once per run.  That is the essence
of what I have implemented.  It cuts the time for make check-world in
half or less, and it saves gigabytes of disk space.

The idea is that we only maintain one temporary installation under the
top-level directory.  By looking at the variable MAKELEVEL, we can tell
whether we are the top-level make invocation.  If so, make a temp
installation.  If not, we know that the upper layers have already done
it and we can just point to the existing temp installation.

I do this by ripping out the temp installation logic from pg_regress and
implementing it directly in the makefiles.  This is much simpler and has
additional potential benefits:

The pg_regress temp install mode is actually a combination of two
functionalities: temp install plus temp instance.  Until now, you could
only get the two together, but the temp instance functionality is
actually quite useful by itself.  It opens up the possibility of
implementing "make check" for external pgxs modules, for example.

Also, you could now run the temp installation step using parallel make,
making it even faster.  This was previously disabled because the make
flags would have to pass through pg_regress.  It still won't quite work
to run make check-world -j8, say, because we can't actually run the
tests in parallel (yet), but something like make -C src/test/regress
check -j8 should work.

To that end, I have renamed the pg_regress --temp-install option to
--temp-instance.  Since --temp-install is only used inside the source
tree, this shouldn't cause any compatibility problems.

MSVC build is not yet adjusted for this.  Looking at vcregress.pl, this
should be fairly straightforward.

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Attachments:

regress-temp-instance.patch (text/x-patch, 30.0K) download diff --git a/.gitignore b/.gitignore index 681af08..823d3ac 100644 --- a/.gitignore +++ b/.gitignore @@ -34,3 +34,4 @@ lib*.pc /pgsql.sln.cache /Debug/ /Release/ +/tmp_install/ diff --git a/GNUmakefile.in b/GNUmakefile.in index 69e0824..5667943 100644 --- a/GNUmakefile.in +++ b/GNUmakefile.in @@ -47,6 +47,7 @@ $(call recurse,distprep,doc src config contrib) # it's not built by default $(call recurse,clean,doc contrib src config) clean: + rm -rf tmp_install/ # Garbage from autoconf: @rm -rf autom4te.cache/ @@ -61,6 +62,8 @@ distclean maintainer-clean: # Garbage from autoconf: @rm -rf autom4te.cache/ +check-world: temp-install + check check-tests: all check check-tests installcheck installcheck-parallel installcheck-tests: diff --git a/contrib/earthdistance/Makefile b/contrib/earthdistance/Makefile index 93dcbe3..cde1ae6 100644 --- a/contrib/earthdistance/Makefile +++ b/contrib/earthdistance/Makefile @@ -7,7 +7,7 @@ DATA = earthdistance--1.0.sql earthdistance--unpackaged--1.0.sql PGFILEDESC = "earthdistance - calculate distances on the surface of the Earth" REGRESS = earthdistance -REGRESS_OPTS = --extra-install=contrib/cube +EXTRA_INSTALL = contrib/cube LDFLAGS_SL += $(filter -lm, $(LIBS)) diff --git a/contrib/pg_upgrade/test.sh b/contrib/pg_upgrade/test.sh index 7bbd2c7..7d493d9 100644 --- a/contrib/pg_upgrade/test.sh +++ b/contrib/pg_upgrade/test.sh @@ -80,7 +80,7 @@ if [ "$1" = '--install' ]; then # use psql from the proper installation directory, which might # be outdated or missing. But don't override anything else that's # already in EXTRA_REGRESS_OPTS. - EXTRA_REGRESS_OPTS="$EXTRA_REGRESS_OPTS --psqldir='$bindir'" + EXTRA_REGRESS_OPTS="$EXTRA_REGRESS_OPTS --bindir='$bindir'" export EXTRA_REGRESS_OPTS fi diff --git a/contrib/test_decoding/Makefile b/contrib/test_decoding/Makefile index d7f32c3..6210ddb 100644 --- a/contrib/test_decoding/Makefile +++ b/contrib/test_decoding/Makefile @@ -39,35 +39,33 @@ submake-test_decoding: REGRESSCHECKS=ddl rewrite toast permissions decoding_in_xact binary prepared -regresscheck: all | submake-regress submake-test_decoding +regresscheck: all | submake-regress submake-test_decoding temp-install $(MKDIR_P) regression_output $(pg_regress_check) \ --temp-config $(top_srcdir)/contrib/test_decoding/logical.conf \ - --temp-install=./tmp_check \ - --extra-install=contrib/test_decoding \ + --temp-instance=./tmp_check \ --outputdir=./regression_output \ $(REGRESSCHECKS) -regresscheck-install-force: | submake-regress submake-test_decoding +regresscheck-install-force: | submake-regress submake-test_decoding temp-install $(pg_regress_installcheck) \ - --extra-install=contrib/test_decoding \ $(REGRESSCHECKS) ISOLATIONCHECKS=mxact delayed_startup concurrent_ddl_dml -isolationcheck: all | submake-isolation submake-test_decoding +isolationcheck: all | submake-isolation submake-test_decoding temp-install $(MKDIR_P) isolation_output $(pg_isolation_regress_check) \ --temp-config $(top_srcdir)/contrib/test_decoding/logical.conf \ - --extra-install=contrib/test_decoding \ --outputdir=./isolation_output \ $(ISOLATIONCHECKS) -isolationcheck-install-force: all | submake-isolation submake-test_decoding +isolationcheck-install-force: all | submake-isolation submake-test_decoding temp-install $(pg_isolation_regress_installcheck) \ - --extra-install=contrib/test_decoding \ $(ISOLATIONCHECKS) PHONY: submake-test_decoding submake-regress check \ regresscheck regresscheck-install-force \ isolationcheck isolationcheck-install-force + +temp-install: EXTRA_INSTALL=contrib/test_decoding diff --git a/src/Makefile.global.in b/src/Makefile.global.in index 0ffc1e8..ae73c85 100644 --- a/src/Makefile.global.in +++ b/src/Makefile.global.in @@ -41,6 +41,7 @@ MAJORVERSION = @PG_MAJORVERSION@ # Support for VPATH builds vpath_build = @vpath_build@ +abs_top_builddir = @abs_top_builddir@ abs_top_srcdir = @abs_top_srcdir@ ifneq ($(vpath_build),yes) @@ -296,6 +297,17 @@ BZIP2 = bzip2 # Testing +check: temp-install + +.PHONY: temp-install +temp-install: +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 +endif + for extra in $(EXTRA_INSTALL); do $(MAKE) -C $(top_builddir)/$$extra DESTDIR=$(abs_top_builddir)/tmp_install install >>$(abs_top_builddir)/tmp_install/log/install.log 2>&1 || exit; done + PROVE = @PROVE@ PG_PROVE_FLAGS = --ext='.pl' -I $(top_srcdir)/src/test/perl/ PROVE_FLAGS = --verbose @@ -310,14 +322,16 @@ define ld_library_path_var $(if $(filter $(PORTNAME),darwin),DYLD_LIBRARY_PATH,$(if $(filter $(PORTNAME),aix),LIBPATH,LD_LIBRARY_PATH)) endef +define with_temp_install +PATH="$(abs_top_builddir)/tmp_install$(bindir):$$PATH" $(call add_to_path,$(ld_library_path_var),$(abs_top_builddir)/tmp_install$(libdir)) +endef + define prove_installcheck cd $(srcdir) && TESTDIR='$(CURDIR)' PATH="$(bindir):$$PATH" PGPORT='6$(DEF_PGPORT)' $(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) endef define prove_check -$(MKDIR_P) tmp_check/log -$(MAKE) -C $(top_builddir) DESTDIR=$(CURDIR)/tmp_check/install install >$(CURDIR)/tmp_check/log/install.log 2>&1 -cd $(srcdir) && TESTDIR='$(CURDIR)' PATH="$(CURDIR)/tmp_check/install$(bindir):$$PATH" $(call add_to_path,$(ld_library_path_var),$(CURDIR)/tmp_check/install$(libdir)) PGPORT='6$(DEF_PGPORT)' $(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) +cd $(srcdir) && TESTDIR='$(CURDIR)' $(with_temp_install) PGPORT='6$(DEF_PGPORT)' $(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) endef # Installation. @@ -491,13 +505,13 @@ endif pg_regress_locale_flags = $(if $(ENCODING),--encoding=$(ENCODING)) $(NOLOCALE) -pg_regress_check = $(top_builddir)/src/test/regress/pg_regress --inputdir=$(srcdir) --temp-install=./tmp_check --top-builddir=$(top_builddir) $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS) -pg_regress_installcheck = $(top_builddir)/src/test/regress/pg_regress --inputdir=$(srcdir) --psqldir='$(PSQLDIR)' $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS) +pg_regress_check = $(with_temp_install) $(top_builddir)/src/test/regress/pg_regress --inputdir=$(srcdir) --temp-instance=./tmp_check --bindir= $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS) +pg_regress_installcheck = $(top_builddir)/src/test/regress/pg_regress --inputdir=$(srcdir) --bindir='$(bindir)' $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS) pg_regress_clean_files = results/ regression.diffs regression.out tmp_check/ log/ -pg_isolation_regress_check = $(top_builddir)/src/test/isolation/pg_isolation_regress --inputdir=$(srcdir) --temp-install=./tmp_check --top-builddir=$(top_builddir) $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS) -pg_isolation_regress_installcheck = $(top_builddir)/src/test/isolation/pg_isolation_regress --inputdir=$(srcdir) --top-builddir=$(top_builddir) $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS) +pg_isolation_regress_check = $(with_temp_install) $(top_builddir)/src/test/isolation/pg_isolation_regress --inputdir=$(srcdir) --temp-instance=./tmp_check --bindir= $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS) +pg_isolation_regress_installcheck = $(top_builddir)/src/test/isolation/pg_isolation_regress --inputdir=$(srcdir) $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS) ########################################################################## # diff --git a/src/interfaces/ecpg/test/Makefile b/src/interfaces/ecpg/test/Makefile index 56f6a17..8454449 100644 --- a/src/interfaces/ecpg/test/Makefile +++ b/src/interfaces/ecpg/test/Makefile @@ -8,16 +8,12 @@ override CPPFLAGS := \ '-I$(top_builddir)/src/port' \ '-I$(top_srcdir)/src/test/regress' \ '-DHOST_TUPLE="$(host_tuple)"' \ - '-DMAKEPROG="$(MAKE)"' \ '-DSHELLPROG="$(SHELL)"' \ '-DDLSUFFIX="$(DLSUFFIX)"' \ $(CPPFLAGS) PGFILEDESC = "ECPG Test - regression tests for ECPG" -# where to find psql for testing an existing installation -PSQLDIR = $(bindir) - # default encoding for regression tests ENCODING = SQL_ASCII @@ -81,11 +77,11 @@ endif REGRESS_OPTS = --dbname=regress1,connectdb --create-role=connectuser,connectdb $(EXTRA_REGRESS_OPTS) check: all - ./pg_regress $(REGRESS_OPTS) --top-builddir=$(top_builddir) --temp-install=./tmp_check $(pg_regress_locale_flags) $(THREAD) --schedule=$(srcdir)/ecpg_schedule + $(with_temp_install) ./pg_regress $(REGRESS_OPTS) --temp-instance=./tmp_check --bindir= $(pg_regress_locale_flags) $(THREAD) --schedule=$(srcdir)/ecpg_schedule # the same options, but with --listen-on-tcp checktcp: all - ./pg_regress $(REGRESS_OPTS) --top-builddir=$(top_builddir) --temp-install=./tmp_check $(pg_regress_locale_flags) $(THREAD) --schedule=$(srcdir)/ecpg_schedule_tcp --host=localhost + $(with_temp_install) ./pg_regress $(REGRESS_OPTS) --temp-instance=./tmp_check --bindir= $(pg_regress_locale_flags) $(THREAD) --schedule=$(srcdir)/ecpg_schedule_tcp --host=localhost installcheck: all - ./pg_regress $(REGRESS_OPTS) --psqldir='$(PSQLDIR)' --top-builddir=$(top_builddir) $(pg_regress_locale_flags) $(THREAD) --schedule=$(srcdir)/ecpg_schedule + ./pg_regress $(REGRESS_OPTS) --bindir='$(bindir)' $(pg_regress_locale_flags) $(THREAD) --schedule=$(srcdir)/ecpg_schedule diff --git a/src/makefiles/pgxs.mk b/src/makefiles/pgxs.mk index cc69c1b..3186a5b 100644 --- a/src/makefiles/pgxs.mk +++ b/src/makefiles/pgxs.mk @@ -265,9 +265,6 @@ else REGRESS_OPTS += --dbname=$(CONTRIB_TESTDB) endif -# where to find psql for running the tests -PSQLDIR = $(bindir) - # When doing a VPATH build, must copy over the data files so that the # driver script can find them. We have to use an absolute path for # the targets, because otherwise make will try to locate the missing @@ -302,7 +299,9 @@ check: @echo 'Do "$(MAKE) install", then "$(MAKE) installcheck" instead.' else check: all submake $(REGRESS_PREP) - $(pg_regress_check) --extra-install=$(subdir) $(REGRESS_OPTS) $(REGRESS) + $(pg_regress_check) $(REGRESS_OPTS) $(REGRESS) + +temp-install: EXTRA_INSTALL=$(subdir) endif endif # REGRESS diff --git a/src/pl/plperl/GNUmakefile b/src/pl/plperl/GNUmakefile index a04a2d3..2b69847 100644 --- a/src/pl/plperl/GNUmakefile +++ b/src/pl/plperl/GNUmakefile @@ -69,8 +69,6 @@ ifeq ($(shell $(PERL) -V:usemultiplicity), usemultiplicity='define';) REGRESS += plperl_plperlu endif endif -# where to find psql for running the tests -PSQLDIR = $(bindir) # where to find xsubpp for building XS. XSUBPPDIR = $(shell $(PERL) -e 'use List::Util qw(first); print first { -r "$$_/ExtUtils/xsubpp" } @INC') diff --git a/src/pl/plpython/Makefile b/src/pl/plpython/Makefile index 9c0fc61..b761688 100644 --- a/src/pl/plpython/Makefile +++ b/src/pl/plpython/Makefile @@ -115,9 +115,6 @@ REGRESS = \ REGRESS_PLPYTHON3_MANGLE := $(REGRESS) -# where to find psql for running the tests -PSQLDIR = $(bindir) - include $(top_srcdir)/src/Makefile.shlib all: all-lib diff --git a/src/pl/tcl/Makefile b/src/pl/tcl/Makefile index 7ea0026..851e3c0 100644 --- a/src/pl/tcl/Makefile +++ b/src/pl/tcl/Makefile @@ -45,8 +45,6 @@ DATA = pltcl.control pltcl--1.0.sql pltcl--unpackaged--1.0.sql \ REGRESS_OPTS = --dbname=$(PL_TESTDB) --load-extension=pltcl REGRESS = pltcl_setup pltcl_queries -# where to find psql for running the tests -PSQLDIR = $(bindir) # Tcl on win32 ships with import libraries only for Microsoft Visual C++, # which are not compatible with mingw gcc. Therefore we need to build a diff --git a/src/test/isolation/Makefile b/src/test/isolation/Makefile index 26bcf22..ef49c5a 100644 --- a/src/test/isolation/Makefile +++ b/src/test/isolation/Makefile @@ -6,9 +6,6 @@ subdir = src/test/isolation top_builddir = ../../.. include $(top_builddir)/src/Makefile.global -# where to find psql for testing an existing installation -PSQLDIR = $(bindir) - override CPPFLAGS := -I$(srcdir) -I$(libpq_srcdir) -I$(srcdir)/../regress $(CPPFLAGS) OBJS = specparse.o isolationtester.o @@ -52,17 +49,17 @@ maintainer-clean: distclean rm -f specparse.c specscanner.c installcheck: all - ./pg_isolation_regress --psqldir='$(PSQLDIR)' $(EXTRA_REGRESS_OPTS) --inputdir=$(srcdir) --schedule=$(srcdir)/isolation_schedule + ./pg_isolation_regress --bindir='$(bindir)' $(EXTRA_REGRESS_OPTS) --inputdir=$(srcdir) --schedule=$(srcdir)/isolation_schedule check: all - ./pg_isolation_regress --temp-install=./tmp_check --inputdir=$(srcdir) --top-builddir=$(top_builddir) $(EXTRA_REGRESS_OPTS) --schedule=$(srcdir)/isolation_schedule + $(with_temp_install) ./pg_isolation_regress --temp-instance=./tmp_check --inputdir=$(srcdir) --bindir= $(EXTRA_REGRESS_OPTS) --schedule=$(srcdir)/isolation_schedule # Versions of the check tests that include the prepared_transactions test # 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 - ./pg_isolation_regress --psqldir='$(PSQLDIR)' $(EXTRA_REGRESS_OPTS) --inputdir=$(srcdir) --schedule=$(srcdir)/isolation_schedule prepared-transactions +installcheck-prepared-txns: all temp-install + ./pg_isolation_regress --bindir='$(bindir)' $(EXTRA_REGRESS_OPTS) --inputdir=$(srcdir) --schedule=$(srcdir)/isolation_schedule prepared-transactions -check-prepared-txns: all - ./pg_isolation_regress --temp-install=./tmp_check $(EXTRA_REGRESS_OPTS) --inputdir=$(srcdir) --top-builddir=$(top_builddir) --schedule=$(srcdir)/isolation_schedule prepared-transactions +check-prepared-txns: all temp-install + ./pg_isolation_regress --temp-instance=./tmp_check $(EXTRA_REGRESS_OPTS) --inputdir=$(srcdir) --schedule=$(srcdir)/isolation_schedule prepared-transactions diff --git a/src/test/regress/GNUmakefile b/src/test/regress/GNUmakefile index b084e0a..26a451d 100644 --- a/src/test/regress/GNUmakefile +++ b/src/test/regress/GNUmakefile @@ -20,9 +20,6 @@ ifdef TEMP_CONFIG TEMP_CONF += --temp-config=$(TEMP_CONFIG) endif -# where to find psql for testing an existing installation -PSQLDIR = $(bindir) - # maximum simultaneous connections for parallel tests MAXCONNOPT = ifdef MAX_CONNECTIONS @@ -31,7 +28,6 @@ endif # stuff to pass into build of pg_regress EXTRADEFS = '-DHOST_TUPLE="$(host_tuple)"' \ - '-DMAKEPROG="$(MAKE)"' \ '-DSHELLPROG="$(SHELL)"' \ '-DDLSUFFIX="$(DLSUFFIX)"' diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c index 27c46ab..cca942e 100644 --- a/src/test/regress/pg_regress.c +++ b/src/test/regress/pg_regress.c @@ -43,25 +43,10 @@ typedef struct _resultmap } _resultmap; /* - * Values obtained from pg_config_paths.h and Makefile. The PG installation - * paths are only used in temp_install mode: we use these strings to find - * out where "make install" will put stuff under the temp_install directory. - * In non-temp_install mode, the only thing we need is the location of psql, - * which we expect to find in psqldir, or in the PATH if psqldir isn't given. - * - * XXX Because pg_regress is not installed in bindir, we can't support - * this for relocatable trees as it is. --psqldir would need to be - * specified in those cases. + * Values obtained from Makefile. */ -char *bindir = PGBINDIR; -char *libdir = LIBDIR; -char *datadir = PGSHAREDIR; char *host_platform = HOST_TUPLE; -#ifndef WIN32_ONLY_COMPILER -static char *makeprog = MAKEPROG; -#endif - #ifndef WIN32 /* not used in WIN32 case */ static char *shellprog = SHELLPROG; #endif @@ -84,7 +69,7 @@ _stringlist *dblist = NULL; bool debug = false; char *inputdir = "."; char *outputdir = "."; -char *psqldir = PGBINDIR; +char *bindir = PGBINDIR; char *launcher = NULL; static _stringlist *loadlanguage = NULL; static _stringlist *loadextension = NULL; @@ -92,9 +77,8 @@ static int max_connections = 0; static char *encoding = NULL; static _stringlist *schedulelist = NULL; static _stringlist *extra_tests = NULL; -static char *temp_install = NULL; +static char *temp_instance = NULL; static char *temp_config = NULL; -static char *top_builddir = NULL; static bool nolocale = false; static bool use_existing = false; static char *hostname = NULL; @@ -103,7 +87,6 @@ static bool port_specified_by_user = false; static char *dlpath = PKGLIBDIR; static char *user = NULL; static _stringlist *extraroles = NULL; -static _stringlist *extra_install = NULL; /* internal variables */ static const char *progname; @@ -300,8 +283,10 @@ stop_postmaster(void) fflush(stderr); snprintf(buf, sizeof(buf), - "\"%s/pg_ctl\" stop -D \"%s/data\" -s -m fast", - bindir, temp_install); + "\"%s%spg_ctl\" stop -D \"%s/data\" -s -m fast", + bindir ? bindir : "", + bindir ? "/" : "", + temp_instance); r = system(buf); if (r != 0) { @@ -749,27 +734,6 @@ doputenv(const char *var, const char *val) } /* - * Set the environment variable "pathname", prepending "addval" to its - * old value (if any). - */ -static void -add_to_path(const char *pathname, char separator, const char *addval) -{ - char *oldval = getenv(pathname); - char *newval; - - if (!oldval || !oldval[0]) - { - /* no previous value */ - newval = psprintf("%s=%s", pathname, addval); - } - else - newval = psprintf("%s=%s%c%s", pathname, addval, separator, oldval); - - putenv(newval); -} - -/* * Prepare environment variables for running regression tests */ static void @@ -835,7 +799,7 @@ initialize_environment(void) putenv(new_pgoptions); } - if (temp_install) + if (temp_instance) { /* * Clear out any environment vars that might cause psql to connect to @@ -873,49 +837,6 @@ initialize_environment(void) sprintf(s, "%d", port); doputenv("PGPORT", s); } - - /* - * GNU make stores some flags in the MAKEFLAGS environment variable to - * pass arguments to its own children. If we are invoked by make, - * that causes the make invoked by us to think its part of the make - * task invoking us, and so it tries to communicate with the toplevel - * make. Which fails. - * - * Unset the variable to protect against such problems. We also reset - * MAKELEVEL to be certain the child doesn't notice the make above us. - */ - unsetenv("MAKEFLAGS"); - unsetenv("MAKELEVEL"); - - /* - * Adjust path variables to point into the temp-install tree - */ - bindir = psprintf("%s/install/%s", temp_install, bindir); - - libdir = psprintf("%s/install/%s", temp_install, libdir); - - datadir = psprintf("%s/install/%s", temp_install, datadir); - - /* psql will be installed into temp-install bindir */ - psqldir = bindir; - - /* - * Set up shared library paths to include the temp install. - * - * LD_LIBRARY_PATH covers many platforms. DYLD_LIBRARY_PATH works on - * Darwin, and maybe other Mach-based systems. LIBPATH is for AIX. - * Windows needs shared libraries in PATH (only those linked into - * executables, not dlopen'ed ones). Feel free to account for others - * as well. - */ - add_to_path("LD_LIBRARY_PATH", ':', libdir); - add_to_path("DYLD_LIBRARY_PATH", ':', libdir); - add_to_path("LIBPATH", ':', libdir); -#if defined(WIN32) - add_to_path("PATH", ';', libdir); -#elif defined(__CYGWIN__) - add_to_path("PATH", ':', libdir); -#endif } else { @@ -998,8 +919,8 @@ psql_command(const char *database, const char *query,...) /* And now we can build and execute the shell command */ snprintf(psql_cmd, sizeof(psql_cmd), "\"%s%spsql\" -X -c \"%s\" \"%s\"", - psqldir ? psqldir : "", - psqldir ? "/" : "", + bindir ? bindir : "", + bindir ? "/" : "", query_escaped, database); @@ -1957,6 +1878,7 @@ help(void) printf(_("Usage:\n %s [OPTION]... [EXTRA-TEST]...\n"), progname); printf(_("\n")); printf(_("Options:\n")); + printf(_(" --bindir=DIR use programs in DIR (default: configured bindir)\n")); printf(_(" --create-role=ROLE create the specified role before testing\n")); printf(_(" --dbname=DB use database DB (default \"regression\")\n")); printf(_(" --debug turn on debug mode in programs that are run\n")); @@ -1973,21 +1895,18 @@ help(void) printf(_(" --outputdir=DIR place output files in DIR (default \".\")\n")); printf(_(" --schedule=FILE use test ordering schedule from FILE\n")); printf(_(" (can be used multiple times to concatenate)\n")); - printf(_(" --temp-install=DIR create a temporary installation in DIR\n")); - printf(_(" --use-existing use an existing installation\n")); + printf(_(" --temp-instance=DIR create a temporary instance in DIR\n")); + printf(_(" --use-existing use an existing installation\n")); // XXX printf(_("\n")); - printf(_("Options for \"temp-install\" mode:\n")); - printf(_(" --extra-install=DIR additional directory to install (e.g., contrib)\n")); + printf(_("Options for \"temp-instance\" mode:\n")); printf(_(" --no-locale use C locale\n")); printf(_(" --port=PORT start postmaster on PORT\n")); printf(_(" --temp-config=FILE append contents of FILE to temporary config\n")); - printf(_(" --top-builddir=DIR (relative) path to top level build directory\n")); printf(_("\n")); printf(_("Options for using an existing installation:\n")); printf(_(" --host=HOST use postmaster running on HOST\n")); printf(_(" --port=PORT use postmaster running at PORT\n")); printf(_(" --user=USER connect as USER\n")); - printf(_(" --psqldir=DIR use psql in DIR (default: configured bindir)\n")); printf(_("\n")); printf(_("The exit status is 0 if all tests passed, 1 if some tests failed, and 2\n")); printf(_("if the tests could not be run for some reason.\n")); @@ -2009,20 +1928,19 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc {"encoding", required_argument, NULL, 6}, {"outputdir", required_argument, NULL, 7}, {"schedule", required_argument, NULL, 8}, - {"temp-install", required_argument, NULL, 9}, + {"temp-instance", required_argument, NULL, 9}, {"no-locale", no_argument, NULL, 10}, - {"top-builddir", required_argument, NULL, 11}, + {"datadir", required_argument, NULL, 12}, {"host", required_argument, NULL, 13}, {"port", required_argument, NULL, 14}, {"user", required_argument, NULL, 15}, - {"psqldir", required_argument, NULL, 16}, + {"bindir", required_argument, NULL, 16}, {"dlpath", required_argument, NULL, 17}, {"create-role", required_argument, NULL, 18}, {"temp-config", required_argument, NULL, 19}, {"use-existing", no_argument, NULL, 20}, {"launcher", required_argument, NULL, 21}, {"load-extension", required_argument, NULL, 22}, - {"extra-install", required_argument, NULL, 23}, {NULL, 0, NULL, 0} }; @@ -2093,14 +2011,11 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc add_stringlist_item(&schedulelist, optarg); break; case 9: - temp_install = make_absolute_path(optarg); + temp_instance = make_absolute_path(optarg); break; case 10: nolocale = true; break; - case 11: - top_builddir = strdup(optarg); - break; case 13: hostname = strdup(optarg); break; @@ -2112,9 +2027,11 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc user = strdup(optarg); break; case 16: - /* "--psqldir=" should mean to use PATH */ + /* "--bindir=" means to use PATH */ if (strlen(optarg)) - psqldir = strdup(optarg); + bindir = strdup(optarg); + else + bindir = NULL; break; case 17: dlpath = strdup(optarg); @@ -2134,9 +2051,6 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc case 22: add_stringlist_item(&loadextension, optarg); break; - case 23: - add_stringlist_item(&extra_install, optarg); - break; default: /* getopt_long already emitted a complaint */ fprintf(stderr, _("\nTry \"%s -h\" for more information.\n"), @@ -2154,7 +2068,7 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc optind++; } - if (temp_install && !port_specified_by_user) + if (temp_instance && !port_specified_by_user) /* * To reduce chances of interference with parallel installations, use @@ -2180,82 +2094,44 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc unlimit_core_size(); #endif - if (temp_install) + if (temp_instance) { FILE *pg_conf; - _stringlist *sl; /* - * Prepare the temp installation + * Prepare the temp instance */ - if (!top_builddir) - { - fprintf(stderr, _("--top-builddir must be specified when using --temp-install\n")); - exit(2); - } - if (directory_exists(temp_install)) + if (directory_exists(temp_instance)) { - header(_("removing existing temp installation")); - if (!rmtree(temp_install, true)) + header(_("removing existing temp instance")); + if (!rmtree(temp_instance, true)) { - fprintf(stderr, _("\n%s: could not remove temp installation \"%s\": %s\n"), progname, temp_install, strerror(errno)); + fprintf(stderr, _("\n%s: could not remove temp instance \"%s\": %s\n"), progname, temp_instance, strerror(errno)); exit(2); } } - header(_("creating temporary installation")); + header(_("creating temporary instance")); - /* make the temp install top directory */ - make_directory(temp_install); + /* make the temp instance top directory */ + make_directory(temp_instance); /* and a directory for log files */ - snprintf(buf, sizeof(buf), "%s/log", outputdir); + snprintf(buf, sizeof(buf), "%s/log", temp_instance); if (!directory_exists(buf)) make_directory(buf); - /* "make install" */ -#ifndef WIN32_ONLY_COMPILER - snprintf(buf, sizeof(buf), - "\"%s\" -C \"%s\" DESTDIR=\"%s/install\" install > \"%s/log/install.log\" 2>&1", - makeprog, top_builddir, temp_install, outputdir); -#else - snprintf(buf, sizeof(buf), - "perl \"%s/src/tools/msvc/install.pl\" \"%s/install\" >\"%s/log/install.log\" 2>&1", - top_builddir, temp_install, outputdir); -#endif - if (system(buf)) - { - fprintf(stderr, _("\n%s: installation failed\nExamine %s/log/install.log for the reason.\nCommand was: %s\n"), progname, outputdir, buf); - exit(2); - } - - for (sl = extra_install; sl != NULL; sl = sl->next) - { -#ifndef WIN32_ONLY_COMPILER - snprintf(buf, sizeof(buf), - "\"%s\" -C \"%s/%s\" DESTDIR=\"%s/install\" install >> \"%s/log/install.log\" 2>&1", - makeprog, top_builddir, sl->str, temp_install, outputdir); -#else - fprintf(stderr, _("\n%s: --extra-install option not supported on this platform\n"), progname); - exit(2); -#endif - - if (system(buf)) - { - fprintf(stderr, _("\n%s: installation failed\nExamine %s/log/install.log for the reason.\nCommand was: %s\n"), progname, outputdir, buf); - exit(2); - } - } - /* initdb */ header(_("initializing database system")); snprintf(buf, sizeof(buf), - "\"%s/initdb\" -D \"%s/data\" -L \"%s\" --noclean --nosync%s%s > \"%s/log/initdb.log\" 2>&1", - bindir, temp_install, datadir, + "\"%s%sinitdb\" -D \"%s/data\" --noclean --nosync%s%s > \"%s/log/initdb.log\" 2>&1", + bindir ? bindir : "", + bindir ? "/" : "", + temp_instance, debug ? " --debug" : "", nolocale ? " --no-locale" : "", - outputdir); + temp_instance); if (system(buf)) { fprintf(stderr, _("\n%s: initdb failed\nExamine %s/log/initdb.log for the reason.\nCommand was: %s\n"), progname, outputdir, buf); @@ -2270,7 +2146,7 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc * don't set max_prepared_transactions any higher than actually needed * by the prepared_xacts regression test.) */ - snprintf(buf, sizeof(buf), "%s/data/postgresql.conf", temp_install); + snprintf(buf, sizeof(buf), "%s/data/postgresql.conf", temp_instance); pg_conf = fopen(buf, "a"); if (pg_conf == NULL) { @@ -2302,8 +2178,10 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc * Check if there is a postmaster running already. */ snprintf(buf2, sizeof(buf2), - "\"%s/psql\" -X postgres <%s 2>%s", - bindir, DEVNULL, DEVNULL); + "\"%s%spsql\" -X postgres <%s 2>%s", + bindir ? bindir : "", + bindir ? "/" : "", + DEVNULL, DEVNULL); for (i = 0; i < 16; i++) { @@ -2334,12 +2212,14 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc */ header(_("starting postmaster")); snprintf(buf, sizeof(buf), - "\"%s/postgres\" -D \"%s/data\" -F%s " + "\"%s%spostgres\" -D \"%s/data\" -F%s " "-c \"listen_addresses=%s\" -k \"%s\" " "> \"%s/log/postmaster.log\" 2>&1", - bindir, temp_install, debug ? " -d 5" : "", + bindir ? bindir : "", + bindir ? "/" : "", + temp_instance, debug ? " -d 5" : "", hostname ? hostname : "", sockdir ? sockdir : "", - outputdir); + temp_instance); postmaster_pid = spawn_process(buf); if (postmaster_pid == INVALID_PID) { @@ -2453,7 +2333,7 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc /* * Shut down temp installation's postmaster */ - if (temp_install) + if (temp_instance) { header(_("shutting down postmaster")); stop_postmaster(); diff --git a/src/test/regress/pg_regress.h b/src/test/regress/pg_regress.h index 942d761..992c0bf 100644 --- a/src/test/regress/pg_regress.h +++ b/src/test/regress/pg_regress.h @@ -43,12 +43,6 @@ extern char *inputdir; extern char *outputdir; extern char *launcher; -/* - * This should not be global but every module should be able to read command - * line parameters. - */ -extern char *psqldir; - extern const char *basic_diff_opts; extern const char *pretty_diff_opts; diff --git a/src/test/regress/pg_regress_main.c b/src/test/regress/pg_regress_main.c index 22197aa..3b7b952 100644 --- a/src/test/regress/pg_regress_main.c +++ b/src/test/regress/pg_regress_main.c @@ -65,8 +65,8 @@ psql_start_test(const char *testname, snprintf(psql_cmd + offset, sizeof(psql_cmd) - offset, "\"%s%spsql\" -X -a -q -d \"%s\" < \"%s\" > \"%s\" 2>&1", - psqldir ? psqldir : "", - psqldir ? "/" : "", + bindir ? bindir : "", + bindir ? "/" : "", dblist->str, infile, outfile);
^ permalink raw reply [nested|flat] 22+ messages in thread

* Re: improving speed of make check-world
  2014-08-15 05:45 improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
@ 2014-08-25 17:32 ` Heikki Linnakangas <hlinnakangas@vmware.com>
  2014-08-26 01:32   ` Re: improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
  2 siblings, 1 reply; 22+ messages in thread

From: Heikki Linnakangas @ 2014-08-25 17:32 UTC (permalink / raw)
  To: Peter Eisentraut <peter_e@gmx.net>; pgsql-hackers

On 08/15/2014 08:45 AM, Peter Eisentraut wrote:
> make check-world creates a temporary installation in every subdirectory
> it runs a test in, which is stupid: it's very slow and uses a lot of
> disk space.  It's enough to do this once per run.  That is the essence
> of what I have implemented.  It cuts the time for make check-world in
> half or less, and it saves gigabytes of disk space.

Nice!

> The idea is that we only maintain one temporary installation under the
> top-level directory.  By looking at the variable MAKELEVEL, we can tell
> whether we are the top-level make invocation.  If so, make a temp
> installation.  If not, we know that the upper layers have already done
> it and we can just point to the existing temp installation.
>
> I do this by ripping out the temp installation logic from pg_regress and
> implementing it directly in the makefiles.  This is much simpler and has
> additional potential benefits:
>
> The pg_regress temp install mode is actually a combination of two
> functionalities: temp install plus temp instance.  Until now, you could
> only get the two together, but the temp instance functionality is
> actually quite useful by itself.  It opens up the possibility of
> implementing "make check" for external pgxs modules, for example.
>
> Also, you could now run the temp installation step using parallel make,
> making it even faster.  This was previously disabled because the make
> flags would have to pass through pg_regress.  It still won't quite work
> to run make check-world -j8, say, because we can't actually run the
> tests in parallel (yet), but something like make -C src/test/regress
> check -j8 should work.
>
> To that end, I have renamed the pg_regress --temp-install option to
> --temp-instance.  Since --temp-install is only used inside the source
> tree, this shouldn't cause any compatibility problems.

Yeah, that all makes a lot of sense.

The new EXTRA_INSTALL makefile variable ought to be documented in 
extend.sgml, where we list REGRESS_OPTS and others.

- Heikki



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



^ permalink  raw  reply  [nested|flat] 22+ messages in thread

* Re: improving speed of make check-world
  2014-08-15 05:45 improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
  2014-08-25 17:32 ` Re: improving speed of make check-world Heikki Linnakangas <hlinnakangas@vmware.com>
@ 2014-08-26 01:32   ` Peter Eisentraut <peter_e@gmx.net>
  2014-08-31 03:52     ` Re: improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
  0 siblings, 1 reply; 22+ messages in thread

From: Peter Eisentraut @ 2014-08-26 01:32 UTC (permalink / raw)
  To: Heikki Linnakangas <hlinnakangas@vmware.com>; pgsql-hackers

On 8/25/14 1:32 PM, Heikki Linnakangas wrote:
> The new EXTRA_INSTALL makefile variable ought to be documented in
> extend.sgml, where we list REGRESS_OPTS and others.

But EXTRA_INSTALL is only of use inside the main source tree, not by
extensions.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



^ permalink  raw  reply  [nested|flat] 22+ messages in thread

* Re: improving speed of make check-world
  2014-08-15 05:45 improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
  2014-08-25 17:32 ` Re: improving speed of make check-world Heikki Linnakangas <hlinnakangas@vmware.com>
  2014-08-26 01:32   ` Re: improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
@ 2014-08-31 03:52     ` Peter Eisentraut <peter_e@gmx.net>
  0 siblings, 0 replies; 22+ messages in thread

From: Peter Eisentraut @ 2014-08-31 03:52 UTC (permalink / raw)
  To: pgsql-hackers

Updated, rebased patch.

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Attachments:

regress-temp-instance-v2.patch (text/x-patch, 30.0K) download diff --git a/.gitignore b/.gitignore index 681af08..823d3ac 100644 --- a/.gitignore +++ b/.gitignore @@ -34,3 +34,4 @@ lib*.pc /pgsql.sln.cache /Debug/ /Release/ +/tmp_install/ diff --git a/GNUmakefile.in b/GNUmakefile.in index 69e0824..5667943 100644 --- a/GNUmakefile.in +++ b/GNUmakefile.in @@ -47,6 +47,7 @@ $(call recurse,distprep,doc src config contrib) # it's not built by default $(call recurse,clean,doc contrib src config) clean: + rm -rf tmp_install/ # Garbage from autoconf: @rm -rf autom4te.cache/ @@ -61,6 +62,8 @@ distclean maintainer-clean: # Garbage from autoconf: @rm -rf autom4te.cache/ +check-world: temp-install + check check-tests: all check check-tests installcheck installcheck-parallel installcheck-tests: diff --git a/contrib/earthdistance/Makefile b/contrib/earthdistance/Makefile index 93dcbe3..cde1ae6 100644 --- a/contrib/earthdistance/Makefile +++ b/contrib/earthdistance/Makefile @@ -7,7 +7,7 @@ DATA = earthdistance--1.0.sql earthdistance--unpackaged--1.0.sql PGFILEDESC = "earthdistance - calculate distances on the surface of the Earth" REGRESS = earthdistance -REGRESS_OPTS = --extra-install=contrib/cube +EXTRA_INSTALL = contrib/cube LDFLAGS_SL += $(filter -lm, $(LIBS)) diff --git a/contrib/pg_upgrade/test.sh b/contrib/pg_upgrade/test.sh index 7bbd2c7..7d493d9 100644 --- a/contrib/pg_upgrade/test.sh +++ b/contrib/pg_upgrade/test.sh @@ -80,7 +80,7 @@ if [ "$1" = '--install' ]; then # use psql from the proper installation directory, which might # be outdated or missing. But don't override anything else that's # already in EXTRA_REGRESS_OPTS. - EXTRA_REGRESS_OPTS="$EXTRA_REGRESS_OPTS --psqldir='$bindir'" + EXTRA_REGRESS_OPTS="$EXTRA_REGRESS_OPTS --bindir='$bindir'" export EXTRA_REGRESS_OPTS fi diff --git a/contrib/test_decoding/Makefile b/contrib/test_decoding/Makefile index d7f32c3..6210ddb 100644 --- a/contrib/test_decoding/Makefile +++ b/contrib/test_decoding/Makefile @@ -39,35 +39,33 @@ submake-test_decoding: REGRESSCHECKS=ddl rewrite toast permissions decoding_in_xact binary prepared -regresscheck: all | submake-regress submake-test_decoding +regresscheck: all | submake-regress submake-test_decoding temp-install $(MKDIR_P) regression_output $(pg_regress_check) \ --temp-config $(top_srcdir)/contrib/test_decoding/logical.conf \ - --temp-install=./tmp_check \ - --extra-install=contrib/test_decoding \ + --temp-instance=./tmp_check \ --outputdir=./regression_output \ $(REGRESSCHECKS) -regresscheck-install-force: | submake-regress submake-test_decoding +regresscheck-install-force: | submake-regress submake-test_decoding temp-install $(pg_regress_installcheck) \ - --extra-install=contrib/test_decoding \ $(REGRESSCHECKS) ISOLATIONCHECKS=mxact delayed_startup concurrent_ddl_dml -isolationcheck: all | submake-isolation submake-test_decoding +isolationcheck: all | submake-isolation submake-test_decoding temp-install $(MKDIR_P) isolation_output $(pg_isolation_regress_check) \ --temp-config $(top_srcdir)/contrib/test_decoding/logical.conf \ - --extra-install=contrib/test_decoding \ --outputdir=./isolation_output \ $(ISOLATIONCHECKS) -isolationcheck-install-force: all | submake-isolation submake-test_decoding +isolationcheck-install-force: all | submake-isolation submake-test_decoding temp-install $(pg_isolation_regress_installcheck) \ - --extra-install=contrib/test_decoding \ $(ISOLATIONCHECKS) PHONY: submake-test_decoding submake-regress check \ regresscheck regresscheck-install-force \ isolationcheck isolationcheck-install-force + +temp-install: EXTRA_INSTALL=contrib/test_decoding diff --git a/src/Makefile.global.in b/src/Makefile.global.in index 0ffc1e8..3238c5c 100644 --- a/src/Makefile.global.in +++ b/src/Makefile.global.in @@ -41,6 +41,7 @@ MAJORVERSION = @PG_MAJORVERSION@ # Support for VPATH builds vpath_build = @vpath_build@ +abs_top_builddir = @abs_top_builddir@ abs_top_srcdir = @abs_top_srcdir@ ifneq ($(vpath_build),yes) @@ -296,6 +297,17 @@ BZIP2 = bzip2 # Testing +check: all temp-install + +.PHONY: temp-install +temp-install: +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 +endif + for extra in $(EXTRA_INSTALL); do $(MAKE) -C $(top_builddir)/$$extra DESTDIR=$(abs_top_builddir)/tmp_install install >>$(abs_top_builddir)/tmp_install/log/install.log 2>&1 || exit; done + PROVE = @PROVE@ PG_PROVE_FLAGS = --ext='.pl' -I $(top_srcdir)/src/test/perl/ PROVE_FLAGS = --verbose @@ -310,14 +322,16 @@ define ld_library_path_var $(if $(filter $(PORTNAME),darwin),DYLD_LIBRARY_PATH,$(if $(filter $(PORTNAME),aix),LIBPATH,LD_LIBRARY_PATH)) endef +define with_temp_install +PATH="$(abs_top_builddir)/tmp_install$(bindir):$$PATH" $(call add_to_path,$(ld_library_path_var),$(abs_top_builddir)/tmp_install$(libdir)) +endef + define prove_installcheck cd $(srcdir) && TESTDIR='$(CURDIR)' PATH="$(bindir):$$PATH" PGPORT='6$(DEF_PGPORT)' $(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) endef define prove_check -$(MKDIR_P) tmp_check/log -$(MAKE) -C $(top_builddir) DESTDIR=$(CURDIR)/tmp_check/install install >$(CURDIR)/tmp_check/log/install.log 2>&1 -cd $(srcdir) && TESTDIR='$(CURDIR)' PATH="$(CURDIR)/tmp_check/install$(bindir):$$PATH" $(call add_to_path,$(ld_library_path_var),$(CURDIR)/tmp_check/install$(libdir)) PGPORT='6$(DEF_PGPORT)' $(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) +cd $(srcdir) && TESTDIR='$(CURDIR)' $(with_temp_install) PGPORT='6$(DEF_PGPORT)' $(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) endef # Installation. @@ -491,13 +505,13 @@ endif pg_regress_locale_flags = $(if $(ENCODING),--encoding=$(ENCODING)) $(NOLOCALE) -pg_regress_check = $(top_builddir)/src/test/regress/pg_regress --inputdir=$(srcdir) --temp-install=./tmp_check --top-builddir=$(top_builddir) $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS) -pg_regress_installcheck = $(top_builddir)/src/test/regress/pg_regress --inputdir=$(srcdir) --psqldir='$(PSQLDIR)' $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS) +pg_regress_check = $(with_temp_install) $(top_builddir)/src/test/regress/pg_regress --inputdir=$(srcdir) --temp-instance=./tmp_check --bindir= $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS) +pg_regress_installcheck = $(top_builddir)/src/test/regress/pg_regress --inputdir=$(srcdir) --bindir='$(bindir)' $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS) pg_regress_clean_files = results/ regression.diffs regression.out tmp_check/ log/ -pg_isolation_regress_check = $(top_builddir)/src/test/isolation/pg_isolation_regress --inputdir=$(srcdir) --temp-install=./tmp_check --top-builddir=$(top_builddir) $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS) -pg_isolation_regress_installcheck = $(top_builddir)/src/test/isolation/pg_isolation_regress --inputdir=$(srcdir) --top-builddir=$(top_builddir) $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS) +pg_isolation_regress_check = $(with_temp_install) $(top_builddir)/src/test/isolation/pg_isolation_regress --inputdir=$(srcdir) --temp-instance=./tmp_check --bindir= $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS) +pg_isolation_regress_installcheck = $(top_builddir)/src/test/isolation/pg_isolation_regress --inputdir=$(srcdir) $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS) ########################################################################## # diff --git a/src/interfaces/ecpg/test/Makefile b/src/interfaces/ecpg/test/Makefile index 387729b..a4ac021 100644 --- a/src/interfaces/ecpg/test/Makefile +++ b/src/interfaces/ecpg/test/Makefile @@ -11,14 +11,10 @@ override CPPFLAGS := \ '-I$(top_builddir)/src/port' \ '-I$(top_srcdir)/src/test/regress' \ '-DHOST_TUPLE="$(host_tuple)"' \ - '-DMAKEPROG="$(MAKE)"' \ '-DSHELLPROG="$(SHELL)"' \ '-DDLSUFFIX="$(DLSUFFIX)"' \ $(CPPFLAGS) -# where to find psql for testing an existing installation -PSQLDIR = $(bindir) - # default encoding for regression tests ENCODING = SQL_ASCII @@ -82,11 +78,11 @@ endif REGRESS_OPTS = --dbname=regress1,connectdb --create-role=connectuser,connectdb $(EXTRA_REGRESS_OPTS) check: all - ./pg_regress $(REGRESS_OPTS) --top-builddir=$(top_builddir) --temp-install=./tmp_check $(pg_regress_locale_flags) $(THREAD) --schedule=$(srcdir)/ecpg_schedule + $(with_temp_install) ./pg_regress $(REGRESS_OPTS) --temp-instance=./tmp_check --bindir= $(pg_regress_locale_flags) $(THREAD) --schedule=$(srcdir)/ecpg_schedule # the same options, but with --listen-on-tcp checktcp: all - ./pg_regress $(REGRESS_OPTS) --top-builddir=$(top_builddir) --temp-install=./tmp_check $(pg_regress_locale_flags) $(THREAD) --schedule=$(srcdir)/ecpg_schedule_tcp --host=localhost + $(with_temp_install) ./pg_regress $(REGRESS_OPTS) --temp-instance=./tmp_check --bindir= $(pg_regress_locale_flags) $(THREAD) --schedule=$(srcdir)/ecpg_schedule_tcp --host=localhost installcheck: all - ./pg_regress $(REGRESS_OPTS) --psqldir='$(PSQLDIR)' --top-builddir=$(top_builddir) $(pg_regress_locale_flags) $(THREAD) --schedule=$(srcdir)/ecpg_schedule + ./pg_regress $(REGRESS_OPTS) --bindir='$(bindir)' $(pg_regress_locale_flags) $(THREAD) --schedule=$(srcdir)/ecpg_schedule diff --git a/src/makefiles/pgxs.mk b/src/makefiles/pgxs.mk index cc69c1b..3186a5b 100644 --- a/src/makefiles/pgxs.mk +++ b/src/makefiles/pgxs.mk @@ -265,9 +265,6 @@ else REGRESS_OPTS += --dbname=$(CONTRIB_TESTDB) endif -# where to find psql for running the tests -PSQLDIR = $(bindir) - # When doing a VPATH build, must copy over the data files so that the # driver script can find them. We have to use an absolute path for # the targets, because otherwise make will try to locate the missing @@ -302,7 +299,9 @@ check: @echo 'Do "$(MAKE) install", then "$(MAKE) installcheck" instead.' else check: all submake $(REGRESS_PREP) - $(pg_regress_check) --extra-install=$(subdir) $(REGRESS_OPTS) $(REGRESS) + $(pg_regress_check) $(REGRESS_OPTS) $(REGRESS) + +temp-install: EXTRA_INSTALL=$(subdir) endif endif # REGRESS diff --git a/src/pl/plperl/GNUmakefile b/src/pl/plperl/GNUmakefile index a04a2d3..2b69847 100644 --- a/src/pl/plperl/GNUmakefile +++ b/src/pl/plperl/GNUmakefile @@ -69,8 +69,6 @@ ifeq ($(shell $(PERL) -V:usemultiplicity), usemultiplicity='define';) REGRESS += plperl_plperlu endif endif -# where to find psql for running the tests -PSQLDIR = $(bindir) # where to find xsubpp for building XS. XSUBPPDIR = $(shell $(PERL) -e 'use List::Util qw(first); print first { -r "$$_/ExtUtils/xsubpp" } @INC') diff --git a/src/pl/plpython/Makefile b/src/pl/plpython/Makefile index 9c0fc61..b761688 100644 --- a/src/pl/plpython/Makefile +++ b/src/pl/plpython/Makefile @@ -115,9 +115,6 @@ REGRESS = \ REGRESS_PLPYTHON3_MANGLE := $(REGRESS) -# where to find psql for running the tests -PSQLDIR = $(bindir) - include $(top_srcdir)/src/Makefile.shlib all: all-lib diff --git a/src/pl/tcl/Makefile b/src/pl/tcl/Makefile index 7ea0026..851e3c0 100644 --- a/src/pl/tcl/Makefile +++ b/src/pl/tcl/Makefile @@ -45,8 +45,6 @@ DATA = pltcl.control pltcl--1.0.sql pltcl--unpackaged--1.0.sql \ REGRESS_OPTS = --dbname=$(PL_TESTDB) --load-extension=pltcl REGRESS = pltcl_setup pltcl_queries -# where to find psql for running the tests -PSQLDIR = $(bindir) # Tcl on win32 ships with import libraries only for Microsoft Visual C++, # which are not compatible with mingw gcc. Therefore we need to build a diff --git a/src/test/isolation/Makefile b/src/test/isolation/Makefile index a88257a..4577509 100644 --- a/src/test/isolation/Makefile +++ b/src/test/isolation/Makefile @@ -9,9 +9,6 @@ subdir = src/test/isolation top_builddir = ../../.. include $(top_builddir)/src/Makefile.global -# where to find psql for testing an existing installation -PSQLDIR = $(bindir) - override CPPFLAGS := -I$(srcdir) -I$(libpq_srcdir) -I$(srcdir)/../regress $(CPPFLAGS) OBJS = specparse.o isolationtester.o $(WIN32RES) @@ -55,17 +52,17 @@ maintainer-clean: distclean rm -f specparse.c specscanner.c installcheck: all - ./pg_isolation_regress --psqldir='$(PSQLDIR)' $(EXTRA_REGRESS_OPTS) --inputdir=$(srcdir) --schedule=$(srcdir)/isolation_schedule + ./pg_isolation_regress --bindir='$(bindir)' $(EXTRA_REGRESS_OPTS) --inputdir=$(srcdir) --schedule=$(srcdir)/isolation_schedule check: all - ./pg_isolation_regress --temp-install=./tmp_check --inputdir=$(srcdir) --top-builddir=$(top_builddir) $(EXTRA_REGRESS_OPTS) --schedule=$(srcdir)/isolation_schedule + $(with_temp_install) ./pg_isolation_regress --temp-instance=./tmp_check --inputdir=$(srcdir) --bindir= $(EXTRA_REGRESS_OPTS) --schedule=$(srcdir)/isolation_schedule # Versions of the check tests that include the prepared_transactions test # 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 - ./pg_isolation_regress --psqldir='$(PSQLDIR)' $(EXTRA_REGRESS_OPTS) --inputdir=$(srcdir) --schedule=$(srcdir)/isolation_schedule prepared-transactions +installcheck-prepared-txns: all temp-install + ./pg_isolation_regress --bindir='$(bindir)' $(EXTRA_REGRESS_OPTS) --inputdir=$(srcdir) --schedule=$(srcdir)/isolation_schedule prepared-transactions -check-prepared-txns: all - ./pg_isolation_regress --temp-install=./tmp_check $(EXTRA_REGRESS_OPTS) --inputdir=$(srcdir) --top-builddir=$(top_builddir) --schedule=$(srcdir)/isolation_schedule prepared-transactions +check-prepared-txns: all temp-install + ./pg_isolation_regress --temp-instance=./tmp_check $(EXTRA_REGRESS_OPTS) --inputdir=$(srcdir) --schedule=$(srcdir)/isolation_schedule prepared-transactions diff --git a/src/test/regress/GNUmakefile b/src/test/regress/GNUmakefile index b40b37c..8cb9fd7 100644 --- a/src/test/regress/GNUmakefile +++ b/src/test/regress/GNUmakefile @@ -23,9 +23,6 @@ ifdef TEMP_CONFIG TEMP_CONF += --temp-config=$(TEMP_CONFIG) endif -# where to find psql for testing an existing installation -PSQLDIR = $(bindir) - # maximum simultaneous connections for parallel tests MAXCONNOPT = ifdef MAX_CONNECTIONS @@ -34,7 +31,6 @@ endif # stuff to pass into build of pg_regress EXTRADEFS = '-DHOST_TUPLE="$(host_tuple)"' \ - '-DMAKEPROG="$(MAKE)"' \ '-DSHELLPROG="$(SHELL)"' \ '-DDLSUFFIX="$(DLSUFFIX)"' diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c index 27c46ab..cca942e 100644 --- a/src/test/regress/pg_regress.c +++ b/src/test/regress/pg_regress.c @@ -43,25 +43,10 @@ typedef struct _resultmap } _resultmap; /* - * Values obtained from pg_config_paths.h and Makefile. The PG installation - * paths are only used in temp_install mode: we use these strings to find - * out where "make install" will put stuff under the temp_install directory. - * In non-temp_install mode, the only thing we need is the location of psql, - * which we expect to find in psqldir, or in the PATH if psqldir isn't given. - * - * XXX Because pg_regress is not installed in bindir, we can't support - * this for relocatable trees as it is. --psqldir would need to be - * specified in those cases. + * Values obtained from Makefile. */ -char *bindir = PGBINDIR; -char *libdir = LIBDIR; -char *datadir = PGSHAREDIR; char *host_platform = HOST_TUPLE; -#ifndef WIN32_ONLY_COMPILER -static char *makeprog = MAKEPROG; -#endif - #ifndef WIN32 /* not used in WIN32 case */ static char *shellprog = SHELLPROG; #endif @@ -84,7 +69,7 @@ _stringlist *dblist = NULL; bool debug = false; char *inputdir = "."; char *outputdir = "."; -char *psqldir = PGBINDIR; +char *bindir = PGBINDIR; char *launcher = NULL; static _stringlist *loadlanguage = NULL; static _stringlist *loadextension = NULL; @@ -92,9 +77,8 @@ static int max_connections = 0; static char *encoding = NULL; static _stringlist *schedulelist = NULL; static _stringlist *extra_tests = NULL; -static char *temp_install = NULL; +static char *temp_instance = NULL; static char *temp_config = NULL; -static char *top_builddir = NULL; static bool nolocale = false; static bool use_existing = false; static char *hostname = NULL; @@ -103,7 +87,6 @@ static bool port_specified_by_user = false; static char *dlpath = PKGLIBDIR; static char *user = NULL; static _stringlist *extraroles = NULL; -static _stringlist *extra_install = NULL; /* internal variables */ static const char *progname; @@ -300,8 +283,10 @@ stop_postmaster(void) fflush(stderr); snprintf(buf, sizeof(buf), - "\"%s/pg_ctl\" stop -D \"%s/data\" -s -m fast", - bindir, temp_install); + "\"%s%spg_ctl\" stop -D \"%s/data\" -s -m fast", + bindir ? bindir : "", + bindir ? "/" : "", + temp_instance); r = system(buf); if (r != 0) { @@ -749,27 +734,6 @@ doputenv(const char *var, const char *val) } /* - * Set the environment variable "pathname", prepending "addval" to its - * old value (if any). - */ -static void -add_to_path(const char *pathname, char separator, const char *addval) -{ - char *oldval = getenv(pathname); - char *newval; - - if (!oldval || !oldval[0]) - { - /* no previous value */ - newval = psprintf("%s=%s", pathname, addval); - } - else - newval = psprintf("%s=%s%c%s", pathname, addval, separator, oldval); - - putenv(newval); -} - -/* * Prepare environment variables for running regression tests */ static void @@ -835,7 +799,7 @@ initialize_environment(void) putenv(new_pgoptions); } - if (temp_install) + if (temp_instance) { /* * Clear out any environment vars that might cause psql to connect to @@ -873,49 +837,6 @@ initialize_environment(void) sprintf(s, "%d", port); doputenv("PGPORT", s); } - - /* - * GNU make stores some flags in the MAKEFLAGS environment variable to - * pass arguments to its own children. If we are invoked by make, - * that causes the make invoked by us to think its part of the make - * task invoking us, and so it tries to communicate with the toplevel - * make. Which fails. - * - * Unset the variable to protect against such problems. We also reset - * MAKELEVEL to be certain the child doesn't notice the make above us. - */ - unsetenv("MAKEFLAGS"); - unsetenv("MAKELEVEL"); - - /* - * Adjust path variables to point into the temp-install tree - */ - bindir = psprintf("%s/install/%s", temp_install, bindir); - - libdir = psprintf("%s/install/%s", temp_install, libdir); - - datadir = psprintf("%s/install/%s", temp_install, datadir); - - /* psql will be installed into temp-install bindir */ - psqldir = bindir; - - /* - * Set up shared library paths to include the temp install. - * - * LD_LIBRARY_PATH covers many platforms. DYLD_LIBRARY_PATH works on - * Darwin, and maybe other Mach-based systems. LIBPATH is for AIX. - * Windows needs shared libraries in PATH (only those linked into - * executables, not dlopen'ed ones). Feel free to account for others - * as well. - */ - add_to_path("LD_LIBRARY_PATH", ':', libdir); - add_to_path("DYLD_LIBRARY_PATH", ':', libdir); - add_to_path("LIBPATH", ':', libdir); -#if defined(WIN32) - add_to_path("PATH", ';', libdir); -#elif defined(__CYGWIN__) - add_to_path("PATH", ':', libdir); -#endif } else { @@ -998,8 +919,8 @@ psql_command(const char *database, const char *query,...) /* And now we can build and execute the shell command */ snprintf(psql_cmd, sizeof(psql_cmd), "\"%s%spsql\" -X -c \"%s\" \"%s\"", - psqldir ? psqldir : "", - psqldir ? "/" : "", + bindir ? bindir : "", + bindir ? "/" : "", query_escaped, database); @@ -1957,6 +1878,7 @@ help(void) printf(_("Usage:\n %s [OPTION]... [EXTRA-TEST]...\n"), progname); printf(_("\n")); printf(_("Options:\n")); + printf(_(" --bindir=DIR use programs in DIR (default: configured bindir)\n")); printf(_(" --create-role=ROLE create the specified role before testing\n")); printf(_(" --dbname=DB use database DB (default \"regression\")\n")); printf(_(" --debug turn on debug mode in programs that are run\n")); @@ -1973,21 +1895,18 @@ help(void) printf(_(" --outputdir=DIR place output files in DIR (default \".\")\n")); printf(_(" --schedule=FILE use test ordering schedule from FILE\n")); printf(_(" (can be used multiple times to concatenate)\n")); - printf(_(" --temp-install=DIR create a temporary installation in DIR\n")); - printf(_(" --use-existing use an existing installation\n")); + printf(_(" --temp-instance=DIR create a temporary instance in DIR\n")); + printf(_(" --use-existing use an existing installation\n")); // XXX printf(_("\n")); - printf(_("Options for \"temp-install\" mode:\n")); - printf(_(" --extra-install=DIR additional directory to install (e.g., contrib)\n")); + printf(_("Options for \"temp-instance\" mode:\n")); printf(_(" --no-locale use C locale\n")); printf(_(" --port=PORT start postmaster on PORT\n")); printf(_(" --temp-config=FILE append contents of FILE to temporary config\n")); - printf(_(" --top-builddir=DIR (relative) path to top level build directory\n")); printf(_("\n")); printf(_("Options for using an existing installation:\n")); printf(_(" --host=HOST use postmaster running on HOST\n")); printf(_(" --port=PORT use postmaster running at PORT\n")); printf(_(" --user=USER connect as USER\n")); - printf(_(" --psqldir=DIR use psql in DIR (default: configured bindir)\n")); printf(_("\n")); printf(_("The exit status is 0 if all tests passed, 1 if some tests failed, and 2\n")); printf(_("if the tests could not be run for some reason.\n")); @@ -2009,20 +1928,19 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc {"encoding", required_argument, NULL, 6}, {"outputdir", required_argument, NULL, 7}, {"schedule", required_argument, NULL, 8}, - {"temp-install", required_argument, NULL, 9}, + {"temp-instance", required_argument, NULL, 9}, {"no-locale", no_argument, NULL, 10}, - {"top-builddir", required_argument, NULL, 11}, + {"datadir", required_argument, NULL, 12}, {"host", required_argument, NULL, 13}, {"port", required_argument, NULL, 14}, {"user", required_argument, NULL, 15}, - {"psqldir", required_argument, NULL, 16}, + {"bindir", required_argument, NULL, 16}, {"dlpath", required_argument, NULL, 17}, {"create-role", required_argument, NULL, 18}, {"temp-config", required_argument, NULL, 19}, {"use-existing", no_argument, NULL, 20}, {"launcher", required_argument, NULL, 21}, {"load-extension", required_argument, NULL, 22}, - {"extra-install", required_argument, NULL, 23}, {NULL, 0, NULL, 0} }; @@ -2093,14 +2011,11 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc add_stringlist_item(&schedulelist, optarg); break; case 9: - temp_install = make_absolute_path(optarg); + temp_instance = make_absolute_path(optarg); break; case 10: nolocale = true; break; - case 11: - top_builddir = strdup(optarg); - break; case 13: hostname = strdup(optarg); break; @@ -2112,9 +2027,11 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc user = strdup(optarg); break; case 16: - /* "--psqldir=" should mean to use PATH */ + /* "--bindir=" means to use PATH */ if (strlen(optarg)) - psqldir = strdup(optarg); + bindir = strdup(optarg); + else + bindir = NULL; break; case 17: dlpath = strdup(optarg); @@ -2134,9 +2051,6 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc case 22: add_stringlist_item(&loadextension, optarg); break; - case 23: - add_stringlist_item(&extra_install, optarg); - break; default: /* getopt_long already emitted a complaint */ fprintf(stderr, _("\nTry \"%s -h\" for more information.\n"), @@ -2154,7 +2068,7 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc optind++; } - if (temp_install && !port_specified_by_user) + if (temp_instance && !port_specified_by_user) /* * To reduce chances of interference with parallel installations, use @@ -2180,82 +2094,44 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc unlimit_core_size(); #endif - if (temp_install) + if (temp_instance) { FILE *pg_conf; - _stringlist *sl; /* - * Prepare the temp installation + * Prepare the temp instance */ - if (!top_builddir) - { - fprintf(stderr, _("--top-builddir must be specified when using --temp-install\n")); - exit(2); - } - if (directory_exists(temp_install)) + if (directory_exists(temp_instance)) { - header(_("removing existing temp installation")); - if (!rmtree(temp_install, true)) + header(_("removing existing temp instance")); + if (!rmtree(temp_instance, true)) { - fprintf(stderr, _("\n%s: could not remove temp installation \"%s\": %s\n"), progname, temp_install, strerror(errno)); + fprintf(stderr, _("\n%s: could not remove temp instance \"%s\": %s\n"), progname, temp_instance, strerror(errno)); exit(2); } } - header(_("creating temporary installation")); + header(_("creating temporary instance")); - /* make the temp install top directory */ - make_directory(temp_install); + /* make the temp instance top directory */ + make_directory(temp_instance); /* and a directory for log files */ - snprintf(buf, sizeof(buf), "%s/log", outputdir); + snprintf(buf, sizeof(buf), "%s/log", temp_instance); if (!directory_exists(buf)) make_directory(buf); - /* "make install" */ -#ifndef WIN32_ONLY_COMPILER - snprintf(buf, sizeof(buf), - "\"%s\" -C \"%s\" DESTDIR=\"%s/install\" install > \"%s/log/install.log\" 2>&1", - makeprog, top_builddir, temp_install, outputdir); -#else - snprintf(buf, sizeof(buf), - "perl \"%s/src/tools/msvc/install.pl\" \"%s/install\" >\"%s/log/install.log\" 2>&1", - top_builddir, temp_install, outputdir); -#endif - if (system(buf)) - { - fprintf(stderr, _("\n%s: installation failed\nExamine %s/log/install.log for the reason.\nCommand was: %s\n"), progname, outputdir, buf); - exit(2); - } - - for (sl = extra_install; sl != NULL; sl = sl->next) - { -#ifndef WIN32_ONLY_COMPILER - snprintf(buf, sizeof(buf), - "\"%s\" -C \"%s/%s\" DESTDIR=\"%s/install\" install >> \"%s/log/install.log\" 2>&1", - makeprog, top_builddir, sl->str, temp_install, outputdir); -#else - fprintf(stderr, _("\n%s: --extra-install option not supported on this platform\n"), progname); - exit(2); -#endif - - if (system(buf)) - { - fprintf(stderr, _("\n%s: installation failed\nExamine %s/log/install.log for the reason.\nCommand was: %s\n"), progname, outputdir, buf); - exit(2); - } - } - /* initdb */ header(_("initializing database system")); snprintf(buf, sizeof(buf), - "\"%s/initdb\" -D \"%s/data\" -L \"%s\" --noclean --nosync%s%s > \"%s/log/initdb.log\" 2>&1", - bindir, temp_install, datadir, + "\"%s%sinitdb\" -D \"%s/data\" --noclean --nosync%s%s > \"%s/log/initdb.log\" 2>&1", + bindir ? bindir : "", + bindir ? "/" : "", + temp_instance, debug ? " --debug" : "", nolocale ? " --no-locale" : "", - outputdir); + temp_instance); if (system(buf)) { fprintf(stderr, _("\n%s: initdb failed\nExamine %s/log/initdb.log for the reason.\nCommand was: %s\n"), progname, outputdir, buf); @@ -2270,7 +2146,7 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc * don't set max_prepared_transactions any higher than actually needed * by the prepared_xacts regression test.) */ - snprintf(buf, sizeof(buf), "%s/data/postgresql.conf", temp_install); + snprintf(buf, sizeof(buf), "%s/data/postgresql.conf", temp_instance); pg_conf = fopen(buf, "a"); if (pg_conf == NULL) { @@ -2302,8 +2178,10 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc * Check if there is a postmaster running already. */ snprintf(buf2, sizeof(buf2), - "\"%s/psql\" -X postgres <%s 2>%s", - bindir, DEVNULL, DEVNULL); + "\"%s%spsql\" -X postgres <%s 2>%s", + bindir ? bindir : "", + bindir ? "/" : "", + DEVNULL, DEVNULL); for (i = 0; i < 16; i++) { @@ -2334,12 +2212,14 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc */ header(_("starting postmaster")); snprintf(buf, sizeof(buf), - "\"%s/postgres\" -D \"%s/data\" -F%s " + "\"%s%spostgres\" -D \"%s/data\" -F%s " "-c \"listen_addresses=%s\" -k \"%s\" " "> \"%s/log/postmaster.log\" 2>&1", - bindir, temp_install, debug ? " -d 5" : "", + bindir ? bindir : "", + bindir ? "/" : "", + temp_instance, debug ? " -d 5" : "", hostname ? hostname : "", sockdir ? sockdir : "", - outputdir); + temp_instance); postmaster_pid = spawn_process(buf); if (postmaster_pid == INVALID_PID) { @@ -2453,7 +2333,7 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc /* * Shut down temp installation's postmaster */ - if (temp_install) + if (temp_instance) { header(_("shutting down postmaster")); stop_postmaster(); diff --git a/src/test/regress/pg_regress.h b/src/test/regress/pg_regress.h index 942d761..992c0bf 100644 --- a/src/test/regress/pg_regress.h +++ b/src/test/regress/pg_regress.h @@ -43,12 +43,6 @@ extern char *inputdir; extern char *outputdir; extern char *launcher; -/* - * This should not be global but every module should be able to read command - * line parameters. - */ -extern char *psqldir; - extern const char *basic_diff_opts; extern const char *pretty_diff_opts; diff --git a/src/test/regress/pg_regress_main.c b/src/test/regress/pg_regress_main.c index 22197aa..3b7b952 100644 --- a/src/test/regress/pg_regress_main.c +++ b/src/test/regress/pg_regress_main.c @@ -65,8 +65,8 @@ psql_start_test(const char *testname, snprintf(psql_cmd + offset, sizeof(psql_cmd) - offset, "\"%s%spsql\" -X -a -q -d \"%s\" < \"%s\" > \"%s\" 2>&1", - psqldir ? psqldir : "", - psqldir ? "/" : "", + bindir ? bindir : "", + bindir ? "/" : "", dblist->str, infile, outfile);
^ permalink raw reply [nested|flat] 22+ messages in thread

* Re: improving speed of make check-world
  2014-08-15 05:45 improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
@ 2014-08-31 09:36 ` Fabien COELHO <coelho@cri.ensmp.fr>
  2014-08-31 16:08   ` Re: improving speed of make check-world Fabien COELHO <coelho@cri.ensmp.fr>
  2015-02-15 02:01   ` Re: improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
  2 siblings, 2 replies; 22+ messages in thread

From: Fabien COELHO @ 2014-08-31 09:36 UTC (permalink / raw)
  To: Peter Eisentraut <peter_e@gmx.net>; +Cc: pgsql-hackers



Hello Peter,

Here is a review:

The version 2 of the patch applies cleanly on current head.

The ability to generate and reuse a temporary installation for different 
tests looks quite useful, thus putting install out of pg_regress and in 
make seems reasonnable.

However I'm wondering whether there could be some use case of pg_regress 
where doing the install might be useful nevertheless, say for manually 
doing things on the command line, in which case keeping the feature, even 
if not used by default, could be nice?

About changes: I think that more comments would be welcome, eg 
with_temp_install.

There is not a single documentation change. Should there be some? Hmmm... 
I have not found much documentation about "pg_regress"...

I have tested make check, check-world, as well as make check in contrib 
sub-directories. It seems to work fine in sequential mode.

Running "make -j2 check-world" does not work because "initdb" is not found 
by "pg_regress". but "make -j1 check-world" does work fine. It seems that 
some dependencies might be missing and there is a race condition between 
temporary install and running some checks?? Maybe it is not expected to 
work anyway? See below suggestions to make it work.

However in this case the error message passed by pg_regress contains an 
error:

  pg_regress: initdb failed
  Examine /home/fabien/DEV/GIT/postgresql/contrib/btree_gin/log/initdb.log for the reason.
  Command was: "initdb" -D "/home/fabien/DEV/GIT/postgresql/contrib/btree_gin/./tmp_check/data" --noclean --nosync > "/home/fabien/DEV/GIT/postgresql/contrib/btree_gin/./tmp_check/log/initdb.log" 2>&1
  make[2]: *** [check] Error 2

The error messages point to non existing log file (./tmp_check is 
missing), the temp_instance variable should be used in the error message 
as well as in the commands. Maybe other error messages have the same 
issues.

I do not like much the MAKELEVEL hack in a phony target. When running in 
parallel, several makes may have the same level anyway, not sure what 
would happen... Maybe it is the origin of the race condition which makes 
initdb not to be found above. I would suggest to try to rely on 
dependences, maybe something like the following could work to ensure that 
an installation is done once and only once per make invocation, whatever 
the underlying parallelism & levels, as well as a possibility to reuse the 
previous installation.

   # must be defined somewhere common to all makefiles
   ifndef MAKE_NONCE
   # define a nanosecond timestamp
   export MAKE_NONCE := $(shell date +%s.%N)
   endif

   # actual new tmp installation
   .tmp_install:
 	$(RM) ./.tmp_install.*
 	$(RM) -r ./tmp_install
 	# create tmp installation...
 	touch $@

   # tmp installation for the nonce
   .tmp_install.$(MAKE_NONCE): .tmp_install
   	touch $@

   # generate a new tmp installation by default
   # "make USE_TMP_INSTALL=1 ..." reuses a previous installation if available
   ifdef USE_TMP_INSTALL
   TMP_INSTALL = .tmp_install
   else
   TMP_INSTALL = .tmp_install.$(MAKE_NONCE)
   endif # USE_TMP_INSTALL

   .PHONY: main-temp-install
   main-temp-install: $(TMP_INSTALL)

   .PHONY: extra-temp-install
   extra-temp-install: main-temp-install
 	# do EXTRA_INSTALL


> MSVC build is not yet adjusted for this.  Looking at vcregress.pl, this 
> should be fairly straightforward.

No idea about that point.

-- 
Fabien.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



^ permalink  raw  reply  [nested|flat] 22+ messages in thread

* Re: improving speed of make check-world
  2014-08-15 05:45 improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
  2014-08-31 09:36 ` Re: improving speed of make check-world Fabien COELHO <coelho@cri.ensmp.fr>
@ 2014-08-31 16:08   ` Fabien COELHO <coelho@cri.ensmp.fr>
  1 sibling, 0 replies; 22+ messages in thread

From: Fabien COELHO @ 2014-08-31 16:08 UTC (permalink / raw)
  To: Peter Eisentraut <peter_e@gmx.net>; +Cc: pgsql-hackers


>  # actual new tmp installation
>  .tmp_install:
> 	$(RM) ./.tmp_install.*
> 	$(RM) -r ./tmp_install
> 	# create tmp installation...
> 	touch $@
>
>  # tmp installation for the nonce
>  .tmp_install.$(MAKE_NONCE): .tmp_install
>  	touch $@

Oops, I got it wrong, the install would not be reexecuted the second time.

Maybe someting more like:

   ifdef USE_ONE_INSTALL
   TMP_INSTALL = .tmp_install.once
   else
   TMP_INSTALL = .tmp_install.$(MAKE_NONCE)
   endif

   $(TMP_INSTALL):
   	$(RM) -r ./tmp_install .tmp_install.*
   	# do installation...
   	touch $@

So that the file target is different each time it is run. Hopefully.

-- 
Fabien.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



^ permalink  raw  reply  [nested|flat] 22+ messages in thread

* Re: improving speed of make check-world
  2014-08-15 05:45 improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
  2014-08-31 09:36 ` Re: improving speed of make check-world Fabien COELHO <coelho@cri.ensmp.fr>
@ 2015-02-15 02:01   ` Peter Eisentraut <peter_e@gmx.net>
  2015-02-15 02:09     ` Re: improving speed of make check-world Tom Lane <tgl@sss.pgh.pa.us>
  2015-02-24 08:06     ` Re: improving speed of make check-world Michael Paquier <michael.paquier@gmail.com>
  1 sibling, 2 replies; 22+ messages in thread

From: Peter Eisentraut @ 2015-02-15 02:01 UTC (permalink / raw)
  To: pgsql-hackers; +Cc: Fabien COELHO <coelho@cri.ensmp.fr>

On 8/31/14 5:36 AM, Fabien COELHO wrote:
> Running "make -j2 check-world" does not work because "initdb" is not
> found by "pg_regress". but "make -j1 check-world" does work fine. It
> seems that some dependencies might be missing and there is a race
> condition between temporary install and running some checks?? Maybe it
> is not expected to work anyway? See below suggestions to make it work.

Here is an updated patch that fixes this problem.

The previous problem was simply a case were the make rules were not
parallel-safe.

For recursive make, we (through magic) set up targets like this:

    check: check-subdir1-check check-subdir2-check

And with my old patch we added

    check: temp-install

So the aggregate prerequisites were in effect something like

    check: check-subdir1-check check-subdir2-check temp-install

And so there was nothing stopping a parallel make to descend into the
subdirectories before the temp install was set up.

What we need is additional prerequisites like

    check-subdir1-check check-subdir2-check: temp-install

I have hacked this directly into the $(recurse) function, which is ugly.
 This could possibly be improved somehow, but the effect would be the
same in any case.

With this, I can now run things like

    make -C src/pl check -j3
    make -C src/bin check -j8

A full make check-world -j$X is still prone to fail because some test
suites can't run in parallel with others, but that's a separate problem
to fix.


Note:  We have in the meantime added logic to pg_regress to clean up the
temporary installation after the run.  This changes that because
pg_regress is no longer responsible for the temporary installation.
pg_regress still cleans up the temporary data directory, so you still
get quite a bit of space savings.  But the temporary installation is not
cleaned up.  But since we would now only use a single temporary
installation, the disk space usage still stays in the same order of
magnitude.

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Attachments:

regress-temp-instance-v3.patch (text/x-patch, 31.0K) download diff --git a/.gitignore b/.gitignore index 715f817..77feb4c 100644 --- a/.gitignore +++ b/.gitignore @@ -35,3 +35,4 @@ lib*.pc /pgsql.sln.cache /Debug/ /Release/ +/tmp_install/ diff --git a/GNUmakefile.in b/GNUmakefile.in index 69e0824..361897a 100644 --- a/GNUmakefile.in +++ b/GNUmakefile.in @@ -47,6 +47,7 @@ $(call recurse,distprep,doc src config contrib) # it's not built by default $(call recurse,clean,doc contrib src config) clean: + rm -rf tmp_install/ # Garbage from autoconf: @rm -rf autom4te.cache/ diff --git a/contrib/earthdistance/Makefile b/contrib/earthdistance/Makefile index 93dcbe3..cde1ae6 100644 --- a/contrib/earthdistance/Makefile +++ b/contrib/earthdistance/Makefile @@ -7,7 +7,7 @@ DATA = earthdistance--1.0.sql earthdistance--unpackaged--1.0.sql PGFILEDESC = "earthdistance - calculate distances on the surface of the Earth" REGRESS = earthdistance -REGRESS_OPTS = --extra-install=contrib/cube +EXTRA_INSTALL = contrib/cube LDFLAGS_SL += $(filter -lm, $(LIBS)) diff --git a/contrib/pg_upgrade/test.sh b/contrib/pg_upgrade/test.sh index 75b6357..3012ed0 100644 --- a/contrib/pg_upgrade/test.sh +++ b/contrib/pg_upgrade/test.sh @@ -87,7 +87,7 @@ if [ "$1" = '--install' ]; then # use psql from the proper installation directory, which might # be outdated or missing. But don't override anything else that's # already in EXTRA_REGRESS_OPTS. - EXTRA_REGRESS_OPTS="$EXTRA_REGRESS_OPTS --psqldir='$bindir'" + EXTRA_REGRESS_OPTS="$EXTRA_REGRESS_OPTS --bindir='$bindir'" export EXTRA_REGRESS_OPTS fi diff --git a/contrib/test_decoding/Makefile b/contrib/test_decoding/Makefile index 438be44..613e9c3 100644 --- a/contrib/test_decoding/Makefile +++ b/contrib/test_decoding/Makefile @@ -39,35 +39,33 @@ submake-test_decoding: REGRESSCHECKS=ddl rewrite toast permissions decoding_in_xact decoding_into_rel binary prepared -regresscheck: all | submake-regress submake-test_decoding +regresscheck: all | submake-regress submake-test_decoding temp-install $(MKDIR_P) regression_output $(pg_regress_check) \ --temp-config $(top_srcdir)/contrib/test_decoding/logical.conf \ - --temp-install=./tmp_check \ - --extra-install=contrib/test_decoding \ + --temp-instance=./tmp_check \ --outputdir=./regression_output \ $(REGRESSCHECKS) -regresscheck-install-force: | submake-regress submake-test_decoding +regresscheck-install-force: | submake-regress submake-test_decoding temp-install $(pg_regress_installcheck) \ - --extra-install=contrib/test_decoding \ $(REGRESSCHECKS) ISOLATIONCHECKS=mxact delayed_startup ondisk_startup concurrent_ddl_dml -isolationcheck: all | submake-isolation submake-test_decoding +isolationcheck: all | submake-isolation submake-test_decoding temp-install $(MKDIR_P) isolation_output $(pg_isolation_regress_check) \ --temp-config $(top_srcdir)/contrib/test_decoding/logical.conf \ - --extra-install=contrib/test_decoding \ --outputdir=./isolation_output \ $(ISOLATIONCHECKS) -isolationcheck-install-force: all | submake-isolation submake-test_decoding +isolationcheck-install-force: all | submake-isolation submake-test_decoding temp-install $(pg_isolation_regress_installcheck) \ - --extra-install=contrib/test_decoding \ $(ISOLATIONCHECKS) PHONY: submake-test_decoding submake-regress check \ regresscheck regresscheck-install-force \ isolationcheck isolationcheck-install-force + +temp-install: EXTRA_INSTALL=contrib/test_decoding diff --git a/src/Makefile.global.in b/src/Makefile.global.in index 7c39d82..435bea6 100644 --- a/src/Makefile.global.in +++ b/src/Makefile.global.in @@ -43,6 +43,7 @@ MAJORVERSION = @PG_MAJORVERSION@ # (PGXS VPATH support is handled separately in pgxs.mk) ifndef PGXS vpath_build = @vpath_build@ +abs_top_builddir = @abs_top_builddir@ abs_top_srcdir = @abs_top_srcdir@ ifneq ($(vpath_build),yes) @@ -300,6 +301,17 @@ BZIP2 = bzip2 # Testing +check: temp-install + +.PHONY: temp-install +temp-install: +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 +endif + for extra in $(EXTRA_INSTALL); do $(MAKE) -C '$(top_builddir)'/$$extra DESTDIR='$(abs_top_builddir)'/tmp_install install >>'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1 || exit; done + PROVE = @PROVE@ PG_PROVE_FLAGS = -I $(top_srcdir)/src/test/perl/ PROVE_FLAGS = --verbose @@ -314,6 +326,10 @@ define ld_library_path_var $(if $(filter $(PORTNAME),darwin),DYLD_LIBRARY_PATH,$(if $(filter $(PORTNAME),aix),LIBPATH,LD_LIBRARY_PATH)) endef +define with_temp_install +PATH="$(abs_top_builddir)/tmp_install$(bindir):$$PATH" $(call add_to_path,$(ld_library_path_var),$(abs_top_builddir)/tmp_install$(libdir)) +endef + ifeq ($(enable_tap_tests),yes) define prove_installcheck @@ -321,9 +337,7 @@ cd $(srcdir) && TESTDIR='$(CURDIR)' PATH="$(bindir):$$PATH" PGPORT='6$(DEF_PGPOR endef define prove_check -$(MKDIR_P) tmp_check/log -$(MAKE) -C $(top_builddir) DESTDIR='$(CURDIR)'/tmp_check/install install >'$(CURDIR)'/tmp_check/log/install.log 2>&1 -cd $(srcdir) && TESTDIR='$(CURDIR)' PATH="$(CURDIR)/tmp_check/install$(bindir):$$PATH" $(call add_to_path,$(ld_library_path_var),$(CURDIR)/tmp_check/install$(libdir)) top_builddir='$(CURDIR)/$(top_builddir)' PGPORT='6$(DEF_PGPORT)' $(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) t/*.pl +cd $(srcdir) && TESTDIR='$(CURDIR)' $(with_temp_install) PGPORT='6$(DEF_PGPORT)' top_builddir='$(CURDIR)/$(top_builddir)' $(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) t/*.pl endef else @@ -494,13 +508,13 @@ endif pg_regress_locale_flags = $(if $(ENCODING),--encoding=$(ENCODING)) $(NOLOCALE) -pg_regress_check = $(top_builddir)/src/test/regress/pg_regress --inputdir=$(srcdir) --temp-install=./tmp_check --top-builddir=$(top_builddir) $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS) -pg_regress_installcheck = $(top_builddir)/src/test/regress/pg_regress --inputdir=$(srcdir) --psqldir='$(PSQLDIR)' $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS) +pg_regress_check = $(with_temp_install) $(top_builddir)/src/test/regress/pg_regress --inputdir=$(srcdir) --temp-instance=./tmp_check --bindir= $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS) +pg_regress_installcheck = $(top_builddir)/src/test/regress/pg_regress --inputdir=$(srcdir) --bindir='$(bindir)' $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS) pg_regress_clean_files = results/ regression.diffs regression.out tmp_check/ log/ -pg_isolation_regress_check = $(top_builddir)/src/test/isolation/pg_isolation_regress --inputdir=$(srcdir) --temp-install=./tmp_check --top-builddir=$(top_builddir) $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS) -pg_isolation_regress_installcheck = $(top_builddir)/src/test/isolation/pg_isolation_regress --inputdir=$(srcdir) --top-builddir=$(top_builddir) $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS) +pg_isolation_regress_check = $(with_temp_install) $(top_builddir)/src/test/isolation/pg_isolation_regress --inputdir=$(srcdir) --temp-instance=./tmp_check --bindir= $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS) +pg_isolation_regress_installcheck = $(top_builddir)/src/test/isolation/pg_isolation_regress --inputdir=$(srcdir) $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS) ########################################################################## # @@ -679,7 +693,7 @@ endif define _create_recursive_target .PHONY: $(1)-$(2)-recurse $(1): $(1)-$(2)-recurse -$(1)-$(2)-recurse: +$(1)-$(2)-recurse: $(if $(filter check, $(3)), temp-install) $$(MAKE) -C $(2) $(3) endef # Note that the use of $$ on the last line above is important; we want diff --git a/src/interfaces/ecpg/test/Makefile b/src/interfaces/ecpg/test/Makefile index 387729b..a4ac021 100644 --- a/src/interfaces/ecpg/test/Makefile +++ b/src/interfaces/ecpg/test/Makefile @@ -11,14 +11,10 @@ override CPPFLAGS := \ '-I$(top_builddir)/src/port' \ '-I$(top_srcdir)/src/test/regress' \ '-DHOST_TUPLE="$(host_tuple)"' \ - '-DMAKEPROG="$(MAKE)"' \ '-DSHELLPROG="$(SHELL)"' \ '-DDLSUFFIX="$(DLSUFFIX)"' \ $(CPPFLAGS) -# where to find psql for testing an existing installation -PSQLDIR = $(bindir) - # default encoding for regression tests ENCODING = SQL_ASCII @@ -82,11 +78,11 @@ endif REGRESS_OPTS = --dbname=regress1,connectdb --create-role=connectuser,connectdb $(EXTRA_REGRESS_OPTS) check: all - ./pg_regress $(REGRESS_OPTS) --top-builddir=$(top_builddir) --temp-install=./tmp_check $(pg_regress_locale_flags) $(THREAD) --schedule=$(srcdir)/ecpg_schedule + $(with_temp_install) ./pg_regress $(REGRESS_OPTS) --temp-instance=./tmp_check --bindir= $(pg_regress_locale_flags) $(THREAD) --schedule=$(srcdir)/ecpg_schedule # the same options, but with --listen-on-tcp checktcp: all - ./pg_regress $(REGRESS_OPTS) --top-builddir=$(top_builddir) --temp-install=./tmp_check $(pg_regress_locale_flags) $(THREAD) --schedule=$(srcdir)/ecpg_schedule_tcp --host=localhost + $(with_temp_install) ./pg_regress $(REGRESS_OPTS) --temp-instance=./tmp_check --bindir= $(pg_regress_locale_flags) $(THREAD) --schedule=$(srcdir)/ecpg_schedule_tcp --host=localhost installcheck: all - ./pg_regress $(REGRESS_OPTS) --psqldir='$(PSQLDIR)' --top-builddir=$(top_builddir) $(pg_regress_locale_flags) $(THREAD) --schedule=$(srcdir)/ecpg_schedule + ./pg_regress $(REGRESS_OPTS) --bindir='$(bindir)' $(pg_regress_locale_flags) $(THREAD) --schedule=$(srcdir)/ecpg_schedule diff --git a/src/makefiles/pgxs.mk b/src/makefiles/pgxs.mk index 24ddba9..ea17b1c 100644 --- a/src/makefiles/pgxs.mk +++ b/src/makefiles/pgxs.mk @@ -258,9 +258,6 @@ else REGRESS_OPTS += --dbname=$(CONTRIB_TESTDB) endif -# where to find psql for running the tests -PSQLDIR = $(bindir) - # When doing a VPATH build, must copy over the data files so that the # driver script can find them. We have to use an absolute path for # the targets, because otherwise make will try to locate the missing @@ -295,7 +292,9 @@ check: @echo 'Do "$(MAKE) install", then "$(MAKE) installcheck" instead.' else check: all submake $(REGRESS_PREP) - $(pg_regress_check) --extra-install=$(subdir) $(REGRESS_OPTS) $(REGRESS) + $(pg_regress_check) $(REGRESS_OPTS) $(REGRESS) + +temp-install: EXTRA_INSTALL=$(subdir) endif endif # REGRESS diff --git a/src/pl/plperl/GNUmakefile b/src/pl/plperl/GNUmakefile index a04a2d3..2b69847 100644 --- a/src/pl/plperl/GNUmakefile +++ b/src/pl/plperl/GNUmakefile @@ -69,8 +69,6 @@ ifeq ($(shell $(PERL) -V:usemultiplicity), usemultiplicity='define';) REGRESS += plperl_plperlu endif endif -# where to find psql for running the tests -PSQLDIR = $(bindir) # where to find xsubpp for building XS. XSUBPPDIR = $(shell $(PERL) -e 'use List::Util qw(first); print first { -r "$$_/ExtUtils/xsubpp" } @INC') diff --git a/src/pl/plpython/Makefile b/src/pl/plpython/Makefile index 430aa89..c58e56b 100644 --- a/src/pl/plpython/Makefile +++ b/src/pl/plpython/Makefile @@ -115,9 +115,6 @@ REGRESS = \ REGRESS_PLPYTHON3_MANGLE := $(REGRESS) -# where to find psql for running the tests -PSQLDIR = $(bindir) - include $(top_srcdir)/src/Makefile.shlib all: all-lib diff --git a/src/pl/tcl/Makefile b/src/pl/tcl/Makefile index 7ea0026..851e3c0 100644 --- a/src/pl/tcl/Makefile +++ b/src/pl/tcl/Makefile @@ -45,8 +45,6 @@ DATA = pltcl.control pltcl--1.0.sql pltcl--unpackaged--1.0.sql \ REGRESS_OPTS = --dbname=$(PL_TESTDB) --load-extension=pltcl REGRESS = pltcl_setup pltcl_queries -# where to find psql for running the tests -PSQLDIR = $(bindir) # Tcl on win32 ships with import libraries only for Microsoft Visual C++, # which are not compatible with mingw gcc. Therefore we need to build a diff --git a/src/test/isolation/Makefile b/src/test/isolation/Makefile index a88257a..4577509 100644 --- a/src/test/isolation/Makefile +++ b/src/test/isolation/Makefile @@ -9,9 +9,6 @@ subdir = src/test/isolation top_builddir = ../../.. include $(top_builddir)/src/Makefile.global -# where to find psql for testing an existing installation -PSQLDIR = $(bindir) - override CPPFLAGS := -I$(srcdir) -I$(libpq_srcdir) -I$(srcdir)/../regress $(CPPFLAGS) OBJS = specparse.o isolationtester.o $(WIN32RES) @@ -55,17 +52,17 @@ maintainer-clean: distclean rm -f specparse.c specscanner.c installcheck: all - ./pg_isolation_regress --psqldir='$(PSQLDIR)' $(EXTRA_REGRESS_OPTS) --inputdir=$(srcdir) --schedule=$(srcdir)/isolation_schedule + ./pg_isolation_regress --bindir='$(bindir)' $(EXTRA_REGRESS_OPTS) --inputdir=$(srcdir) --schedule=$(srcdir)/isolation_schedule check: all - ./pg_isolation_regress --temp-install=./tmp_check --inputdir=$(srcdir) --top-builddir=$(top_builddir) $(EXTRA_REGRESS_OPTS) --schedule=$(srcdir)/isolation_schedule + $(with_temp_install) ./pg_isolation_regress --temp-instance=./tmp_check --inputdir=$(srcdir) --bindir= $(EXTRA_REGRESS_OPTS) --schedule=$(srcdir)/isolation_schedule # Versions of the check tests that include the prepared_transactions test # 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 - ./pg_isolation_regress --psqldir='$(PSQLDIR)' $(EXTRA_REGRESS_OPTS) --inputdir=$(srcdir) --schedule=$(srcdir)/isolation_schedule prepared-transactions +installcheck-prepared-txns: all temp-install + ./pg_isolation_regress --bindir='$(bindir)' $(EXTRA_REGRESS_OPTS) --inputdir=$(srcdir) --schedule=$(srcdir)/isolation_schedule prepared-transactions -check-prepared-txns: all - ./pg_isolation_regress --temp-install=./tmp_check $(EXTRA_REGRESS_OPTS) --inputdir=$(srcdir) --top-builddir=$(top_builddir) --schedule=$(srcdir)/isolation_schedule prepared-transactions +check-prepared-txns: all temp-install + ./pg_isolation_regress --temp-instance=./tmp_check $(EXTRA_REGRESS_OPTS) --inputdir=$(srcdir) --schedule=$(srcdir)/isolation_schedule prepared-transactions diff --git a/src/test/regress/GNUmakefile b/src/test/regress/GNUmakefile index 110eb80..4d6849e 100644 --- a/src/test/regress/GNUmakefile +++ b/src/test/regress/GNUmakefile @@ -23,9 +23,6 @@ ifdef TEMP_CONFIG TEMP_CONF += --temp-config=$(TEMP_CONFIG) endif -# where to find psql for testing an existing installation -PSQLDIR = $(bindir) - # maximum simultaneous connections for parallel tests MAXCONNOPT = ifdef MAX_CONNECTIONS @@ -34,7 +31,6 @@ endif # stuff to pass into build of pg_regress EXTRADEFS = '-DHOST_TUPLE="$(host_tuple)"' \ - '-DMAKEPROG="$(MAKE)"' \ '-DSHELLPROG="$(SHELL)"' \ '-DDLSUFFIX="$(DLSUFFIX)"' diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c index 0de1af6..7ad4ee1 100644 --- a/src/test/regress/pg_regress.c +++ b/src/test/regress/pg_regress.c @@ -44,25 +44,10 @@ typedef struct _resultmap } _resultmap; /* - * Values obtained from pg_config_paths.h and Makefile. The PG installation - * paths are only used in temp_install mode: we use these strings to find - * out where "make install" will put stuff under the temp_install directory. - * In non-temp_install mode, the only thing we need is the location of psql, - * which we expect to find in psqldir, or in the PATH if psqldir isn't given. - * - * XXX Because pg_regress is not installed in bindir, we can't support - * this for relocatable trees as it is. --psqldir would need to be - * specified in those cases. + * Values obtained from Makefile. */ -char *bindir = PGBINDIR; -char *libdir = LIBDIR; -char *datadir = PGSHAREDIR; char *host_platform = HOST_TUPLE; -#ifndef WIN32_ONLY_COMPILER -static char *makeprog = MAKEPROG; -#endif - #ifndef WIN32 /* not used in WIN32 case */ static char *shellprog = SHELLPROG; #endif @@ -85,7 +70,7 @@ _stringlist *dblist = NULL; bool debug = false; char *inputdir = "."; char *outputdir = "."; -char *psqldir = PGBINDIR; +char *bindir = PGBINDIR; char *launcher = NULL; static _stringlist *loadlanguage = NULL; static _stringlist *loadextension = NULL; @@ -93,9 +78,8 @@ static int max_connections = 0; static char *encoding = NULL; static _stringlist *schedulelist = NULL; static _stringlist *extra_tests = NULL; -static char *temp_install = NULL; +static char *temp_instance = NULL; static char *temp_config = NULL; -static char *top_builddir = NULL; static bool nolocale = false; static bool use_existing = false; static char *hostname = NULL; @@ -104,7 +88,6 @@ static bool port_specified_by_user = false; static char *dlpath = PKGLIBDIR; static char *user = NULL; static _stringlist *extraroles = NULL; -static _stringlist *extra_install = NULL; static char *config_auth_datadir = NULL; /* internal variables */ @@ -302,8 +285,10 @@ stop_postmaster(void) fflush(stderr); snprintf(buf, sizeof(buf), - "\"%s/pg_ctl\" stop -D \"%s/data\" -s -m fast", - bindir, temp_install); + "\"%s%spg_ctl\" stop -D \"%s/data\" -s -m fast", + bindir ? bindir : "", + bindir ? "/" : "", + temp_instance); r = system(buf); if (r != 0) { @@ -751,27 +736,6 @@ doputenv(const char *var, const char *val) } /* - * Set the environment variable "pathname", prepending "addval" to its - * old value (if any). - */ -static void -add_to_path(const char *pathname, char separator, const char *addval) -{ - char *oldval = getenv(pathname); - char *newval; - - if (!oldval || !oldval[0]) - { - /* no previous value */ - newval = psprintf("%s=%s", pathname, addval); - } - else - newval = psprintf("%s=%s%c%s", pathname, addval, separator, oldval); - - putenv(newval); -} - -/* * Prepare environment variables for running regression tests */ static void @@ -845,7 +809,7 @@ initialize_environment(void) putenv(new_pgoptions); } - if (temp_install) + if (temp_instance) { /* * Clear out any environment vars that might cause psql to connect to @@ -883,49 +847,6 @@ initialize_environment(void) sprintf(s, "%d", port); doputenv("PGPORT", s); } - - /* - * GNU make stores some flags in the MAKEFLAGS environment variable to - * pass arguments to its own children. If we are invoked by make, - * that causes the make invoked by us to think its part of the make - * task invoking us, and so it tries to communicate with the toplevel - * make. Which fails. - * - * Unset the variable to protect against such problems. We also reset - * MAKELEVEL to be certain the child doesn't notice the make above us. - */ - unsetenv("MAKEFLAGS"); - unsetenv("MAKELEVEL"); - - /* - * Adjust path variables to point into the temp-install tree - */ - bindir = psprintf("%s/install/%s", temp_install, bindir); - - libdir = psprintf("%s/install/%s", temp_install, libdir); - - datadir = psprintf("%s/install/%s", temp_install, datadir); - - /* psql will be installed into temp-install bindir */ - psqldir = bindir; - - /* - * Set up shared library paths to include the temp install. - * - * LD_LIBRARY_PATH covers many platforms. DYLD_LIBRARY_PATH works on - * Darwin, and maybe other Mach-based systems. LIBPATH is for AIX. - * Windows needs shared libraries in PATH (only those linked into - * executables, not dlopen'ed ones). Feel free to account for others - * as well. - */ - add_to_path("LD_LIBRARY_PATH", ':', libdir); - add_to_path("DYLD_LIBRARY_PATH", ':', libdir); - add_to_path("LIBPATH", ':', libdir); -#if defined(WIN32) - add_to_path("PATH", ';', libdir); -#elif defined(__CYGWIN__) - add_to_path("PATH", ':', libdir); -#endif } else { @@ -1178,8 +1099,8 @@ psql_command(const char *database, const char *query,...) /* And now we can build and execute the shell command */ snprintf(psql_cmd, sizeof(psql_cmd), "\"%s%spsql\" -X -c \"%s\" \"%s\"", - psqldir ? psqldir : "", - psqldir ? "/" : "", + bindir ? bindir : "", + bindir ? "/" : "", query_escaped, database); @@ -2154,21 +2075,18 @@ help(void) printf(_(" --outputdir=DIR place output files in DIR (default \".\")\n")); printf(_(" --schedule=FILE use test ordering schedule from FILE\n")); printf(_(" (can be used multiple times to concatenate)\n")); - printf(_(" --temp-install=DIR create a temporary installation in DIR\n")); - printf(_(" --use-existing use an existing installation\n")); + printf(_(" --temp-instance=DIR create a temporary instance in DIR\n")); + printf(_(" --use-existing use an existing installation\n")); // XXX printf(_("\n")); - printf(_("Options for \"temp-install\" mode:\n")); - printf(_(" --extra-install=DIR additional directory to install (e.g., contrib)\n")); + printf(_("Options for \"temp-instance\" mode:\n")); printf(_(" --no-locale use C locale\n")); printf(_(" --port=PORT start postmaster on PORT\n")); printf(_(" --temp-config=FILE append contents of FILE to temporary config\n")); - printf(_(" --top-builddir=DIR (relative) path to top level build directory\n")); printf(_("\n")); printf(_("Options for using an existing installation:\n")); printf(_(" --host=HOST use postmaster running on HOST\n")); printf(_(" --port=PORT use postmaster running at PORT\n")); printf(_(" --user=USER connect as USER\n")); - printf(_(" --psqldir=DIR use psql in DIR (default: configured bindir)\n")); printf(_("\n")); printf(_("The exit status is 0 if all tests passed, 1 if some tests failed, and 2\n")); printf(_("if the tests could not be run for some reason.\n")); @@ -2190,20 +2108,19 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc {"encoding", required_argument, NULL, 6}, {"outputdir", required_argument, NULL, 7}, {"schedule", required_argument, NULL, 8}, - {"temp-install", required_argument, NULL, 9}, + {"temp-instance", required_argument, NULL, 9}, {"no-locale", no_argument, NULL, 10}, - {"top-builddir", required_argument, NULL, 11}, + {"datadir", required_argument, NULL, 12}, {"host", required_argument, NULL, 13}, {"port", required_argument, NULL, 14}, {"user", required_argument, NULL, 15}, - {"psqldir", required_argument, NULL, 16}, + {"bindir", required_argument, NULL, 16}, {"dlpath", required_argument, NULL, 17}, {"create-role", required_argument, NULL, 18}, {"temp-config", required_argument, NULL, 19}, {"use-existing", no_argument, NULL, 20}, {"launcher", required_argument, NULL, 21}, {"load-extension", required_argument, NULL, 22}, - {"extra-install", required_argument, NULL, 23}, {"config-auth", required_argument, NULL, 24}, {NULL, 0, NULL, 0} }; @@ -2275,14 +2192,11 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc add_stringlist_item(&schedulelist, optarg); break; case 9: - temp_install = make_absolute_path(optarg); + temp_instance = make_absolute_path(optarg); break; case 10: nolocale = true; break; - case 11: - top_builddir = strdup(optarg); - break; case 13: hostname = strdup(optarg); break; @@ -2294,9 +2208,11 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc user = strdup(optarg); break; case 16: - /* "--psqldir=" should mean to use PATH */ + /* "--bindir=" means to use PATH */ if (strlen(optarg)) - psqldir = strdup(optarg); + bindir = strdup(optarg); + else + bindir = NULL; break; case 17: dlpath = strdup(optarg); @@ -2316,9 +2232,6 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc case 22: add_stringlist_item(&loadextension, optarg); break; - case 23: - add_stringlist_item(&extra_install, optarg); - break; case 24: config_auth_datadir = pstrdup(optarg); break; @@ -2347,7 +2260,7 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc exit(0); } - if (temp_install && !port_specified_by_user) + if (temp_instance && !port_specified_by_user) /* * To reduce chances of interference with parallel installations, use @@ -2373,83 +2286,45 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc unlimit_core_size(); #endif - if (temp_install) + if (temp_instance) { FILE *pg_conf; - _stringlist *sl; /* - * Prepare the temp installation + * Prepare the temp instance */ - if (!top_builddir) - { - fprintf(stderr, _("--top-builddir must be specified when using --temp-install\n")); - exit(2); - } - if (directory_exists(temp_install)) + if (directory_exists(temp_instance)) { - header(_("removing existing temp installation")); - if (!rmtree(temp_install, true)) + header(_("removing existing temp instance")); + if (!rmtree(temp_instance, true)) { - fprintf(stderr, _("\n%s: could not remove temp installation \"%s\"\n"), - progname, temp_install); + fprintf(stderr, _("\n%s: could not remove temp instance \"%s\"\n"), + progname, temp_instance); exit(2); } } - header(_("creating temporary installation")); + header(_("creating temporary instance")); - /* make the temp install top directory */ - make_directory(temp_install); + /* make the temp instance top directory */ + make_directory(temp_instance); /* and a directory for log files */ - snprintf(buf, sizeof(buf), "%s/log", outputdir); + snprintf(buf, sizeof(buf), "%s/log", temp_instance); if (!directory_exists(buf)) make_directory(buf); - /* "make install" */ -#ifndef WIN32_ONLY_COMPILER - snprintf(buf, sizeof(buf), - "\"%s\" -C \"%s\" DESTDIR=\"%s/install\" install > \"%s/log/install.log\" 2>&1", - makeprog, top_builddir, temp_install, outputdir); -#else - snprintf(buf, sizeof(buf), - "perl \"%s/src/tools/msvc/install.pl\" \"%s/install\" >\"%s/log/install.log\" 2>&1", - top_builddir, temp_install, outputdir); -#endif - if (system(buf)) - { - fprintf(stderr, _("\n%s: installation failed\nExamine %s/log/install.log for the reason.\nCommand was: %s\n"), progname, outputdir, buf); - exit(2); - } - - for (sl = extra_install; sl != NULL; sl = sl->next) - { -#ifndef WIN32_ONLY_COMPILER - snprintf(buf, sizeof(buf), - "\"%s\" -C \"%s/%s\" DESTDIR=\"%s/install\" install >> \"%s/log/install.log\" 2>&1", - makeprog, top_builddir, sl->str, temp_install, outputdir); -#else - fprintf(stderr, _("\n%s: --extra-install option not supported on this platform\n"), progname); - exit(2); -#endif - - if (system(buf)) - { - fprintf(stderr, _("\n%s: installation failed\nExamine %s/log/install.log for the reason.\nCommand was: %s\n"), progname, outputdir, buf); - exit(2); - } - } - /* initdb */ header(_("initializing database system")); snprintf(buf, sizeof(buf), - "\"%s/initdb\" -D \"%s/data\" -L \"%s\" --noclean --nosync%s%s > \"%s/log/initdb.log\" 2>&1", - bindir, temp_install, datadir, + "\"%s%sinitdb\" -D \"%s/data\" --noclean --nosync%s%s > \"%s/log/initdb.log\" 2>&1", + bindir ? bindir : "", + bindir ? "/" : "", + temp_instance, debug ? " --debug" : "", nolocale ? " --no-locale" : "", - outputdir); + temp_instance); if (system(buf)) { fprintf(stderr, _("\n%s: initdb failed\nExamine %s/log/initdb.log for the reason.\nCommand was: %s\n"), progname, outputdir, buf); @@ -2464,7 +2339,7 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc * failures, don't set max_prepared_transactions any higher than * actually needed by the prepared_xacts regression test.) */ - snprintf(buf, sizeof(buf), "%s/data/postgresql.conf", temp_install); + snprintf(buf, sizeof(buf), "%s/data/postgresql.conf", temp_instance); pg_conf = fopen(buf, "a"); if (pg_conf == NULL) { @@ -2502,7 +2377,7 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc * Since we successfully used the same buffer for the much-longer * "initdb" command, this can't truncate. */ - snprintf(buf, sizeof(buf), "%s/data", temp_install); + snprintf(buf, sizeof(buf), "%s/data", temp_instance); config_sspi_auth(buf); #elif !defined(HAVE_UNIX_SOCKETS) #error Platform has no means to secure the test installation. @@ -2512,8 +2387,10 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc * Check if there is a postmaster running already. */ snprintf(buf2, sizeof(buf2), - "\"%s/psql\" -X postgres <%s 2>%s", - bindir, DEVNULL, DEVNULL); + "\"%s%spsql\" -X postgres <%s 2>%s", + bindir ? bindir : "", + bindir ? "/" : "", + DEVNULL, DEVNULL); for (i = 0; i < 16; i++) { @@ -2544,12 +2421,14 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc */ header(_("starting postmaster")); snprintf(buf, sizeof(buf), - "\"%s/postgres\" -D \"%s/data\" -F%s " + "\"%s%spostgres\" -D \"%s/data\" -F%s " "-c \"listen_addresses=%s\" -k \"%s\" " "> \"%s/log/postmaster.log\" 2>&1", - bindir, temp_install, debug ? " -d 5" : "", + bindir ? bindir : "", + bindir ? "/" : "", + temp_instance, debug ? " -d 5" : "", hostname ? hostname : "", sockdir ? sockdir : "", - outputdir); + temp_instance); postmaster_pid = spawn_process(buf); if (postmaster_pid == INVALID_PID) { @@ -2663,23 +2542,23 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc /* * Shut down temp installation's postmaster */ - if (temp_install) + if (temp_instance) { header(_("shutting down postmaster")); stop_postmaster(); } /* - * If there were no errors, remove the temp installation immediately to - * conserve disk space. (If there were errors, we leave the installation + * If there were no errors, remove the temp instance immediately to + * conserve disk space. (If there were errors, we leave the instance * in place for possible manual investigation.) */ - if (temp_install && fail_count == 0 && fail_ignore_count == 0) + if (temp_instance && fail_count == 0 && fail_ignore_count == 0) { - header(_("removing temporary installation")); - if (!rmtree(temp_install, true)) - fprintf(stderr, _("\n%s: could not remove temp installation \"%s\"\n"), - progname, temp_install); + header(_("removing temporary instance")); + if (!rmtree(temp_instance, true)) + fprintf(stderr, _("\n%s: could not remove temp instance \"%s\"\n"), + progname, temp_instance); } fclose(logfile); diff --git a/src/test/regress/pg_regress.h b/src/test/regress/pg_regress.h index a7af7a5..0fc9db7 100644 --- a/src/test/regress/pg_regress.h +++ b/src/test/regress/pg_regress.h @@ -43,12 +43,6 @@ extern char *inputdir; extern char *outputdir; extern char *launcher; -/* - * This should not be global but every module should be able to read command - * line parameters. - */ -extern char *psqldir; - extern const char *basic_diff_opts; extern const char *pretty_diff_opts; diff --git a/src/test/regress/pg_regress_main.c b/src/test/regress/pg_regress_main.c index a403965..860431b 100644 --- a/src/test/regress/pg_regress_main.c +++ b/src/test/regress/pg_regress_main.c @@ -65,8 +65,8 @@ psql_start_test(const char *testname, snprintf(psql_cmd + offset, sizeof(psql_cmd) - offset, "\"%s%spsql\" -X -a -q -d \"%s\" < \"%s\" > \"%s\" 2>&1", - psqldir ? psqldir : "", - psqldir ? "/" : "", + bindir ? bindir : "", + bindir ? "/" : "", dblist->str, infile, outfile);
^ permalink raw reply [nested|flat] 22+ messages in thread

* Re: improving speed of make check-world
  2014-08-15 05:45 improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
  2014-08-31 09:36 ` Re: improving speed of make check-world Fabien COELHO <coelho@cri.ensmp.fr>
  2015-02-15 02:01   ` Re: improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
@ 2015-02-15 02:09     ` Tom Lane <tgl@sss.pgh.pa.us>
  1 sibling, 0 replies; 22+ messages in thread

From: Tom Lane @ 2015-02-15 02:09 UTC (permalink / raw)
  To: Peter Eisentraut <peter_e@gmx.net>; +Cc: pgsql-hackers; Fabien COELHO <coelho@cri.ensmp.fr>

Peter Eisentraut <peter_e@gmx.net> writes:
> On 8/31/14 5:36 AM, Fabien COELHO wrote:
>> Running "make -j2 check-world" does not work because "initdb" is not
>> found by "pg_regress". but "make -j1 check-world" does work fine. It
>> seems that some dependencies might be missing and there is a race
>> condition between temporary install and running some checks?? Maybe it
>> is not expected to work anyway? See below suggestions to make it work.

> Here is an updated patch that fixes this problem.

Doesn't the Windows side of the house still depend on that functionality
you removed from pg_regress?  Perhaps that's not a big deal to fix, but
it seems like a commit-blocker from here.

			regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



^ permalink  raw  reply  [nested|flat] 22+ messages in thread

* Re: improving speed of make check-world
  2014-08-15 05:45 improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
  2014-08-31 09:36 ` Re: improving speed of make check-world Fabien COELHO <coelho@cri.ensmp.fr>
  2015-02-15 02:01   ` Re: improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
@ 2015-02-24 08:06     ` Michael Paquier <michael.paquier@gmail.com>
  2015-03-08 13:22       ` Re: improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
  1 sibling, 1 reply; 22+ messages in thread

From: Michael Paquier @ 2015-02-24 08:06 UTC (permalink / raw)
  To: Peter Eisentraut <peter_e@gmx.net>; +Cc: pgsql-hackers; Fabien COELHO <coelho@cri.ensmp.fr>

On Sun, Feb 15, 2015 at 11:01 AM, Peter Eisentraut wrote:
> Here is an updated patch.

Nice patch. This is going to save a lot of resources.

An update of vcregress.pl is necessary. This visibly just consists in
updating the options that have been renamed in pg_regress (don't mind
testing any code sent out).

-               {"top-builddir", required_argument, NULL, 11},
+               {"datadir", required_argument, NULL, 12},
In pg_regress.c datadir is a new option but it is used nowhere, so it
could be as well removed.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



^ permalink  raw  reply  [nested|flat] 22+ messages in thread

* Re: improving speed of make check-world
  2014-08-15 05:45 improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
  2014-08-31 09:36 ` Re: improving speed of make check-world Fabien COELHO <coelho@cri.ensmp.fr>
  2015-02-15 02:01   ` Re: improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
  2015-02-24 08:06     ` Re: improving speed of make check-world Michael Paquier <michael.paquier@gmail.com>
@ 2015-03-08 13:22       ` Peter Eisentraut <peter_e@gmx.net>
  2015-03-09 05:46         ` Re: improving speed of make check-world Michael Paquier <michael.paquier@gmail.com>
  0 siblings, 1 reply; 22+ messages in thread

From: Peter Eisentraut @ 2015-03-08 13:22 UTC (permalink / raw)
  To: Michael Paquier <michael.paquier@gmail.com>; +Cc: pgsql-hackers; Fabien COELHO <coelho@cri.ensmp.fr>

On 2/24/15 3:06 AM, Michael Paquier wrote:
> On Sun, Feb 15, 2015 at 11:01 AM, Peter Eisentraut wrote:
>> Here is an updated patch.
> 
> Nice patch. This is going to save a lot of resources.
> 
> An update of vcregress.pl is necessary. This visibly just consists in
> updating the options that have been renamed in pg_regress (don't mind
> testing any code sent out).

Well, that turns out to be more complicated than initially thought.
Apparently, the msvc has a bit of a different idea of what check and
installcheck do with respect to temporary installs.  For instance,
vcregress installcheck does not use psql from the installation but from
the build tree.  vcregress check uses psql from the build tree but other
binaries (initdb, pg_ctl) from the temporary installation.  It is hard
for me to straighten this out by just looking at the code.  Attached is
a patch that shows the idea, but I can't easily take it further than that.

> -               {"top-builddir", required_argument, NULL, 11},
> +               {"datadir", required_argument, NULL, 12},
> In pg_regress.c datadir is a new option but it is used nowhere, so it
> could be as well removed.

Yeah, that's an oversight that is easily corrected.

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Attachments:

regress-temp-instance-msvc.patch (application/x-patch, 2.9K) download diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl index bd3dd2c..1e09c74 100644 --- a/src/tools/msvc/vcregress.pl +++ b/src/tools/msvc/vcregress.pl @@ -94,7 +94,7 @@ sub installcheck my @args = ( "../../../$Config/pg_regress/pg_regress", "--dlpath=.", - "--psqldir=../../../$Config/psql", + "--bindir=../../../$Config/psql", "--schedule=${schedule}_schedule", "--encoding=SQL_ASCII", "--no-locale"); @@ -106,39 +106,56 @@ sub installcheck sub check { + my $cwd = getcwd(); + + system($0, 'install.pl', "$cwd/tmp_install"); + my $status = $? >> 8; + exit $status if $status; + + $ENV{PATH} = "$cwd/tmp_install/bin;$cwd/tmp_install/lib;$ENV{PATH}"; + my @args = ( "../../../$Config/pg_regress/pg_regress", "--dlpath=.", - "--psqldir=../../../$Config/psql", + "--bindir=", "--schedule=${schedule}_schedule", "--encoding=SQL_ASCII", "--no-locale", - "--temp-install=./tmp_check", - "--top-builddir=\"$topdir\""); + "--temp-instance=./tmp_check"); push(@args, $maxconn) if $maxconn; push(@args, $temp_config) if $temp_config; system(@args); - my $status = $? >> 8; + $status = $? >> 8; exit $status if $status; } sub ecpgcheck { chdir $startdir; + system("msbuild ecpg_regression.proj /p:config=$Config"); my $status = $? >> 8; exit $status if $status; + + my $cwd = getcwd(); + + system($0, 'install.pl', "$cwd/tmp_install"); + $status = $? >> 8; + exit $status if $status; + + $ENV{PATH} = "$cwd/tmp_install/bin;$cwd/tmp_install/lib;$ENV{PATH}"; + chdir "$topdir/src/interfaces/ecpg/test"; $schedule = "ecpg"; my @args = ( "../../../../$Config/pg_regress_ecpg/pg_regress_ecpg", - "--psqldir=../../../$Config/psql", + "--bindir=", "--dbname=regress1,connectdb", "--create-role=connectuser,connectdb", "--schedule=${schedule}_schedule", "--encoding=SQL_ASCII", "--no-locale", - "--temp-install=./tmp_chk", + "--temp-instance=./tmp_chk", "--top-builddir=\"$topdir\""); push(@args, $maxconn) if $maxconn; system(@args); @@ -153,7 +170,7 @@ sub isolationcheck "../../../$Config/pg_isolation_regress"); my @args = ( "../../../$Config/pg_isolation_regress/pg_isolation_regress", - "--psqldir=../../../$Config/psql", + "--bindir=../../../$Config/psql", "--inputdir=.", "--schedule=./isolation_schedule"); push(@args, $maxconn) if $maxconn; @@ -202,7 +219,7 @@ sub plcheck print "Checking $lang\n"; my @args = ( "../../../$Config/pg_regress/pg_regress", - "--psqldir=../../../$Config/psql", + "--bindir=../../../$Config/psql", "--dbname=pl_regression", @lang_args, @tests); system(@args); my $status = $? >> 8; @@ -237,7 +254,7 @@ sub contribcheck my @opts = fetchRegressOpts(); my @args = ( "../../$Config/pg_regress/pg_regress", - "--psqldir=../../$Config/psql", + "--bindir=../../$Config/psql", "--dbname=contrib_regression", @opts, @tests); system(@args); my $status = $? >> 8;
^ permalink raw reply [nested|flat] 22+ messages in thread

* Re: improving speed of make check-world
  2014-08-15 05:45 improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
  2014-08-31 09:36 ` Re: improving speed of make check-world Fabien COELHO <coelho@cri.ensmp.fr>
  2015-02-15 02:01   ` Re: improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
  2015-02-24 08:06     ` Re: improving speed of make check-world Michael Paquier <michael.paquier@gmail.com>
  2015-03-08 13:22       ` Re: improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
@ 2015-03-09 05:46         ` Michael Paquier <michael.paquier@gmail.com>
  2015-03-09 06:51           ` Re: improving speed of make check-world Michael Paquier <michael.paquier@gmail.com>
  0 siblings, 1 reply; 22+ messages in thread

From: Michael Paquier @ 2015-03-09 05:46 UTC (permalink / raw)
  To: Peter Eisentraut <peter_e@gmx.net>; +Cc: pgsql-hackers; Fabien COELHO <coelho@cri.ensmp.fr>

On Sun, Mar 8, 2015 at 10:22 PM, Peter Eisentraut <peter_e@gmx.net> wrote:
> On 2/24/15 3:06 AM, Michael Paquier wrote:
>> On Sun, Feb 15, 2015 at 11:01 AM, Peter Eisentraut wrote:
>>> Here is an updated patch.
>>
>> Nice patch. This is going to save a lot of resources.
>>
>> An update of vcregress.pl is necessary. This visibly just consists in
>> updating the options that have been renamed in pg_regress (don't mind
>> testing any code sent out).
>
> Well, that turns out to be more complicated than initially thought.
> Apparently, the msvc has a bit of a different idea of what check and
> installcheck do with respect to temporary installs.  For instance,
> vcregress installcheck does not use psql from the installation but from
> the build tree.  vcregress check uses psql from the build tree but other
> binaries (initdb, pg_ctl) from the temporary installation.  It is hard
> for me to straighten this out by just looking at the code.  Attached is
> a patch that shows the idea, but I can't easily take it further than that.

Urg. Yes for installcheck I guess that we cannot do much but simply
use the psql from the tree, but on the contrary for all the other
targets we can use the temporary installation as $topdir/tmp_install.

Regarding the patch you sent, IMO it is not a good idea to kick
install with system() as this can fail as an unrecognized command
runnable. And the command that should be used is not "vcregress
install $path" but simply "vcregress install". Hence I think that
calling simply Install() makes more sense. Also, I think that it would
be better to not enforce PATH before kicking the commands.

Speaking of which, attached is a patch rewritten in-line with those
comments, simplifying a bit the whole at the same time. Note this
patch changes ecpgcheck as it should be patched, but as ecpgcheck test
is broken even on HEAD, I'll raise a separate thread about that with a
proper fix (for example bowerbird skips this test).
On my side, with this patch, installcheck, check, plcheck,
upgradecheck work properly and all of them use the common
installation. It would be more adapted to add checks on the existence
of $tmp_installdir/bin though in InstallTemp instead of kicking an
installation all the time. But that's simple enough to change.
Regards,
-- 
Michael

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Attachments:

20150308_regress-temp-instance-msvc_v2.patch (text/x-diff, 5.1K) download From 2b4551a7bc411fc03703f2641b655faf76f2c679 Mon Sep 17 00:00:00 2001 From: Michael Paquier <michael@otacoo.com> Date: Sun, 8 Mar 2015 22:39:10 -0700 Subject: [PATCH] Make vcregress use common installation path for all tests installcheck is let as-is as it depends on psql being present in PATH. --- src/tools/msvc/vcregress.pl | 66 +++++++++++++++++++++++++++++---------------- 1 file changed, 43 insertions(+), 23 deletions(-) diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl index bd3dd2c..aa7fbaa 100644 --- a/src/tools/msvc/vcregress.pl +++ b/src/tools/msvc/vcregress.pl @@ -16,6 +16,7 @@ my $startdir = getcwd(); chdir "../../.." if (-d "../../../src/tools/msvc"); my $topdir = getcwd(); +my $tmp_installdir = "$topdir/tmp_install"; require 'src/tools/msvc/config_default.pl'; require 'src/tools/msvc/config.pl' if (-f 'src/tools/msvc/config.pl'); @@ -94,7 +95,7 @@ sub installcheck my @args = ( "../../../$Config/pg_regress/pg_regress", "--dlpath=.", - "--psqldir=../../../$Config/psql", + "--bindir=../../../$Config/psql", "--schedule=${schedule}_schedule", "--encoding=SQL_ASCII", "--no-locale"); @@ -106,15 +107,19 @@ sub installcheck sub check { + chdir $startdir; + + InstallTemp(); + chdir "${topdir}/src/test/regress"; + my @args = ( - "../../../$Config/pg_regress/pg_regress", + "${tmp_installdir}/bin/pg_regress", "--dlpath=.", - "--psqldir=../../../$Config/psql", + "--bindir=${tmp_installdir}/bin", "--schedule=${schedule}_schedule", "--encoding=SQL_ASCII", "--no-locale", - "--temp-install=./tmp_check", - "--top-builddir=\"$topdir\""); + "--temp-instance=./tmp_check"); push(@args, $maxconn) if $maxconn; push(@args, $temp_config) if $temp_config; system(@args); @@ -125,20 +130,24 @@ sub check sub ecpgcheck { chdir $startdir; + system("msbuild ecpg_regression.proj /p:config=$Config"); my $status = $? >> 8; exit $status if $status; + + InstallTemp(); chdir "$topdir/src/interfaces/ecpg/test"; + $schedule = "ecpg"; my @args = ( - "../../../../$Config/pg_regress_ecpg/pg_regress_ecpg", - "--psqldir=../../../$Config/psql", + "${tmp_installdir}/bin/pg_regress_ecpg", + "--bindir=${tmp_installdir}/bin", "--dbname=regress1,connectdb", "--create-role=connectuser,connectdb", "--schedule=${schedule}_schedule", "--encoding=SQL_ASCII", "--no-locale", - "--temp-install=./tmp_chk", + "--temp-instance=./tmp_chk", "--top-builddir=\"$topdir\""); push(@args, $maxconn) if $maxconn; system(@args); @@ -148,12 +157,14 @@ sub ecpgcheck sub isolationcheck { - chdir "../isolation"; - copy("../../../$Config/isolationtester/isolationtester.exe", - "../../../$Config/pg_isolation_regress"); + chdir $startdir; + + InstallTemp(); + chdir "${topdir}/src/test/isolation"; + my @args = ( - "../../../$Config/pg_isolation_regress/pg_isolation_regress", - "--psqldir=../../../$Config/psql", + "${tmp_installdir}/bin/pg_isolation_regress", + "--bindir=${tmp_installdir}/bin", "--inputdir=.", "--schedule=./isolation_schedule"); push(@args, $maxconn) if $maxconn; @@ -164,7 +175,10 @@ sub isolationcheck sub plcheck { - chdir "../../pl"; + chdir $startdir; + + InstallTemp(); + chdir "${topdir}/src/pl"; foreach my $pl (glob("*")) { @@ -201,8 +215,8 @@ sub plcheck "============================================================\n"; print "Checking $lang\n"; my @args = ( - "../../../$Config/pg_regress/pg_regress", - "--psqldir=../../../$Config/psql", + "${tmp_installdir}/bin/pg_regress", + "--bindir=${tmp_installdir}/bin/psql", "--dbname=pl_regression", @lang_args, @tests); system(@args); my $status = $? >> 8; @@ -236,8 +250,8 @@ sub contribcheck my @tests = fetchTests(); my @opts = fetchRegressOpts(); my @args = ( - "../../$Config/pg_regress/pg_regress", - "--psqldir=../../$Config/psql", + "${tmp_installdir}/bin/pg_regress", + "--bindir=${tmp_installdir}/bin", "--dbname=contrib_regression", @opts, @tests); system(@args); my $status = $? >> 8; @@ -251,8 +265,8 @@ sub contribcheck sub standard_initdb { return ( - system('initdb', '-N') == 0 and system( - "$topdir/$Config/pg_regress/pg_regress", '--config-auth', + system("${tmp_installdir}/bin/initdb", '-N') == 0 and system( + "${tmp_installdir}/bin/pg_regress", '--config-auth', $ENV{PGDATA}) == 0); } @@ -272,13 +286,13 @@ sub upgradecheck my $tmp_root = "$topdir/contrib/pg_upgrade/tmp_check"; (mkdir $tmp_root || die $!) unless -d $tmp_root; my $tmp_install = "$tmp_root/install"; - print "Setting up temp install\n\n"; - Install($tmp_install, "all", $config); + + InstallTemp(); # Install does a chdir, so change back after that chdir $cwd; my ($bindir, $libdir, $oldsrc, $newsrc) = - ("$tmp_install/bin", "$tmp_install/lib", $topdir, $topdir); + ("$tmp_installdir/bin", "$tmp_installdir/lib", $topdir, $topdir); $ENV{PATH} = "$bindir;$ENV{PATH}"; my $data = "$tmp_root/data"; $ENV{PGDATA} = "$data.old"; @@ -413,6 +427,12 @@ sub GetTests return ""; } +sub InstallTemp +{ + print "Setting up temp install\n\n"; + Install("$tmp_installdir", "all", $config); +} + sub usage { print STDERR -- 1.9.2.msysgit.0
^ permalink raw reply [nested|flat] 22+ messages in thread

* Re: improving speed of make check-world
  2014-08-15 05:45 improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
  2014-08-31 09:36 ` Re: improving speed of make check-world Fabien COELHO <coelho@cri.ensmp.fr>
  2015-02-15 02:01   ` Re: improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
  2015-02-24 08:06     ` Re: improving speed of make check-world Michael Paquier <michael.paquier@gmail.com>
  2015-03-08 13:22       ` Re: improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
  2015-03-09 05:46         ` Re: improving speed of make check-world Michael Paquier <michael.paquier@gmail.com>
@ 2015-03-09 06:51           ` Michael Paquier <michael.paquier@gmail.com>
  2015-04-10 19:35             ` Re: improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
  0 siblings, 1 reply; 22+ messages in thread

From: Michael Paquier @ 2015-03-09 06:51 UTC (permalink / raw)
  To: Peter Eisentraut <peter_e@gmx.net>; +Cc: pgsql-hackers; Fabien COELHO <coelho@cri.ensmp.fr>

On Sun, Mar 8, 2015 at 10:46 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> Speaking of which, attached is a patch rewritten in-line with those
> comments, simplifying a bit the whole at the same time. Note this
> patch changes ecpgcheck as it should be patched, but as ecpgcheck test
> is broken even on HEAD, I'll raise a separate thread about that with a
> proper fix (for example bowerbird skips this test).

Correction: HEAD is fine, but previous patch was broken because it
tried to use --top-builddir. Also for ecpgcheck it looks that tricking
PATH is needed to avoid dll loading errors related to libecpg.dll and
libecpg_compat.dll. Updated patch is attached.

Bonus track: pg_regress.c is missing the description of --bindir in help().
-- 
Michael

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Attachments:

20150308_regress-temp-instance-msvc_v3.patch (text/x-diff, 5.2K) download From 5b729058335cf4a77e22de25bce4c90fa6d686c8 Mon Sep 17 00:00:00 2001 From: Michael Paquier <michael@otacoo.com> Date: Sun, 8 Mar 2015 22:39:10 -0700 Subject: [PATCH] Make vcregress use common installation path for all tests installcheck is let as-is as it depends on psql being present in PATH. --- src/tools/msvc/vcregress.pl | 66 ++++++++++++++++++++++++++++----------------- 1 file changed, 42 insertions(+), 24 deletions(-) diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl index bd3dd2c..c7ce5aa 100644 --- a/src/tools/msvc/vcregress.pl +++ b/src/tools/msvc/vcregress.pl @@ -16,6 +16,7 @@ my $startdir = getcwd(); chdir "../../.." if (-d "../../../src/tools/msvc"); my $topdir = getcwd(); +my $tmp_installdir = "$topdir/tmp_install"; require 'src/tools/msvc/config_default.pl'; require 'src/tools/msvc/config.pl' if (-f 'src/tools/msvc/config.pl'); @@ -94,7 +95,7 @@ sub installcheck my @args = ( "../../../$Config/pg_regress/pg_regress", "--dlpath=.", - "--psqldir=../../../$Config/psql", + "--bindir=../../../$Config/psql", "--schedule=${schedule}_schedule", "--encoding=SQL_ASCII", "--no-locale"); @@ -106,15 +107,19 @@ sub installcheck sub check { + chdir $startdir; + + InstallTemp(); + chdir "${topdir}/src/test/regress"; + my @args = ( - "../../../$Config/pg_regress/pg_regress", + "${tmp_installdir}/bin/pg_regress", "--dlpath=.", - "--psqldir=../../../$Config/psql", + "--bindir=${tmp_installdir}/bin", "--schedule=${schedule}_schedule", "--encoding=SQL_ASCII", "--no-locale", - "--temp-install=./tmp_check", - "--top-builddir=\"$topdir\""); + "--temp-instance=./tmp_check"); push(@args, $maxconn) if $maxconn; push(@args, $temp_config) if $temp_config; system(@args); @@ -128,18 +133,20 @@ sub ecpgcheck system("msbuild ecpg_regression.proj /p:config=$Config"); my $status = $? >> 8; exit $status if $status; + InstallTemp(); chdir "$topdir/src/interfaces/ecpg/test"; + + $ENV{PATH} = "${tmp_installdir}/bin;${tmp_installdir}/lib;$ENV{PATH}"; $schedule = "ecpg"; my @args = ( - "../../../../$Config/pg_regress_ecpg/pg_regress_ecpg", - "--psqldir=../../../$Config/psql", + "${tmp_installdir}/bin/pg_regress_ecpg", + "--bindir=", "--dbname=regress1,connectdb", "--create-role=connectuser,connectdb", "--schedule=${schedule}_schedule", "--encoding=SQL_ASCII", "--no-locale", - "--temp-install=./tmp_chk", - "--top-builddir=\"$topdir\""); + "--temp-instance=./tmp_chk"); push(@args, $maxconn) if $maxconn; system(@args); $status = $? >> 8; @@ -148,12 +155,14 @@ sub ecpgcheck sub isolationcheck { - chdir "../isolation"; - copy("../../../$Config/isolationtester/isolationtester.exe", - "../../../$Config/pg_isolation_regress"); + chdir $startdir; + + InstallTemp(); + chdir "${topdir}/src/test/isolation"; + my @args = ( - "../../../$Config/pg_isolation_regress/pg_isolation_regress", - "--psqldir=../../../$Config/psql", + "${tmp_installdir}/bin/pg_isolation_regress", + "--bindir=${tmp_installdir}/bin", "--inputdir=.", "--schedule=./isolation_schedule"); push(@args, $maxconn) if $maxconn; @@ -164,7 +173,10 @@ sub isolationcheck sub plcheck { - chdir "../../pl"; + chdir $startdir; + + InstallTemp(); + chdir "${topdir}/src/pl"; foreach my $pl (glob("*")) { @@ -201,8 +213,8 @@ sub plcheck "============================================================\n"; print "Checking $lang\n"; my @args = ( - "../../../$Config/pg_regress/pg_regress", - "--psqldir=../../../$Config/psql", + "${tmp_installdir}/bin/pg_regress", + "--bindir=${tmp_installdir}/bin/psql", "--dbname=pl_regression", @lang_args, @tests); system(@args); my $status = $? >> 8; @@ -236,8 +248,8 @@ sub contribcheck my @tests = fetchTests(); my @opts = fetchRegressOpts(); my @args = ( - "../../$Config/pg_regress/pg_regress", - "--psqldir=../../$Config/psql", + "${tmp_installdir}/bin/pg_regress", + "--bindir=${tmp_installdir}/bin", "--dbname=contrib_regression", @opts, @tests); system(@args); my $status = $? >> 8; @@ -251,8 +263,8 @@ sub contribcheck sub standard_initdb { return ( - system('initdb', '-N') == 0 and system( - "$topdir/$Config/pg_regress/pg_regress", '--config-auth', + system("${tmp_installdir}/bin/initdb", '-N') == 0 and system( + "${tmp_installdir}/bin/pg_regress", '--config-auth', $ENV{PGDATA}) == 0); } @@ -272,13 +284,13 @@ sub upgradecheck my $tmp_root = "$topdir/contrib/pg_upgrade/tmp_check"; (mkdir $tmp_root || die $!) unless -d $tmp_root; my $tmp_install = "$tmp_root/install"; - print "Setting up temp install\n\n"; - Install($tmp_install, "all", $config); + + InstallTemp(); # Install does a chdir, so change back after that chdir $cwd; my ($bindir, $libdir, $oldsrc, $newsrc) = - ("$tmp_install/bin", "$tmp_install/lib", $topdir, $topdir); + ("$tmp_installdir/bin", "$tmp_installdir/lib", $topdir, $topdir); $ENV{PATH} = "$bindir;$ENV{PATH}"; my $data = "$tmp_root/data"; $ENV{PGDATA} = "$data.old"; @@ -413,6 +425,12 @@ sub GetTests return ""; } +sub InstallTemp +{ + print "Setting up temp install\n\n"; + Install("$tmp_installdir", "all", $config); +} + sub usage { print STDERR -- 1.9.2.msysgit.0
^ permalink raw reply [nested|flat] 22+ messages in thread

* Re: improving speed of make check-world
  2014-08-15 05:45 improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
  2014-08-31 09:36 ` Re: improving speed of make check-world Fabien COELHO <coelho@cri.ensmp.fr>
  2015-02-15 02:01   ` Re: improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
  2015-02-24 08:06     ` Re: improving speed of make check-world Michael Paquier <michael.paquier@gmail.com>
  2015-03-08 13:22       ` Re: improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
  2015-03-09 05:46         ` Re: improving speed of make check-world Michael Paquier <michael.paquier@gmail.com>
  2015-03-09 06:51           ` Re: improving speed of make check-world Michael Paquier <michael.paquier@gmail.com>
@ 2015-04-10 19:35             ` Peter Eisentraut <peter_e@gmx.net>
  2015-04-11 11:48               ` Re: improving speed of make check-world Michael Paquier <michael.paquier@gmail.com>
  0 siblings, 1 reply; 22+ messages in thread

From: Peter Eisentraut @ 2015-04-10 19:35 UTC (permalink / raw)
  To: Michael Paquier <michael.paquier@gmail.com>; +Cc: pgsql-hackers; Fabien COELHO <coelho@cri.ensmp.fr>

On 3/9/15 2:51 AM, Michael Paquier wrote:
> On Sun, Mar 8, 2015 at 10:46 PM, Michael Paquier
> <michael.paquier@gmail.com> wrote:
>> Speaking of which, attached is a patch rewritten in-line with those
>> comments, simplifying a bit the whole at the same time. Note this
>> patch changes ecpgcheck as it should be patched, but as ecpgcheck test
>> is broken even on HEAD, I'll raise a separate thread about that with a
>> proper fix (for example bowerbird skips this test).
> 
> Correction: HEAD is fine, but previous patch was broken because it
> tried to use --top-builddir. Also for ecpgcheck it looks that tricking
> PATH is needed to avoid dll loading errors related to libecpg.dll and
> libecpg_compat.dll. Updated patch is attached.

To clarify: Are you of the opinion that your patch (on top of my base
patch) is sufficient to make all of this work on Windows?



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



^ permalink  raw  reply  [nested|flat] 22+ messages in thread

* Re: improving speed of make check-world
  2014-08-15 05:45 improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
  2014-08-31 09:36 ` Re: improving speed of make check-world Fabien COELHO <coelho@cri.ensmp.fr>
  2015-02-15 02:01   ` Re: improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
  2015-02-24 08:06     ` Re: improving speed of make check-world Michael Paquier <michael.paquier@gmail.com>
  2015-03-08 13:22       ` Re: improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
  2015-03-09 05:46         ` Re: improving speed of make check-world Michael Paquier <michael.paquier@gmail.com>
  2015-03-09 06:51           ` Re: improving speed of make check-world Michael Paquier <michael.paquier@gmail.com>
  2015-04-10 19:35             ` Re: improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
@ 2015-04-11 11:48               ` Michael Paquier <michael.paquier@gmail.com>
  2015-04-11 22:36                 ` Re: improving speed of make check-world Michael Paquier <michael.paquier@gmail.com>
  0 siblings, 1 reply; 22+ messages in thread

From: Michael Paquier @ 2015-04-11 11:48 UTC (permalink / raw)
  To: Peter Eisentraut <peter_e@gmx.net>; +Cc: pgsql-hackers; Fabien COELHO <coelho@cri.ensmp.fr>

On Sat, Apr 11, 2015 at 4:35 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
> On 3/9/15 2:51 AM, Michael Paquier wrote:
>> On Sun, Mar 8, 2015 at 10:46 PM, Michael Paquier
>> <michael.paquier@gmail.com> wrote:
>>> Speaking of which, attached is a patch rewritten in-line with those
>>> comments, simplifying a bit the whole at the same time. Note this
>>> patch changes ecpgcheck as it should be patched, but as ecpgcheck test
>>> is broken even on HEAD, I'll raise a separate thread about that with a
>>> proper fix (for example bowerbird skips this test).
>>
>> Correction: HEAD is fine, but previous patch was broken because it
>> tried to use --top-builddir. Also for ecpgcheck it looks that tricking
>> PATH is needed to avoid dll loading errors related to libecpg.dll and
>> libecpg_compat.dll. Updated patch is attached.
>
> To clarify: Are you of the opinion that your patch (on top of my base
> patch) is sufficient to make all of this work on Windows?

Things will work. I just tested again each test target with the patch
attached while wrapping again my mind on this stuff (actually found
one bug in plcheck where --bindir was not correct, and removed
tmp_install in upgradecheck). Now, what this patch does is enforcing
the temporary install for each *check target of vcregress.pl. This has
the disadvantage of making the benefits of MAKELEVEL=0 seen for build
methods using the Makefiles go away for MSVC, but it minimizes the use
of ENV variables which is a good thing to me by setting --bindir to a
value as much as possible (ecpgcheck being an exception), and it makes
the set of tests more consistent with each other in the way they run.
Another thing to know that this patch changes is that vcregress does
not rely anymore on the contents of Release/$projects, aka the build
structure of MS stuff, but just fetches binaries from the temporary
installation. That's more consistent with Makefile builds, now perhaps
some people have a different opinion.

What we could add later on a new target, allcheck, that would kick all
the tests at once and installs the temporary installation just once.
It would be straight-forward to write a patch, but this is a separate
discussion as installcheck would need to be skipped. isolationcheck
should as well be changed to be more self-dependent as even of HEAD it
needs to have an instance of PG running to work.
Regards,
-- 
Michael

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Attachments:

20150411_regress-temp-instance-msvc_v3.patch (text/x-patch, 4.7K) download diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl index bd3dd2c..d9ff7ec 100644 --- a/src/tools/msvc/vcregress.pl +++ b/src/tools/msvc/vcregress.pl @@ -16,6 +16,7 @@ my $startdir = getcwd(); chdir "../../.." if (-d "../../../src/tools/msvc"); my $topdir = getcwd(); +my $tmp_installdir = "$topdir/tmp_install"; require 'src/tools/msvc/config_default.pl'; require 'src/tools/msvc/config.pl' if (-f 'src/tools/msvc/config.pl'); @@ -94,7 +95,7 @@ sub installcheck my @args = ( "../../../$Config/pg_regress/pg_regress", "--dlpath=.", - "--psqldir=../../../$Config/psql", + "--bindir=../../../$Config/psql", "--schedule=${schedule}_schedule", "--encoding=SQL_ASCII", "--no-locale"); @@ -106,15 +107,19 @@ sub installcheck sub check { + chdir $startdir; + + InstallTemp(); + chdir "${topdir}/src/test/regress"; + my @args = ( - "../../../$Config/pg_regress/pg_regress", + "${tmp_installdir}/bin/pg_regress", "--dlpath=.", - "--psqldir=../../../$Config/psql", + "--bindir=${tmp_installdir}/bin", "--schedule=${schedule}_schedule", "--encoding=SQL_ASCII", "--no-locale", - "--temp-install=./tmp_check", - "--top-builddir=\"$topdir\""); + "--temp-instance=./tmp_check"); push(@args, $maxconn) if $maxconn; push(@args, $temp_config) if $temp_config; system(@args); @@ -128,18 +133,20 @@ sub ecpgcheck system("msbuild ecpg_regression.proj /p:config=$Config"); my $status = $? >> 8; exit $status if $status; + InstallTemp(); chdir "$topdir/src/interfaces/ecpg/test"; + + $ENV{PATH} = "${tmp_installdir}/bin;${tmp_installdir}/lib;$ENV{PATH}"; $schedule = "ecpg"; my @args = ( - "../../../../$Config/pg_regress_ecpg/pg_regress_ecpg", - "--psqldir=../../../$Config/psql", + "${tmp_installdir}/bin/pg_regress_ecpg", + "--bindir=", "--dbname=regress1,connectdb", "--create-role=connectuser,connectdb", "--schedule=${schedule}_schedule", "--encoding=SQL_ASCII", "--no-locale", - "--temp-install=./tmp_chk", - "--top-builddir=\"$topdir\""); + "--temp-instance=./tmp_chk"); push(@args, $maxconn) if $maxconn; system(@args); $status = $? >> 8; @@ -148,12 +155,14 @@ sub ecpgcheck sub isolationcheck { - chdir "../isolation"; - copy("../../../$Config/isolationtester/isolationtester.exe", - "../../../$Config/pg_isolation_regress"); + chdir $startdir; + + InstallTemp(); + chdir "${topdir}/src/test/isolation"; + my @args = ( - "../../../$Config/pg_isolation_regress/pg_isolation_regress", - "--psqldir=../../../$Config/psql", + "${tmp_installdir}/bin/pg_isolation_regress", + "--bindir=${tmp_installdir}/bin", "--inputdir=.", "--schedule=./isolation_schedule"); push(@args, $maxconn) if $maxconn; @@ -164,7 +173,10 @@ sub isolationcheck sub plcheck { - chdir "../../pl"; + chdir $startdir; + + InstallTemp(); + chdir "${topdir}/src/pl"; foreach my $pl (glob("*")) { @@ -201,8 +213,8 @@ sub plcheck "============================================================\n"; print "Checking $lang\n"; my @args = ( - "../../../$Config/pg_regress/pg_regress", - "--psqldir=../../../$Config/psql", + "${tmp_installdir}/bin/pg_regress", + "--bindir=${tmp_installdir}/bin", "--dbname=pl_regression", @lang_args, @tests); system(@args); my $status = $? >> 8; @@ -236,8 +248,8 @@ sub contribcheck my @tests = fetchTests(); my @opts = fetchRegressOpts(); my @args = ( - "../../$Config/pg_regress/pg_regress", - "--psqldir=../../$Config/psql", + "${tmp_installdir}/bin/pg_regress", + "--bindir=${tmp_installdir}/bin", "--dbname=contrib_regression", @opts, @tests); system(@args); my $status = $? >> 8; @@ -251,8 +263,8 @@ sub contribcheck sub standard_initdb { return ( - system('initdb', '-N') == 0 and system( - "$topdir/$Config/pg_regress/pg_regress", '--config-auth', + system("${tmp_installdir}/bin/initdb", '-N') == 0 and system( + "${tmp_installdir}/bin/pg_regress", '--config-auth', $ENV{PGDATA}) == 0); } @@ -271,14 +283,13 @@ sub upgradecheck $ENV{PGPORT} ||= 50432; my $tmp_root = "$topdir/contrib/pg_upgrade/tmp_check"; (mkdir $tmp_root || die $!) unless -d $tmp_root; - my $tmp_install = "$tmp_root/install"; - print "Setting up temp install\n\n"; - Install($tmp_install, "all", $config); + + InstallTemp(); # Install does a chdir, so change back after that chdir $cwd; my ($bindir, $libdir, $oldsrc, $newsrc) = - ("$tmp_install/bin", "$tmp_install/lib", $topdir, $topdir); + ("$tmp_installdir/bin", "$tmp_installdir/lib", $topdir, $topdir); $ENV{PATH} = "$bindir;$ENV{PATH}"; my $data = "$tmp_root/data"; $ENV{PGDATA} = "$data.old"; @@ -413,6 +424,12 @@ sub GetTests return ""; } +sub InstallTemp +{ + print "Setting up temp install\n\n"; + Install("$tmp_installdir", "all", $config); +} + sub usage { print STDERR
^ permalink raw reply [nested|flat] 22+ messages in thread

* Re: improving speed of make check-world
  2014-08-15 05:45 improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
  2014-08-31 09:36 ` Re: improving speed of make check-world Fabien COELHO <coelho@cri.ensmp.fr>
  2015-02-15 02:01   ` Re: improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
  2015-02-24 08:06     ` Re: improving speed of make check-world Michael Paquier <michael.paquier@gmail.com>
  2015-03-08 13:22       ` Re: improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
  2015-03-09 05:46         ` Re: improving speed of make check-world Michael Paquier <michael.paquier@gmail.com>
  2015-03-09 06:51           ` Re: improving speed of make check-world Michael Paquier <michael.paquier@gmail.com>
  2015-04-10 19:35             ` Re: improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
  2015-04-11 11:48               ` Re: improving speed of make check-world Michael Paquier <michael.paquier@gmail.com>
@ 2015-04-11 22:36                 ` Michael Paquier <michael.paquier@gmail.com>
  0 siblings, 0 replies; 22+ messages in thread

From: Michael Paquier @ 2015-04-11 22:36 UTC (permalink / raw)
  To: Peter Eisentraut <peter_e@gmx.net>; +Cc: pgsql-hackers; Fabien COELHO <coelho@cri.ensmp.fr>

On Sat, Apr 11, 2015 at 8:48 PM, Michael Paquier wrote:
> Now, what this patch does is enforcing
> the temporary install for each *check target of vcregress.pl. This has
> the disadvantage of making the benefits of MAKELEVEL=0 seen for build
> methods using the Makefiles go away for MSVC

A trick that we could use here is to store a timestamp when running up
to completion "build" and the temporary installation in vcregress, and
skip the temporary installation if timestamp of vcregress' temporary
installation is newer than the one of "build".
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



^ permalink  raw  reply  [nested|flat] 22+ messages in thread

* Re: improving speed of make check-world
  2014-08-15 05:45 improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
@ 2015-04-23 17:22 ` Jeff Janes <jeff.janes@gmail.com>
  2015-04-24 22:59   ` Re: improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
  2 siblings, 1 reply; 22+ messages in thread

From: Jeff Janes @ 2015-04-23 17:22 UTC (permalink / raw)
  To: Peter Eisentraut <peter_e@gmx.net>; +Cc: pgsql-hackers

On Thu, Aug 14, 2014 at 10:45 PM, Peter Eisentraut <peter_e@gmx.net> wrote:

> make check-world creates a temporary installation in every subdirectory
> it runs a test in, which is stupid: it's very slow and uses a lot of
> disk space.  It's enough to do this once per run.  That is the essence
> of what I have implemented.  It cuts the time for make check-world in
> half or less, and it saves gigabytes of disk space.
>

Something about this commit (dcae5faccab64776376d354d) broke "make check"
in parallel conditions when started from a clean directory.  It fails with
a different error each time, one example:

make -j4 check > /dev/null

In file included from gram.y:14515:
scan.c: In function 'yy_try_NUL_trans':
scan.c:10307: warning: unused variable 'yyg'
/usr/bin/ld: tab-complete.o: No such file: No such file or directory
collect2: ld returned 1 exit status
make[3]: *** [psql] Error 1
make[2]: *** [all-psql-recurse] Error 2
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [all-bin-recurse] Error 2
make: *** [all-src-recurse] Error 2
make: *** Waiting for unfinished jobs....

If I rerun it without cleaning the tree, is usually passes the second
time.  Or I can just separate the make and the check like "make -j4 >
/dev/null && make check > /dev/null" but I've grown accustomed to being
able to combine them since this problem was first fixed a couple years ago
(in a commit I can't seem to find)

I have:
GNU Make 4.0
Built for x86_64-unknown-linux-gnu

I was using ccache, but I still get the problem without using it.

Cheers,

Jeff

^ permalink  raw  reply  [nested|flat] 22+ messages in thread

* Re: improving speed of make check-world
  2014-08-15 05:45 improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
  2015-04-23 17:22 ` Re: improving speed of make check-world Jeff Janes <jeff.janes@gmail.com>
@ 2015-04-24 22:59   ` Peter Eisentraut <peter_e@gmx.net>
  2015-04-25 14:23     ` Re: improving speed of make check-world Michael Paquier <michael.paquier@gmail.com>
  0 siblings, 1 reply; 22+ messages in thread

From: Peter Eisentraut @ 2015-04-24 22:59 UTC (permalink / raw)
  To: Jeff Janes <jeff.janes@gmail.com>; +Cc: pgsql-hackers

On 4/23/15 1:22 PM, Jeff Janes wrote:
> Something about this commit (dcae5faccab64776376d354d) broke "make
> check" in parallel conditions when started from a clean directory.  It
> fails with a different error each time, one example:
> 
> make -j4 check > /dev/null
> 
> In file included from gram.y:14515:
> scan.c: In function 'yy_try_NUL_trans':
> scan.c:10307: warning: unused variable 'yyg'
> /usr/bin/ld: tab-complete.o: No such file: No such file or directory
> collect2: ld returned 1 exit status
> make[3]: *** [psql] Error 1
> make[2]: *** [all-psql-recurse] Error 2
> make[2]: *** Waiting for unfinished jobs....
> make[1]: *** [all-bin-recurse] Error 2
> make: *** [all-src-recurse] Error 2
> make: *** Waiting for unfinished jobs....

I think the problem is that "check" depends on "all", but now also
depends on temp-install, which in turn runs install and all.  With a
sufficient amount of parallelism, you end up running two "all"s on top
of each other.

It seems this can be fixed by removing the check: all dependency.  Try
removing that in the top-level GNUmakefile.in and see if the problem
goes away.  For completeness, we should then also remove it in the other
makefiles.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



^ permalink  raw  reply  [nested|flat] 22+ messages in thread

* Re: improving speed of make check-world
  2014-08-15 05:45 improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
  2015-04-23 17:22 ` Re: improving speed of make check-world Jeff Janes <jeff.janes@gmail.com>
  2015-04-24 22:59   ` Re: improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
@ 2015-04-25 14:23     ` Michael Paquier <michael.paquier@gmail.com>
  2015-04-27 16:46       ` Re: improving speed of make check-world Jeff Janes <jeff.janes@gmail.com>
  0 siblings, 1 reply; 22+ messages in thread

From: Michael Paquier @ 2015-04-25 14:23 UTC (permalink / raw)
  To: Peter Eisentraut <peter_e@gmx.net>; +Cc: Jeff Janes <jeff.janes@gmail.com>; pgsql-hackers

On Sat, Apr 25, 2015 at 7:59 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
> On 4/23/15 1:22 PM, Jeff Janes wrote:
>> Something about this commit (dcae5faccab64776376d354d) broke "make
>> check" in parallel conditions when started from a clean directory.  It
>> fails with a different error each time, one example:
>>
>> make -j4 check > /dev/null
>>
>> In file included from gram.y:14515:
>> scan.c: In function 'yy_try_NUL_trans':
>> scan.c:10307: warning: unused variable 'yyg'
>> /usr/bin/ld: tab-complete.o: No such file: No such file or directory
>> collect2: ld returned 1 exit status
>> make[3]: *** [psql] Error 1
>> make[2]: *** [all-psql-recurse] Error 2
>> make[2]: *** Waiting for unfinished jobs....
>> make[1]: *** [all-bin-recurse] Error 2
>> make: *** [all-src-recurse] Error 2
>> make: *** Waiting for unfinished jobs....
>
> I think the problem is that "check" depends on "all", but now also
> depends on temp-install, which in turn runs install and all.  With a
> sufficient amount of parallelism, you end up running two "all"s on top
> of each other.
>
> It seems this can be fixed by removing the check: all dependency.  Try
> removing that in the top-level GNUmakefile.in and see if the problem
> goes away.  For completeness, we should then also remove it in the other
> makefiles.

Yep. I spent some time yesterday and today poking at that, but I
clearly missed that dependency.. Attached is a patch fixing the
problem. I tested check and check-world with some parallel jobs and
both worked. In the case of check, the amount of logs is very reduced
because all the install process is done by temp-install which
redirects everything into tmp_install/log/install.log.
-- 
Michael

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Attachments:

20150425_fix_makej.patch (text/x-diff, 5.2K) download diff --git a/GNUmakefile.in b/GNUmakefile.in index 361897a..42aeaa6 100644 --- a/GNUmakefile.in +++ b/GNUmakefile.in @@ -62,7 +62,7 @@ distclean maintainer-clean: # Garbage from autoconf: @rm -rf autom4te.cache/ -check check-tests: all +check check-tests: check check-tests installcheck installcheck-parallel installcheck-tests: $(MAKE) -C src/test/regress $@ diff --git a/contrib/test_decoding/Makefile b/contrib/test_decoding/Makefile index 613e9c3..e47ebab 100644 --- a/contrib/test_decoding/Makefile +++ b/contrib/test_decoding/Makefile @@ -39,7 +39,7 @@ submake-test_decoding: REGRESSCHECKS=ddl rewrite toast permissions decoding_in_xact decoding_into_rel binary prepared -regresscheck: all | submake-regress submake-test_decoding temp-install +regresscheck: | submake-regress submake-test_decoding temp-install $(MKDIR_P) regression_output $(pg_regress_check) \ --temp-config $(top_srcdir)/contrib/test_decoding/logical.conf \ @@ -53,7 +53,7 @@ regresscheck-install-force: | submake-regress submake-test_decoding temp-install ISOLATIONCHECKS=mxact delayed_startup ondisk_startup concurrent_ddl_dml -isolationcheck: all | submake-isolation submake-test_decoding temp-install +isolationcheck: | submake-isolation submake-test_decoding temp-install $(MKDIR_P) isolation_output $(pg_isolation_regress_check) \ --temp-config $(top_srcdir)/contrib/test_decoding/logical.conf \ diff --git a/src/bin/initdb/Makefile b/src/bin/initdb/Makefile index fc809a0..d479788 100644 --- a/src/bin/initdb/Makefile +++ b/src/bin/initdb/Makefile @@ -58,7 +58,7 @@ clean distclean maintainer-clean: # ensure that changes in datadir propagate into object file initdb.o: initdb.c $(top_builddir)/src/Makefile.global -check: all +check: $(prove_check) installcheck: diff --git a/src/bin/pg_basebackup/Makefile b/src/bin/pg_basebackup/Makefile index 58f8b66..0d8421a 100644 --- a/src/bin/pg_basebackup/Makefile +++ b/src/bin/pg_basebackup/Makefile @@ -50,7 +50,7 @@ clean distclean maintainer-clean: $(OBJS) rm -rf tmp_check -check: all +check: $(prove_check) installcheck: diff --git a/src/bin/pg_config/Makefile b/src/bin/pg_config/Makefile index 71ce236..dbc9899 100644 --- a/src/bin/pg_config/Makefile +++ b/src/bin/pg_config/Makefile @@ -49,7 +49,7 @@ clean distclean maintainer-clean: rm -f pg_config$(X) $(OBJS) rm -rf tmp_check -check: all +check: $(prove_check) installcheck: diff --git a/src/bin/pg_controldata/Makefile b/src/bin/pg_controldata/Makefile index f7a4010..fd7399b 100644 --- a/src/bin/pg_controldata/Makefile +++ b/src/bin/pg_controldata/Makefile @@ -35,7 +35,7 @@ clean distclean maintainer-clean: rm -f pg_controldata$(X) $(OBJS) rm -rf tmp_check -check: all +check: $(prove_check) installcheck: diff --git a/src/bin/pg_ctl/Makefile b/src/bin/pg_ctl/Makefile index 525e1d4..37eb482 100644 --- a/src/bin/pg_ctl/Makefile +++ b/src/bin/pg_ctl/Makefile @@ -38,7 +38,7 @@ clean distclean maintainer-clean: rm -f pg_ctl$(X) $(OBJS) rm -rf tmp_check -check: all +check: $(prove_check) installcheck: diff --git a/src/bin/scripts/Makefile b/src/bin/scripts/Makefile index 8b6f54c..c831716 100644 --- a/src/bin/scripts/Makefile +++ b/src/bin/scripts/Makefile @@ -69,7 +69,7 @@ clean distclean maintainer-clean: rm -f dumputils.c print.c mbprint.c kwlookup.c keywords.c rm -rf tmp_check -check: all +check: $(prove_check) installcheck: diff --git a/src/interfaces/ecpg/Makefile b/src/interfaces/ecpg/Makefile index b80de4e..41460a1 100644 --- a/src/interfaces/ecpg/Makefile +++ b/src/interfaces/ecpg/Makefile @@ -26,5 +26,5 @@ install-ecpglib-recurse: install-pgtypeslib-recurse clean distclean maintainer-clean: $(MAKE) -C test clean -check checktcp installcheck: all +check checktcp installcheck: $(MAKE) -C test $@ diff --git a/src/makefiles/pgxs.mk b/src/makefiles/pgxs.mk index ea17b1c..583a503 100644 --- a/src/makefiles/pgxs.mk +++ b/src/makefiles/pgxs.mk @@ -291,7 +291,7 @@ check: @echo '"$(MAKE) check" is not supported.' @echo 'Do "$(MAKE) install", then "$(MAKE) installcheck" instead.' else -check: all submake $(REGRESS_PREP) +check: submake $(REGRESS_PREP) $(pg_regress_check) $(REGRESS_OPTS) $(REGRESS) temp-install: EXTRA_INSTALL=$(subdir) diff --git a/src/pl/plperl/GNUmakefile b/src/pl/plperl/GNUmakefile index 2b69847..7f48067 100644 --- a/src/pl/plperl/GNUmakefile +++ b/src/pl/plperl/GNUmakefile @@ -112,7 +112,7 @@ uninstall-data: .PHONY: install-data uninstall-data -check: all submake +check: submake $(pg_regress_check) $(REGRESS_OPTS) $(REGRESS) installcheck: submake diff --git a/src/pl/plpython/Makefile b/src/pl/plpython/Makefile index c58e56b..cbd0123 100644 --- a/src/pl/plpython/Makefile +++ b/src/pl/plpython/Makefile @@ -173,7 +173,7 @@ pg_regress_clean_files += sql/python3/ expected/python3/ results/python3/ endif # Python 3 -check: all submake +check: submake $(pg_regress_check) $(REGRESS_OPTS) $(REGRESS) installcheck: submake diff --git a/src/pl/tcl/Makefile b/src/pl/tcl/Makefile index 851e3c0..533d3b4 100644 --- a/src/pl/tcl/Makefile +++ b/src/pl/tcl/Makefile @@ -92,7 +92,7 @@ uninstall-data: .PHONY: install-data uninstall-data -check: all submake +check: submake $(pg_regress_check) $(REGRESS_OPTS) $(REGRESS) installcheck: submake
^ permalink raw reply [nested|flat] 22+ messages in thread

* Re: improving speed of make check-world
  2014-08-15 05:45 improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
  2015-04-23 17:22 ` Re: improving speed of make check-world Jeff Janes <jeff.janes@gmail.com>
  2015-04-24 22:59   ` Re: improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
  2015-04-25 14:23     ` Re: improving speed of make check-world Michael Paquier <michael.paquier@gmail.com>
@ 2015-04-27 16:46       ` Jeff Janes <jeff.janes@gmail.com>
  2015-04-28 13:09         ` Re: improving speed of make check-world Michael Paquier <michael.paquier@gmail.com>
  0 siblings, 1 reply; 22+ messages in thread

From: Jeff Janes @ 2015-04-27 16:46 UTC (permalink / raw)
  To: Michael Paquier <michael.paquier@gmail.com>; +Cc: Peter Eisentraut <peter_e@gmx.net>; pgsql-hackers

On Sat, Apr 25, 2015 at 7:23 AM, Michael Paquier <michael.paquier@gmail.com>
wrote:

> On Sat, Apr 25, 2015 at 7:59 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
> > On 4/23/15 1:22 PM, Jeff Janes wrote:
> >> Something about this commit (dcae5faccab64776376d354d) broke "make
> >> check" in parallel conditions when started from a clean directory.  It
> >> fails with a different error each time, one example:
> >>
> >> make -j4 check > /dev/null
> >>
> >> In file included from gram.y:14515:
> >> scan.c: In function 'yy_try_NUL_trans':
> >> scan.c:10307: warning: unused variable 'yyg'
> >> /usr/bin/ld: tab-complete.o: No such file: No such file or directory
> >> collect2: ld returned 1 exit status
> >> make[3]: *** [psql] Error 1
> >> make[2]: *** [all-psql-recurse] Error 2
> >> make[2]: *** Waiting for unfinished jobs....
> >> make[1]: *** [all-bin-recurse] Error 2
> >> make: *** [all-src-recurse] Error 2
> >> make: *** Waiting for unfinished jobs....
> >
> > I think the problem is that "check" depends on "all", but now also
> > depends on temp-install, which in turn runs install and all.  With a
> > sufficient amount of parallelism, you end up running two "all"s on top
> > of each other.
> >
> > It seems this can be fixed by removing the check: all dependency.  Try
> > removing that in the top-level GNUmakefile.in and see if the problem
> > goes away.  For completeness, we should then also remove it in the other
> > makefiles.
>
> Yep. I spent some time yesterday and today poking at that, but I
> clearly missed that dependency.. Attached is a patch fixing the
> problem. I tested check and check-world with some parallel jobs and
> both worked. In the case of check, the amount of logs is very reduced
> because all the install process is done by temp-install which
> redirects everything into tmp_install/log/install.log.
>

This change fixed the problem for me.

It also made this age-old compiler warning go away:

In file included from gram.y:14515:
scan.c: In function 'yy_try_NUL_trans':
scan.c:10307: warning: unused variable 'yyg'

I guess by redirecting it into the log file you indicated, but is that a
good idea to redirect stderr?

Cheers,

Jeff



--
> Michael
>

^ permalink  raw  reply  [nested|flat] 22+ messages in thread

* Re: improving speed of make check-world
  2014-08-15 05:45 improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
  2015-04-23 17:22 ` Re: improving speed of make check-world Jeff Janes <jeff.janes@gmail.com>
  2015-04-24 22:59   ` Re: improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
  2015-04-25 14:23     ` Re: improving speed of make check-world Michael Paquier <michael.paquier@gmail.com>
  2015-04-27 16:46       ` Re: improving speed of make check-world Jeff Janes <jeff.janes@gmail.com>
@ 2015-04-28 13:09         ` Michael Paquier <michael.paquier@gmail.com>
  2015-04-28 20:05           ` Re: improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
  2015-04-29 04:12           ` Re: improving speed of make check-world Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 2 replies; 22+ messages in thread

From: Michael Paquier @ 2015-04-28 13:09 UTC (permalink / raw)
  To: Jeff Janes <jeff.janes@gmail.com>; +Cc: Peter Eisentraut <peter_e@gmx.net>; pgsql-hackers

On Tue, Apr 28, 2015 at 1:46 AM, Jeff Janes <jeff.janes@gmail.com> wrote:
> On Sat, Apr 25, 2015 at 7:23 AM, Michael Paquier <michael.paquier@gmail.com>
> wrote:
>>
>> On Sat, Apr 25, 2015 at 7:59 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
>> > On 4/23/15 1:22 PM, Jeff Janes wrote:
>> >> Something about this commit (dcae5faccab64776376d354d) broke "make
>> >> check" in parallel conditions when started from a clean directory.  It
>> >> fails with a different error each time, one example:
>> >>
>> >> make -j4 check > /dev/null
>> >>
>> >> In file included from gram.y:14515:
>> >> scan.c: In function 'yy_try_NUL_trans':
>> >> scan.c:10307: warning: unused variable 'yyg'
>> >> /usr/bin/ld: tab-complete.o: No such file: No such file or directory
>> >> collect2: ld returned 1 exit status
>> >> make[3]: *** [psql] Error 1
>> >> make[2]: *** [all-psql-recurse] Error 2
>> >> make[2]: *** Waiting for unfinished jobs....
>> >> make[1]: *** [all-bin-recurse] Error 2
>> >> make: *** [all-src-recurse] Error 2
>> >> make: *** Waiting for unfinished jobs....
>> >
>> > I think the problem is that "check" depends on "all", but now also
>> > depends on temp-install, which in turn runs install and all.  With a
>> > sufficient amount of parallelism, you end up running two "all"s on top
>> > of each other.
>> >
>> > It seems this can be fixed by removing the check: all dependency.  Try
>> > removing that in the top-level GNUmakefile.in and see if the problem
>> > goes away.  For completeness, we should then also remove it in the other
>> > makefiles.
>>
>> Yep. I spent some time yesterday and today poking at that, but I
>> clearly missed that dependency.. Attached is a patch fixing the
>> problem. I tested check and check-world with some parallel jobs and
>> both worked. In the case of check, the amount of logs is very reduced
>> because all the install process is done by temp-install which
>> redirects everything into tmp_install/log/install.log.
>
>
> This change fixed the problem for me.
>
> It also made this age-old compiler warning go away:
>
> In file included from gram.y:14515:
> scan.c: In function 'yy_try_NUL_trans':
> scan.c:10307: warning: unused variable 'yyg'
>
> I guess by redirecting it into the log file you indicated, but is that a
> good idea to redirect stderr?

I am sure that Peter did that on purpose, both approaches having
advantages and disadvantages. Personally I don't mind looking at the
install log file in tmp_install to see the state of the installation,
but it is true that this change is a bit disturbing regarding the fact
that everything was directly outputted to stderr and stdout for many
years.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



^ permalink  raw  reply  [nested|flat] 22+ messages in thread

* Re: improving speed of make check-world
  2014-08-15 05:45 improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
  2015-04-23 17:22 ` Re: improving speed of make check-world Jeff Janes <jeff.janes@gmail.com>
  2015-04-24 22:59   ` Re: improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
  2015-04-25 14:23     ` Re: improving speed of make check-world Michael Paquier <michael.paquier@gmail.com>
  2015-04-27 16:46       ` Re: improving speed of make check-world Jeff Janes <jeff.janes@gmail.com>
  2015-04-28 13:09         ` Re: improving speed of make check-world Michael Paquier <michael.paquier@gmail.com>
@ 2015-04-28 20:05           ` Peter Eisentraut <peter_e@gmx.net>
  1 sibling, 0 replies; 22+ messages in thread

From: Peter Eisentraut @ 2015-04-28 20:05 UTC (permalink / raw)
  To: Michael Paquier <michael.paquier@gmail.com>; Jeff Janes <jeff.janes@gmail.com>; +Cc: pgsql-hackers

On 4/28/15 9:09 AM, Michael Paquier wrote:
>> I guess by redirecting it into the log file you indicated, but is that a
>> > good idea to redirect stderr?
> I am sure that Peter did that on purpose, both approaches having
> advantages and disadvantages. Personally I don't mind looking at the
> install log file in tmp_install to see the state of the installation,
> but it is true that this change is a bit disturbing regarding the fact
> that everything was directly outputted to stderr and stdout for many
> years.

Not really.  Before, pg_regress put the output of its internal make
install run into a log file.  Now make is just replicating that
behavior.  I would agree that that seems kind of obsolete now, because
it's really just another sub-make.  So if no one disagrees, I'd just
remove the log file redirection in temp-install.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



^ permalink  raw  reply  [nested|flat] 22+ messages in thread

* Re: improving speed of make check-world
  2014-08-15 05:45 improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
  2015-04-23 17:22 ` Re: improving speed of make check-world Jeff Janes <jeff.janes@gmail.com>
  2015-04-24 22:59   ` Re: improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
  2015-04-25 14:23     ` Re: improving speed of make check-world Michael Paquier <michael.paquier@gmail.com>
  2015-04-27 16:46       ` Re: improving speed of make check-world Jeff Janes <jeff.janes@gmail.com>
  2015-04-28 13:09         ` Re: improving speed of make check-world Michael Paquier <michael.paquier@gmail.com>
@ 2015-04-29 04:12           ` Tom Lane <tgl@sss.pgh.pa.us>
  1 sibling, 0 replies; 22+ messages in thread

From: Tom Lane @ 2015-04-29 04:12 UTC (permalink / raw)
  To: Michael Paquier <michael.paquier@gmail.com>; +Cc: Jeff Janes <jeff.janes@gmail.com>; Peter Eisentraut <peter_e@gmx.net>; pgsql-hackers

Michael Paquier <michael.paquier@gmail.com> writes:
> On Tue, Apr 28, 2015 at 1:46 AM, Jeff Janes <jeff.janes@gmail.com> wrote:
>> This change fixed the problem for me.
>> It also made this age-old compiler warning go away:
>> 
>> In file included from gram.y:14515:
>> scan.c: In function 'yy_try_NUL_trans':
>> scan.c:10307: warning: unused variable 'yyg'
>> 
>> I guess by redirecting it into the log file you indicated, but is that a
>> good idea to redirect stderr?

> I am sure that Peter did that on purpose, both approaches having
> advantages and disadvantages. Personally I don't mind looking at the
> install log file in tmp_install to see the state of the installation,
> but it is true that this change is a bit disturbing regarding the fact
> that everything was directly outputted to stderr and stdout for many
> years.

Hm.  I don't really like the idea that "make all" behaves differently
if invoked by "make check" than if invoked directly.  Hiding warnings
from you is not very good, and hiding errors would be even more annoying.

			regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



^ permalink  raw  reply  [nested|flat] 22+ messages in thread


end of thread, other threads:[~2015-04-29 04:12 UTC | newest]

Thread overview: 22+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2014-08-15 05:45 improving speed of make check-world Peter Eisentraut <peter_e@gmx.net>
2014-08-25 17:32 ` Heikki Linnakangas <hlinnakangas@vmware.com>
2014-08-26 01:32   ` Peter Eisentraut <peter_e@gmx.net>
2014-08-31 03:52     ` Peter Eisentraut <peter_e@gmx.net>
2014-08-31 09:36 ` Fabien COELHO <coelho@cri.ensmp.fr>
2014-08-31 16:08   ` Fabien COELHO <coelho@cri.ensmp.fr>
2015-02-15 02:01   ` Peter Eisentraut <peter_e@gmx.net>
2015-02-15 02:09     ` Tom Lane <tgl@sss.pgh.pa.us>
2015-02-24 08:06     ` Michael Paquier <michael.paquier@gmail.com>
2015-03-08 13:22       ` Peter Eisentraut <peter_e@gmx.net>
2015-03-09 05:46         ` Michael Paquier <michael.paquier@gmail.com>
2015-03-09 06:51           ` Michael Paquier <michael.paquier@gmail.com>
2015-04-10 19:35             ` Peter Eisentraut <peter_e@gmx.net>
2015-04-11 11:48               ` Michael Paquier <michael.paquier@gmail.com>
2015-04-11 22:36                 ` Michael Paquier <michael.paquier@gmail.com>
2015-04-23 17:22 ` Jeff Janes <jeff.janes@gmail.com>
2015-04-24 22:59   ` Peter Eisentraut <peter_e@gmx.net>
2015-04-25 14:23     ` Michael Paquier <michael.paquier@gmail.com>
2015-04-27 16:46       ` Jeff Janes <jeff.janes@gmail.com>
2015-04-28 13:09         ` Michael Paquier <michael.paquier@gmail.com>
2015-04-28 20:05           ` Peter Eisentraut <peter_e@gmx.net>
2015-04-29 04:12           ` Tom Lane <tgl@sss.pgh.pa.us>

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox