Re: Documentation for building with meson

[vignesh C]

On Wed, 29 Mar 2023 at 00:57, samay sharma  wrote:
>
> Hi,
>
> On Wed, Mar 15, 2023 at 4:28 AM Peter Eisentraut 
>  wrote:
>>
>>  > [PATCH v8 1/5] Make minor additions and corrections to meson docs
>>
>> The last hunk revealed that there is some mixing up between meson setup
>> and meson configure.  This goes a bit further.  For example, earlier it
>> says that to get a list of meson setup options, call meson configure
>> --help and look at https://mesonbuild.com/Commands.html#configure, which
>> are both wrong.  Also later throughout the text it uses one or the
>> other.  I think this has the potential to be very confusing, and we
>> should clean this up carefully.
>>
>> The text about additional meson test options maybe should go into the
>> regress.sgml chapter?
>
>
> I tried to make the meson setup and meson configure usage consistent. I've 
> removed the text for the test options.
>>
>>
>>
>>  > [PATCH v8 2/5] Add data layout options sub-section in installation
>>   docs
>>
>> This makes sense.  Please double check your patch for correct title
>> casing, vertical spacing of XML, to keep everything looking consistent.
>
>
> Thanks for noticing. Made it consistent on both sides.
>>
>>
>> This text isn't yours, but since your patch emphasizes it, I wonder if
>> it could use some clarification:
>>
>> + These options affect how PostgreSQL lays out data on disk.
>> + Note that changing these breaks on-disk database compatibility,
>> + meaning you cannot use pg_upgrade to upgrade to
>> + a build with different values of these options.
>>
>> This isn't really correct.  What breaking on-disk compatibility means is
>> that you can't use a server compiled one way with a data directory
>> initialized by binaries compiled another way.  pg_upgrade may well have
>> the ability to upgrade between one or the other; that's up to pg_upgrade
>> to figure out but not an intrinsic property.  (I wonder why pg_upgrade
>> cares about the WAL block size.)
>
>
>  Fixed.
>>
>>
>>
>>  > [PATCH v8 3/5] Remove Anti-Features section from Installation from
>>   source docs
>>
>> Makes sense.  But is "--disable-thread-safety" really a developer
>> feature?  I think not.
>>
>
> Moved to PostgreSQL features. Do you think there's a better place for it?
>
>>
>>
>>  > [PATCH v8 4/5] Re-organize Miscellaneous section
>>
>> This moves the Miscellaneous section after Developer Features.  I think
>> Developer Features should be last.
>>
>> Maybe should remove this section and add the options to the regular
>> PostgreSQL Features section.
>
>
> Yes, that makes sense. Made this change.
>>
>>
>> Also consider the grouping in meson_options.txt, which is slightly
>> different yet.
>
>
> Removed Misc options section from meson_options.txt too.
>>
>>
>>
>>  > [PATCH v8 5/5] Change Short Version for meson installation guide
>>
>> +# create working directory
>> +mkdir postgres
>> +cd postgres
>> +
>> +# fetch source code
>> +git clone https://git.postgresql.org/git/postgresql.git src
>>
>> This comes after the "Getting the Source" section, so at this point they
>> already have the source and don't need to do "git clone" etc. again.
>>
>> +# setup and enter build directory (done only first time)
>> +## Unix based platforms
>> +meson setup build src --prefix=$PWD/install
>> +
>> +## Windows
>> +meson setup build src --prefix=%cd%/install
>>
>> Maybe some people work this way, but to me the directory structures you
>> create here are completely weird.
>
>
> I'd like to discuss what you think is a good directory structure to work 
> with. I've mentioned some of the drawbacks I see with the current structure 
> for the short version. I know this structure can feel different but it 
> feeling weird is not ideal. Do you have a directory structure in mind which 
> is different but doesn't feel odd to you?
>
>>
>>
>> +# Initialize a new database
>> +../install/bin/initdb -D ../data
>> +
>> +# Start database
>> +../install/bin/pg_ctl -D ../data/ -l logfile start
>> +
>> +# Connect to the database
>> +../install/bin/psql -d postgres
>>
>> The terminology here needs to be tightened up.  You are using "database"
>> here to mean three different things.
>
>
> I'll address this together once we are aligned on the overall directory 
> structure etc.
>
>> There are a few reasons why I had done this. Some reasons Andres has 
>> described in his previous email and I'll add a few specific examples on why 
>> having the same section for both might not be a good idea.
>>
>>  * Having readline and zlib as required for building PostgreSQL is now wrong 
>> because they are not required for meson builds. Also, the name of the 
>> configs are different for make and meson and the current section only lists 
>> the make ones.
>>  * There are many references to configure in that section which don't apply 
>> to meson.
>>  * Last I checked Flex and Bison were always required to build via meson but 
>> not for make and the current section doesn't explain those 

Re: Documentation for building with meson

[Peter Eisentraut]

On 10.06.23 06:00, Andres Freund wrote:

 From c5e637a54c2b83e5bd8c4155784d97e82937eb51 Mon Sep 17 00:00:00 2001
From: Samay Sharma
Date: Mon, 6 Feb 2023 16:09:42 -0800
Subject: [PATCH v9 2/5] Add data layout options sub-section in installation
  docs

This commit separates out blocksize, segsize and wal_blocksize
options into a separate Data layout options sub-section in both
the make and meson docs. They were earlier in a miscellaneous
section which included several unrelated options. This change
also helps reduce the repetition of the warnings that changing
these parameters breaks on-disk compatibility.

I still like this change, but ISTM that the "Data Layout" section should
follow the "PostgreSQL Features" section, rather than follow "Anti Features",
"Build Process Details" and "Miscellaneous". I realize some of these are
reorganized later on, but even then "Build Process Details"

Would anybody mind if I swapped these around?


I don't mind a Data Layout section in principle, but I wonder whether 
it's worth changing now.  The segsize option is proposed to be turned 
into a run-time option (and/or removed).  For the WAL block size, I had 
previously mentioned, I don't think it is correct that pg_upgrade should 
actually care about it.  So I wouldn't spend too much time trying to 
carefully refactor the notes on the data layout options if we're going 
to have to change them around before long again anyway.

Re: Documentation for building with meson

[Andres Freund]

Hi,

On 2023-03-28 12:27:26 -0700, samay sharma wrote:
> +   
> +If you want to build the docs, you can type:
> +
> +ninja docs
> +
> +   

To me 'you can type' sounds odd. To me even just "To build the docs:" would
sound better. But the make docs do it "your" way, so I'll just go along with
it for now.


> From c5e637a54c2b83e5bd8c4155784d97e82937eb51 Mon Sep 17 00:00:00 2001
> From: Samay Sharma 
> Date: Mon, 6 Feb 2023 16:09:42 -0800
> Subject: [PATCH v9 2/5] Add data layout options sub-section in installation
>  docs
>
> This commit separates out blocksize, segsize and wal_blocksize
> options into a separate Data layout options sub-section in both
> the make and meson docs. They were earlier in a miscellaneous
> section which included several unrelated options. This change
> also helps reduce the repetition of the warnings that changing
> these parameters breaks on-disk compatibility.

I still like this change, but ISTM that the "Data Layout" section should
follow the "PostgreSQL Features" section, rather than follow "Anti Features",
"Build Process Details" and "Miscellaneous". I realize some of these are
reorganized later on, but even then "Build Process Details"

Would anybody mind if I swapped these around?


> +  

I don't quite understand the "-with" added to the ids?


