[libvirt] [PATCH] freebsd: Fix build problem due to picking up the wrong libvirt.h

[Eric Blake]

On 05/31/2011 02:51 PM, Matthias Bolte wrote:
> AM_GNU_GETTEXT calls AM_ICONV_LINK. AM_ICONV_LINK saves and alters
> CPPFLAGS, but doesn't restore it when it finds libiconv. This
> results in /usr/local/include ending up in the gcc command line
> before the include path for the local include directory. This makes
> gcc pick a previous installed libvirt.h instead of the correct one
> from the source tree.
> 
> Workaround this problem by saving and restoring CPPFLAGS around
> the AM_GNU_GETTEXT call.
> ---
>  configure.ac |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index b2ba930..8f46dbd 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -2011,8 +2011,16 @@ dnl Enable building libvirtd?
>  AM_CONDITIONAL([WITH_LIBVIRTD],[test "x$with_libvirtd" = "xyes"])
>  
>  dnl Check for gettext - don't go any newer than what RHEL 5 supports
> +dnl
> +dnl save and restore CPPFLAGS around gettext check as the internal iconv
> +dnl check might leave -I/usr/local/include in CPPFLAGS on FreeBSD resulting
> +dnl in the build picking up previously installed libvirt/libvirt.h instead
> +dnl of the correct one from the soucre tree
> +
> +save_CPPFLAGS="$CPPFLAGS"
>  AM_GNU_GETTEXT_VERSION([0.17])
>  AM_GNU_GETTEXT([external])
> +CPPFLAGS="$save_CPPFLAGS"

Does this still pick up /usr/local/include later in the command line,
for the portion of the build that actually needs to include iconf
headers, but in a position after our internal headers have been found first?

I was actually thinking that we should instead work around this by
setting AM_CPPFLAGS somewhere in our Makefile.am files.  That is, when
AM_CPPFLAGS is set, then automake outputs $(AM_CPPFLAGS) $(CPPFLAGS)
when producing compilation rules, such that our internal CPPFLAGS
settings come first in the command line, prior to anything inherited
during configure (including the internal iconv settings).  That is, with
new enough automake, I think this alternative patch solve your BSD
compilation (although other directories need the same treatment),
without needing any configure.ac hacks.

On the other hand, it looks like RHEL 5 only supports automake 1.9.6,
and the automake NEWS doesn't mention AM_CPPFLAGS until 1.10 :(

So even if the approach in my patch solves your issue, I think it's a
non-starter, and something like your patch may be the best we can do.

But I'm still worried that we have to use -I/usr/local/include somewhere
in the command line, which means we would have to modify src/Makefile.am
(and friends) to have:

INCLUDES= ... $(GETTEXT_CPPFLAGS)

where GETTEXT_CPPFLAGS is substituted with the difference in
$save_CPPFLAGS and $CPPFLAGS prior to the point where we restore $CPPFLAGS.

diff --git i/src/Makefile.am w/src/Makefile.am
index 02d53ee..793fb9b 100644
--- i/src/Makefile.am
+++ w/src/Makefile.am
@@ -3,20 +3,20 @@
 # No libraries with the exception of LIBXML should be listed
 # here. List them against the individual XXX_la_CFLAGS targets
 # that actually use them
-INCLUDES =							\
+AM_CPPFLAGS =							\
 		-I$(top_srcdir)/gnulib/lib			\
 		-I../gnulib/lib					\
 		-I../include					\
 		-I at top_srcdir@/src/util				\
 		-I at top_srcdir@/include				\
-		$(DRIVER_MODULE_CFLAGS)				\
+		-DIN_LIBVIRT
+
+AM_CFLAGS =	$(DRIVER_MODULE_CFLAGS)				\
 		$(LIBXML_CFLAGS)				\
 		$(WARN_CFLAGS)					\
-		$(LOCK_CHECKING_CFLAGS)			\
-		-DIN_LIBVIRT				\
-		$(WIN32_EXTRA_CFLAGS)
-
-AM_CFLAGS = $(COVERAGE_CFLAGS)
+		$(LOCK_CHECKING_CFLAGS)				\
+		$(WIN32_EXTRA_CFLAGS)				\
+		$(COVERAGE_CFLAGS)
 AM_LDFLAGS = $(COVERAGE_LDFLAGS)

 EXTRA_DIST = $(conf_DATA)


-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20110531/f92a613b/attachment-0001.sig>