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 <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
v9-0004-Remove-Miscellaneous-section.patch
Description: Binary data
v9-0002-Add-data-layout-options-sub-section-in-installati.patch
Description: Binary data
v9-0001-Make-minor-additions-and-corrections-to-meson-doc.patch
Description: Binary data
v9-0003-Remove-Anti-Features-section-from-Installation-fr.patch
Description: Binary data
v9-0005-Change-Short-Version-for-meson-installation-guide.patch
Description: Binary data
