Subject: Re: Documentation for building with meson

From: Peter Eisentraut <[email-masked]>
Date: Wed, 15 Mar 2023 12:28:36 +0100

> [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 <command>pg_upgrade</command> 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.

---

From: samay sharma <[email-masked]>
Date: Tue, 28 Mar 2023 12:27:26 -0700

Hi,

> The text about additional meson test options maybe should go into the

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

> casing, vertical spacing of XML, to keep everything looking consistent.

Thanks for noticing. Made it consistent on both sides.

> cares about the WAL block size.)

 Fixed.

> Makes sense.  But is "--disable-thread-safety" really a developer
Moved to PostgreSQL features. Do you think there's a better place for it?

> PostgreSQL Features section.

Yes, that makes sense. Made this change.

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

Removed Misc options section from meson_options.txt too.

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

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

Do you have thoughts on the requirements section and the motivation to have
two different versions I had mentioned upthread?

Regards,
Samay

---

From: Andres Freund <[email-masked]>
Date: Tue, 11 Apr 2023 10:18:07 -0700

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
>  <screen>
>  meson configure -Dcassert=true
>  </screen>
> -    <command>meson configure</command>'s commonly used command-line options
> -    are explained in <xref linkend="meson-options"/>.
> +    Commonly used build options for <command>meson configure</command> (and <command>meson setup</command>) are explained in <xref linkend="meson-options"/>.
>     </para>
>    </step>
>  
> @@ -2078,6 +2077,13 @@ ninja
>      processes used with the command line argument <literal>-j</literal>.
>     </para>
>  
> +   <para>
> +    If you want to build the docs, you can type:
> +<screen>
> +ninja docs
> +</screen>
> +   </para>

"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 <[email-masked]>
> 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

---

From: samay sharma <[email-masked]>
Date: Tue, 11 Apr 2023 14:41:38 -0700

Hi,

On Tue, Apr 11, 2023 at 10:18 AM Andres Freund <[email-masked]> wrote:

> > +    Commonly used build options for <command>meson configure</command>
> (and <command>meson setup</command>) are explained in <xref
> linkend="meson-options"/>.
> >     </para>
> >    </step>
> >

Sure, that works.

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

Cool.

---

From: Peter Eisentraut <[email-masked]>
Date: Wed, 12 Apr 2023 23:19:23 +0200

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

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: "Tristan Partin" <[email-masked]>
Date: Tue, 30 May 2023 13:09:47 -0500

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)

---

From: "Tristan Partin" <[email-masked]>
Date: Tue, 30 May 2023 13:13:52 -0500

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

---

From: Andres Freund <[email-masked]>
Date: Fri, 9 Jun 2023 21:00:05 -0700

Hi,

> +    If you want to build the docs, you can type:

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
> Date: Mon, 6 Feb 2023 16:09:42 -0800

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?

> +      <varlistentry id="meson-option-with-blocksize">

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

>  doc/src/sgml/installation.sgml | 140 ++++++++++++++-------------------
>  1 file changed, 57 insertions(+), 83 deletions(-)
>
> index 7e65cdd72e..d7ab0c205e 100644
> @@ -1214,23 +1214,6 @@ build-postgresql:
>         </listitem>
>        </varlistentry>
>
> -     </variablelist>
> -
> -   </sect3>
> -
> -   <sect3 id="configure-options-anti-features">
> -    <title>Anti-Features</title>
> -
> -    <para>
> -     The options described in this section allow disabling
> -     certain <productname>PostgreSQL</productname> 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.
> -    </para>
> -
> -     <variablelist>
> -
>        <varlistentry id="configure-option-without-readline">
>         <term><option>--without-readline</option></term>
>         <listitem>

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

---

From: Peter Eisentraut <[email-masked]>
Date: Mon, 12 Jun 2023 22:33:16 +0200

On 10.06.23 06:00, Andres Freund wrote:
>> From: Samay Sharma<[email-masked]>

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.

---

From: vignesh C <[email-masked]>
Date: Sat, 20 Jan 2024 07:23:36 +0530

On Wed, 29 Mar 2023 at 00:57, samay sharma <[email-masked]> wrote:
>
> Hi,
>
> On Wed, Mar 15, 2023 at 4:28 AM Peter Eisentraut <[email-masked]> wrote:
>>
> I tried to make the meson setup and meson configure usage consistent. I've removed the text for the test options.
>>
>>
>>
> 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?
>
>>
>>
> 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 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.
>
>
> Do you have thoughts on the requirements section and the motivation to have two different versions I had mentioned upthread?

I have changed the status of commitfest entry to "Returned with
Feedback" as there was no followup on Tristan Partin and Andres's
comments from many months. Please handle the comments and add a new
commitfest entry if required for any pending tasks left.

Regards,
Vignesh