Re: [RFC] building postgres with meson -v6

On 08.03.22 03:56, Andres Freund wrote:
> Attached is v6 of the meson patchset. There are a lots of changes since the
> last version posted. These include:
> - python2 removal is now committed, so not needed in here anymore
> - CI changed to be based on the CI now merged into postgres
> - CI also tests suse, rhel, fedora (Nazir Bilal Yavuz). Found several bugs. I
> don't think we'd merge all of those, but while working on the meson branch,
> it's really useful.
> - all dependencies, except for pl/tcl (should be done soon)
> - several missing options added (segsize, extra_{lib,include}_dirs, enable-tap-tests
> - several portability fixes, builds on net/openbsd without changes now
> - improvements to a number of "configure" tests
> - lots of ongoing rebasing changes
> - ...

I looked at this today mainly to consider whether some of the prereq
work is ready for adoption now. A lot of the work has to do with
making various scripts write the output to other directories. I
suspect this has something to do with how meson handles separate build
directories and how we have so far handled files created in the
distribution tarball. But the whole picture isn't clear to me.

More generally, I don't see a distprep target in the meson build
files. I wonder what your plan for that is, or whether that would
even work under meson. In [0], I argued for getting rid of the
distprep step. Perhaps it is time to reconsider that now.

[0]:
https://www.postgresql.org/message-id/flat/cf0bec33-d965-664d-e0ec-fb15290f2273%402ndquadrant.com

For the short term, I think the patches 0002, 0008, 0010, and 0011
could be adopted, if they are finished as described.

Patch 0007 seems unrelated, or at least independently significant, and
should be discussed separately.

The rest is really all part of the same
put-things-in-the-right-directory issue.

For the overall patch set, I did a quick test with

meson setup build
cd build
ninja

which failed with

Undefined symbols for architecture x86_64:
"_bbsink_zstd_new", referenced from:
_SendBaseBackup in replication_basebackup.c.o

So maybe your patch set is not up to date with this new zstd build
option.

Details:

v6-0001-meson-prereq-output-and-depencency-tracking-work.patch.gz

This all looks kind of reasonable, but lacks explanation in some
cases, so I can't fully judge it.

v6-0002-meson-prereq-move-snowball_create.sql-creation-in.patch.gz

Looks like a reasonable direction, would be good to deduplicate with
Install.pm.

v6-0003-meson-prereq-add-output-path-arg-in-generate-lwlo.patch.gz

Ok. Similar to 0001. (But unlike 0001, nothing in this patch
actually uses the new output dir option. That only comes in 0013.)

v6-0004-meson-prereq-add-src-tools-gen_versioning_script..patch.gz

This isn't used until 0013, and there it is patched again, so I'm not
sure if this is in the right position of this patch series.

v6-0005-meson-prereq-generate-errcodes.pl-accept-output-f.patch.gz

Also similar to 0001.

v6-0006-meson-prereq-remove-unhelpful-chattiness-in-snowb.patch.gz

Might as well include this into 0002.

v6-0007-meson-prereq-Can-we-get-away-with-not-export-all-.patch.gz

This is a separate discussion. It's not clear to me why this is part
of this patch series.

v6-0008-meson-prereq-Handle-DLSUFFIX-in-msvc-builds-simil.patch.gz

Part of this was already done in 0001, so check if these patches are
split correctly.

I think the right way here is actually to go the other way around:
Move DLSUFFIX into header files for all platforms. Move the DLSUFFIX
assignment from src/makefiles/ to src/templates/, have configure read
it, and then substitute it into Makefile.global and pg_config.h.

Then we also don't have to patch the Windows build code a bunch of
times to add the DLSUFFIX define everywhere.

There is code in configure already that would benefit from this, which
currently says

# We don't know the platform DLSUFFIX here, so check 'em all.

v6-0009-prereq-make-unicode-targets-work-in-vpath-builds.patch.gz

Another directory issue

v6-0010-ldap-tests-Don-t-run-on-unsupported-operating-sys.patch.gz

Not sure what this is supposed to do, but it looks independent of this
patch series. Does it currently not work on "unsupported" operating
systems?

v6-0011-ldap-tests-Add-paths-for-openbsd.patch.gz

The more the merrier, although I'm a little bit worried about pointing
to a /usr/local/share/examples/ directory.

v6-0012-wip-split-TESTDIR-into-two.patch.gz
v6-0013-meson-Add-meson-based-buildsystem.patch.gz
v6-0014-meson-ci-Build-both-with-meson-and-as-before.patch.gz

I suggest in the interim to add a README.meson to show how to invoke
this. Eventually, of course, we'd rewrite the installation
instructions.