> From 11d82aa49efb3d1cbc08f14562a757f115053c8b Mon Sep 17 00:00:00 2001
> From: Samay Sharma 
> Date: Mon, 13 Feb 2023 16:23:52 -0800
> Subject: [PATCH v9 3/5] Remove Anti-Features section from Installation from
>  source docs
>
> Currently, several meson setup options are listed in anti-features.
> However, they are similar to most other options in the postgres
> features list as they are 'auto' features themselves. Also, other
> options are likely better suited to the developer options section.
> This commit, therefore, moves the options listed in the anti-features
> section into other sections and removes that section.
> For consistency, this reorganization has been done on the make section
> of the docs as well.
> ---
>  doc/src/sgml/installation.sgml | 140 ++---
>  1 file changed, 57 insertions(+), 83 deletions(-)
>
> diff --git a/doc/src/sgml/installation.sgml b/doc/src/sgml/installation.sgml
> index 7e65cdd72e..d7ab0c205e 100644
> --- a/doc/src/sgml/installation.sgml
> +++ b/doc/src/sgml/installation.sgml
> @@ -1214,23 +1214,6 @@ build-postgresql:
> 
>
>
> - 
> -
> -   
> -
> -   
> -Anti-Features
> -
> -
> - The options described in this section allow disabling
> - certain PostgreSQL features that are built
> - by default, but which might need to be turned off if the required
> - software or system features are not available.  Using these options is
> - not recommended unless really necessary.
> -
> -
> - 
> -
>
> --without-readline
> 

I don't think this is quite right. The section above the list says

"The options described in this section enable building of various PostgreSQL
features that are not built by default. Most of these are non-default only
because they require additional software, as described in Section 17.1."

So just merging --without-icu, --without-readline, --without-zlib,
--disable-thread-safety, in with the rest doesn't quite seem right.

I suspect that the easiest way for that is to just move --disable-atomics,
--disable-spinlocks to the developer section and then to leave the
anti-features section around for autoconf.

Any better idea?

Greetings,

Andres Freund

Re: Documentation for building with meson

[Tristan Partin]

On Wed Apr 12, 2023 at 4:19 PM CDT, Peter Eisentraut wrote:
> On 11.04.23 19:18, Andres Freund wrote:
> >> Subject: [PATCH v9 2/5] Add data layout options sub-section in installation
> >>   docs
> >>
> >> This commit separates out blocksize, segsize and wal_blocksize
> >> options into a separate Data layout options sub-section in both
> >> the make and meson docs. They were earlier in a miscellaneous
> >> section which included several unrelated options. This change
> >> also helps reduce the repetition of the warnings that changing
> >> these parameters breaks on-disk compatibility.
> > 
> > Makes sense. I'm planning to apply this unless Peter or somebody else has
> > further feedback.
>
> I'm okay with patches 0001 through 0004.
>
> I don't like 0005.  I think we should drop that for now and maybe have a 
> separate discussion under a separate heading about that.

With regard to 0005, perhaps it makes the most sense to use the XDG
Directory Specification as an example. Install into ~/.local or
~/.local/postgres (as the Meson default prefix is). Put the data into
~/.var/lib/postgres or something similar.

