Bogosity in contrib/xml2/Makefile

Started by Tom Laneabout 18 years ago3 messageshackers
Jump to latest
#1Tom Lane
tgl@sss.pgh.pa.us

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

#2Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#1)
Re: Bogosity in contrib/xml2/Makefile

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. +

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#2)
Re: Bogosity in contrib/xml2/Makefile

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