Re: [RFC] building postgres with meson - v11

[samay sharma]

On Tue, Sep 6, 2022 at 9:46 PM Peter Eisentraut <
peter.eisentr...@enterprisedb.com> wrote:

> On 02.09.22 01:12, samay sharma wrote:
> > So, I was thinking of the following structure:
> > - Supported Platforms
> > - Getting the Source
> > - Building with make and autoconf
> >-- Short version
> >-- Requirements
> >-- Installation Procedure and it's subsections
> > - Building with Meson
> >-- Short version
> >-- Requirements
> >-- Installation Procedure and it's subsections
> > - Post-installation Setup
> > - Platform specific notes
>
> I like that.
>

Attached is a docs-only patch with that structure. We need to update the
platform specific notes section to add meson specific nuances. Also, in
terms of supported platforms, if there are platforms which work with make
but not with meson, we have to add that too.

Regards,
Samay

>
> > As a follow up patch, we could also try to fit the Windows part into
> > this model. We could add a Building with visual C++ or Microsoft windows
> > SDK section. It doesn't have a short version but follows the remaining
> > template of requirements and installation procedure subsections
> > (Building, Cleaning and Installing and Running Regression tests) well.
>
> We were thinking about removing the old Windows build system for PG 16.
> Let's see how that goes.  Otherwise, yes, that would be good as well.
>


v1-0001-Add-meson-docs-to-existing-build-from-source.patch
Description: Binary data

Re: [RFC] building postgres with meson - v11

[samay sharma]

Hi,

On Tue, Sep 6, 2022 at 9:48 PM Peter Eisentraut <
peter.eisentr...@enterprisedb.com> wrote:

> On 02.09.22 19:16, samay sharma wrote:
> > Another thing I'd like input on. A common question I've heard from
> > people who've tried out the docs is How do we do the equivalent of X in
> > make with meson. As meson will be new for a bunch of people who are
> > fluent with make, I won't be surprised if this is a common ask. To
> > address that, I was planning to add a page to specify the key things one
> > needs to keep in mind while "migrating" from make to meson and having a
> > translation table of commonly used commands.
> >
> > I was planning to add it in the meson section, but if we go ahead with
> > the structure proposed above, it doesn't fit it into one as cleanly.
> > Maybe, it still goes in the meson section? Thoughts?
>
> This could go into the wiki.
>
> For example, we have
> , which was added
> during the CVS->Git transition.
>

That's a good idea. I'll add a page to the wiki about this topic and share
it on the list for review.


>
> This avoids that we make the PostgreSQL documentation a substitute
> manual for a third-party product.
>

Regards,
Samay

Re: [RFC] building postgres with meson - v11

[Peter Eisentraut]

On 02.09.22 19:16, samay sharma wrote:
Another thing I'd like input on. A common question I've heard from 
people who've tried out the docs is How do we do the equivalent of X in 
make with meson. As meson will be new for a bunch of people who are 
fluent with make, I won't be surprised if this is a common ask. To 
address that, I was planning to add a page to specify the key things one 
needs to keep in mind while "migrating" from make to meson and having a 
translation table of commonly used commands.


I was planning to add it in the meson section, but if we go ahead with 
the structure proposed above, it doesn't fit it into one as cleanly. 
Maybe, it still goes in the meson section? Thoughts?


This could go into the wiki.

For example, we have 
, which was added 
during the CVS->Git transition.


This avoids that we make the PostgreSQL documentation a substitute 
manual for a third-party product.

Re: [RFC] building postgres with meson - v11

[Peter Eisentraut]

On 02.09.22 01:12, samay sharma wrote:

So, I was thinking of the following structure:
- Supported Platforms
- Getting the Source
- Building with make and autoconf
   -- Short version
   -- Requirements
   -- Installation Procedure and it's subsections
- Building with Meson
   -- Short version
   -- Requirements
   -- Installation Procedure and it's subsections
- Post-installation Setup
- Platform specific notes


I like that.

As a follow up patch, we could also try to fit the Windows part into 
this model. We could add a Building with visual C++ or Microsoft windows 
SDK section. It doesn't have a short version but follows the remaining 
template of requirements and installation procedure subsections 
(Building, Cleaning and Installing and Running Regression tests) well.


We were thinking about removing the old Windows build system for PG 16. 
Let's see how that goes.  Otherwise, yes, that would be good as well.

Re: [RFC] building postgres with meson - v11

