[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 <bruce@momjian.us> http://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

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 <bruce@momjian.us> http://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

On Tue, Dec 10, 2013 at 12:08 PM, Bruce Momjian <bruce@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

Bruce Momjian <bruce@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: Bruce Momjian 2013-12-04 <20131204151533.GB17114@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
--
cb@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

On Thu, Mar 27, 2014 at 06:03:24PM +0100, Christoph Berg wrote:

Re: Bruce Momjian 2013-12-04 <20131204151533.GB17114@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 <bruce@momjian.us> http://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