Hi, On 2022-03-09 13:37:23 +0100, Peter Eisentraut wrote: > I looked at this today mainly to consider whether some of the prereq > work is ready for adoption now.
Thanks! > 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. A big part of it is that when building with ninja tools are invoked in the top-level build directory, but right now a bunch of our scripts put their output in CWD. > 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 I think it should be doable to add something roughly like the current distprep. The cleanest way would be to use https://mesonbuild.com/Reference-manual_builtin_meson.html#mesonadd_dist_script to copy the files into the generated tarball. Of course not adding it would be even easier ;) > For the short term, I think the patches 0002, 0008, 0010, and 0011 > could be adopted, if they are finished as described. Cool. > Patch 0007 seems unrelated, or at least independently significant, and > should be discussed separately. It's related - it saves us from doing a lot of extra complexity on windows. I've brought it up as a separate thread too: https://postgr.es/m/20211101020311.av6hphdl6xbjbuif%40alap3.anarazel.de I'm currently a bit stuck implementing this properly for the configure / make system, as outlined in: https://www.postgresql.org/message-id/20220111025328.iq5g6uck53j5qtin%40alap3.anarazel.de > 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. Yep, I posted it before "7cf085f077d - Add support for zstd base backup compression." went in, but after 6c417bbcc8f. So the meson build knew about the zstd dependency, but didn't yet specify it for postgres / pg_basebackup. So all that's needed was / is adding the dependency to those two places. Updated patches attached. This just contains the fix for this issue, doesn't yet address review comments. FWIW, I'd already pushed those fixes out to the git tree. There's frequent enough small changes that reposting everytime seems too noisy. > 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. I'll try to clean it up. > 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. See above. > 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. I'll try it out. > v6-0009-prereq-make-unicode-targets-work-in-vpath-builds.patch.gz > > Another directory issue I think it's a tad different, in that it's fixing something that's currently broken in VPATH builds. > 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? Right now if you run the ldap tests on windows, openbsd, ... the tests fail. The only reason it doesn't cause trouble on the buildfarm is that we currently don't run those tests by default... > 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. It's where the files are in the package :/. > 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. Good idea. Greetings, Andres Freund
v7-0001-meson-prereq-output-and-depencency-tracking-work.patch.gz
Description: application/patch-gzip
bina5C1JO0TT2.bin
Description: application/patch-gzip
binPaf6qrD3cX.bin
Description: application/patch-gzip
binPlektQMp7A.bin
Description: application/patch-gzip
binLzdTk8dWyw.bin
Description: application/patch-gzip
bin3xDqa2rrlV.bin
Description: application/patch-gzip
binjM2QtSW2aD.bin
Description: application/patch-gzip
binyceI74Zu3k.bin
Description: application/patch-gzip
v7-0009-prereq-make-unicode-targets-work-in-vpath-builds.patch.gz
Description: application/patch-gzip
biniNtNv6vcUb.bin
Description: application/patch-gzip
v7-0011-ldap-tests-Add-paths-for-openbsd.patch.gz
Description: application/patch-gzip
v7-0012-wip-split-TESTDIR-into-two.patch.gz
Description: application/patch-gzip
v7-0013-meson-Add-meson-based-buildsystem.patch.gz
Description: application/patch-gzip
v7-0014-meson-ci-Build-both-with-meson-and-as-before.patch.gz
Description: application/patch-gzip