[samay sharma]

On Thu, Sep 1, 2022 at 4:12 PM samay sharma  wrote:

> Hi,
>
> On Wed, Aug 31, 2022 at 1:42 AM Peter Eisentraut <
> peter.eisentr...@enterprisedb.com> wrote:
>
>> On 24.08.22 17:30, Andres Freund wrote:
>> >> 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.
>>
>> If you look at the current structure of the installation chapter
>>
>> 17.1. Short Version
>> 17.2. Requirements
>> 17.3. Getting the Source
>> 17.4. Installation Procedure
>> 17.5. Post-Installation Setup
>> 17.6. Supported Platforms
>> 17.7. Platform-Specific Notes
>>
>> only 17.1, small parts of 12.2, and 17.4 should differ between make and
>> meson.  There is no conceivable reason why the meson installation
>> chapter should have a different "Getting the Source" section.  And some
>> of the post-installation and platform-specific information doesn't
>> appear at all on the meson chapter.
>>
>> I think we can try to be a bit more ingenious in how we weave this
>> together in the best way.  What I really wouldn't want is two separate
>> chapters that duplicate the entire process.  I think we could do one
>> chapter, like
>>
>> - Short Version
>> - Requirements
>> - Getting the Source
>> - Installation Procedure
>> - Installation Procedure using Meson
>> - Post-Installation Setup
>> - Supported Platforms
>> - Platform-Specific Notes
>>
>
> I spent some more time thinking about the structure of the docs. The
> getting the source, supported platforms, post installation setup and
> platform specific notes sections are going to be mostly common. We do
> expect some differences in supported platforms and platform specific notes
> but I think they should be manageable without confusing readers.
>
> The others; short version, requirements, and installation procedure are
> pretty different and I feel combining them will end up confusing readers or
> require creating autoconf / make and meson versions of many things at many
> different places. Also, if we keep it separate, it'll be easier to remove
> make / autoconf specific sections if (when?) we want to do that.
>
> So, I was thinking of the following structure:
> - Supported Platforms
> - Getting the Source
> - Building with make and autoconf
>   -- Short version
>   -- Requirements
>   -- Installation Procedure and it's subsections
> - Building with Meson
>   -- Short version
>   -- Requirements
>   -- Installation Procedure and it's subsections
> - Post-installation Setup
> - Platform specific notes
>
> It has the disadvantage of short version moving to a bit later in the
> chapter but I think it's a good structure to reduce duplication and also
> keep sections which are different separate. Thoughts on this approach? If
> this looks good, I can submit a patch rearranging things this way.
>

Another thing I'd like input on. A common question I've heard from people
who've tried out the docs is How do we do the equivalent of X in make with
meson. As meson will be new for a bunch of people who are fluent with make,
I won't be surprised if this is a common ask. To address that, I was
planning to add a page to specify the key things one needs to keep in mind
while "migrating" from make to meson and having a translation table of
commonly used commands.

I was planning to add it in the meson section, but if we go ahead with the
structure proposed above, it doesn't fit it into one as cleanly. Maybe, it
still goes in the meson section? Thoughts?

Regards,
Samay


>
> As a follow up patch, we could also try to fit the Windows part into this
> model. We could add a Building with visual C++ or Microsoft windows SDK
> section. It doesn't have a short version but follows the remaining template
> of requirements and installation procedure subsections (Building, Cleaning
> and Installing and Running Regression tests) well.
>
> Regards,
> Samay
>
>>
>> Alternatively, if people prefer two separate chapters, let's think about
>> some source-code level techniques to share the common contents.
>>
>

Re: [RFC] building postgres with meson - v11

[samay sharma]

Hi,

On Wed, Aug 31, 2022 at 1:42 AM Peter Eisentraut <
peter.eisentr...@enterprisedb.com> wrote:

> On 24.08.22 17:30, Andres Freund wrote:
> >> 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.
>
> If you look at the current structure of the installation chapter
>
> 17.1. Short Version
> 17.2. Requirements
> 17.3. Getting the Source
> 17.4. Installation Procedure
> 17.5. Post-Installation Setup
> 17.6. Supported Platforms
> 17.7. Platform-Specific Notes
>
> only 17.1, small parts of 12.2, and 17.4 should differ between make and
> meson.  There is no conceivable reason why the meson installation
> chapter should have a different "Getting the Source" section.  And some
> of the post-installation and platform-specific information doesn't
> appear at all on the meson chapter.
>
> I think we can try to be a bit more ingenious in how we weave this
> together in the best way.  What I really wouldn't want is two separate
> chapters that duplicate the entire process.  I think we could do one
> chapter, like
>
> - Short Version
> - Requirements
> - Getting the Source
> - Installation Procedure
> - Installation Procedure using Meson
> - Post-Installation Setup
> - Supported Platforms
> - Platform-Specific Notes
>

