Re: [HACKERS] [patch] Adding EXTRA_REGRESS_OPTS to all pg_regress invocations
On Thu, Mar 27, 2014 at 06:03:24PM +0100, Christoph Berg wrote: Re: Bruce Momjian 2013-12-04 20131204151533.gb17...@momjian.us On Mon, May 6, 2013 at 11:51:47PM -0700, Christoph Berg wrote: make check supports EXTRA_REGRESS_OPTS to pass extra options to pg_regress, but all the other places where pg_regress is used do not allow this. The attached patch adds EXTRA_REGRESS_OPTS to Makefile.global.in (for contrib modules) and two more special Makefiles (isolation and pg_upgrade). The use case here is that Debian needs to be able to redirect the unix socket directory used to /tmp, because /var/run/postgresql isn't writable for the buildd user. The matching part for this inside pg_regress is still in discussion here, but the addition of EXTRA_REGRESS_OPTS is an independent step that is also useful for others, so I'd like to propose it for inclusion. Thanks, patch applied. This will appear in PG 9.4. I suppose we could backpatch this but I would need community feedback on that. Thanks for pushing this. In the meantime, a new bit has appeared: The new contrib/test_decoding checks make use of the pg_isolation_regress_check macros (which the isolation test itself doesn't). These macros also need EXTRA_REGRESS_OPTS, on top of 86ef4796f5120c55d1a48cfab52e51df8ed271b5: Applied. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [patch] Adding EXTRA_REGRESS_OPTS to all pg_regress invocations
Re: Bruce Momjian 2013-12-04 20131204151533.gb17...@momjian.us On Mon, May 6, 2013 at 11:51:47PM -0700, Christoph Berg wrote: make check supports EXTRA_REGRESS_OPTS to pass extra options to pg_regress, but all the other places where pg_regress is used do not allow this. The attached patch adds EXTRA_REGRESS_OPTS to Makefile.global.in (for contrib modules) and two more special Makefiles (isolation and pg_upgrade). The use case here is that Debian needs to be able to redirect the unix socket directory used to /tmp, because /var/run/postgresql isn't writable for the buildd user. The matching part for this inside pg_regress is still in discussion here, but the addition of EXTRA_REGRESS_OPTS is an independent step that is also useful for others, so I'd like to propose it for inclusion. Thanks, patch applied. This will appear in PG 9.4. I suppose we could backpatch this but I would need community feedback on that. Thanks for pushing this. In the meantime, a new bit has appeared: The new contrib/test_decoding checks make use of the pg_isolation_regress_check macros (which the isolation test itself doesn't). These macros also need EXTRA_REGRESS_OPTS, on top of 86ef4796f5120c55d1a48cfab52e51df8ed271b5: diff --git a/src/Makefile.global.in b/src/Makefile.global.in new file mode 100644 index cdddf49..8d08d19 *** a/src/Makefile.global.in --- b/src/Makefile.global.in *** pg_regress_installcheck = $(top_builddir *** 468,475 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) ! pg_isolation_regress_installcheck = $(top_builddir)/src/test/isolation/pg_isolation_regress --inputdir=$(srcdir) --top-builddir=$(top_builddir) $(pg_regress_locale_flags) ## # --- 468,475 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) ## # Christoph -- c...@df7cb.de | http://www.df7cb.de/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [patch] Adding EXTRA_REGRESS_OPTS to all pg_regress invocations
On Thu, Dec 5, 2013 at 09:52:27AM +0100, Christoph Berg wrote:
The change is sane in itself. It won't affect anyone who doesn't use
EXTRA_REGRESS_OPTS. Why would we want to make packagers do MORE
work?
The patch has been in the Debian/Ubuntu/apt.pg.o packages for some
time, for 8.3+. I'm attaching the patches used there.
(Sidenote: To enable building of several package flavors in parallel
on the same machine we use
make -C build check-world EXTRA_REGRESS_OPTS='--host=/tmp --port=$(shell perl
-le 'print 1024 + int(rand(64000))')'
so pg_regress' static per-version ports do not conflict. But 9.2's
contrib/pg_upgrade/{Makefile/test.sh} do not like --port in there, so
the 9.2 patch has an extra sed hack in there to remove --port for
pg_upgrade. That bit should probably not be applied for general use.
The rest is safe, though.)
OK, Christoph has provided a full set of tested patches back to 8.4.
Should I backpatch these? Peter says no, but two others say yes.
--
Bruce Momjian br...@momjian.ushttp://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [patch] Adding EXTRA_REGRESS_OPTS to all pg_regress invocations
On Tue, Dec 10, 2013 at 12:08 PM, Bruce Momjian br...@momjian.us wrote:
On Thu, Dec 5, 2013 at 09:52:27AM +0100, Christoph Berg wrote:
The change is sane in itself. It won't affect anyone who doesn't use
EXTRA_REGRESS_OPTS. Why would we want to make packagers do MORE
work?
The patch has been in the Debian/Ubuntu/apt.pg.o packages for some
time, for 8.3+. I'm attaching the patches used there.
(Sidenote: To enable building of several package flavors in parallel
on the same machine we use
make -C build check-world EXTRA_REGRESS_OPTS='--host=/tmp --port=$(shell
perl -le 'print 1024 + int(rand(64000))')'
so pg_regress' static per-version ports do not conflict. But 9.2's
contrib/pg_upgrade/{Makefile/test.sh} do not like --port in there, so
the 9.2 patch has an extra sed hack in there to remove --port for
pg_upgrade. That bit should probably not be applied for general use.
The rest is safe, though.)
OK, Christoph has provided a full set of tested patches back to 8.4.
Should I backpatch these? Peter says no, but two others say yes.
My 2c. Adding a new feature in a maintenance branch is usually not
done, so I'd vote no.
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
Re: [HACKERS] [patch] Adding EXTRA_REGRESS_OPTS to all pg_regress invocations
Bruce Momjian br...@momjian.us writes: OK, Christoph has provided a full set of tested patches back to 8.4. Should I backpatch these? Peter says no, but two others say yes. It's hard to paint that as a bug fix, so I'd vote for HEAD only. 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
Re: [HACKERS] [patch] Adding EXTRA_REGRESS_OPTS to all pg_regress invocations
On Mon, May 6, 2013 at 11:51:47PM -0700, Christoph Berg wrote: make check supports EXTRA_REGRESS_OPTS to pass extra options to pg_regress, but all the other places where pg_regress is used do not allow this. The attached patch adds EXTRA_REGRESS_OPTS to Makefile.global.in (for contrib modules) and two more special Makefiles (isolation and pg_upgrade). The use case here is that Debian needs to be able to redirect the unix socket directory used to /tmp, because /var/run/postgresql isn't writable for the buildd user. The matching part for this inside pg_regress is still in discussion here, but the addition of EXTRA_REGRESS_OPTS is an independent step that is also useful for others, so I'd like to propose it for inclusion. Thanks, patch applied. This will appear in PG 9.4. I suppose we could backpatch this but I would need community feedback on that. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] [patch] Adding EXTRA_REGRESS_OPTS to all pg_regress invocations
make check supports EXTRA_REGRESS_OPTS to pass extra options to pg_regress, but all the other places where pg_regress is used do not allow this. The attached patch adds EXTRA_REGRESS_OPTS to Makefile.global.in (for contrib modules) and two more special Makefiles (isolation and pg_upgrade). The use case here is that Debian needs to be able to redirect the unix socket directory used to /tmp, because /var/run/postgresql isn't writable for the buildd user. The matching part for this inside pg_regress is still in discussion here, but the addition of EXTRA_REGRESS_OPTS is an independent step that is also useful for others, so I'd like to propose it for inclusion. Christoph -- c...@df7cb.de | http://www.df7cb.de/ diff --git a/contrib/pg_upgrade/Makefile b/contrib/pg_upgrade/Makefile new file mode 100644 index bbb14a1..ffcdad8 *** a/contrib/pg_upgrade/Makefile --- b/contrib/pg_upgrade/Makefile *** include $(top_srcdir)/contrib/contrib-gl *** 25,31 endif check: test.sh all ! MAKE=$(MAKE) bindir=$(bindir) libdir=$(libdir) $(SHELL) $ --install # disabled because it upsets the build farm #installcheck: test.sh --- 25,31 endif check: test.sh all ! MAKE=$(MAKE) bindir=$(bindir) libdir=$(libdir) EXTRA_REGRESS_OPTS=$(EXTRA_REGRESS_OPTS) $(SHELL) $ --install # disabled because it upsets the build farm #installcheck: test.sh diff --git a/src/Makefile.global.in b/src/Makefile.global.in new file mode 100644 index 89e39d2..d0f08a9 *** a/src/Makefile.global.in --- b/src/Makefile.global.in *** endif *** 448,455 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) ! pg_regress_installcheck = $(top_builddir)/src/test/regress/pg_regress --inputdir=$(srcdir) --psqldir='$(PSQLDIR)' $(pg_regress_locale_flags) pg_regress_clean_files = results/ regression.diffs regression.out tmp_check/ log/ --- 448,455 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_clean_files = results/ regression.diffs regression.out tmp_check/ log/ diff --git a/src/test/isolation/Makefile b/src/test/isolation/Makefile new file mode 100644 index 5e5c9bb..26bcf22 *** a/src/test/isolation/Makefile --- b/src/test/isolation/Makefile *** maintainer-clean: distclean *** 52,68 rm -f specparse.c specscanner.c installcheck: all ! ./pg_isolation_regress --psqldir='$(PSQLDIR)' --inputdir=$(srcdir) --schedule=$(srcdir)/isolation_schedule check: all ! ./pg_isolation_regress --temp-install=./tmp_check --inputdir=$(srcdir) --top-builddir=$(top_builddir) --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)' --inputdir=$(srcdir) --schedule=$(srcdir)/isolation_schedule prepared-transactions check-prepared-txns: all ! ./pg_isolation_regress --temp-install=./tmp_check --inputdir=$(srcdir) --top-builddir=$(top_builddir) --schedule=$(srcdir)/isolation_schedule prepared-transactions --- 52,68 rm -f specparse.c specscanner.c installcheck: all ! ./pg_isolation_regress --psqldir='$(PSQLDIR)' $(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 # 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 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 signature.asc Description: Digital signature
