Horton

Thread

Commits

  1. Revert "Add TAP test to automate the equivalent of check_guc"

  2. Add TAP test to automate the equivalent of check_guc

  1. pgsql: Add TAP test to automate the equivalent of check_guc

    Michael Paquier <michael@paquier.xyz> — 2022-02-09T01:15:56Z

    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
its format is in line with what guc.c has.  It has never been run
automatically, and has rotten over the years, creating a lot of false
positives as per a report from Justin Pryzby.

d10e41d has introduced a SQL function to publish the most relevant flags
associated to a GUC, with tests added in the main regression test suite
to make sure that we avoid most of the inconsistencies in the GUC
settings, based on recent reports, but there was nothing able to
cross-check postgresql.conf.sample with the contents of guc.c.

This commit adds a TAP test that covers the remaining gap.  It emulates
the most relevant checks that check_guc does, so as any format mistakes
are detected in postgresql.conf.sample at development stage, with the
following checks:
- Check that parameters marked as NOT_IN_SAMPLE are not in the sample
file.
- Check that there are no dead entries in postgresql.conf.sample for
parameters not marked as NOT_IN_SAMPLE.
- Check that no parameters are missing from the sample file if listed in
guc.c without NOT_IN_SAMPLE.

The idea of building a list of the GUCs by parsing the sample file comes
from Justin, and he wrote the regex used in the patch to find all the
GUCs (this same formatting rule basically applies for the last 20~ years
or so).  In order to test this patch, I have played with manual
modifications of postgresql.conf.sample and guc.c, making sure that we
detect problems with the GUC rules and the sample file format.

The test is located in src/test/modules/test_misc, which is the best
location I could think about for such sanity checks.

Reviewed-by: Justin Pryzby
Discussion: https://postgr.es/m/Yf9YGSwPiMu0c7fP@paquier.xyz

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/b0a55f4d4ad58e4bc152f8c1696b4af46525e3f1

Modified Files
--------------
src/test/modules/test_misc/t/003_check_guc.pl | 108 ++++++++++++++++++++++++++
1 file changed, 108 insertions(+)

  2. Re: pgsql: Add TAP test to automate the equivalent of check_guc

    Christoph Berg <myon@debian.org> — 2022-02-11T09:48:11Z

    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=main start
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

  3. Re: pgsql: Add TAP test to automate the equivalent of check_guc

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-02-11T14:59:55Z

    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

  4. Re: pgsql: Add TAP test to automate the equivalent of check_guc

    Justin Pryzby <pryzby@telsasoft.com> — 2022-02-11T15:35:49Z

    On Fri, Feb 11, 2022 at 09:59:55AM -0500, Tom Lane wrote:
> 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".

My original way used the source tree, but Michael thought it would be an issue
for "installcheck" where the config may not be available.

https://www.postgresql.org/message-id/YfTg/WHNLVVygy8v%40paquier.xyz

This is what I had written:

-- test that GUCS are in postgresql.conf
SELECT lower(name) FROM tab_settings_flags WHERE NOT not_in_sample EXCEPT
SELECT regexp_replace(ln, '^#?([_[:alpha:]]+) (= .*|[^ ]*$)', '\1') AS guc
FROM (SELECT regexp_split_to_table(pg_read_file('postgresql.conf'), '\n') AS ln) conf
WHERE ln ~ '^#?[[:alpha:]]'
ORDER BY 1;
            lower            
-----------------------------
 config_file
 plpgsql.check_asserts
 plpgsql.extra_errors
 plpgsql.extra_warnings
 plpgsql.print_strict_params
 plpgsql.variable_conflict
(6 rows)

-- test that lines in postgresql.conf that look like GUCs are GUCs
SELECT regexp_replace(ln, '^#?([_[:alpha:]]+) (= .*|[^ ]*$)', '\1') AS guc
FROM (SELECT regexp_split_to_table(pg_read_file('postgresql.conf'), '\n') AS ln) conf
WHERE ln ~ '^#?[[:alpha:]]'
EXCEPT SELECT lower(name) FROM tab_settings_flags WHERE NOT not_in_sample
ORDER BY 1;
        guc        
-------------------
 include
 include_dir
 include_if_exists
(3 rows)

-- 
Justin

  5. Re: pgsql: Add TAP test to automate the equivalent of check_guc

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-02-11T15:41:27Z

    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

  6. Re: pgsql: Add TAP test to automate the equivalent of check_guc

    Justin Pryzby <pryzby@telsasoft.com> — 2022-02-11T17:29:29Z

    On Fri, Feb 11, 2022 at 10:41:27AM -0500, Tom Lane wrote:
> 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.

