Re: Documentation for building with meson

On Wed, 29 Mar 2023 at 00:57, samay sharma <smilingsamay(at)gmail(dot)com> wrote:
>
> 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?

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