Обсуждение: meson: pgxs Makefile.global differences
I started digging into a warning I noticed on my FDW builds where Postgres is built with meson, e.g. <https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=crake&dt=2023-08-16%2018%3A37%3A25&stg=FileTextArrayFDW-build> which has this:
cc1: warning: ‘-Wformat-security’ ignored without ‘-Wformat’ [-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:
HEAD: CFLAGS =-Wshadow=compatible-local
REL_15_STABLE: CFLAGS =-Wall -g
The warning is apparently due to the missing -Wall.
Shouldn't we be aiming for pretty much identical settings?
cheers
andrew
-- Andrew Dunstan EDB: https://www.enterprisedb.com
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/cgi-bin/show_stage_log.pl?nm=crake&dt=2023-08-16%2018%3A37%3A25&stg=FileTextArrayFDW-build> > which has this: > > cc1: warning: ‘-Wformat-security’ ignored without ‘-Wformat’ > [-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: > > HEAD: CFLAGS =-Wshadow=compatible-local > REL_15_STABLE: CFLAGS =-Wall -g > > The warning is apparently due to the missing -Wall. > > Shouldn't we be aiming for pretty much identical settings? I agree that they should be identical. The meson bild should definitely be aiming for 100% compatibility for the Makefile.global. -- Tristan Partin Neon (https://neon.tech)
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/cgi-bin/show_stage_log.pl?nm=crake&dt=2023-08-16%2018%3A37%3A25&stg=FileTextArrayFDW-build> > > which has this: > > > > cc1: warning: ‘-Wformat-security’ ignored without ‘-Wformat’ > > [-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? > > HEAD: CFLAGS =-Wshadow=compatible-local > > REL_15_STABLE: CFLAGS =-Wall -g > > > > The warning is apparently due to the missing -Wall. > > > > Shouldn't we be aiming for pretty much identical settings? The difference for -Wshadow=compatible-local is due to changes between 15 and HEAD. We're indeed not adding -Wall right now (the warning level is handled by meson, so it doesn't show up in our cflags right now). > I agree that they should be identical. The meson bild should definitely 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 most important contents to match, while skipping lots of things that are primarily 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. Greetings, Andres Freund PS: I don't have andres@anarazel.dev , just .de :)
> On Aug 17, 2023, at 4:51 PM, Andres Freund <andres@anarazel.de> 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/cgi-bin/show_stage_log.pl?nm=crake&dt=2023-08-16%2018%3A37%3A25&stg=FileTextArrayFDW-build> >>> which has this: >>> >>> cc1: warning: ‘-Wformat-security’ ignored without ‘-Wformat’ >>> [-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? Yes, sorry, meant to type common. > > >>> HEAD: CFLAGS =-Wshadow=compatible-local >>> REL_15_STABLE: CFLAGS =-Wall -g >>> >>> The warning is apparently due to the missing -Wall. >>> >>> Shouldn't we be aiming for pretty much identical settings? > > The difference for -Wshadow=compatible-local is due to changes between 15 and > HEAD. > > We're indeed not adding -Wall right now (the warning level is handled by > meson, so it doesn't show up in our cflags right now). > > >> I agree that they should be identical. The meson bild should definitely 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 most > important contents to match, while skipping lots of things that are primarily > 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. > > Cool Cheers Andrew
On Thu Aug 17, 2023 at 3:51 PM CDT, Andres Freund wrote: > PS: I don't have andres@anarazel.dev , just .de :) Fat fingered a "v" somehow. -- Tristan Partin Neon (https://neon.tech)
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/cgi-bin/show_stage_log.pl?nm=crake&dt=2023-08-16%2018%3A37%3A25&stg=FileTextArrayFDW-build> which has this: cc1: warning: ‘-Wformat-security’ ignored without ‘-Wformat’ [-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?HEAD: CFLAGS =-Wshadow=compatible-local REL_15_STABLE: CFLAGS =-Wall -g The warning is apparently due to the missing -Wall. Shouldn't we be aiming for pretty much identical settings?The difference for -Wshadow=compatible-local is due to changes between 15 and HEAD. We're indeed not adding -Wall right now (the warning level is handled by meson, so it doesn't show up in our cflags right now).I agree that they should be identical. The meson bild should definitely 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 most important contents to match, while skipping lots of things that are primarily 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 debug-enabled builds?
cheers
andrew
-- Andrew Dunstan EDB: https://www.enterprisedb.com
On 21.08.23 17:33, Andrew Dunstan wrote: > Where should we do that? And how about the -g that's also missing for > debug-enabled builds? I think it's the options in these two tables that meson handles internally and that we should explicitly reproduce for Makefile.global: https://mesonbuild.com/Builtin-options.html#details-for-buildtype https://mesonbuild.com/Builtin-options.html#details-for-warning_level
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/cgi-bin/show_stage_log.pl?nm=crake&dt=2023-08-16%2018%3A37%3A25&stg=FileTextArrayFDW-build> > >>> which has this: > >>> > >>> cc1: warning: ‘-Wformat-security’ ignored without ‘-Wformat’ > >>> [-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? > > > > > >>> HEAD: CFLAGS =-Wshadow=compatible-local > >>> REL_15_STABLE: CFLAGS =-Wall -g > >>> > >>> The warning is apparently due to the missing -Wall. > >>> > >>> Shouldn't we be aiming for pretty much identical settings? > > The difference for -Wshadow=compatible-local is due to changes between 15 and > > HEAD. > > > > We're indeed not adding -Wall right now (the warning level is handled by > > meson, so it doesn't show up in our cflags right now). > > > > > >> I agree that they should be identical. The meson bild should definitely 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 most > > important contents to match, while skipping lots of things that are primarily > > 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 > 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 = var_cflags + cc.get_supported_arguments('-Wxxx') if get_option('debug') # Populate for debug flags that aren't -g debug_flags = {} pgxs_cflags += debug_flags.get(cc.get_id(), cc.get_supported_arguments('-g') endif ... CFLAGS: pgxs_cflags, ... -- Tristan Partin Neon (https://neon.tech)