The important test is that "new GUCs exist in sample.conf".  The converse test
is less interesting, so I think the important half would be okay.

I see the BF client appends to postgresql.conf.
https://github.com/PGBuildFarm/client-code/blob/main/run_build.pl#L1374

It could use "include", which was added in v8.2.  Or it could add a marker,
like pg_regress does:
src/test/regress/pg_regress.c:          fputs("\n# Configuration added by pg_regress\n\n", pg_conf);

If it were desirable to also check that "things that look like GUCs in the conf
are actually GUCs", either of those would avoid false positives.

Or, is it okay to use ABS_SRCDIR?

+\getenv abs_srcdir PG_ABS_SRCDIR
+\set filename :abs_srcdir '../../../../src/backend/utils/misc/postgresql.conf.sample'
+-- test that GUCS are in postgresql.conf
+SELECT lower(name) FROM tab_settings_flags WHERE NOT not_in_sample EXCEPT
+SELECT regexp_replace(ln, '^#?([_[:alpha:]]+) (= .*|[^ ]*$)', '\1') AS guc
+FROM (SELECT regexp_split_to_table(pg_read_file(:'filename'), '\n') AS ln) conf
+WHERE ln ~ '^#?[[:alpha:]]'
+ORDER BY 1;
+
+-- test that lines in postgresql.conf that look like GUCs are GUCs
+SELECT regexp_replace(ln, '^#?([_[:alpha:]]+) (= .*|[^ ]*$)', '\1') AS guc
+FROM (SELECT regexp_split_to_table(pg_read_file(:'filename'), '\n') AS ln) conf
+WHERE ln ~ '^#?[[:alpha:]]'
+EXCEPT SELECT lower(name) FROM tab_settings_flags WHERE NOT not_in_sample
+ORDER BY 1;

-- 
Justin

  7. Re: pgsql: Add TAP test to automate the equivalent of check_guc

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-02-11T17:34:31Z

    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

  8. Re: pgsql: Add TAP test to automate the equivalent of check_guc

    Michael Paquier <michael@paquier.xyz> — 2022-02-12T00:46:49Z

    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

  9. Re: pgsql: Add TAP test to automate the equivalent of check_guc

    Michael Paquier <michael@paquier.xyz> — 2022-02-12T00:49:47Z

    On Fri, Feb 11, 2022 at 12:34:31PM -0500, Tom Lane wrote:
> Justin Pryzby <pryzby@telsasoft.com> writes:
>> Or, is it okay to use ABS_SRCDIR?
> 
> Don't see why not --- other tests certainly do.

Another solution would be to rely on TESTDIR, which is always set as
of the Makefile invocations and vcregress.pl, but that's doomed for
VPATH builds as this points to the build directory, not the source
directory so we would not have access to the sample file.  The root
issue here is that we don't have access to top_srcdir when running the
TAP tests, if we want to keep this stuff as a TAP test.

libpq's regress.pl tweaks things by passing down a SRCDIR to take care
of the VPATH problem, but there is nothing in %ENV that points to the
source directory in the context of a TAP test by default, if we cannot
rely on pg_config to find where the sample file is located.  That's
different than this thread, but should we make an effort and expand
the data available here for TAP tests?

Hmm.  Relying on ABS_SRCDIR in the main test suite would be fine,
though.  It also looks that there would be nothing preventing the
addition of the three tests in the TAP test, as of three SQL queries
instead.  These had better be written with one or more CTEs, for
readability, at least, with the inner query reading the contents of
the sample file to grab the list of all the GUCs.

I guess that it would be better to revert the TAP test and rework all
that for the time being, then.
--
Michael

  10. Re: pgsql: Add TAP test to automate the equivalent of check_guc

    Michael Paquier <michael@paquier.xyz> — 2022-02-12T04:29:54Z

    On Sat, Feb 12, 2022 at 09:49:47AM +0900, Michael Paquier wrote:
> I guess that it would be better to revert the TAP test and rework all
> that for the time being, then.

And this part is done.
--
Michael

  11. Re: pgsql: Add TAP test to automate the equivalent of check_guc

    Christoph Berg <myon@debian.org> — 2022-02-12T08:49:18Z

    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

  12. Re: pgsql: Add TAP test to automate the equivalent of check_guc

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-02-12T16:23:23Z

    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

  13. Re: pgsql: Add TAP test to automate the equivalent of check_guc

    Christoph Berg <myon@debian.org> — 2022-02-12T16:31:55Z

    Re: Tom Lane
> 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?

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.

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.

