Обсуждение: Bogosity in contrib/xml2/Makefile
Whilst fooling with bug #4058 I noticed that xml2's .c files were being
compiled without -g or any of the various warning flags we normally use.
I saw this:
gcc -I/usr/include/libxml2 -fpic -I. -I../../src/include -D_GNU_SOURCE -I/usr/include/libxml2 -c -o xpath.o xpath.c
when I expected something like this:
gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith -Winline -Wdeclaration-after-statement -Wendif-labels
-fno-strict-aliasing-fwrapv -g -fpic -I. -I../../src/include -D_GNU_SOURCE -I/usr/include/libxml2 -c -o xpath.o
xpath.c
The reason is apparently this line in its Makefile:
override CFLAGS += $(shell xml2-config --cflags)
It seems the "override" locks down the value so that the subsequent
assignment in Makefile.global does nothing. I didn't try the PGXS
case but I imagine it doesn't do the right thing either.
Now, in HEAD and 8.3 I think we could just remove this line, because
configure knows how to pull the needed -I and -L flags out of
xml2-config's output and stick them into appropriate flag variables
(neither of which is CFLAGS btw...). I am not sure what to do in older
branches though --- there doesn't seem to be any real nice solution.
Even though xml2 is deprecated and may go away for 8.4, I think this is
important to fix in the back branches. Failing to use the -f flags for
instance could be resulting in outright wrong code, and we'd be unlikely
to notice since there's no regression test at all for this module.
Thoughts?
regards, tom lane
Tom, did we come to any conclusion on this? --------------------------------------------------------------------------- Tom Lane wrote: > Whilst fooling with bug #4058 I noticed that xml2's .c files were being > compiled without -g or any of the various warning flags we normally use. > I saw this: > > gcc -I/usr/include/libxml2 -fpic -I. -I../../src/include -D_GNU_SOURCE -I/usr/include/libxml2 -c -o xpath.o xpath.c > > when I expected something like this: > > gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith -Winline -Wdeclaration-after-statement -Wendif-labels -fno-strict-aliasing-fwrapv -g -fpic -I. -I../../src/include -D_GNU_SOURCE -I/usr/include/libxml2 -c -o xpath.o xpath.c > > The reason is apparently this line in its Makefile: > > override CFLAGS += $(shell xml2-config --cflags) > > It seems the "override" locks down the value so that the subsequent > assignment in Makefile.global does nothing. I didn't try the PGXS > case but I imagine it doesn't do the right thing either. > > Now, in HEAD and 8.3 I think we could just remove this line, because > configure knows how to pull the needed -I and -L flags out of > xml2-config's output and stick them into appropriate flag variables > (neither of which is CFLAGS btw...). I am not sure what to do in older > branches though --- there doesn't seem to be any real nice solution. > > Even though xml2 is deprecated and may go away for 8.4, I think this is > important to fix in the back branches. Failing to use the -f flags for > instance could be resulting in outright wrong code, and we'd be unlikely > to notice since there's no regression test at all for this module. > > Thoughts? > > regards, tom lane > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian <bruce@momjian.us> writes:
> Tom, did we come to any conclusion on this?
No, nobody suggested anything :-(
As I said, I think we can just drop the assignment to CFLAGS in HEAD
and 8.3, relying on configure to get it right.
After looking at the pgxs documentation, I think that the right
thing is to set PG_CPPFLAGS rather than CFLAGS from
the output of xml2-config --cflags. That should work for as far
back as we were using pgxs, anyway.
regards, tom lane