Re: [RFC] building postgres with meson - v11 - Mailing list pgsql-hackers

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