Feedback-ID: i131946ab:Fastmail
Message-ID: <0517adcd-4fea-f20d-0a74-31f75627138e@enterprisedb.com>
Date: Wed, 15 Mar 2023 12:28:36 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.8.0
Subject: Re: Documentation for building with meson
Content-Language: en-US
To: samay sharma <smilingsamay@gmail.com>, Andres Freund <andres@anarazel.de>
Cc: Justin Pryzby <pryzby@telsasoft.com>,
 Nazir Bilal Yavuz <byavuz81@gmail.com>,
 John Naylor <john.naylor@enterprisedb.com>, pgsql-hackers@postgresql.org
References: <CAJxrbywTPhfAxE52N=oXUDsNFJ5TYPzboeDxdijkFT2TwOZsUw@mail.gmail.com>
 <20221105213903.fc7igvsbhnijdmqf@awork3.anarazel.de>
 <CAJxrbyzHyfvgESwspORMU1j8sDyQK1pbJ=yx3mDLXtwCdqOLAQ@mail.gmail.com>
 <d3852278-7e67-c73b-0a51-4da20c599a22@gmail.com>
 <CAJxrbyzcMM_TLtUPWkv4R+VSYJEKo-p_fCka1X69V_YHamFhpQ@mail.gmail.com>
 <20221123063654.GH11463@telsasoft.com>
 <CAJxrbyyk8jVYCvjyiQ49DOgMzB6skmDgnWjJH_LeA8v__8dOAg@mail.gmail.com>
 <20221123201620.GK11463@telsasoft.com>
 <CAJxrbyxYyrJvx2DGogw-6ZD_evyF_iW=6-_0mJAmy-3SqYAewQ@mail.gmail.com>
 <6f9c0595-1de0-ca15-b5fc-c650c3f57016@enterprisedb.com>
 <20221201172139.ua64fs5m7soyunqk@awork3.anarazel.de>
 <CAJxrbyx2nw3NvSixGqCfmfYCzQbJkSmUx95uESr9ssWJoyMCtQ@mail.gmail.com>
From: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
In-Reply-To: <CAJxrbyx2nw3NvSixGqCfmfYCzQbJkSmUx95uESr9ssWJoyMCtQ@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: <https://www.postgresql.org/message-id/0517adcd-4fea-f20d-0a74-31f75627138e%40enterprisedb.com>
Precedence: bulk

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