Thread: Re: pgsql: Add TAP test to automate the equivalent of check_guc
Re: Michael Paquier
> Add TAP test to automate the equivalent of check_guc
>
> src/backend/utils/misc/check_guc is a script that cross-checks the
> consistency of the GUCs with postgresql.conf.sample, making sure that
Hi,
this test is failing at Debian package compile time:
00:55:07 ok 18 - drop tablespace 2
00:55:07 ok 19 - drop tablespace 3
00:55:07 ok 20 - drop tablespace 4
00:55:07 ok
00:55:07 # Looks like your test exited with 2 before it could output anything.
00:55:09 t/003_check_guc.pl ..............
00:55:09 1..3
00:55:09 Dubious, test returned 2 (wstat 512, 0x200)
00:55:09 Failed 3/3 subtests
00:55:09
00:55:09 Test Summary Report
00:55:09 -------------------
00:55:09 t/003_check_guc.pl (Wstat: 512 Tests: 0 Failed: 0)
00:55:09 Non-zero exit status: 2
00:55:09 Parse errors: Bad plan. You planned 3 tests but ran 0.
00:55:09 Files=3, Tests=62, 6 wallclock secs ( 0.05 usr 0.01 sys + 3.31 cusr 1.35 csys = 4.72 CPU)
00:55:09 Result: FAIL
00:55:09 make[3]: *** [/<<PKGBUILDDIR>>/build/../src/makefiles/pgxs.mk:457: check] Error 1
00:55:09 make[3]: Leaving directory '/<<PKGBUILDDIR>>/build/src/test/modules/test_misc'
### Starting node "main"
# Running: pg_ctl -w -D
/srv/projects/postgresql/pg/master/build/src/test/modules/test_misc/tmp_check/t_003_check_guc_main_data/pgdata-l
/srv/projects/postgresql/pg/master/build/src/test/modules/test_misc/tmp_check/log/003_check_guc_main.log-o
--cluster-name=mainstart
waiting for server to start.... done
server started
# Postmaster PID for node "main" is 1432398
Could not open /usr/share/postgresql/15/postgresql.conf.sample: No such file or directory at t/003_check_guc.pl line
47.
### Stopping node "main" using mode immediate
So it's trying to read from /usr/share/postgresql which doesn't exist
yet at build time.
The relevant part of the test is this:
# Find the location of postgresql.conf.sample, based on the information
# provided by pg_config.
my $sample_file =
$node->config_data('--sharedir') . '/postgresql.conf.sample';
It never caused any problem in the 12+ years we have been doing this,
but Debian is patching pg_config not to be relocatable since we are
installing into /usr/lib/postgresql/NN /usr/share/postgresql/NN, so
not a single prefix.
https://salsa.debian.org/postgresql/postgresql/-/blob/15/debian/patches/50-per-version-dirs.patch
Christoph
Christoph Berg <myon@debian.org> writes:
> this test is failing at Debian package compile time:
> Could not open /usr/share/postgresql/15/postgresql.conf.sample: No such file or directory at t/003_check_guc.pl line
47.
> So it's trying to read from /usr/share/postgresql which doesn't exist
> yet at build time.
> The relevant part of the test is this:
> # Find the location of postgresql.conf.sample, based on the information
> # provided by pg_config.
> my $sample_file =
> $node->config_data('--sharedir') . '/postgresql.conf.sample';
This seems like a pretty bad idea even if it weren't failing outright.
We should be examining the version of the file that's in the source
tree; the one in the installation tree might have version-skew
problems, if you've not yet done "make install".
regards, tom lane
Justin Pryzby <pryzby@telsasoft.com> writes:
> On Fri, Feb 11, 2022 at 09:59:55AM -0500, Tom Lane wrote:
>> This seems like a pretty bad idea even if it weren't failing outright.
>> We should be examining the version of the file that's in the source
>> tree; the one in the installation tree might have version-skew
>> problems, if you've not yet done "make install".
> My original way used the source tree, but Michael thought it would be an issue
> for "installcheck" where the config may not be available.
Yeah, you are at risk either way, but in practice nobody is going to be
running these TAP tests without a source tree.
> This is what I had written:
> FROM (SELECT regexp_split_to_table(pg_read_file('postgresql.conf'), '\n') AS ln) conf
That's not using the source tree either, but the copy in the
cluster-under-test. I'd fear it to be unstable in the buildfarm, where
animals can append whatever they please to the config file being used by
tests.
regards, tom lane
Justin Pryzby <pryzby@telsasoft.com> writes:
> Or, is it okay to use ABS_SRCDIR?
Don't see why not --- other tests certainly do.
regards, tom lane
On Fri, Feb 11, 2022 at 10:48:11AM +0100, Christoph Berg wrote: > It never caused any problem in the 12+ years we have been doing this, > but Debian is patching pg_config not to be relocatable since we are > installing into /usr/lib/postgresql/NN /usr/share/postgresql/NN, so > not a single prefix. > > https://salsa.debian.org/postgresql/postgresql/-/blob/15/debian/patches/50-per-version-dirs.patch Wow. This is the way for Debian to bypass the fact that ./configure is itself patched, hence you cannot rely on things like --libdir, --bindir and the kind at build time? That's.. Err.. Fancy, I'd say. -- Michael
Attachment
Re: Michael Paquier > On Fri, Feb 11, 2022 at 10:48:11AM +0100, Christoph Berg wrote: > > It never caused any problem in the 12+ years we have been doing this, > > but Debian is patching pg_config not to be relocatable since we are > > installing into /usr/lib/postgresql/NN /usr/share/postgresql/NN, so > > not a single prefix. > > > > https://salsa.debian.org/postgresql/postgresql/-/blob/15/debian/patches/50-per-version-dirs.patch > > Wow. This is the way for Debian to bypass the fact that ./configure > is itself patched, hence you cannot rely on things like --libdir, > --bindir and the kind at build time? That's.. Err.. Fancy, I'd > say. --libdir and --bindir will point at the final install locations. I think the "bug" here is that vanilla PG doesn't support installing in FHS locations with /usr/lib and /usr/share split, hence the Debian patch. Christoph
Christoph Berg <myon@debian.org> writes:
> I think the "bug" here is that vanilla PG doesn't support installing
> in FHS locations with /usr/lib and /usr/share split, hence the Debian
> patch.
I'm confused by this statement. An out-of-the-box installation
produces trees under $PREFIX/lib and $PREFIX/share. What about
that is not FHS compliant? And couldn't you fix it using the
installation fine-tuning switches that configure already provides?
regards, tom lane
Christoph Berg <myon@debian.org> writes:
> I was confusing that with this: The problem that led to the pg_config
> patch years ago was that we have a /usr/bin/pg_config in
> (non-major-version-dependant) libpq-dev, and
> /usr/lib/postgresql/NN/bin/pg_config in the individual
> postgresql-server-dev-NN packages, and iirc the /usr/bin version
> didn't particularly like the other binaries being in
> /usr/lib/postgresql/NN/bin/.
Ah. That seems a bit problematic anyway ... if the version-dependent
pg_configs don't all return identical results, what's the
non-version-dependent one supposed to do?
regards, tom lane
On Sat, Feb 12, 2022 at 05:31:55PM +0100, Christoph Berg wrote: > To support multiple major versions in parallel, we need > /usr/lib/postgresql/NN/lib and /usr/share/postgresql/NN, so it's not a > single $PREFIX. But you are correct, and ./configure supports that. Yep, I don't quite see the problem, but I don't have the years of experience in terms of packaging you have, either. (NB: I use various --libdir and --bindir combinations that for some of packaging of PG I maintain for pg_upgrade, but that's not as complex as Debian, I guess). > I was confusing that with this: The problem that led to the pg_config > patch years ago was that we have a /usr/bin/pg_config in > (non-major-version-dependant) libpq-dev, and > /usr/lib/postgresql/NN/bin/pg_config in the individual > postgresql-server-dev-NN packages, and iirc the /usr/bin version > didn't particularly like the other binaries being in > /usr/lib/postgresql/NN/bin/. > > I guess it's time to revisit that problem now and see if it can be > solved more pretty today on our side. If you can solve that, my take is that it could make things nicer in the long-run for some of the TAP test facilities. Still, I'll try to look at a solution for this thread that does not interact badly with what you are doing, and I am going to grab your patch when testing things. -- Michael
Attachment
On Wed, Feb 16, 2022 at 7:24 PM Michael Paquier <michael@paquier.xyz> wrote: > On Wed, Feb 16, 2022 at 01:38:51PM +0100, Christoph Berg wrote: > > The build works with the "Add TAP test to automate the equivalent of > > check_guc, take two" commit. Thanks! > > Thanks for double-checking, Christoph! > > > (Tests are still failing for > > https://www.postgresql.org/message-id/YgjwrkEvNEqoz4Vm%40msg.df7cb.de > > though) > > This has been unanswered for four weeks now. I'll see about jumping > into it, then.. Sorry, I saw that and then forgot about it ... but isn't it 3 days, not 4 weeks? In any case, I'm happy to have you take care of it, but I can also look at it tomorrow if you prefer. -- Robert Haas EDB: http://www.enterprisedb.com
On Wed, Feb 16, 2022 at 07:39:26PM -0500, Robert Haas wrote: > Sorry, I saw that and then forgot about it ... but isn't it 3 days, > not 4 weeks? In any case, I'm happy to have you take care of it, but I > can also look at it tomorrow if you prefer. Ugh. I looked at the top of the thread and saw January the 17th :) So that means that I need more caffeine. If you are planning to look at it, please feel free. Thanks! -- Michael
Attachment
Re: Michael Paquier > > I was confusing that with this: The problem that led to the pg_config > > patch years ago was that we have a /usr/bin/pg_config in > > (non-major-version-dependant) libpq-dev, and > > /usr/lib/postgresql/NN/bin/pg_config in the individual > > postgresql-server-dev-NN packages, and iirc the /usr/bin version > > didn't particularly like the other binaries being in > > /usr/lib/postgresql/NN/bin/. > > > > I guess it's time to revisit that problem now and see if it can be > > solved more pretty today on our side. > > If you can solve that, my take is that it could make things nicer in > the long-run for some of the TAP test facilities. Still, I'll try to > look at a solution for this thread that does not interact badly with > what you are doing, and I am going to grab your patch when testing > things. tl;dr: This is no longer an issue. Since build-time testing broke again about two weeks ago due to Debian's pg_config patch, I revisited the situation and found that the patch is in fact no longer necessary to support pg_config in /usr/bin: To support cross-compilation against libpq, some years ago I had already replaced /usr/bin/pg_config (in libpq-dev) with a perl script that collects path info at build time and outputs them statically at run time [1]. The "real" /usr/lib/postgresql/15/bin/pg_config (in postgresql-server-dev-15) is still the C version, now unpatched with full relocatability support. [1] https://salsa.debian.org/postgresql/postgresql-common/-/blob/master/server/postgresql.mk#L189-194 https://salsa.debian.org/postgresql/postgresql-common/-/blob/master/server/pg_config.pl Christoph
On Fri, Apr 15, 2022 at 04:49:28PM +0200, Christoph Berg wrote: > Since build-time testing broke again about two weeks ago due to > Debian's pg_config patch, I revisited the situation and found that the > patch is in fact no longer necessary to support pg_config in /usr/bin: > > To support cross-compilation against libpq, some years ago I had > already replaced /usr/bin/pg_config (in libpq-dev) with a perl script > that collects path info at build time and outputs them statically at > run time [1]. The "real" /usr/lib/postgresql/15/bin/pg_config (in > postgresql-server-dev-15) is still the C version, now unpatched with > full relocatability support. Nice! Thanks for letting us know, Christoph. -- Michael