-- 
Tristan Partin
Neon (https://neon.tech)

Re: Documentation for building with meson

[Tristan Partin]

Hey!

Nice work organizing the docs. Looks pretty good. I had a single comment
regarding the Meson docs in general.

It seems like Postgres cares a bit about Windows/Mac development. Is
ninja the blessed way to build Postgres on Windows and Mac? Otherwise, I
would suggest just being more generic and replace instances of `ninja
xxx` in the docs with `meson compile xxx`, making the command
backend-agnostic to accomodate XCode and VS project users.

-- 
Tristan Partin
Neon (https://neon.tech)

Re: Documentation for building with meson

[Peter Eisentraut]

On 11.04.23 19:18, Andres Freund wrote:

Subject: [PATCH v9 2/5] Add data layout options sub-section in installation
  docs

This commit separates out blocksize, segsize and wal_blocksize
options into a separate Data layout options sub-section in both
the make and meson docs. They were earlier in a miscellaneous
section which included several unrelated options. This change
also helps reduce the repetition of the warnings that changing
these parameters breaks on-disk compatibility.


Makes sense. I'm planning to apply this unless Peter or somebody else has
further feedback.


I'm okay with patches 0001 through 0004.

I don't like 0005.  I think we should drop that for now and maybe have a 
separate discussion under a separate heading about that.






 From 11d82aa49efb3d1cbc08f14562a757f115053c8b Mon Sep 17 00:00:00 2001
From: Samay Sharma 
Date: Mon, 13 Feb 2023 16:23:52 -0800
Subject: [PATCH v9 3/5] Remove Anti-Features section from Installation from
  source docs

Currently, several meson setup options are listed in anti-features.
However, they are similar to most other options in the postgres
features list as they are 'auto' features themselves. Also, other
options are likely better suited to the developer options section.
This commit, therefore, moves the options listed in the anti-features
section into other sections and removes that section.

For consistency, this reorganization has been done on the make section
of the docs as well.


Makes sense to me. "Anti-Features" is confusing as a name to start with.

Re: Documentation for building with meson

[samay sharma]

Hi,

On Tue, Apr 11, 2023 at 10:18 AM Andres Freund  wrote:

> Hi,
>
> On 2023-03-28 12:27:26 -0700, samay sharma wrote:
> > Subject: [PATCH v9 1/5] Make minor additions and corrections to meson
> docs
> >
> > This commit makes a few corrections to the meson docs
> > and adds a few instructions and links for better clarity.
> > ---
> >  doc/src/sgml/installation.sgml | 24 +++-
> >  1 file changed, 15 insertions(+), 9 deletions(-)
> >
> > diff --git a/doc/src/sgml/installation.sgml
> b/doc/src/sgml/installation.sgml
> > index 70ab5b77d8..e3b9b6c0d0 100644
> > --- a/doc/src/sgml/installation.sgml
> > +++ b/doc/src/sgml/installation.sgml
> > @@ -2057,8 +2057,7 @@ meson setup build -Dssl=openssl
> >  
> >  meson configure -Dcassert=true
> >  
> > -meson configure's commonly used command-line
> options
> > -are explained in .
> > +Commonly used build options for meson configure
> (and meson setup) are explained in  linkend="meson-options"/>.
> > 
> >
> >
> > @@ -2078,6 +2077,13 @@ ninja
> >  processes used with the command line argument -j.
> > 
> >
> > +   
> > +If you want to build the docs, you can type:
> > +
> > +ninja docs
> > +
> > +   
>
> "type" sounds a bit too, IDK, process oriented. "To build the docs, use"?
>

Sure, that works.

>
>
> > Subject: [PATCH v9 2/5] Add data layout options sub-section in
> installation
> >  docs
> >
> > This commit separates out blocksize, segsize and wal_blocksize
> > options into a separate Data layout options sub-section in both
> > the make and meson docs. They were earlier in a miscellaneous
> > section which included several unrelated options. This change
> > also helps reduce the repetition of the warnings that changing
> > these parameters breaks on-disk compatibility.
>
> Makes sense. I'm planning to apply this unless Peter or somebody else has
> further feedback.
>

Cool.

>
>
> > From 11d82aa49efb3d1cbc08f14562a757f115053c8b Mon Sep 17 00:00:00 2001
> > From: Samay Sharma 
> > Date: Mon, 13 Feb 2023 16:23:52 -0800
> > Subject: [PATCH v9 3/5] Remove Anti-Features section from Installation
> from
> >  source docs
> >
> > Currently, several meson setup options are listed in anti-features.
> > However, they are similar to most other options in the postgres
> > features list as they are 'auto' features themselves. Also, other
> > options are likely better suited to the developer options section.
> > This commit, therefore, moves the options listed in the anti-features
> > section into other sections and removes that section.
> >
> > For consistency, this reorganization has been done on the make section
> > of the docs as well.
>
> Makes sense to me. "Anti-Features" is confusing as a name to start with.
>
> Greetings,
>
> Andres Freund
>

Re: Documentation for building with meson

[Andres Freund]

Hi,

On 2023-03-28 12:27:26 -0700, samay sharma wrote:
> Subject: [PATCH v9 1/5] Make minor additions and corrections to meson docs
> 
> This commit makes a few corrections to the meson docs
> and adds a few instructions and links for better clarity.
> ---
>  doc/src/sgml/installation.sgml | 24 +++-
>  1 file changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/doc/src/sgml/installation.sgml b/doc/src/sgml/installation.sgml
> index 70ab5b77d8..e3b9b6c0d0 100644
> --- a/doc/src/sgml/installation.sgml
> +++ b/doc/src/sgml/installation.sgml
> @@ -2057,8 +2057,7 @@ meson setup build -Dssl=openssl
>  
>  meson configure -Dcassert=true
>  
> -meson configure's commonly used command-line options
> -are explained in .
> +Commonly used build options for meson configure (and 
> meson setup) are explained in  linkend="meson-options"/>.
> 
>
>  
> @@ -2078,6 +2077,13 @@ ninja
>  processes used with the command line argument -j.
> 
>  
> +   
> +If you want to build the docs, you can type:
> +
> +ninja docs
> +
> +   

"type" sounds a bit too, IDK, process oriented. "To build the docs, use"?


> Subject: [PATCH v9 2/5] Add data layout options sub-section in installation
>  docs
> 
> This commit separates out blocksize, segsize and wal_blocksize
> options into a separate Data layout options sub-section in both
> the make and meson docs. They were earlier in a miscellaneous
> section which included several unrelated options. This change
> also helps reduce the repetition of the warnings that changing
> these parameters breaks on-disk compatibility.

Makes sense. I'm planning to apply this unless Peter or somebody else has
further feedback.


> From 11d82aa49efb3d1cbc08f14562a757f115053c8b Mon Sep 17 00:00:00 2001
> From: Samay Sharma 
> Date: Mon, 13 Feb 2023 16:23:52 -0800
> Subject: [PATCH v9 3/5] Remove Anti-Features section from Installation from
>  source docs
> 
> Currently, several meson setup options are listed in anti-features.
> However, they are similar to most other options in the postgres
> features list as they are 'auto' features themselves. Also, other
> options are likely better suited to the developer options section.
> This commit, therefore, moves the options listed in the anti-features
> section into other sections and removes that section.
> 
> For consistency, this reorganization has been done on the make section
> of the docs as well.

Makes sense to me. "Anti-Features" is confusing as a name to start with.

Greetings,

Andres Freund

Re: Documentation for building with meson

[samay sharma]

Hi,

On Wed, Mar 15, 2023 at 4:28 AM Peter Eisentraut <
peter.eisentr...@enterprisedb.com> wrote:

>  > [PATCH v8 1/5] Make minor additions and corrections to meson docs
>
> The last hunk revealed that there is some mixing up between meson setup
> and meson configure.  This goes a bit further.  For example, earlier it
> says that to get a list of meson setup options, call meson configure
> --help and look at https://mesonbuild.com/Commands.html#configure, which
> are both wrong.  Also later throughout the text it uses one or the
> other.  I think this has the potential to be very confusing, and we
> should clean this up carefully.
>
> The text about additional meson test options maybe should go into the
> regress.sgml chapter?
>

I tried to make the meson setup and meson configure usage consistent. I've
removed the text for the test options.

>
>
>  > [PATCH v8 2/5] Add data layout options sub-section in installation
>   docs
>
> This makes sense.  Please double check your patch for correct title
> casing, vertical spacing of XML, to keep everything looking consistent.
>

Thanks for noticing. Made it consistent on both sides.

>
> This text isn't yours, but since your patch emphasizes it, I wonder if
> it could use some clarification:
>
> + These options affect how PostgreSQL lays out data on disk.
> + Note that changing these breaks on-disk database compatibility,
> + meaning you cannot use pg_upgrade to upgrade to
> + a build with different values of these options.
>
> This isn't really correct.  What breaking on-disk compatibility means is
> that you can't use a server compiled one way with a data directory
> initialized by binaries compiled another way.  pg_upgrade may well have
> the ability to upgrade between one or the other; that's up to pg_upgrade
> to figure out but not an intrinsic property.  (I wonder why pg_upgrade
> cares about the WAL block size.)
>

 Fixed.

>
>
>  > [PATCH v8 3/5] Remove Anti-Features section from Installation from
>   source docs
>
> Makes sense.  But is "--disable-thread-safety" really a developer
> feature?  I think not.
>
>
Moved to PostgreSQL features. Do you think there's a better place for it?


>
>  > [PATCH v8 4/5] Re-organize Miscellaneous section
>
> This moves the Miscellaneous section after Developer Features.  I think
> Developer Features should be last.
>
> Maybe should remove this section and add the options to the regular
> PostgreSQL Features section.
>

Yes, that makes sense. Made this change.

>
> Also consider the grouping in meson_options.txt, which is slightly
> different yet.


Removed Misc options section from meson_options.txt too.

>
>
>  > [PATCH v8 5/5] Change Short Version for meson installation guide
>
> +# create working directory
> +mkdir postgres
> +cd postgres
> +
> +# fetch source code
> +git clone https://git.postgresql.org/git/postgresql.git src
>
> This comes after the "Getting the Source" section, so at this point they
> already have the source and don't need to do "git clone" etc. again.
>
> +# setup and enter build directory (done only first time)
> +## Unix based platforms
> +meson setup build src --prefix=$PWD/install
> +
> +## Windows
> +meson setup build src --prefix=%cd%/install
>
> Maybe some people work this way, but to me the directory structures you
> create here are completely weird.
>

I'd like to discuss what you think is a good directory structure to work
with. I've mentioned some of the drawbacks I see with the current structure
for the short version. I know this structure can feel different but it
feeling weird is not ideal. Do you have a directory structure in mind which
is different but doesn't feel odd to you?


>
> +# Initialize a new database
> +../install/bin/initdb -D ../data
> +
> +# Start database
> +../install/bin/pg_ctl -D ../data/ -l logfile start
> +
> +# Connect to the database
> +../install/bin/psql -d postgres
>
> The terminology here needs to be tightened up.  You are using "database"
> here to mean three different things.
>

I'll address this together once we are aligned on the overall directory
structure etc.

There are a few reasons why I had done this. Some reasons Andres has
> described in his previous email and I'll add a few specific examples on why
> having the same section for both might not be a good idea.
>
>  * Having readline and zlib as required for building PostgreSQL is now
> wrong because they are not required for meson builds. Also, the name of the
> configs are different for make and meson and the current section only lists
> the make ones.
>  * There are many references to configure in that section which don't
> apply to meson.
>  * Last I checked Flex and Bison were always required to build via meson
> but not for make and the current section doesn't explain those differences.
>
> I spent a good amount of time thinking if we could have a single section,
> clarify these differences to make it correct and not confuse the users. I
> couldn't find a way to do all 

Re: Documentation for building with meson

[Peter Eisentraut]

> [PATCH v8 1/5] Make minor additions and corrections to meson docs

The last hunk revealed that there is some mixing up between meson setup 
and meson configure.  This goes a bit further.  For example, earlier it 
says that to get a list of meson setup options, call meson configure 
--help and look at https://mesonbuild.com/Commands.html#configure, which 
are both wrong.  Also later throughout the text it uses one or the 
other.  I think this has the potential to be very confusing, and we 
should clean this up carefully.


The text about additional meson test options maybe should go into the 
regress.sgml chapter?



> [PATCH v8 2/5] Add data layout options sub-section in installation
 docs

This makes sense.  Please double check your patch for correct title 
casing, vertical spacing of XML, to keep everything looking consistent.


This text isn't yours, but since your patch emphasizes it, I wonder if 
it could use some clarification:


+ These options affect how PostgreSQL lays out data on disk.
+ Note that changing these breaks on-disk database compatibility,
+ meaning you cannot use pg_upgrade to upgrade to
+ a build with different values of these options.

This isn't really correct.  What breaking on-disk compatibility means is 
that you can't use a server compiled one way with a data directory 
initialized by binaries compiled another way.  pg_upgrade may well have 
the ability to upgrade between one or the other; that's up to pg_upgrade 
to figure out but not an intrinsic property.  (I wonder why pg_upgrade 
cares about the WAL block size.)



> [PATCH v8 3/5] Remove Anti-Features section from Installation from
 source docs

Makes sense.  But is "--disable-thread-safety" really a developer 
feature?  I think not.



> [PATCH v8 4/5] Re-organize Miscellaneous section

This moves the Miscellaneous section after Developer Features.  I think 
Developer Features should be last.


Maybe should remove this section and add the options to the regular 
PostgreSQL Features section.


Also consider the grouping in meson_options.txt, which is slightly 
different yet.



> [PATCH v8 5/5] Change Short Version for meson installation guide

+# create working directory
+mkdir postgres
+cd postgres
+
+# fetch source code
+git clone https://git.postgresql.org/git/postgresql.git src

This comes after the "Getting the Source" section, so at this point they 
already have the source and don't need to do "git clone" etc. again.


+# setup and enter build directory (done only first time)
+## Unix based platforms
+meson setup build src --prefix=$PWD/install
+
+## Windows
+meson setup build src --prefix=%cd%/install

Maybe some people work this way, but to me the directory structures you 
create here are completely weird.


+# Initialize a new database
+../install/bin/initdb -D ../data
+
+# Start database
+../install/bin/pg_ctl -D ../data/ -l logfile start
+
+# Connect to the database
+../install/bin/psql -d postgres

The terminology here needs to be tightened up.  You are using "database" 
here to mean three different things.

Re: Documentation for building with meson

[samay sharma]

Hi,

On Thu, Dec 1, 2022 at 9:21 AM Andres Freund  wrote:

> Hi,
>
> On 2022-12-01 15:58:39 +0100, Peter Eisentraut wrote:
> > On 23.11.22 22:24, samay sharma wrote:
> > > Thank you. Attaching v7 addressing most of the points below.
> >
> > I have committed this, after some editing and making some structural
> > changes.
>
> Thanks. I was working on that too, but somehow felt a bit stuck...
>
> I'll try if I can adapt my pending changes.
>

I got back to working on the meson docs. I'm attaching a new patch set
proposing some improvements to the current installation docs. I've tried to
make some corrections and improvements suggested in this thread while
trying to maintain consistency across make and meson docs as per Peter's
ask. There are 5 patches in the patch-set:

v8-0001: Makes minor corrections, adds instructions to build docs and adds
a few links to meson docs.
v8-0002, v8-0003 and v8-0004 make changes to restructure the configure
options based on reasoning discussed below. To maintain consistency, I've
made those changes on both the make and meson side.
v8-0005 Reproposes the Short Version I had proposed in v7 as I feel we
should discuss that proposal. I think it improves things in terms of
installation docs. More details below.


>
> > I moved the "Requirements" section back to the top level.  It did
> > not look appealing to have to maintain two copies of this that have
> almost
> > no substantial difference (but for some reason were written with separate
> > structure and wording).
>

There are a few reasons why I had done this. Some reasons Andres has
described in his previous email and I'll add a few specific examples on why
having the same section for both might not be a good idea.

* Having readline and zlib as required for building PostgreSQL is now wrong
because they are not required for meson builds. Also, the name of the
configs are different for make and meson and the current section only lists
the make ones.
* There are many references to configure in that section which don't apply
to meson.
* Last I checked Flex and Bison were always required to build via meson but
not for make and the current section doesn't explain those differences.

I spent a good amount of time thinking if we could have a single section,
clarify these differences to make it correct and not confuse the users. I
couldn't find a way to do all three. Therefore, I think we should move to a
different requirements section for both. I'm happy to re-propose the
previous version which separates them but wanted to see if anybody has
better ideas.


>
> I don't think this is good. The whole "The following software packages are
> required for building PostgreSQL" section is wrong now.  "They are not
> required in the default configuration, but they are needed when certain
> build
> options are enabled, as explained below:" section is misleading as well.
>
> By the time we fix all of those we'll end up with a different section
> again.
>
>
> > Also, I rearranged the Building with Meson section to use the same
> internal
> > structure as the Building with Autoconf and Make section.  This will
> make it
> > easier to maintain going forward.  For example if someone adds a new
> option,
> > it will be easier to find the corresponding places in the lists where to
> add
> > them.


> I don't know. The existing list order makes very little sense to me. The
> E.g. --enable-nls is before the rest in configure, presumably because it
> sorts
> there alphabetically. But that's not the case for meson.
>
> Copying "Anti-Features" as a structure element to the meson docs seems
> bogus
> (also the name is bogus, but that's a pre-existing issue). There's no
> difference in -Dreadline= to the other options meson-options-features list.


> Nor does -Dspinlocks= -Datomics= make sense in the "anti features"
> section. It
> made some sense for autoconf because of the --without- prefix, but that's
> not
> at play in meson.  Their placement in the "Developer Options" made a whole
> lot
> more sense.
>

I agree "Anti-Features" desn't make sense in the meson context. One of my
patches removes that section and moves some options into the "Postgres
Features" section and others into the "Developer Options" section. I've
proposed to make those changes on both sides to make it easier to maintain.


>
> I don't like "Miscellaneous" bit containing minor stuff like krb_srvnam and
> data layout changing options like blocksize,segsize,wal_blocksize. But it
> makes sense to change that for both at the same time.
>

I've proposed a patch to add a new "Data Layout Options" section which
includes: blocksize, segsize and wal_blocksize. I've created that section
on both sides.

>
>
> > We will likely keep iterating on the contents for the next little while,
> but
> > I'm glad we now have a structure in place that we should be able to live
> > with.
>

I feel that there are a few shortcomings of the current "Short Version". I
tried to address them in my previous 

Re: Documentation for building with meson

[Andres Freund]

Hi,

On 2022-12-01 15:58:39 +0100, Peter Eisentraut wrote:
> On 23.11.22 22:24, samay sharma wrote:
> > Thank you. Attaching v7 addressing most of the points below.
> 
> I have committed this, after some editing and making some structural
> changes.

Thanks. I was working on that too, but somehow felt a bit stuck...

I'll try if I can adapt my pending changes.


> I moved the "Requirements" section back to the top level.  It did
> not look appealing to have to maintain two copies of this that have almost
> no substantial difference (but for some reason were written with separate
> structure and wording).

I don't think this is good. The whole "The following software packages are
required for building PostgreSQL" section is wrong now.  "They are not
required in the default configuration, but they are needed when certain build
options are enabled, as explained below:" section is misleading as well.

By the time we fix all of those we'll end up with a different section again.


> Also, I rearranged the Building with Meson section to use the same internal
> structure as the Building with Autoconf and Make section.  This will make it
> easier to maintain going forward.  For example if someone adds a new option,
> it will be easier to find the corresponding places in the lists where to add
> them.

I don't know. The existing list order makes very little sense to me. The
E.g. --enable-nls is before the rest in configure, presumably because it sorts
there alphabetically. But that's not the case for meson.

Copying "Anti-Features" as a structure element to the meson docs seems bogus
(also the name is bogus, but that's a pre-existing issue). There's no
difference in -Dreadline= to the other options meson-options-features list.

Nor does -Dspinlocks= -Datomics= make sense in the "anti features" section. It
made some sense for autoconf because of the --without- prefix, but that's not
at play in meson.  Their placement in the "Developer Options" made a whole lot
more sense.


I don't like "Miscellaneous" bit containing minor stuff like krb_srvnam and
data layout changing options like blocksize,segsize,wal_blocksize. But it
makes sense to change that for both at the same time.


> We will likely keep iterating on the contents for the next little while, but
> I'm glad we now have a structure in place that we should be able to live
> with.

I agree that it's good to have something we can work from more
iteratively. But I don't think this is a structure that we can live with.


I'm not particularly happy about this level of structural change made without
discussing it prior.

Greetings,

Andres Freund

Re: Documentation for building with meson

[Peter Eisentraut]

On 23.11.22 22:24, samay sharma wrote:

Thank you. Attaching v7 addressing most of the points below.


I have committed this, after some editing and making some structural 
changes.  I moved the "Requirements" section back to the top level.  It 
did not look appealing to have to maintain two copies of this that have 
almost no substantial difference (but for some reason were written with 
separate structure and wording).  Also, I rearranged the Building with 
Meson section to use the same internal structure as the Building with 
Autoconf and Make section.  This will make it easier to maintain going 
forward.  For example if someone adds a new option, it will be easier to 
find the corresponding places in the lists where to add them.


We will likely keep iterating on the contents for the next little while, 
but I'm glad we now have a structure in place that we should be able to 
live with.

Re: Documentation for building with meson

[Justin Pryzby]

Thanks; two more things I saw:

 - In other docs,  isn't used around { a | b } lists:
   git grep '[^<]*|' doc
 - I think this is(was) missing a word;
   Setting this option allows you to override THE value of all 'auto' features

-- 
Justin

Re: Documentation for building with meson

[samay sharma]

Hi,

On Wed, Nov 23, 2022 at 12:16 PM Justin Pryzby  wrote:

> On Wed, Nov 23, 2022 at 11:30:54AM -0800, samay sharma wrote:
> > Thanks for the feedback. Addressed all and added markup at a few more
> > places in v6 (attached).
>
> Thanks.  It looks good to me.  A couple thoughts, maybe they're not
> important.
>

Thank you. Attaching v7 addressing most of the points below.


>
>  - LZ4 and Zstd refer to wal_compression and default_toast_compression,
>but not to any corresponding option to basebackup.
>
>  - There's no space after the hash mark here; but above, there was:
>#Setup build directory with a different installation prefix
>

Added a space as that looks better.

>
>  - You use slash to show enumerated options, but it's more typical to
>use braces: {a | b | c}:
>-Dnls=auto/enabled/disabled
>

Changed.

>
>  - There's no earlier description/definition of an "auto" feature, but
>still says this:
>"Setting this option allows you to override value of all 'auto'
> features"
>

Described what an "auto" feature is in ().

>
>  - Currently the documentation always refers to "PostgreSQL", but you
>added two references to "Postgres":
>+ If a program required to build Postgres...
>+ Once Postgres is built...
>

Good catch. Changed to PostgreSQL.

Regards,
Samay

>
> --
> Justin
>


v7-0001-Add-docs-for-building-with-meson.patch
Description: Binary data

Re: Documentation for building with meson

[Justin Pryzby]

On Wed, Nov 23, 2022 at 11:30:54AM -0800, samay sharma wrote:
> Thanks for the feedback. Addressed all and added markup at a few more
> places in v6 (attached).

Thanks.  It looks good to me.  A couple thoughts, maybe they're not
important.

 - LZ4 and Zstd refer to wal_compression and default_toast_compression,
   but not to any corresponding option to basebackup.

 - There's no space after the hash mark here; but above, there was:
   #Setup build directory with a different installation prefix

 - You use slash to show enumerated options, but it's more typical to
   use braces: {a | b | c}:
   -Dnls=auto/enabled/disabled

 - There's no earlier description/definition of an "auto" feature, but
   still says this:
   "Setting this option allows you to override value of all 'auto' features"

 - Currently the documentation always refers to "PostgreSQL", but you
   added two references to "Postgres":
   + If a program required to build Postgres...
   + Once Postgres is built...

-- 
Justin

Re: Documentation for building with meson

[samay sharma]

Hi,

On Tue, Nov 22, 2022 at 10:36 PM Justin Pryzby  wrote:

> On Mon, Nov 14, 2022 at 10:41:21AM -0800, samay sharma wrote:
>
> > You need LZ4, if you want to support compression of data with that
> > method; see default_toast_compression and wal_compression.
>
> => The first comma is odd.  Maybe it should say "LZ4 is needed to
> support .."
>
> > You need Zstandard, if you want to support compression of data or
> > backups with that method; see wal_compression. The minimum required
> > version is 1.4.0.
>
> Same.
>
> Also, since v15, LZ4 and zstd can both be used by basebackup.
>
> >Some commonly used ones are mentioned in the subsequent sections
>
> => Some commonly used options ...
>
> >  Most of these require additional software, as described in Section
> >  17.3.2, and are set to be auto features.
>
> => "Are set to be auto features" sounds odd.  I think it should say
> something like " .. and are automatically enabled if the required
> software is detected.".
>
> > You can change this behavior by manually setting the auto features to
> > enabled to require them or disabled to not build with them.
>
> remove "auto".  Maybe "enabled" and "disabled" need markup.
>
> > On Windows, the default WinLDAP library is used. It defults to auto
>
> typo: defults
>
> > It defaults to auto and libsystemd and the associated header files need
> > to be installed to use this option.
>
> => write this as two separate sentences.  Same for libxml.
>
> > bsd to use the UUID functions found in FreeBSD, NetBSD, and some other
> > BSD-derived systems
>
> => should remove mention of netbsd, like c4b6d218e
>
> > Enables use of the Zlib library. It defaults to auto and enables
> > support for compressed archives in pg_dump ,pg_restore and
> > pg_basebackup and is recommended.
>
> => The comma is mis-placed.
>
> > The default backend meson uses is ninja and that should suffice for
> > most use cases. However, if you'd like to fully integrate with
> > visual studio, you can set the BACKEND to vs.
>
> => BACKEND is missing markup.
>
> > This option can be used to specify the buildtype to use; defaults to
> > release
>
> => release is missing markup
>
> >  Specify the optimization level. LEVEL can be set to any of
> >  {0,g,1,2,3,s}.
>
> => LEVEL is missing markup
>

Thanks for the feedback. Addressed all and added markup at a few more
places in v6 (attached).

Regards,
Samay

>
> Thanks,
> --
> Justin
>


v6-0001-Add-docs-for-building-with-meson.patch
Description: Binary data

Re: Documentation for building with meson

[Justin Pryzby]

On Mon, Nov 14, 2022 at 10:41:21AM -0800, samay sharma wrote:

> You need LZ4, if you want to support compression of data with that
> method; see default_toast_compression and wal_compression. 

=> The first comma is odd.  Maybe it should say "LZ4 is needed to
support .."

> You need Zstandard, if you want to support compression of data or
> backups with that method; see wal_compression. The minimum required
> version is 1.4.0. 

Same.

Also, since v15, LZ4 and zstd can both be used by basebackup.

>Some commonly used ones are mentioned in the subsequent sections

=> Some commonly used options ...

>  Most of these require additional software, as described in Section
>  17.3.2, and are set to be auto features.

=> "Are set to be auto features" sounds odd.  I think it should say
something like " .. and are automatically enabled if the required
software is detected.".

> You can change this behavior by manually setting the auto features to
> enabled to require them or disabled to not build with them. 

remove "auto".  Maybe "enabled" and "disabled" need markup.

> On Windows, the default WinLDAP library is used. It defults to auto

typo: defults

> It defaults to auto and libsystemd and the associated header files need
> to be installed to use this option.

=> write this as two separate sentences.  Same for libxml.

> bsd to use the UUID functions found in FreeBSD, NetBSD, and some other
> BSD-derived systems 

=> should remove mention of netbsd, like c4b6d218e

> Enables use of the Zlib library. It defaults to auto and enables
> support for compressed archives in pg_dump ,pg_restore and
> pg_basebackup and is recommended. 

=> The comma is mis-placed.

> The default backend meson uses is ninja and that should suffice for
> most use cases. However, if you'd like to fully integrate with
> visual studio, you can set the BACKEND to vs. 

=> BACKEND is missing markup.

> This option can be used to specify the buildtype to use; defaults to
> release

=> release is missing markup

>  Specify the optimization level. LEVEL can be set to any of
>  {0,g,1,2,3,s}. 

=> LEVEL is missing markup

Thanks,
-- 
Justin

Re: Documentation for building with meson

[Justin Pryzby]

On Wed, Nov 16, 2022 at 10:52:35AM +0900, Ian Lawrence Barwick wrote:
> 2022年10月20日(木) 11:43 Justin Pryzby :
> >
> > On Wed, Oct 19, 2022 at 11:35:10AM -0700, samay sharma wrote:
> > > Creating a new thread focussed on adding docs for building Postgres with
> > > meson. This is a spinoff from the original thread [1] and I've attempted 
> > > to
> > > address all the feedback provided there in the attached patch.
> > >
> > > Please let me know your thoughts.
> >
> > It's easier to review rendered documentation.
> > I made a rendered copy available here:
> > https://api.cirrus-ci.com/v1/artifact/task/6084297781149696/html_docs/html_docs/install-meson.html
> 
> For reference, are there any instructions anywhere on how to do this?  It'd be
> useful to be able to provide a link to the latest version of this 
> documentation
> (and also document the process for patch authors in general).

I've submitted patches which would do that for every patch (ideally,
excluding patches that don't touch the docs, although it looks like the
"exclusion" isn't working).
https://commitfest.postgresql.org/40/3709/

The most recent patches on that thread don't include the "docs as
artifacts" patch, but only the preparatory "build docs as a separate
task".  I think the other part is stalled waiting for some updates to
cfbot to allow knowing how many commits are in the patchset.

FYI, you can navigate from the cirrus task's URL to the git commit (and
its parents)

https://cirrus-ci.com/task/6084297781149696 =>
https://github.com/justinpryzby/postgres/commit/7b57f3323fc77e9b04ef2e76976776090eb8b5b5

-- 
Justin

Re: Documentation for building with meson

[Ian Lawrence Barwick]

2022年10月20日(木) 11:43 Justin Pryzby :
>
> On Wed, Oct 19, 2022 at 11:35:10AM -0700, samay sharma wrote:
> > Creating a new thread focussed on adding docs for building Postgres with
> > meson. This is a spinoff from the original thread [1] and I've attempted to
> > address all the feedback provided there in the attached patch.
> >
> > Please let me know your thoughts.
>
> It's easier to review rendered documentation.
> I made a rendered copy available here:
> https://api.cirrus-ci.com/v1/artifact/task/6084297781149696/html_docs/html_docs/install-meson.html

For reference, are there any instructions anywhere on how to do this?  It'd be
useful to be able to provide a link to the latest version of this documentation
(and also document the process for patch authors in general).

Regards

Ian Barwick

Re: Documentation for building with meson

[samay sharma]

Hi,

On Thu, Nov 10, 2022 at 4:46 AM Nazir Bilal Yavuz 
wrote:

> Hi,
>
> I did some tests on windows. I used 'ninja' as a backend.
> On 11/8/2022 9:23 PM, samay sharma wrote:
>
> Hi,
>
> On Sat, Nov 5, 2022 at 2:39 PM Andres Freund  wrote:
>
>> Hi,
>>
>> On 2022-10-30 20:51:59 -0700, samay sharma wrote:
>> > +# setup and enter build directory (done only first time)
>> > +meson setup build src --prefix=$PWD/install
>>
>> This command won't work on windows, I think.
>>
>
> Yes, $PWD isn't recognized on windows, %CD% could be alternative.
>
Added.

>
> > +  
>> > +
>>  
>> --sysconfdir=DIRECTORY
>> > +   
>> > +
>> > + Sets the directory for various configuration files,
>> > + PREFIX/etc by
>> default.
>> > +
>> > +   
>> > +  
>>
>> Need to check what windows does here.
>>
>
> It is same on windows: 'PREFIX/etc'.
>
> I also checked other dirs(bindir, sysconfdir, libdir, includedir, datadir,
> localedir, mandir), default path is correct for all of them.
>

Thanks. I've made a few windows specific fixes in the latest version.
Attached v5.

Regards,
Samay


>
> Regards,
> Nazir Bilal Yavuz
>
>
>
>


v5-0001-Add-docs-for-building-with-meson.patch
Description: Binary data

Re: Documentation for building with meson

[Nazir Bilal Yavuz]

Hi,

I did some tests on windows. I used 'ninja' as a backend.

On 11/8/2022 9:23 PM, samay sharma wrote:

Hi,

On Sat, Nov 5, 2022 at 2:39 PM Andres Freund  wrote:

Hi,

On 2022-10-30 20:51:59 -0700, samay sharma wrote:
> +# setup and enter build directory (done only first time)
> +meson setup build src --prefix=$PWD/install

This command won't work on windows, I think.



Yes, $PWD isn't recognized on windows, %CD% could be alternative.



> +      
> +
 
--sysconfdir=DIRECTORY
> +       
> +        
> +         Sets the directory for various configuration files,
> +  PREFIX/etc by
default.
> +        
> +       
> +      

Need to check what windows does here.



It is same on windows: 'PREFIX/etc'.

I also checked other dirs(bindir, sysconfdir, libdir, includedir, 
datadir, localedir, mandir), default path is correct for all of them.


Regards,
Nazir Bilal Yavuz

Re: Documentation for building with meson

[Andres Freund]

Hi,

On 2022-10-30 20:51:59 -0700, samay sharma wrote:
> +# setup and enter build directory (done only first time)
> +meson setup build src --prefix=$PWD/install

This command won't work on windows, I think.


> + 
> +  Configuring the build
> +
> +   
> +The first step of the installation procedure is to configure the
> +source tree for your system and choose the options you would like. To

s/source tree/build tree/?


> +create and configure the build directory, you can start with the
> +meson setup command.
> +   
> +
> +
> +meson setup build
> +
> +
> +   
> +The setup command takes a builddir and a 
> srcdir
> +argument. If no srcdir is given, Meson will deduce the
> +srcdir based on the current directory and the location
> +of meson.build. The builddir is 
> mandatory.
> +   
> +
> +   
> +Meson then loads the build configuration file and sets up the build 
> directory.
> +Additionally, the invocation can pass options to Meson. The list of 
> commonly
> +used options is in subsequent sections. A few examples of specifying 
> different
> +build options are:

Somehow the "tone" is descriptive in a distanced way, rather than instructing
what to do.


> +   
> +Installation Locations
> +
> + 
> +  These options control where ninja install (or meson 
> install) will put
> +  the files.  The --prefix option is sufficient for
> +  most cases.

Perhaps the short version use of prefix could be a link here? Not sure if
that's a good idea.



> + 
> +  
> +   
> --prefix=PREFIX
> +   
> +
> + Install all files under the directory 
> PREFIX
> + instead of /usr/local/pgsql. The actual
> + files will be installed into various subdirectories; no files
> + will ever be installed directly into the
> + PREFIX directory.
> +
> +   
> +  

Hm, need to mention windows here likely. By default the installation will go
to :/usr/local/pgsql.


> +  
> +   
> --bindir=DIRECTORY
> +   
> +
> + Specifies the directory for executable programs. The default
> + is PREFIX/bin, which
> + normally means /usr/local/pgsql/bin.
> +
> +   
> +  

Hm, do we really want the "which normally means" part? That'll make the OS
stuff more complicated.


> +  
> +   
> --sysconfdir=DIRECTORY
> +   
> +
> + Sets the directory for various configuration files,
> + PREFIX/etc by 
> default.
> +
> +   
> +  

Need to check what windows does here.


> +  
> +   
> -Dnls=auto/enabled/disabled

Wonder if we should define a entity for
auto/enabled/disabled? There's a lot of repetitions
of it.


> +   
> +
> + Enables or disables Native Language Support 
> (NLS),
> + that is, the ability to display a program's messages in a
> + language other than English. It defaults to auto, meaning that it
> + will be enabled automatically if an implementation of the
> + Gettext API is found.
> +

Do we really want to repeat the "It defaults to auto, meaning that it will be
enabled automatically if ..." for each of these? Perhaps we could say
'defaults to auto'?


> +
> + By default,
> + 
> pkg-configpkg-config
> + will be used to find the required compilation options.  This is
> + supported for ICU4C version 4.6 and 
> later.
> + 
> +

I'd just remove this paragraph. Right now the meson build will just use solely
use pkg-config config files for icu.  I don't think we need to care about 4.6
(from 2010) anymore.


> +  
> +   
> -Dllvm=auto/enabled/disabled
> +   
> +
> + Build with support for LLVM based
> + JIT compilation + condition="standalone-ignore"> (see  + linkend="jit"/>).  This
> + requires the LLVM library to be 
> installed.
> + The minimum required version of LLVM is
> + currently 3.9. It is set to disabled by default.
> +
> +
> + 
> llvm-configllvm-config
> + will be used to find the required compilation options.
> + llvm-config, and then
> + llvm-config-$major-$minor for all supported
> + versions, will be searched for in your PATH.
> + 
> +

Probably a link to the docs for meson native files suffices here for
now. Since the autoconf docs have been written there's only
llvm-config-$version, llvm stopped having separate major/minor versions
somewhere around llvm 4. I think it'd suffice to say llvm-config-$version?


> +
> + LLVM support requires a compatible
> + clang compiler (specified, if necessary, using 
> the
> + CLANG environment variable), and a working C++
> + compiler (specified, if necessary, using the CXX
> + environment variable).
> +

For clang we don't look for 

Re: Documentation for building with meson

[samay sharma]

Hi,

On Thu, Oct 27, 2022 at 1:04 AM John Naylor 
wrote:

> +# Run the main pg_regress and isolation tests
> +meson test --suite main
>
> This does not work for me in a fresh install until running
>
> meson test --suite setup
>
> In fact, we see in
>
> https://wiki.postgresql.org/wiki/Meson
>
> meson test --suite setup --suite main
>

You are right that this will be needed for a new install. I've added
--suite setup in the testing section in the v3 of the patch (attached).


> That was just an eyeball check from a naive user -- it would be good to
> try running everything documented here.
>

I retried all the instructions as suggested and they work for me.

Regards,
Samay


>
> --
> John Naylor
> EDB: http://www.enterprisedb.com
>


v3-0001-Add-docs-for-building-with-meson.patch
Description: Binary data

Re: Documentation for building with meson

[Jacob Champion]

On Thu, Oct 27, 2022 at 4:03 PM Andres Freund  wrote:
> Tests can have dependencies, and they're correctly built. The problem however
> is that, for historical reasons if I understand correctly, dependencies of
> tests are automatically included in the default 'all' target. Which means if
> you just type in 'ninja', it'd automatically create the test installation -
> which is probably not what we want, given that that's not a fast step on some
> platforms.

And I see that between-suite dependencies were rejected as a feature
[1]. Ah well, `--suite setup` is not so bad once you learn it.

Thanks!
--Jacob

[1] https://github.com/mesonbuild/meson/issues/2740

Re: Documentation for building with meson

[Andres Freund]

Hi,

On 2022-10-27 14:15:32 -0700, Jacob Champion wrote:
> On Thu, Oct 27, 2022 at 1:04 AM John Naylor
>  wrote:
> > This does not work for me in a fresh install until running
> >
> > meson test --suite setup
> >
> > In fact, we see in
> >
> > https://wiki.postgresql.org/wiki/Meson
> >
> > meson test --suite setup --suite main
> 
> (Is there a way to declare a dependency on the setup suite in Meson,
> so that we don't have to specify it manually? I was bitten by this
> recently; if you make a code change and forget to run setup, it'll
> recompile locally but then skip reinstallation, giving false test
> results.)

Tests can have dependencies, and they're correctly built. The problem however
is that, for historical reasons if I understand correctly, dependencies of
tests are automatically included in the default 'all' target. Which means if
you just type in 'ninja', it'd automatically create the test installation -
which is probably not what we want, given that that's not a fast step on some
platforms.

Greetings,

Andres Freund

Re: Documentation for building with meson

[Jacob Champion]

On Thu, Oct 27, 2022 at 1:04 AM John Naylor
 wrote:
> This does not work for me in a fresh install until running
>
> meson test --suite setup
>
> In fact, we see in
>
> https://wiki.postgresql.org/wiki/Meson
>
> meson test --suite setup --suite main

(Is there a way to declare a dependency on the setup suite in Meson,
so that we don't have to specify it manually? I was bitten by this
recently; if you make a code change and forget to run setup, it'll
recompile locally but then skip reinstallation, giving false test
results.)

--Jacob

Re: Documentation for building with meson

[John Naylor]

+# Run the main pg_regress and isolation tests
+meson test --suite main

This does not work for me in a fresh install until running

meson test --suite setup

In fact, we see in

https://wiki.postgresql.org/wiki/Meson

meson test --suite setup --suite main

That was just an eyeball check from a naive user -- it would be good to try
running everything documented here.

--
John Naylor
EDB: http://www.enterprisedb.com

Re: Documentation for building with meson

[samay sharma]

Hi,

On Wed, Oct 19, 2022 at 7:43 PM Justin Pryzby  wrote:

> On Wed, Oct 19, 2022 at 11:35:10AM -0700, samay sharma wrote:
> > Creating a new thread focussed on adding docs for building Postgres with
> > meson. This is a spinoff from the original thread [1] and I've attempted
> to
> > address all the feedback provided there in the attached patch.
> >
> > Please let me know your thoughts.
>
> It's easier to review rendered documentation.
> I made a rendered copy available here:
>
> https://api.cirrus-ci.com/v1/artifact/task/6084297781149696/html_docs/html_docs/install-meson.html


Thanks for your for review. Attached v2 of the patch here.


>
>
> +  Flex and Bison
> +  are needed to build PostgreSQL using
> +  meson. Be sure to get
> +  Flex 2.5.31 or later and
> +  Bison 1.875 or later from your package
> manager.
> +  Other lex and
> yacc
> +  programs cannot be used.
>
> These versions need to be updated, see also: 57bab3330:
>   - b086a47a270 "Bump minimum version of Bison to 2.3"
>   - 8b878bffa8d "Bump minimum version of Flex to 2.5.35"
>

Changed


>
> + will be enabled automatically if the required packages are found.
>
> should refer to files/libraries/headers/dependencies rather than
> "packages" ?
>

Changed to dependencies


>
> + default is false that is to use
> Readline.
>
> "that is to use" should be parenthesized or separate with commas, like
> | default is false, that is to use Readline.
>
> zlib is mentioned twice, the first being "strongly recommended".
> Is that intended?  Also, basebackup can use zlib, too.
>

Yes, the first is in the requirements section where we just list packages
required / recommended. The other mention is in the list of configure
options. This is similar to how the documentation looks today for make /
autoconf. Added pg_basebackup as a use case too.


>
> + If you have the binaries for certain by programs required to
> build
>
> remove "by" ?
>

Done


>
> + Postgres (with or without optional flags) stored at non-standard
> + paths, you could specify them manually to meson configure. The
> complete
> + list of programs for whom this is supported can be found by
> running
>
> for *which
>
> Actually, I suggest to say:
> |If a program required to build Postgres (with or without optional flags)
> |is stored in a non-standard path, ...
>

Liked this framing better. Changed.

>
> + a build with a different value of these options.
>
> .. with different values ..
>

Done

>
> + the server, it is recommended to use atleast the
> --buildtype=debug
>
> at least
>
Done

>
> +and it's options in the meson documentation.
>
> its
>
Done

>
> Maybe other things should have  ?
>
>  Git
>  Libxml2
>  libxslt
>  visual studio
>  DTrace
>  ninja
>
> +  Flex and Bison
>
> Maybe these should use  ?
>

I'm unsure of the right protocol for this. I tried to follow the precedent
set in the make / autoconf part of the documentation, which uses
 at certain places and  at others. Is there a
reference or guidance on which to use where or is it mostly a case by case
decision?


> +  be installed with pip.
>
> Should be  ?
>

Changed.

>
> This part is redundant with prior text:
> " To use this option, you will need an implementation of the Gettext API. "
>

Modified.

>
> + Enabls use of the Zlib library
>
> typo: Enabls
>

Fixed.

>
> + This option is set to true by default and setting it to false will
>
> change "and" to ";" for spinlocks and atomics?
>

Done

>
> + Debug info is generated but the result is not optimized.
>
> Maybe say the "code" is not optimized ?
>

Changed

>
> + the tests can slow down the server significantly
>
> remove "can"
>

Done.


>
> + You can override the amount of parallel processes used with
>
> s/amount/number/
>

Done

>
> + If you'd like to build with a backend other that ninja
>
> other *than
>

Fixed.

>
> +  the GNU C library then you will additionally
>
> library comma
>

Added

>
> +argument. If no srcdir is given Meson will deduce
> the
>
> given comma
>

Added

>
> + It should be noted that after the initial configure step
>
> step comma
>

Added

>
> +After the installation you can free disk space by removing the built
>
> installation comma
>

Added

>
> +To learn more about running the tests and how to interpret the results
> +you can refer to the documentation for interpreting test results.
>
> interpret the results comma
> "for interpreting test results" seems redundant.
>

Changed.

>
> + ninja install should work for most cases but if you'd like to use more
> options
>
> cases comma
>

Added

>
> Starting with "Developer Options", this intermixes postgres
> project-specific options like cassert and tap-tests with meson's stuff
> like buildtype and werror.  IMO there's too much detail about meson's
> options, which I think is better left to that project's own
> documentation, and postgres docs should include only a brief 

Re: Documentation for building with meson

[Justin Pryzby]

On Wed, Oct 19, 2022 at 11:35:10AM -0700, samay sharma wrote:
> Creating a new thread focussed on adding docs for building Postgres with
> meson. This is a spinoff from the original thread [1] and I've attempted to
> address all the feedback provided there in the attached patch.
> 
> Please let me know your thoughts.

It's easier to review rendered documentation.
I made a rendered copy available here:
https://api.cirrus-ci.com/v1/artifact/task/6084297781149696/html_docs/html_docs/install-meson.html

+  Flex and Bison
+  are needed to build PostgreSQL using
+  meson. Be sure to get
+  Flex 2.5.31 or later and
+  Bison 1.875 or later from your package 
manager.
+  Other lex and yacc
+  programs cannot be used.

These versions need to be updated, see also: 57bab3330:
  - b086a47a270 "Bump minimum version of Bison to 2.3"
  - 8b878bffa8d "Bump minimum version of Flex to 2.5.35"

+ will be enabled automatically if the required packages are found.

should refer to files/libraries/headers/dependencies rather than
"packages" ?

+ default is false that is to use Readline.

"that is to use" should be parenthesized or separate with commas, like
| default is false, that is to use Readline.

zlib is mentioned twice, the first being "strongly recommended".
Is that intended?  Also, basebackup can use zlib, too.

+ If you have the binaries for certain by programs required to build

remove "by" ?

+ Postgres (with or without optional flags) stored at non-standard
+ paths, you could specify them manually to meson configure. The 
complete
+ list of programs for whom this is supported can be found by running

for *which

Actually, I suggest to say:
|If a program required to build Postgres (with or without optional flags)
|is stored in a non-standard path, ...

+ a build with a different value of these options.

.. with different values ..

+ the server, it is recommended to use atleast the 
--buildtype=debug

at least

+and it's options in the meson documentation.

its

Maybe other things should have  ?

 Git
 Libxml2
 libxslt
 visual studio
 DTrace
 ninja

+  Flex and Bison

Maybe these should use  ?

+  be installed with pip.

Should be  ?

This part is redundant with prior text:
" To use this option, you will need an implementation of the Gettext API. "

+ Enabls use of the Zlib library

typo: Enabls

+ This option is set to true by default and setting it to false will 

change "and" to ";" for spinlocks and atomics?

+ Debug info is generated but the result is not optimized. 

Maybe say the "code" is not optimized ?

+ the tests can slow down the server significantly

remove "can"

+ You can override the amount of parallel processes used with

s/amount/number/

+ If you'd like to build with a backend other that ninja

other *than

+  the GNU C library then you will additionally

library comma

+argument. If no srcdir is given Meson will deduce the

given comma

+ It should be noted that after the initial configure step

step comma

+After the installation you can free disk space by removing the built

installation comma

+To learn more about running the tests and how to interpret the results
+you can refer to the documentation for interpreting test results.

interpret the results comma
"for interpreting test results" seems redundant.

+ ninja install should work for most cases but if you'd like to use more options

cases comma

Starting with "Developer Options", this intermixes postgres
project-specific options like cassert and tap-tests with meson's stuff
like buildtype and werror.  IMO there's too much detail about meson's
options, which I think is better left to that project's own
documentation, and postgres docs should include only a brief mention and
a reference to their docs.

+ Ninja will automatically detect the number of CPUs in your computer and
+ parallelize itself accordingly. You can override the amount of parallel
+ processes used with the command line argument -j. 

too much detail for my taste..

-- 
Justin

Documentation for building with meson

[samay sharma]

Hi,

Creating a new thread focussed on adding docs for building Postgres with
meson. This is a spinoff from the original thread [1] and I've attempted to
address all the feedback provided there in the attached patch.

Please let me know your thoughts.

[1]
https://www.postgresql.org/message-id/28de92b5-a514-fe1b-1637-ba228aa2cccf%40enterprisedb.com

Regards,
Samay


v1-0001-Add-docs-for-building-with-meson.patch
Description: Binary data