Re: Documentation for building with meson - Mailing list pgsql-hackers

On Wed, Oct 19, 2022 at 11:35:10AM -0700, samay sharma wrote:
> Creating a new thread focussed on adding docs for building Postgres with
> meson. This is a spinoff from the original thread [1] and I've attempted to
> address all the feedback provided there in the attached patch.
>
> Please let me know your thoughts.

It's easier to review rendered documentation.
I made a rendered copy available here:
https://api.cirrus-ci.com/v1/artifact/task/6084297781149696/html_docs/html_docs/install-meson.html

+      <application>Flex</application> and <application>Bison</application>
+      are needed to build <productname>PostgreSQL</productname> using
+      <application>meson</application>. Be sure to get
+      <application>Flex</application> 2.5.31 or later and
+      <application>Bison</application> 1.875 or later from your package manager.
+      Other <application>lex</application> and <application>yacc</application>
+      programs cannot be used.

These versions need to be updated, see also: 57bab3330:
  - b086a47a270 "Bump minimum version of Bison to 2.3"
  - 8b878bffa8d "Bump minimum version of Flex to 2.5.35"

+         will be enabled automatically if the required packages are found.

should refer to files/libraries/headers/dependencies rather than
"packages" ?

+         default is false that is to use <application>Readline</application>.

"that is to use" should be parenthesized or separate with commas, like
| default is false, that is to use <application>Readline</application>.

zlib is mentioned twice, the first being "strongly recommended".
Is that intended?  Also, basebackup can use zlib, too.

+         If you have the binaries for certain by programs required to build

remove "by" ?

+         Postgres (with or without optional flags) stored at non-standard
+         paths, you could specify them manually to meson configure. The complete
+         list of programs for whom this is supported can be found by running

for *which

Actually, I suggest to say:
|If a program required to build Postgres (with or without optional flags)
|is stored in a non-standard path, ...

+     a build with a different value of these options.

.. with different values ..

+     the server, it is recommended to use atleast the <option>--buildtype=debug</option>

at least

+    and it's options in the meson documentation.

its

Maybe other things should have <productname> ?

 Git
 Libxml2
 libxslt
 visual studio
 DTrace
 ninja

+      <application>Flex</application> and <application>Bison</application>

Maybe these should use <productname> ?

+      be installed with <literal>pip</literal>.

Should be <application> ?

This part is redundant with prior text:
" To use this option, you will need an implementation of the Gettext API. "

+ Enabls use of the Zlib library

typo: Enabls

+ This option is set to true by default and setting it to false will

change "and" to ";" for spinlocks and atomics?

+ Debug info is generated but the result is not optimized.

Maybe say the "code" is not optimized ?

+ the tests can slow down the server significantly

remove "can"

+ You can override the amount of parallel processes used with

s/amount/number/

+ If you'd like to build with a backend other that ninja

other *than

+      the <acronym>GNU</acronym> C library then you will additionally

library comma

+    argument. If no <literal>srcdir</literal> is given Meson will deduce the

given comma

+     It should be noted that after the initial configure step

step comma

+    After the installation you can free disk space by removing the built

installation comma

+    To learn more about running the tests and how to interpret the results
+    you can refer to the documentation for interpreting test results.

interpret the results comma
"for interpreting test results" seems redundant.

+ ninja install should work for most cases but if you'd like to use more options

cases comma

Starting with "Developer Options", this intermixes postgres
project-specific options like cassert and tap-tests with meson's stuff
like buildtype and werror.  IMO there's too much detail about meson's
options, which I think is better left to that project's own
documentation, and postgres docs should include only a brief mention and
a reference to their docs.

+ Ninja will automatically detect the number of CPUs in your computer and
+ parallelize itself accordingly. You can override the amount of parallel
+ processes used with the command line argument -j.

too much detail for my taste..

--
Justin