Обсуждение: Minor configure tweak to simplify adjusting gcc warnings
Would anyone object to modifying configure.in like this:if test "$GCC" = yes -a "$ICC" = no; then
- CFLAGS="$CFLAGS -Wall -Wmissing-prototypes -Wpointer-arith"
+ CFLAGS="-Wall $CFLAGS -Wmissing-prototypes -Wpointer-arith" # These work in some but not all gcc versions
PGAC_PROG_CC_CFLAGS_OPT([-Wdeclaration-after-statement])
The reason I got interested in this is that I attempted to pass in
"CFLAGS=-Wno-format" to configure, to suppress format warnings on
buildfarm member gaur (whose gcc is too old to recognize z modifiers).
That doesn't work because -Wall turns the warnings right back on again.
If the user-supplied CFLAGS were inserted after -Wall then it would work.
A slightly more complicated change could be applied to make sure that
*all* of the CFLAGS forcibly inserted by configure appear before any
externally-sourced CFLAGS, allowing any of them to be overridden from the
environment variable. I'm not sure if it's worth the trouble to do that,
but if there's interest I could make it happen.
regards, tom lane
Hi Tom, On 2015-01-13 22:19:30 -0500, Tom Lane wrote: > Would anyone object to modifying configure.in like this: > > if test "$GCC" = yes -a "$ICC" = no; then > - CFLAGS="$CFLAGS -Wall -Wmissing-prototypes -Wpointer-arith" > + CFLAGS="-Wall $CFLAGS -Wmissing-prototypes -Wpointer-arith" > # These work in some but not all gcc versions > PGAC_PROG_CC_CFLAGS_OPT([-Wdeclaration-after-statement]) I'd actually vote for moving $CFLAGS - no point in not allowing everything to be overwritten/overruled. > The reason I got interested in this is that I attempted to pass in > "CFLAGS=-Wno-format" to configure, to suppress format warnings on > buildfarm member gaur (whose gcc is too old to recognize z modifiers). > That doesn't work because -Wall turns the warnings right back on again. > If the user-supplied CFLAGS were inserted after -Wall then it would work. In the line of http://archives.postgresql.org/message-id/54B58BA3.8040302%40ohmu.fi in wonder if the better fix isn't to define pg_format_attribute(...) and define it empty that if the compiler doesn't support what we want. > A slightly more complicated change could be applied to make sure that > *all* of the CFLAGS forcibly inserted by configure appear before any > externally-sourced CFLAGS, allowing any of them to be overridden from the > environment variable. I'm not sure if it's worth the trouble to do that, > but if there's interest I could make it happen. I think it'd be good idea, but unless you're enthusiastic I guess there are more important things. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Andres Freund <andres@2ndquadrant.com> writes: > On 2015-01-13 22:19:30 -0500, Tom Lane wrote: >> The reason I got interested in this is that I attempted to pass in >> "CFLAGS=-Wno-format" to configure, to suppress format warnings on >> buildfarm member gaur (whose gcc is too old to recognize z modifiers). >> That doesn't work because -Wall turns the warnings right back on again. >> If the user-supplied CFLAGS were inserted after -Wall then it would work. > In the line of > http://archives.postgresql.org/message-id/54B58BA3.8040302%40ohmu.fi in > wonder if the better fix isn't to define pg_format_attribute(...) and > define it empty that if the compiler doesn't support what we want. Well, that would only fix my problem if we added a configure-time test for whether gcc recognizes "z", which frankly seems like a waste of cycles. I've probably got the last one left in captivity that doesn't. Not that I have any great objection to improving the attribute-slinging logic. It just doesn't seem like a good fix for this particular issue. >> A slightly more complicated change could be applied to make sure that >> *all* of the CFLAGS forcibly inserted by configure appear before any >> externally-sourced CFLAGS, allowing any of them to be overridden from the >> environment variable. I'm not sure if it's worth the trouble to do that, >> but if there's interest I could make it happen. > I think it'd be good idea, but unless you're enthusiastic I guess there > are more important things. Nah, I'm fine with doing it, it's just a couple more lines of code. I feared people might think it wasn't worth adding extra complexity for, but as long as someone else likes the idea I'll go do it. regards, tom lane
On 2015-01-14 09:34:23 -0500, Tom Lane wrote: > Andres Freund <andres@2ndquadrant.com> writes: > > On 2015-01-13 22:19:30 -0500, Tom Lane wrote: > >> The reason I got interested in this is that I attempted to pass in > >> "CFLAGS=-Wno-format" to configure, to suppress format warnings on > >> buildfarm member gaur (whose gcc is too old to recognize z modifiers). > >> That doesn't work because -Wall turns the warnings right back on again. > >> If the user-supplied CFLAGS were inserted after -Wall then it would work. > > > In the line of > > http://archives.postgresql.org/message-id/54B58BA3.8040302%40ohmu.fi in > > wonder if the better fix isn't to define pg_format_attribute(...) and > > define it empty that if the compiler doesn't support what we want. > > Well, that would only fix my problem if we added a configure-time test > for whether gcc recognizes "z", which frankly seems like a waste of > cycles. I've probably got the last one left in captivity that doesn't. Hm. I had kinda assumed that %z support for sprintf and gcc's recognition of the format string would coincide and we could just use the %z result. But gull's output doesn't actually that way. > >> A slightly more complicated change could be applied to make sure that > >> *all* of the CFLAGS forcibly inserted by configure appear before any > >> externally-sourced CFLAGS, allowing any of them to be overridden from the > >> environment variable. I'm not sure if it's worth the trouble to do that, > >> but if there's interest I could make it happen. > > > I think it'd be good idea, but unless you're enthusiastic I guess there > > are more important things. > > Nah, I'm fine with doing it, it's just a couple more lines of code. > I feared people might think it wasn't worth adding extra complexity for, > but as long as someone else likes the idea I'll go do it. Ok cool. I had thought the templates, subdirectory overrides would make it slightly less than trivial. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Andres Freund <andres@2ndquadrant.com> writes:
> On 2015-01-14 09:34:23 -0500, Tom Lane wrote:
>> Well, that would only fix my problem if we added a configure-time test
>> for whether gcc recognizes "z", which frankly seems like a waste of
>> cycles. I've probably got the last one left in captivity that doesn't.
> Hm. I had kinda assumed that %z support for sprintf and gcc's
> recognition of the format string would coincide and we could just use
> the %z result. But gull's output doesn't actually that way.
It's only reasonable to assume that gcc matches sprintf if gcc is the
native (vendor-supplied) compiler for the platform. That's not the
case on gaur. It used to be very very commonly not the case, though
I think a lot of vendors have now abandoned their proprietary compilers.
If we were to test for this at all, I think we'd need to make the test
separate from the one for sprintf's behavior.
regards, tom lane
On 2015-01-14 10:01:39 -0500, Tom Lane wrote: > Andres Freund <andres@2ndquadrant.com> writes: > > On 2015-01-14 09:34:23 -0500, Tom Lane wrote: > >> Well, that would only fix my problem if we added a configure-time test > >> for whether gcc recognizes "z", which frankly seems like a waste of > >> cycles. I've probably got the last one left in captivity that doesn't. > > > Hm. I had kinda assumed that %z support for sprintf and gcc's > > recognition of the format string would coincide and we could just use > > the %z result. But gull's output doesn't actually that way. > > It's only reasonable to assume that gcc matches sprintf if gcc is the > native (vendor-supplied) compiler for the platform. That's not the > case on gaur. It used to be very very commonly not the case, though > I think a lot of vendors have now abandoned their proprietary compilers. > If we were to test for this at all, I think we'd need to make the test > separate from the one for sprintf's behavior. I've already given up... Given how infrequent it is, suppressing it for gull seems sufficient. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Andres Freund <andres@2ndquadrant.com> writes:
> I've already given up... Given how infrequent it is, suppressing it for
> gull seems sufficient.
I'm confused --- I see no format warnings in gull's current reports.
regards, tom lane
On 2015-01-14 11:09:10 -0500, Tom Lane wrote: > Andres Freund <andres@2ndquadrant.com> writes: > > I've already given up... Given how infrequent it is, suppressing it for > > gull seems sufficient. > > I'm confused --- I see no format warnings in gull's current reports. Sorry it was me being confused. I somehow switched gaur and gull because I had tabs open for both. Turns out that gaur doesn't doesn't do %z at all... Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On 2015-01-14 09:34:23 -0500, Tom Lane wrote: > Andres Freund <andres@2ndquadrant.com> writes: > > On 2015-01-13 22:19:30 -0500, Tom Lane wrote: > >> A slightly more complicated change could be applied to make sure that > >> *all* of the CFLAGS forcibly inserted by configure appear before any > >> externally-sourced CFLAGS, allowing any of them to be overridden from the > >> environment variable. I'm not sure if it's worth the trouble to do that, > >> but if there's interest I could make it happen. > > > I think it'd be good idea, but unless you're enthusiastic I guess there > > are more important things. > > Nah, I'm fine with doing it, it's just a couple more lines of code. > I feared people might think it wasn't worth adding extra complexity for, > but as long as someone else likes the idea I'll go do it. FWIW, if we moved the CFLAGS="$CFLAGS $user_CFLAGS" further down, it'd have advantage that compiling with -Werror would be more realistic. Right now doing so breaks about half of the feature checking configure checks because of warnings. E.g. on my platform it fails to detect 64bit integers, inline, ... Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Andres Freund <andres@2ndquadrant.com> writes:
> FWIW, if we moved the
> CFLAGS="$CFLAGS $user_CFLAGS"
> further down, it'd have advantage that compiling with -Werror would be
> more realistic. Right now doing so breaks about half of the feature
> checking configure checks because of warnings. E.g. on my platform it
> fails to detect 64bit integers, inline, ...
Given the way autoconf works, I think trying to run the configure tests
with -Werror is a fool's errand. OTOH, not applying the user's CFLAGS
during configure is a nonstarter as well. So rather than trying to inject
-Werror via generic CFLAGS, it would likely be better to have some means
of injecting it only into the actual build and not into the configure run.
There is at least one way to do that already (Makefile.custom). Not
sure if it's worth inventing an --enable-warnings-as-errors type of
switch to do it more directly.
regards, tom lane
On 2015-01-15 09:25:29 -0500, Tom Lane wrote: > Andres Freund <andres@2ndquadrant.com> writes: > > FWIW, if we moved the > > CFLAGS="$CFLAGS $user_CFLAGS" > > further down, it'd have advantage that compiling with -Werror would be > > more realistic. Right now doing so breaks about half of the feature > > checking configure checks because of warnings. E.g. on my platform it > > fails to detect 64bit integers, inline, ... > > Given the way autoconf works, I think trying to run the configure tests > with -Werror is a fool's errand. Yea, agreed. > OTOH, not applying the user's CFLAGS during configure is a nonstarter > as well. Fair enough. What about just filtering out -Werror during configure alone? Or just specifying -Wno-error during it? Given that it really can't work properly, that seems like a relatively simple solution. > So rather than trying to inject -Werror via generic CFLAGS, it would > likely be better to have some means of injecting it only into the > actual build and not into the configure run. > > There is at least one way to do that already (Makefile.custom). Not > sure if it's worth inventing an --enable-warnings-as-errors type of > switch to do it more directly. I think Makefile.custom is really rather hard to discover for new developers. That its inclusion is commented with # NOTE: Makefile.custom is from the pre-Autoconf days of PostgreSQL. # You are liable to shoot yourself in the foot if you use it without # knowing exactly what you're doing. The preferred (and more # reliable) method is to communicate what you want to do to the # configure script, and leave the makefiles alone. doesn't help... I'd also like to have a easy way of adding CFLAGS to configure, instead of overwriting them. There's COPT for make, but that doesn't persist... Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services