Mime-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset=UTF-8; format=Flowed
Date: Mon, 21 Aug 2023 10:48:23 -0500
Message-Id: <CUYCAD19HJ25.36D6QV4YQHQY2@gonk>
Subject: Re: meson: pgxs Makefile.global differences
To: "Andrew Dunstan" <andrew@dunslane.net>, "Andres Freund"
 <andres@anarazel.de>
From: "Tristan Partin" <tristan@neon.tech>
Cc: "PostgreSQL Hackers" <pgsql-hackers@lists.postgresql.org>
References: <61d629b6-d5ad-a49e-e905-b66d3ac4903a@dunslane.net>
 <CUV2U1MR5T8E.2T55FGZG23INJ@gonk>
 <20230817205142.lkinsfndjabpdejv@alap3.anarazel.de>
 <bee50945-c841-5e6d-f62e-74037e8cb352@dunslane.net>
In-Reply-To: <bee50945-c841-5e6d-f62e-74037e8cb352@dunslane.net>
Archived-At: <https://www.postgresql.org/message-id/CUYCAD19HJ25.36D6QV4YQHQY2%40gonk>
Precedence: bulk

On Mon Aug 21, 2023 at 10:33 AM CDT, Andrew Dunstan wrote:
>
> On 2023-08-17 Th 16:51, Andres Freund wrote:
> > Hi,
> >
> > On 2023-08-17 14:45:54 -0500, Tristan Partin wrote:
> >> On Thu Aug 17, 2023 at 2:32 PM CDT, Andrew Dunstan wrote:
> >>> I started digging into a warning I noticed on my FDW builds where
> >>> Postgres is built with meson, e.g.<https://buildfarm.postgresql.org/c=
gi-bin/show_stage_log.pl?nm=3Dcrake&dt=3D2023-08-16%2018%3A37%3A25&stg=3DFi=
leTextArrayFDW-build>
> >>> which has this:
> >>>
> >>> cc1: warning: =E2=80=98-Wformat-security=E2=80=99 ignored without =E2=
=80=98-Wformat=E2=80=99
> >>> [-Wformat-security]
> >>>
> >>> I found that the pgxs Makefile.global built under meson is a bit
> >>> different. On debug builds for both this is what I get on HEAD (meson=
)
> >>> and REL_15_STABLE (autoconf), stripped of the current components:
> > I assume "current" means the flags that are present in both cases?
> >
> >
> >>>   =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 HEAD: CFLAGS =3D-W=
shadow=3Dcompatible-local
> >>> REL_15_STABLE: CFLAGS =3D-Wall=C2=A0 -g
> >>>
> >>> The warning is apparently due to the missing -Wall.
> >>>
> >>> Shouldn't we be aiming for pretty much identical settings?
> > The difference for -Wshadow=3Dcompatible-local is due to changes betwee=
n 15 and
> > HEAD.
> >
> > We're indeed not adding -Wall right now (the warning level is handled b=
y
> > meson, so it doesn't show up in our cflags right now).
> >
> >
> >> I agree that they should be identical. The meson bild should definitel=
y be
> >> aiming for 100% compatibility for the Makefile.global.
> > I don't think that's feasible. It was a fair bit of work to get the mos=
t
> > important contents to match, while skipping lots of things that are pri=
marily
> > relevant for building the server (which isn't relevant for pgxs).
> >
> > That said, in this specific case, I agree, we should likely emit -Wall =
to
> > Makefile.global in meson as well.
> >
>
> Where should we do that? And how about the -g that's also missing for=20
> debug-enabled builds?

Look in src/makefiles/meson.build. You will see a line like
'CFLAGS': var_cflags. You probably want to do something like:

	pgxs_cflags =3D var_cflags + cc.get_supported_arguments('-Wxxx')
	if get_option('debug')
		# Populate for debug flags that aren't -g
		debug_flags =3D {}

		pgxs_cflags +=3D debug_flags.get(cc.get_id(),=20
			cc.get_supported_arguments('-g')
	endif

	...
	CFLAGS: pgxs_cflags,
	...

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