Sorry for the rumbling,
Christoph

  14. Re: pgsql: Add TAP test to automate the equivalent of check_guc

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-02-12T16:50:00Z

    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

  15. Re: pgsql: Add TAP test to automate the equivalent of check_guc

    Christoph Berg <myon@debian.org> — 2022-02-12T17:40:06Z

    Re: Tom Lane
> 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?

It still returns a correct --includedir for client apps that need it.

(Unfortunately many need --includedir-server since that's where the
type OIDs live.)

Christoph

  16. Re: pgsql: Add TAP test to automate the equivalent of check_guc

    Andres Freund <andres@anarazel.de> — 2022-02-12T20:10:46Z

    Hi,

On 2022-02-12 13:29:54 +0900, Michael Paquier wrote:
> On Sat, Feb 12, 2022 at 09:49:47AM +0900, Michael Paquier wrote:
> > I guess that it would be better to revert the TAP test and rework all
> > that for the time being, then.
> 
> And this part is done.

You wrote in the commit message that:

"However, this is also an issue as a TAP test only offers the build directory
as of TESTDIR in the environment context, so this would fail with VPATH
builds."

Tap tests currently are always executed in the source directory above t/, as
far as I can tell. So I don't think VPATH/non-VPATH or windows / non-windows
would break using a relative reference from the test dir.

Greetings,

Andres Freund

  17. Re: pgsql: Add TAP test to automate the equivalent of check_guc

    Michael Paquier <michael@paquier.xyz> — 2022-02-14T02:03:44Z

    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

  18. Re: pgsql: Add TAP test to automate the equivalent of check_guc

    Michael Paquier <michael@paquier.xyz> — 2022-02-14T05:11:37Z

    On Sat, Feb 12, 2022 at 12:10:46PM -0800, Andres Freund wrote:
> Tap tests currently are always executed in the source directory above t/, as
> far as I can tell. So I don't think VPATH/non-VPATH or windows / non-windows
> would break using a relative reference from the test dir.

That would mean to rely on $ENV{PWD} for this job, as of something
like that:
-# 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';
+my $rootdir = $ENV{PWD} . "/../../../..";
+my $sample_file = "$rootdir/src/backend/utils/misc/postgresql.conf.sample";

A run through the CI shows that this is working, and it also works
with Debian's patch for the config data.  I still tend to prefer the
readability of a TAP test over the main regression test suite for this
facility.  Even if we could use the same trick with PG_ABS_SRCDIR, I'd
rather not add this kind of dependency with a file in the source
tree in src/test/regress/.  Do others have any opinions on the matter?
--
Michael

  19. Re: pgsql: Add TAP test to automate the equivalent of check_guc

    Michael Paquier <michael@paquier.xyz> — 2022-02-16T02:23:17Z

    On Mon, Feb 14, 2022 at 02:11:37PM +0900, Michael Paquier wrote:
> A run through the CI shows that this is working, and it also works
> with Debian's patch for the config data.  I still tend to prefer the
> readability of a TAP test over the main regression test suite for this
> facility.

Hearing nothing, I have looked once again at this stuff and tested it
with what Debian was doing.  And that looks to work fine for
everything we care about, so applied with the use of a relative path
to find postgresql.conf.sample in the source tree.
--
Michael

  20. Re: pgsql: Add TAP test to automate the equivalent of check_guc

    Christoph Berg <myon@debian.org> — 2022-02-16T12:38:51Z

    Re: Michael Paquier
> Hearing nothing, I have looked once again at this stuff and tested it
> with what Debian was doing.  And that looks to work fine for
> everything we care about, so applied with the use of a relative path
> to find postgresql.conf.sample in the source tree.

The build works with the "Add TAP test to automate the equivalent of
check_guc, take two" commit. Thanks!

(Tests are still failing for
https://www.postgresql.org/message-id/YgjwrkEvNEqoz4Vm%40msg.df7cb.de
though)

Christoph

  21. Re: pgsql: Add TAP test to automate the equivalent of check_guc

    Michael Paquier <michael@paquier.xyz> — 2022-02-17T00:24:23Z

    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..
--
Michael

  22. Re: pgsql: Add TAP test to automate the equivalent of check_guc

    Robert Haas <robertmhaas@gmail.com> — 2022-02-17T00:39:26Z

    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

  23. Re: pgsql: Add TAP test to automate the equivalent of check_guc

    Michael Paquier <michael@paquier.xyz> — 2022-02-17T00:48:12Z

    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

  24. Re: pgsql: Add TAP test to automate the equivalent of check_guc

    Christoph Berg <myon@debian.org> — 2022-04-15T14:49:28Z

    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

  25. Re: pgsql: Add TAP test to automate the equivalent of check_guc

    Michael Paquier <michael@paquier.xyz> — 2022-04-16T01:02:51Z

    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