Re: Documentation for building with meson

Hi,

On Wed, Oct 19, 2022 at 7:43 PM Justin Pryzby <pryzby(at)telsasoft(dot)com> wrote:

> 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

Thanks for your for review. Attached v2 of the patch here.

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

Changed

>
> + will be enabled automatically if the required packages are found.
>
> should refer to files/libraries/headers/dependencies rather than
> "packages" ?
>

Changed to dependencies

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

Yes, the first is in the requirements section where we just list packages
required / recommended. The other mention is in the list of configure
options. This is similar to how the documentation looks today for make /
autoconf. Added pg_basebackup as a use case too.

>
> + If you have the binaries for certain by programs required to
> build
>
> remove "by" ?
>

Done

>
> + 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, ...
>

Liked this framing better. Changed.

>
> + a build with a different value of these options.
>
> .. with different values ..
>

Done

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

>
> + and it's options in the meson documentation.
>
> its
>
Done

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

I'm unsure of the right protocol for this. I tried to follow the precedent
set in the make / autoconf part of the documentation, which uses
<productname> at certain places and <application> at others. Is there a
reference or guidance on which to use where or is it mostly a case by case
decision?

> + be installed with <literal>pip</literal>.
>
> Should be <application> ?
>

Changed.

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

Modified.

>
> + Enabls use of the Zlib library
>
> typo: Enabls
>

Fixed.

>
> + This option is set to true by default and setting it to false will
>
> change "and" to ";" for spinlocks and atomics?
>

Done

>
> + Debug info is generated but the result is not optimized.
>
> Maybe say the "code" is not optimized ?
>

Changed

>
> + the tests can slow down the server significantly
>
> remove "can"
>

Done.

>
> + You can override the amount of parallel processes used with
>
> s/amount/number/
>

Done

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

Fixed.

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

Added

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

Added

>
> + It should be noted that after the initial configure step
>
> step comma
>

Added

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

Added

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

Changed.

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

Added

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

The meson specific options I've chosen to document are: auto_features,
backend, c_args, c_link_args, buildtype, optimization, werror,
errorlogs and b_coverage as I felt they might be used often and are useful
to know. But, it's very possible that some of them might be obvious and
others may not be as useful as I thought. Are there specific ones you'd
suggest we can remove? Also, if you're curious, this is the list I picked
from: https://mesonbuild.com/Commands.html#configure.

In terms of detail about individual options, I think the descriptions about
most of them are brief but buildtype was pretty verbose. I have shortened
it.

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

I added this as make / autoconf doesn't do something like this. So, it
might be useful to know for people switching over.

Regards,
Samay

>
> --
> Justin
>