Hi, On Wed, Oct 19, 2022 at 7:43 PM Justin Pryzby <pry...@telsasoft.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 >
v2-0001-Add-docs-for-building-with-meson.patch
Description: Binary data
