Thread: Clean up build warnings of plperl with clang-12+
Hi all, This is a follow-up of the discussion that happened here: https://www.postgresql.org/message-id/YYoJYO475dsqYhta@paquier.xyz Recently, we are getting warnings in the build of plperl when using at least clang-12 because of their new flag -Wcompound-token-split-by-macro, all the warnings coming from Perl itself. Upstream has posted a fix about that but it will take time before this propagates across the buildfarm (five~ animals are now impacted?): https://www.nntp.perl.org/group/perl.perl5.changes/2021/07/msg57758.html dangomushi is impacted by that, and I'd like to stick an -Werror in it as it is proving to be good in detecting incorrect placeholders. Tom has suggested that we could add -Wno-compound-token-split-by-macro to take care of the issue on our side, and attached is a patch to do so. Any objections? I'd like to get this back-patched. Thanks, -- Michael
Attachment
On Wed, Nov 10, 2021 at 07:35:21AM +0900, Michael Paquier wrote: > Tom has suggested that we could add -Wno-compound-token-split-by-macro > to take care of the issue on our side, and attached is a patch to do > so. > > Any objections? I'd like to get this back-patched. Backpatched this one as of 9ff47ea. That should allow the addition of -Werror on dangomushi. -- Michael
Attachment
Michael Paquier <michael@paquier.xyz> writes: > Backpatched this one as of 9ff47ea. That should allow the addition of > -Werror on dangomushi. Cool. I have also enabled -Werror on florican. regards, tom lane
Michael Paquier <michael@paquier.xyz> writes: > Backpatched this one as of 9ff47ea. That should allow the addition of > -Werror on dangomushi. I see you tried to do that by adding -Werror to CPPFLAGS. That doesn't work because it breaks configure's tests. (I wonder if meson is any smarter than autoconf about that.) The way I do it on my own animals is like if ($branch eq 'HEAD' or $branch ge 'REL9_3') { # Add -Werror so we get errors for warning conditions. # Pre-9.3 PG doesn't compile cleanly with Sierra's cc. # Can't put this in CFLAGS because it breaks configure, # so use COPT instead. $conf{build_env}{COPT} = "-Werror"; } Guess I could drop the branch check now. regards, tom lane
On 11/11/21 13:18, Tom Lane wrote: > Michael Paquier <michael@paquier.xyz> writes: >> Backpatched this one as of 9ff47ea. That should allow the addition of >> -Werror on dangomushi. > I see you tried to do that by adding -Werror to CPPFLAGS. That doesn't > work because it breaks configure's tests. (I wonder if meson is any > smarter than autoconf about that.) The way I do it on my own > animals is like > > if ($branch eq 'HEAD' or $branch ge 'REL9_3') > { > # Add -Werror so we get errors for warning conditions. > # Pre-9.3 PG doesn't compile cleanly with Sierra's cc. > # Can't put this in CFLAGS because it breaks configure, > # so use COPT instead. > $conf{build_env}{COPT} = "-Werror"; > } > > Guess I could drop the branch check now. > > Wouldn't it be better in any case just to add the clang fix for building plperl rather than globally? cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Andrew Dunstan <andrew@dunslane.net> writes: > Wouldn't it be better in any case just to add the clang fix for building > plperl rather than globally? That'd be considerably messier wouldn't it? (And I wonder if the meson system will be able to do it at all.) If you feel motivated to make that happen, go for it, but I don't. regards, tom lane
On Thu, Nov 11, 2021 at 01:18:32PM -0500, Tom Lane wrote: > I see you tried to do that by adding -Werror to CPPFLAGS. That doesn't > work because it breaks configure's tests. (I wonder if meson is any > smarter than autoconf about that.) The way I do it on my own > animals is like Nya. (Japanized Meh) > if ($branch eq 'HEAD' or $branch ge 'REL9_3') > { > # Add -Werror so we get errors for warning conditions. > # Pre-9.3 PG doesn't compile cleanly with Sierra's cc. > # Can't put this in CFLAGS because it breaks configure, > # so use COPT instead. > $conf{build_env}{COPT} = "-Werror"; > } > > Guess I could drop the branch check now. Thanks. I'll switch to something like that. -- Michael
Attachment
On Thu, Nov 11, 2021 at 03:51:35PM -0500, Tom Lane wrote: > That'd be considerably messier wouldn't it? Yes, that would be a bit messier. For example, we could do that with something saved in Makefile.global.in by ./configure that plperl feeds on to add this extra CFLAGS in its own local Makefile. > (And I wonder if the > meson system will be able to do it at all.) If you feel motivated > to make that happen, go for it, but I don't. Neither am I. 9ff47ea is the least invasive solution, at least. -- Michael