I spent some more time thinking about the structure of the docs. The
getting the source, supported platforms, post installation setup and
platform specific notes sections are going to be mostly common. We do
expect some differences in supported platforms and platform specific notes
but I think they should be manageable without confusing readers.

The others; short version, requirements, and installation procedure are
pretty different and I feel combining them will end up confusing readers or
require creating autoconf / make and meson versions of many things at many
different places. Also, if we keep it separate, it'll be easier to remove
make / autoconf specific sections if (when?) we want to do that.

So, I was thinking of the following structure:
- Supported Platforms
- Getting the Source
- Building with make and autoconf
  -- Short version
  -- Requirements
  -- Installation Procedure and it's subsections
- Building with Meson
  -- Short version
  -- Requirements
  -- Installation Procedure and it's subsections
- Post-installation Setup
- Platform specific notes

It has the disadvantage of short version moving to a bit later in the
chapter but I think it's a good structure to reduce duplication and also
keep sections which are different separate. Thoughts on this approach? If
this looks good, I can submit a patch rearranging things this way.

As a follow up patch, we could also try to fit the Windows part into this
model. We could add a Building with visual C++ or Microsoft windows SDK
section. It doesn't have a short version but follows the remaining template
of requirements and installation procedure subsections (Building, Cleaning
and Installing and Running Regression tests) well.

Regards,
Samay

>
> Alternatively, if people prefer two separate chapters, let's think about
> some source-code level techniques to share the common contents.
>

Re: [RFC] building postgres with meson - v11

[Peter Eisentraut]

On 24.08.22 17:30, Andres Freund wrote:

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.


If you look at the current structure of the installation chapter

17.1. Short Version
17.2. Requirements
17.3. Getting the Source
17.4. Installation Procedure
17.5. Post-Installation Setup
17.6. Supported Platforms
17.7. Platform-Specific Notes

only 17.1, small parts of 12.2, and 17.4 should differ between make and 
meson.  There is no conceivable reason why the meson installation 
chapter should have a different "Getting the Source" section.  And some 
of the post-installation and platform-specific information doesn't 
appear at all on the meson chapter.


I think we can try to be a bit more ingenious in how we weave this 
together in the best way.  What I really wouldn't want is two separate 
chapters that duplicate the entire process.  I think we could do one 
chapter, like


- Short Version
- Requirements
- Getting the Source
- Installation Procedure
- Installation Procedure using Meson
- Post-Installation Setup
- Supported Platforms
- Platform-Specific Notes

Alternatively, if people prefer two separate chapters, let's think about 
some source-code level techniques to share the common contents.

Re: [RFC] building postgres with meson - v11

[Peter Eisentraut]

On 24.08.22 17:30, Andres Freund wrote:

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?


Ok, I see now how this works.  It's a bit weird since the meaning of 
TESTDIR is changed.  I'm not sure if this could create cross-branch 
confusion.



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


Maybe for the 'gnu' format, but on darwin (and others) it's just a flat 
list, so indenting it is pointless.



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.


Given that the code is already written, I wouldn't do it.  Introducing 
Python into the toolchain would require considerations of minimum 
versions, finding the right binaries, formatting, style checking, etc., 
which would probably be a distraction right now.  I also think that this 
script, whose purpose is to read an input file line by line and print it 
back out slightly differently, is not going to be done better in Python.

Re: [RFC] building postgres with meson - v11

[samay sharma]

Hi,

On Wed, Aug 24, 2022 at 8:30 AM Andres Freund  wrote:

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

On Configure options:

To add to the above, very few sections are an exact copy paste. The
arguments and default behaviors of quite a few configure options are
different. The change in default behavior and arguments is primarily due to
"auto" features which get enabled if the dependencies are found. Whereas
with make, we have explicit --enable or --disable options which don't take
any arguments.

Also, a few instructions / commands which worked with make will need to be
done a bit differently due to environment variables etc. which also had to
be communicated.

