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