Thread: 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