Communicating these differences and nuances with cross referencing would
make it confusing as most of this information is in the explanation
paragraph.

On requirements:

They are also a bit different eg. readline is not a "required" thing
anymore, perl, flex, bison are required etc. Also, these are bullet points
with information inlined and not separate sections, so cross-referencing
here also would be hard.

Regards,
Samay


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

Re: [RFC] building postgres with meson - v11

[Andres Freund]

Hi,

On 2022-08-17 14:53:17 -0700, Andres Freund wrote:
> > - In the top-level meson.build, the "renaming" of the Windows system
> >   name
> >
> >   host_system = host_machine.system() == 'windows' ? 'win32' :
> > host_machine.system()
> >   build_system = build_machine.system() == 'windows' ? 'win32' :
> > build_machine.system()
> >
> >   seems unnecessary to me.  Why not stick with the provided names?
> 
> Because right now we also use it for things like choosing the "source" for
> pg_config_os.h (i.e. include/port/{darwin,linux,win32,..}.h). And it seemed
> easier to just have one variable name for all of it.

I am now changing this so that there's an additional 'portname' variable for
this purpose. Otherwise the meson names are used.

Greetings,

Andres Freund

Re: [RFC] building postgres with meson - v11

[Andres Freund]

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

Re: [RFC] building postgres with meson - v11

[Peter Eisentraut]

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.From 2f25b48271bceb7aa1551e015b03fc20b9aff162 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Wed, 24 Aug 2022 11:23:52 +0200
Subject: [PATCH 1/3] Fix for perlcritic

---
 src/tools/msvc/gendef.pl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/tools/msvc/gendef.pl b/src/tools/msvc/gendef.pl
