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

On Fri Jun 9, 2023 at 1:36 PM CDT, Andres Freund wrote:
> Hi,
>
> On 2023-06-09 13:15:27 -0500, Tristan Partin wrote:
> > On Fri Jun 9, 2023 at 11:41 AM CDT, Andres Freund wrote:
> > > I like the current version better - depending on the meson version ma=
kes it
> > > easy to find what needs to be removed when we upgrade the meson minim=
um
> > > version.
> >=20
> > 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:
>
> For selinux it's default disabled, so it doesn't make a practical
> difference. Outside of linux newer versions of meson are more common IME,=
 so
> I'm not really worried about it for systemd.
>
>
> > if meson.version().version_compare('>=3D0.59')
> >   # do old stuff
> > else if host_system =3D=3D 'linux'
> >   # do new stuff
> > endif
> >=20
> > Otherwise, I am happy to remove the patch.
>
> Hm, I don't quite know how this would end up looking like.

Actually, nevermind. I don't know what I was talking about. I will just
go ahead and remove this patch from the set.

> > > > From 189d3ac3d5593ce3e475813e4830a29bb4e96f70 Mon Sep 17 00:00:00 2=
001
> > > > 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=
 used
> > > > as subprojects. For instance, say I am building a Postgres extensio=
n. 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 f=
ound
> > > > on the host system.
> > > >=20
> > > > I have also added overrides for the various public-facing exectuabl=
es.
> > > > Though I don't expect them to get much usage, might as well go ahea=
d and
> > > > 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 co=
st. So
> > > I'm not really convinced this is worth doign.
> >=20
> > Could you explain more about what costs you foresee?
>
> Repetitive code that needs to be added to each further binary we add. I d=
on't
> mind doing that if it has a use case, but I'm not sure I see the use case=
 for
> random binaries...

I want to make sure I am only adding it to things that are user-facing.
So if I have added the line to executables that are for internal use
only, please correct me. In general, I override for all user-facing
programs/dependencies because I never know how some end-user might use
the binaries.

Perhaps what might sway you is that the old method (libpq =3D libpq_dep)
is very error prone because variable names become part of the public API
of your build description which no one likes. In the new way, which is
only possible when specifying overrides, as long as binary names or
pkg-config names don't change, you get stability no matter how you name
your variables.

> > > > From 5ee13f09e4101904dbc9887bd4844eb5f1cb4fea Mon Sep 17 00:00:00 2=
001
> > > > 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=
 specifying
> > > >  paths
> > > >=20
> > > > Meson has a built-in way to override paths without polluting projec=
t
> > > > 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 commandlin=
e.
> > >
> > > So I really don't want to remove these options.
> >=20
> > 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 think *requiring* separate files is a really poor experience when you c=
ome
> from some other system, where those could trivially be overwritten on the
> commandline.

Hmm. I could maybe ask around for a --program command line option. Could
you provide the syntax for what autotools does? That way I can come to
the discussion in the Meson channel with prior art. I personally don't
find the files that annoying, but to each their own.

> The biggest change to make them more usable would be to properly reconfig=
ure
> when the contents of machine file change. IIRC configure is rerun, but th=
e
> changes aren't taken into account.

I could not reproduce this. Perhaps you were testing with an older Meson
where that was the case

# meson.build
project('mytest')

myprog =3D find_program('myprog')
message(myprog.full_path())

test('dummy', find_program('echo'), args: [myprog.full_path()])

# file.ini
[binaries]
myprog =3D '/usr/bin/python3'

# CLI
meson setup build
meson test -C build
sed -i 's/python3/python2/' file.ini
meson test -C build

> > I do however disagree with the usability of machine files. Could you ad=
d a
> > little context about what you find unusable about them?
>
> I can quickly change a meson option and run a build and tests. Trivially
> scriptable. Whereas with a machine file I need to write code to edit a ma=
chine
> file, re-configure from scratch, and only then can build / run tests.
>
> It's particularly bad for cross builds, where unfortunately cross files c=
an't
> be avoided. It's imo the one area where autoconf beats meson handily.
> --host=3Dx86_64-w64-mingw32 works for autoconf. Whereas for meson I need =
to
> manually write a cross file with a bunch of binaries set.
> https://github.com/anarazel/postgres/commit/ae53f21d06b4dadf8e6b90df84000=
fad9a769eaf#diff-3420ebab4f1dbe2ba7102565b0b84e4d6d01fb8b3c1e375bd439eed604=
e743f8R1

One thing that would help you with cross files is to use constants[0].

> There's some helpers aiming to generate cross files, but I've not been ab=
le to
> generate something useful for cross compiling to windows, for example.
>
> I've been unable to generate something like referenced in the above commi=
t in
> a reasonably concise way with env2mfile.

I, too, could not generate anything meaningful with the following
command line.

CC=3Dtestcc CXX=3Dtestcxx AR=3Dtestar STRIP=3Dteststrip \
    PKGCONFIG=3Dtestpkgconfig WINDRES=3Dtestwindres meson env2mfile \
    --cross --system windows --cpu x86_64 --cpu-family x86_64 \
    --endian little -o windows.ini

[binaries]
# Compilers
c =3D ['testcc']
cpp =3D ['testcxx']

# Other binaries

[properties]

[host_machine]
cpu =3D 'x86_64'
cpu_family =3D 'x86_64'
endian =3D 'little'
system =3D 'windows'

> In the end, I also just don't see a meaningful benefit in forcing the use=
 of
> machine files.

I think it is best to use patterns tools want you to use. If aren't moved a=
t
all by the reconfigure behavior actually working, I will drop the patch.

[0]: https://mesonbuild.com/Machine-files.html#constants

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