Re: Documentation for building with meson

From: samay sharma <smilingsamay(at)gmail(dot)com>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, Justin Pryzby <pryzby(at)telsasoft(dot)com>, Nazir Bilal Yavuz <byavuz81(at)gmail(dot)com>, John Naylor <john(dot)naylor(at)enterprisedb(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Documentation for building with meson
Date: 2023-02-25 05:40:58
Message-ID: CAJxrbyx2nw3NvSixGqCfmfYCzQbJkSmUx95uESr9ssWJoyMCtQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On Thu, Dec 1, 2022 at 9:21 AM Andres Freund <andres(at)anarazel(dot)de> wrote:

> Hi,
>
> On 2022-12-01 15:58:39 +0100, Peter Eisentraut wrote:
> > On 23.11.22 22:24, samay sharma wrote:
> > > Thank you. Attaching v7 addressing most of the points below.
> >
> > I have committed this, after some editing and making some structural
> > changes.
>
> Thanks. I was working on that too, but somehow felt a bit stuck...
>
> I'll try if I can adapt my pending changes.
>

I got back to working on the meson docs. I'm attaching a new patch set
proposing some improvements to the current installation docs. I've tried to
make some corrections and improvements suggested in this thread while
trying to maintain consistency across make and meson docs as per Peter's
ask. There are 5 patches in the patch-set:

v8-0001: Makes minor corrections, adds instructions to build docs and adds
a few links to meson docs.
v8-0002, v8-0003 and v8-0004 make changes to restructure the configure
options based on reasoning discussed below. To maintain consistency, I've
made those changes on both the make and meson side.
v8-0005 Reproposes the Short Version I had proposed in v7 as I feel we
should discuss that proposal. I think it improves things in terms of
installation docs. More details below.

>
> > I moved the "Requirements" section back to the top level. It did
> > not look appealing to have to maintain two copies of this that have
> almost
> > no substantial difference (but for some reason were written with separate
> > structure and wording).
>

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.

>
> I don't think this is good. The whole "The following software packages are
> required for building PostgreSQL" section is wrong now. "They are not
> required in the default configuration, but they are needed when certain
> build
> options are enabled, as explained below:" section is misleading as well.
>
> By the time we fix all of those we'll end up with a different section
> again.
>
>
> > Also, I rearranged the Building with Meson section to use the same
> internal
> > structure as the Building with Autoconf and Make section. This will
> make it
> > easier to maintain going forward. For example if someone adds a new
> option,
> > it will be easier to find the corresponding places in the lists where to
> add
> > them.

> I don't know. The existing list order makes very little sense to me. The
> E.g. --enable-nls is before the rest in configure, presumably because it
> sorts
> there alphabetically. But that's not the case for meson.
>
> Copying "Anti-Features" as a structure element to the meson docs seems
> bogus
> (also the name is bogus, but that's a pre-existing issue). There's no
> difference in -Dreadline= to the other options meson-options-features list.

> Nor does -Dspinlocks= -Datomics= make sense in the "anti features"
> section. It
> made some sense for autoconf because of the --without- prefix, but that's
> not
> at play in meson. Their placement in the "Developer Options" made a whole
> lot
> more sense.
>

I agree "Anti-Features" desn't make sense in the meson context. One of my
patches removes that section and moves some options into the "Postgres
Features" section and others into the "Developer Options" section. I've
proposed to make those changes on both sides to make it easier to maintain.

>
> I don't like "Miscellaneous" bit containing minor stuff like krb_srvnam and
> data layout changing options like blocksize,segsize,wal_blocksize. But it
> makes sense to change that for both at the same time.
>

I've proposed a patch to add a new "Data Layout Options" section which
includes: blocksize, segsize and wal_blocksize. I've created that section
on both sides.

>
>
> > We will likely keep iterating on the contents for the next little while,
> but
> > I'm glad we now have a structure in place that we should be able to live
> > with.
>

I feel that there are a few shortcomings of the current "Short Version". I
tried to address them in my previous proposal but I noticed that a version
similar to the make version was committed. So, I thought I'd describe why I
proposed a new structure.

1) The current version has OS specific commands (eg. adduser). They don't
work across all platforms.
2) The installation instructions use paths which require sudo and which are
also OS specific. Not every developer who wants to build and try out
Postgres might have sudo permissions.
3) Most developers have a separate installation path where they store their
dev binaries while the current instructions install them in standard paths.
4) I wanted to have a separate directory which can nicely be cleaned once
you're done with building and testing the packages. That's easier to do at
a local path.
5) There's no description of what each instruction does so that developers
can modify the commands if they want to change something.

Due to these reasons, I feel it's worth considering the newer version of
the "Short version". I've proposed to change it only in the meson docs for
now but if there's interest I can modify the make instructions to be the
same. I left them as it is as people might be used to those instructions.

Regards,
Samay

>
> I agree that it's good to have something we can work from more
> iteratively. But I don't think this is a structure that we can live with.
>
>
> I'm not particularly happy about this level of structural change made
> without
> discussing it prior.
>
> Greetings,
>
> Andres Freund
>

Attachment Content-Type Size
v8-0002-Add-data-layout-options-sub-section-in-installati.patch application/octet-stream 8.9 KB
v8-0005-Change-Short-Version-for-meson-installation-guide.patch application/octet-stream 2.0 KB
v8-0001-Make-minor-additions-and-corrections-to-meson-doc.patch application/octet-stream 2.7 KB
v8-0003-Remove-Anti-Features-section-from-Installation-fr.patch application/octet-stream 9.1 KB
v8-0004-Re-organize-Miscellaneous-section.patch application/octet-stream 7.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2023-02-25 05:54:17 Re: pg_upgrade and logical replication
Previous Message Justin Pryzby 2023-02-25 05:02:14 Re: Add LZ4 compression in pg_dump