index f08268e781..cfbdef9007 100644
--- a/src/tools/msvc/gendef.pl
+++ b/src/tools/msvc/gendef.pl
@@ -174,7 +174,7 @@ sub usage
 {
if (-d $in)
{
-   push @files, <$in/*.obj>;
+   push @files, glob "$in/*.obj";
}
else
{
-- 
2.37.1

From a64c90e756d6996b7d8d9d63d42e30c72d4ce098 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Wed, 24 Aug 2022 11:24:22 +0200
Subject: [PATCH 2/3] Fix whitespace in output of gen_export.pl

---
 src/tools/gen_export.pl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/tools/gen_export.pl b/src/tools/gen_export.pl
index 6a8b196ad8..1265564473 100644
--- a/src/tools/gen_export.pl
+++ b/src/tools/gen_export.pl
@@ -56,7 +56,7 @@
}
elsif ($format eq 'darwin')
{
-   print $output_handle "_$1\n";
+   print $output_handle "_$1\n";
}
elsif ($format eq 'gnu')
{
-- 
2.37.1

From 5b79baf6bbf4fa62f153a5e96e97f8d5a6345821 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Wed, 24 Aug 2022 11:24:36 +0200
Subject: [PATCH 3/3] Some Perl code simplification in gen_export.pl

---
 src/tools/gen_export.pl | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/tools/gen_export.pl b/src/tools/gen_export.pl
index 1265564473..727105ba08 100644
--- a/src/tools/gen_export.pl
+++ b/src/tools/gen_export.pl
@@ -48,7 +48,7 @@
{
# don't do anything with a comment
}
-   elsif (/^([^\s]+)\s+([^\s]+)/)
+   elsif (/^(\S+)\s+(\S+)/)
{
if ($format eq 'aix')
{
@@ -79,5 +79,3 @@
 };
 ";
 }
-
-exit(0);
-- 
2.37.1

Re: [RFC] building postgres with meson - v11

[Andres Freund]

Hi,

On 2022-08-20 09:38:48 +0200, Peter Eisentraut wrote:
> On 17.08.22 23:53, Andres Freund wrote:
> > Any comment on the pg_regress_ecpg commit? I'd like to get that out of the
> > way, and it seems considerably cleaner than the hackery we do right now to
> > make VPATH builds work.
> 
> That one looks like a very good improvement.

Thanks for checking! Pushed.

Greetings,

Andres Freund

Re: [RFC] building postgres with meson - v11

[Peter Eisentraut]

On 17.08.22 23:53, Andres Freund wrote:

Any comment on the pg_regress_ecpg commit? I'd like to get that out of the
way, and it seems considerably cleaner than the hackery we do right now to
make VPATH builds work.


That one looks like a very good improvement.

Re: [RFC] building postgres with meson - v11

[Andres Freund]

Hi,

On 2022-08-17 15:50:23 +0200, Peter Eisentraut wrote:
> - There are various references to "pch" (pre-compiled headers).  Is
>   there more discussion anywhere about this?  I don't know what this
>   would entail or whether there are any drawbacks to be aware of.  The
>   new *_pch.h files don't have any comments.  Maybe this should be a
>   separate patch later.

It's mainly to make windows builds a bit slower. I've no objection to
separating this out.


> - About relativize_shared_library_references: We have had several
>   patches over the years for working around SIP stuff, and some of
>   them did essentially this, but we decided not to go ahead with them.
>   We could revisit that, but it should be a separate patch, not mixed
>   in with this.

The prior approaches all had issues because they didn't support relative
references IIRC (and thus broke being able to relocate the installation),
which this does.

I just found it very annoying to work on macs without this. And there were at
least two "bug" reports of testers of the meson branch that were just due to
SIP.

I'm ok with splitting it out, but I also think it's a lower risk opportunity
to test that this works.


> - postgresql-extension.pc: Similarly, this ought to be a separate
>   patch.  If we want people to use this, we'll need it in the makefile
>   build system anyway.

Makes sense. I'd like to keep it in the same patch for a short while longer,
to deduplicate some of the code, but then will split it out.


> - -DFRONTEND is used somewhat differently from the makefiles.  For
>example, meson sets -DFRONTEND for pg_controldata, but the
>makefiles don't.  Conversely, the makefiles set -DFRONTEND for
>ecpglib, but meson does not.  This should be checked again to make
>sure it all matches up.

Yes, should sync that up.

FWIW, meson does add -DFRONTEND for ecpglib. There were a few places that did
add it twice, I'll push a cleanup of that in a bit.


> - Option name spelling should be make consistent about underscores
>   versus hyphens.  Built-in meson options use underscores, so we
>   should make the user-defined ones like that as well (some already
>   do).  (wal-blocksize krb-srvnam system-tzdata tap-tests bsd-auth)

No objection.


> - I have found the variable name "cdata" for configuration_data() to
>   be less than clear.  I see some GNOME projects to it that way, is
>   that where it's from?  systemd uses "conf", maybe that's better.

I don't know where it's from - I don't think I ever looked at gnome
buildsystem stuff. It seems to be the obvious abbreviation for
configuration_data()...  I don't object to conf, but it's not a clear
improvement to me.


> - In the top-level meson.build, the "renaming" of the Windows system
>   name
>
>   host_system = host_machine.system() == 'windows' ? 'win32' :
> host_machine.system()
>   build_system = build_machine.system() == 'windows' ? 'win32' :
> build_machine.system()
>
>   seems unnecessary to me.  Why not stick with the provided names?

Because right now we also use it for things like choosing the "source" for
pg_config_os.h (i.e. include/port/{darwin,linux,win32,..}.h). And it seemed
easier to just have one variable name for all of it.


> - The c99_test ought to be not needed if the c_std project option is
>   used.  Was there a problem with that?

We don't want to force -std=c99 when not necessary, I think. We sometimes use
features from newer (and from gnu) language versions after testing
availability, and if we hardcode the version those will either fail or elicit
warnings.


> - Is there a way to split up the top-level meson.build somehow?  Maybe
>   just throw some stuff into included files?  This might get out of
>   hand at some point.

We can put stuff into config/meson.build or such. But I don't think it's
clearly warranted at this point.


> - The PG_SYSROOT detection gives different results.  On my system,
>   configure produces
>
> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.3.sdk,
>   meson produces
>
> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk.
>   src/template/darwin goes out of its way to get a version-specific
>   result, so we need to carry that over somehow.  (The difference does
>   result in differences in the built binaries.)

TBH, I don't really understand the SYSROOT stuff all that well, never having
used a mac in anger (well, only in anger, but ...).

What do you think about extracting the relevant portion of src/template/darwin
into a dedicated shell script that gets called by both?


> Then, some patches from me:
>
> 0001-Change-shared-library-installation-naming-on-macOS.patch
>
> This changes the makefiles to make the shared library file naming on
> macOS match what meson produces.  I don't know what the situation is
> on other platforms.

No opinion on the matter. Seems best to apply separately if we want to?


> 

Re: [RFC] building postgres with meson - v11

[Peter Eisentraut]

On 11.08.22 02:20, Andres Freund wrote:

Attached is a new version of the meson patchset. Plenty changes:


I have various bits of comments on this.

- There are various references to "pch" (pre-compiled headers).  Is
  there more discussion anywhere about this?  I don't know what this
  would entail or whether there are any drawbacks to be aware of.  The
  new *_pch.h files don't have any comments.  Maybe this should be a
  separate patch later.

- About relativize_shared_library_references: We have had several
  patches over the years for working around SIP stuff, and some of
  them did essentially this, but we decided not to go ahead with them.
  We could revisit that, but it should be a separate patch, not mixed
  in with this.

- postgresql-extension.pc: Similarly, this ought to be a separate
  patch.  If we want people to use this, we'll need it in the makefile
  build system anyway.

- -DFRONTEND is used somewhat differently from the makefiles.  For
   example, meson sets -DFRONTEND for pg_controldata, but the
   makefiles don't.  Conversely, the makefiles set -DFRONTEND for
   ecpglib, but meson does not.  This should be checked again to make
   sure it all matches up.

- Option name spelling should be make consistent about underscores
  versus hyphens.  Built-in meson options use underscores, so we
  should make the user-defined ones like that as well (some already
  do).  (wal-blocksize krb-srvnam system-tzdata tap-tests bsd-auth)

- I have found the variable name "cdata" for configuration_data() to
  be less than clear.  I see some GNOME projects to it that way, is
  that where it's from?  systemd uses "conf", maybe that's better.

- In the top-level meson.build, the "renaming" of the Windows system
  name

  host_system = host_machine.system() == 'windows' ? 'win32' : 
host_machine.system()
  build_system = build_machine.system() == 'windows' ? 'win32' : 
build_machine.system()


  seems unnecessary to me.  Why not stick with the provided names?

- The c99_test ought to be not needed if the c_std project option is
  used.  Was there a problem with that?

- Is there a way to split up the top-level meson.build somehow?  Maybe
  just throw some stuff into included files?  This might get out of
  hand at some point.

- The PG_SYSROOT detection gives different results.  On my system,
  configure produces

/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.3.sdk,
  meson produces

/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk.
  src/template/darwin goes out of its way to get a version-specific
  result, so we need to carry that over somehow.  (The difference does
  result in differences in the built binaries.)


Then, some patches from me:

0001-Change-shared-library-installation-naming-on-macOS.patch

This changes the makefiles to make the shared library file naming on
macOS match what meson produces.  I don't know what the situation is
on other platforms.

0002-meson-Fix-installation-name-of-libpgfeutils.patch

This was presumably an accidental mistake.

0003-meson-Libraries-need-to-be-built-with-DSO_MAJOR_VERS.patch

This is needed to make NLS work for the libraries.

0004-meson-Add-darwin_versions-argument-for-libraries.patch

This is to make the output match what Makefile.shlib produces.

0005-meson-Fix-link-order-of-support-libraries.patch
0006-meson-Make-link-order-of-external-libraries-match-ma.patch
0007-WIP-meson-Make-link-order-of-object-files-match-make.patch

I have analyzed the produced binaries between both build systems to
make sure they match.  If we link the files and libraries in different
orders, that becomes difficult.  So this fixes this up a bit.  0005 is
needed for correctness in general, I think.  0006 is mostly cosmetic.
You probably wanted to make the library order alphabetical in the
meson files, which I'd support, but then we should change the
makefiles to match.  Similarly, 0007, which is clearly a bit messy at
the moment, but we should try to sort that out either in the old or
the new build files.


And finally some comments on your patches:

meson: prereq: Don't add HAVE_LDAP_H HAVE_WINLDAP_H to pg_config.h.

This can go ahead.

meson: prereq: fix warning compat_informix/rnull.pgc with msvc

-   $float f = 3.71;
+   $float f = (float) 3.71;

This could use float literals like

+   $float f = 3.71f;From 054b24e9ae5fc859f13c05d8150ef1168a477ce4 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Wed, 17 Aug 2022 14:18:52 +0200
Subject: [PATCH 1/7] Change shared library installation naming on macOS

It is not customary to install a shared library with a minor version
number (libpq.5.16.dylib) on macOS.  We just need the file with the
major version number (libpq.5.dylib) and the one without version
number (libpq.dylib).  This also matches the installation layout used
by Meson.
---
 src/Makefile.shlib | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git