Re: [RFC] building postgres with meson - v11

Hi,

On 2022-08-24 11:39:06 +0200, Peter Eisentraut wrote:
> 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.

How is that happening with that version of the patch? The test puts
tap_comp_dir under TESTDIR, and TESTDIR is $(CURDIR)/tmp_check. There was an
earlier version of the patch that was split one more time that did have that
problem, but I don't quite see how that version has it?

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

Thanks.

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

Hm, to me the indentation as is makes more sense, but ...

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

I wonder if we should rewrite this in python - I chose perl because I thought
we could share it, but as you pointed out, that's not possible, because we
don't want to depend on perl during the autoconf build from a tarball.

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

Cool. I looked at this because I was confused about getting warnings with
autoconf that I wasn't getting with meson.

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

The current docs, including the windows ones, are already hard to follow. I
think we should take some care to not make the meson bits even more
confusing. Cross referencing left and right seems problematic from that angle.

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

At the moment creation of the tmp_install is its own test suite. I don't know
if that's the best way, or what the best way is, but that explains that
fact. You can do the above without the issue by specifying
--suite setup --suite recovery.

Greetings,

Andres Freund