Re: [RFC] building postgres with meson - v11

From: Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: pgsql-hackers(at)postgresql(dot)org, samay sharma <smilingsamay(at)gmail(dot)com>, Nazir Bilal Yavuz <byavuz81(at)gmail(dot)com>
Subject: Re: [RFC] building postgres with meson - v11
Date: 2022-08-24 09:39:06
Message-ID: 7dae5979-c6c0-cec5-7a36-76a85aa8053d@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I have looked at your branch at 0545eec895:

258f6dc0a7 Don't hardcode tmp_check/ as test directory for tap tests
8ecc33cf04 Split TESTDIR into TESTLOGDIR and TESTDATADIR

I think these patches are split up a bit incorrectly. If you apply
the first patch by itself, then the output appears in tab_comp_dir/
directly under the source directory. And then the second patch moves
it to tmp_check/tap_comp_dir/. If there is an intent to apply these
patches separately somehow, this should be cleaned up.

I haven't checked the second patch in detail yet, but it looks like
the thought was that the first patch is about ready to go.

834a40e609 meson: prereq: Extend gendef.pl in preparation for meson

I'm not qualified to check that in detail, but it looks reasonable
enough to me.

See attached patch (0001) for a perlcritic fix.

97a0b096e8 meson: prereq: Add src/tools/gen_export.pl

This produces leading whitespace in the output files that at least on
darwin wasn't there before. See attached patch (0002). This should
be checked again on other platforms as well.

Other than that this looks good. Attached is a small cosmetic patch (0003).

40e363b263 meson: prereq: Refactor PG_TEST_EXTRA logic in autoconf build

Since I last looked, this has been turned into a meson option. Which
is probably the best solution. But then we should probably make this
a configure option as well. Otherwise, it could get a bit confusing.
For example, I just unset PG_TEST_EXTRA in my environment to test
something with the meson build, but I was unaware that meson captures
the value at setup time, so my unsetting had no effect.

In any case, maybe adjust the regular expressions to check for word
boundaries, to maintain the original "whitespace-separated"
specification. For example,

elsif ($ENV{PG_TEST_EXTRA} !~ /\bssl\b/)

e0a8387660 solaris: Use versioning scripts instead of -Bsymbolic

This looks like a good idea. The documentation clearly states that
-Bsymbolic shouldn't be used, at least not in the way we have been
doing. Might as well go ahead with this and give it a whirl on the
build farm.

0545eec895 meson: Add docs

We should think more about how to arrange the documentation. We
probably don't want to copy-and-paste all the introductory and
requirements information. I think we can make this initially much
briefer, like the Windows installation chapter. For example, instead
of documenting each setup option again, just mention which ones exist
and then point (link) to the configure chapter for details.

I spent a bit of time with the test suites. I think there is a
problem in that selecting a test suite directly, like

meson test -C _build --suite recovery

doesn't update the tmp_install. So if this is the first thing you run
after a build, everything will fail. Also, if you run this later, the
tmp_install doesn't get updated, so you're not testing up-to-date
code.

Attachment Content-Type Size
0001-Fix-for-perlcritic.patch text/plain 585 bytes
0002-Fix-whitespace-in-output-of-gen_export.pl.patch text/plain 643 bytes
0003-Some-Perl-code-simplification-in-gen_export.pl.patch text/plain 681 bytes

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Егор Чиндяскин 2022-08-24 09:51:12 Stack overflow issue
Previous Message Peter Smith 2022-08-24 09:06:13 Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher