Обсуждение: [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 -- cb@df7cb.de | http://www.df7cb.de/
Вложения
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. +
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. +
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
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
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/
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. +