[libvirt] [PATCH] freebsd: Fix build problem due to picking up the wrong libvirt.h
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" ALL_LINGUAS=`cd "$srcdir/po" > /dev/null && ls *.po | sed 's+\.po$++'` -- 1.7.0.4
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@top_srcdir@/src/util \ -I@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@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
2011/6/1 Eric Blake <eblake@redhat.com>:
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 didn't check, but when we assume that AM_GNU_GETTEXT extends CPPFLAGS for a reason and not just by accident then we need to preserve it's additions to CPPFLAGS but in a later place in the commandline. As the actual problem is that CPPFLAGS comes before -I../include.
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.
That's probably what we should do here. Now how to compute this difference? When we assume that save_CPPFLAGS and CPPFLAGS have a common prefix that we need to strip to get GETTEXT_CPPFLAGS then we could do it like this echo $(CPPFLAGS) | cut -c 1-`expr length $(save_CPPFLAGS)` --complement - This is probably neither the best nor a robustest way to do this. Do you have a better idea? -- Matthias Bolte http://photron.blogspot.com
On 07/26/2011 02:45 PM, Matthias Bolte wrote:
+++ 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"
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.
That's probably what we should do here.
Now how to compute this difference? When we assume that save_CPPFLAGS and CPPFLAGS have a common prefix that we need to strip to get GETTEXT_CPPFLAGS then we could do it like this
echo $(CPPFLAGS) | cut -c 1-`expr length $(save_CPPFLAGS)` --complement -
This is probably neither the best nor a robustest way to do this. Do you have a better idea?
Yep (although I haven't tested it thoroughly): save_CPPFLAGS="$CPPFLAGS" AM_GNU_GETTEXT_VERSION([0.17]) AM_GNU_GETTEXT([external]) GETTEXT_CPPFLAGS= if test "x$save_CPPFLAGS" != "x$CPPFLAGS"; then set dummy $CPPFLAGS for var do case " $var " in " $save_CPPFLAGS ") ;; *) GETTEXT_CPPFLAGS="$GETTEXT_CPPFLAGS $var" ;; esac done fi CPPFLAGS="$save_CPPFLAGS" -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
2011/7/26 Eric Blake <eblake@redhat.com>:
On 07/26/2011 02:45 PM, Matthias Bolte wrote:
+++ 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"
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.
That's probably what we should do here.
Now how to compute this difference? When we assume that save_CPPFLAGS and CPPFLAGS have a common prefix that we need to strip to get GETTEXT_CPPFLAGS then we could do it like this
echo $(CPPFLAGS) | cut -c 1-`expr length $(save_CPPFLAGS)` --complement -
This is probably neither the best nor a robustest way to do this. Do you have a better idea?
Yep (although I haven't tested it thoroughly):
save_CPPFLAGS="$CPPFLAGS" AM_GNU_GETTEXT_VERSION([0.17]) AM_GNU_GETTEXT([external]) GETTEXT_CPPFLAGS= if test "x$save_CPPFLAGS" != "x$CPPFLAGS"; then set dummy $CPPFLAGS for var do case " $var " in " $save_CPPFLAGS ") ;; *) GETTEXT_CPPFLAGS="$GETTEXT_CPPFLAGS $var" ;; esac done fi CPPFLAGS="$save_CPPFLAGS"
Okay, this works for libvirt itself, but it fails for make check as the global CPPFLAGS also affect gnulib, but with this patch it doesn't contain the gettext related parts anymore and the gnulib tests fail to find libintl.h because of this. Making check in gnulib/tests Making check in . In file included from wait-process.c:37: ../../gnulib/lib/gettext.h:28:22: error: libintl.h: No such file or directory Is there any option in gnulib that would allow to inject GETTEXT_CPPFLAGS into the gnulib makefiles, or any other possibility to fix this? I attached a preliminary patch. -- Matthias Bolte http://photron.blogspot.com
On 07/28/2011 06:55 AM, Matthias Bolte wrote:
Okay, this works for libvirt itself, but it fails for make check as the global CPPFLAGS also affect gnulib, but with this patch it doesn't contain the gettext related parts anymore and the gnulib tests fail to find libintl.h because of this.
Is there any option in gnulib that would allow to inject GETTEXT_CPPFLAGS into the gnulib makefiles, or any other possibility to fix this?
Fortunately, I know how to fix that - it involves changing our gnulib-tool to spit out gnulib.mk to be included from a wrapper Makefile.am that we manage, rather than our current practice of letting gnulib-tool spit out the complete Makefile.am (coreutils.git does the same thing).
I attached a preliminary patch.
I'll submit a v2 based on your patch shortly.
+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.
including this typo fix: s/soucre/source/ Everything else you had looks good, just missing the gnulib handling. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 07/28/2011 07:51 AM, Eric Blake wrote:
Is there any option in gnulib that would allow to inject GETTEXT_CPPFLAGS into the gnulib makefiles, or any other possibility to fix this?
Fortunately, I know how to fix that - it involves changing our gnulib-tool to spit out gnulib.mk to be included from a wrapper Makefile.am that we manage, rather than our current practice of letting gnulib-tool spit out the complete Makefile.am (coreutils.git does the same thing).
I attached a preliminary patch.
I'll submit a v2 based on your patch shortly.
https://www.redhat.com/archives/libvir-list/2011-July/msg02019.html -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake -
Matthias Bolte