3:51 p.m.
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
5:04 p.m.
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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
3:45 p.m.
2011/6/1 Eric Blake <eblake(a)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
4:40 p.m.
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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
7:55 a.m.
2011/7/26 Eric Blake <eblake(a)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
8:51 a.m.
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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
2:36 p.m.
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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
4902
days inactive
4960
days old
6 comments
2 participants
participants (2)
-
Eric Blake -
Matthias Bolte