Mime-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset=UTF-8
Date: Fri, 09 Jun 2023 13:15:27 -0500
Message-Id: <CT8BP7BRV4A2.AOE7263UX2SH@gonk>
Cc: <pgsql-hackers@postgresql.org>
Subject: Re: Meson build updates
To: "Andres Freund" <andres@anarazel.de>
From: "Tristan Partin" <tristan@neon.tech>
References: <CSPIJVUDZFKX.3KHMOAVGF94RV@c3po>
 <20230609164135.77fjwqkqzdxfs3f5@awork3.anarazel.de>
In-Reply-To: <20230609164135.77fjwqkqzdxfs3f5@awork3.anarazel.de>
Archived-At: 
 <https://www.postgresql.org/message-id/CT8BP7BRV4A2.AOE7263UX2SH%40gonk>
Precedence: bulk

On Fri Jun 9, 2023 at 11:41 AM CDT, Andres Freund wrote:
> Hi,
>
> On 2023-05-18 10:36:59 -0500, Tristan Partin wrote:
> > From b35ecb2c8dcd71608f98af1e0ec19d965099ceab Mon Sep 17 00:00:00 2001
> > From: Tristan Partin <tristan@neon.tech>
> > Date: Wed, 17 May 2023 09:40:02 -0500
> > Subject: [PATCH postgres v1 12/17] Make finding pkg-config(python3) mor=
e
> >  robust
> >=20
> > It is a possibility that the installation can't be found. Checking for
> > Python.h is redundant with what Meson does internally.
>
> That's not what I saw - we had cases where Python.h was missing, but the
> python dependency was found. It's possible that this is dependent on the
> meson version.

Eli corrected me on this. Please see version 2 of the patchset.
>
> > From 47394ffd113d4170e955bc033841cb7e18fd3ac4 Mon Sep 17 00:00:00 2001
> > From: Tristan Partin <tristan@neon.tech>
> > Date: Wed, 17 May 2023 09:44:49 -0500
> > Subject: [PATCH postgres v1 14/17] Reduce branching on Meson version
> >=20
> > This code had a branch depending on Meson version. Instead, we can just
> > move the system checks to the if statement. I believe this also keeps
> > selinux and systemd from being looked for on non-Linux systems when
> > using Meson < 0.59. Before they would be checked, but obviously fail.
>
> I like the current version better - depending on the meson version makes =
it
> easy to find what needs to be removed when we upgrade the meson minimum
> version.

I think the savings of not looking up selinux/systemd on non-Linux
systems is pretty big. Would you accept a change of something like:

if meson.version().version_compare('>=3D0.59')
  # do old stuff
else if host_system =3D=3D 'linux'
  # do new stuff
endif

Otherwise, I am happy to remove the patch.

> > From 189d3ac3d5593ce3e475813e4830a29bb4e96f70 Mon Sep 17 00:00:00 2001
> > From: Tristan Partin <tristan@neon.tech>
> > Date: Wed, 17 May 2023 10:36:52 -0500
> > Subject: [PATCH postgres v1 16/17] Add Meson overrides
> >=20
> > Meson has the ability to do transparent overrides when projects are use=
d
> > as subprojects. For instance, say I am building a Postgres extension. I
> > can define Postgres to be a subproject of my extension given the
> > following wrap file:
> >=20
> > [wrap-git]
> > url =3D https://git.postgresql.org/git/postgresql.git
> > revision =3D master
> > depth =3D 1
> >=20
> > [provide]
> > dependency_names =3D libpq
> >=20
> > Then in my extension (root project), I can have the following line
> > snippet:
> >=20
> > libpq =3D dependency('libpq')
> >=20
> > This will tell Meson to transparently compile libpq prior to it
> > compiling my extension (because I depend on libpq) if libpq isn't found
> > on the host system.
> >=20
> > I have also added overrides for the various public-facing exectuables.
> > Though I don't expect them to get much usage, might as well go ahead an=
d
> > override them. They can be used by adding the following line to the
> > aforementioned wrap file:
>
> I think adding more boilerplate to all these places does have some cost. =
So
> I'm not really convinced this is worth doign.

Could you explain more about what costs you foresee? I thought this was
a pretty free change :). Most of the meson.build files seems to be
pretty much copy/pastes of each other, so if a new executable came
around, then someone would just get the override line for essentially
free, minus changing the binary name/executable name.

> > From 5ee13f09e4101904dbc9887bd4844eb5f1cb4fea Mon Sep 17 00:00:00 2001
> > From: Tristan Partin <tristan@neon.tech>
> > Date: Wed, 17 May 2023 10:54:53 -0500
> > Subject: [PATCH postgres v1 17/17] Remove Meson program options for spe=
cifying
> >  paths
> >=20
> > Meson has a built-in way to override paths without polluting project
> > build options called machine files.
>
> I think meson machine files are just about unusable. For one, you can't
> actually change any of the options without creating a new build dir. For
> another, it's not something that easily can be done on the commandline.
>
> So I really don't want to remove these options.

I felt like this would be the most controversial change. What could be
done in upstream Meson to make this a more enjoyable experience? I do
however disagree with the usability of machine files. Could you add a
little context about what you find unusable about them?

Happy to revert the change after continuing the discussion, of course.

--=20
Tristan Partin
Neon (https://neon.tech)