Re: Documentation for building with meson

From: samay sharma <smilingsamay(at)gmail(dot)com>
To: Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Justin Pryzby <pryzby(at)telsasoft(dot)com>, Nazir Bilal Yavuz <byavuz81(at)gmail(dot)com>, John Naylor <john(dot)naylor(at)enterprisedb(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Documentation for building with meson
Date: 2023-03-28 19:27:26
Message-ID: CAJxrbywZDLXga8kz3brvDd81M6QNbTN2DHM5v7uDgSm8=RROEQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On Wed, Mar 15, 2023 at 4:28 AM Peter Eisentraut <
peter(dot)eisentraut(at)enterprisedb(dot)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 <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.)
>

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

Attachment Content-Type Size
v9-0004-Remove-Miscellaneous-section.patch application/octet-stream 8.8 KB
v9-0002-Add-data-layout-options-sub-section-in-installati.patch application/octet-stream 9.0 KB
v9-0001-Make-minor-additions-and-corrections-to-meson-doc.patch application/octet-stream 2.8 KB
v9-0003-Remove-Anti-Features-section-from-Installation-fr.patch application/octet-stream 8.1 KB
v9-0005-Change-Short-Version-for-meson-installation-guide.patch application/octet-stream 2.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Pavel Stehule 2023-03-28 19:34:20 Re: Schema variables - new implementation for Postgres 15
Previous Message Isaac Morland 2023-03-28 19:25:05 Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions