[Qemu-devel] [PATCH] configure: improve multiarch support for pkgconfig

[Qemu-devel] [PATCH] configure: improve multiarch support for pkgconfig

[John Snow]

This will improve the multi-arch compilation for hosts using gcc.
configurations using clang won't see an improvement, but also won't
see a regression.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 configure | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/configure b/configure
index 826d6fd..d27729a 100755
--- a/configure
+++ b/configure
@@ -1759,6 +1759,20 @@ if ! has "$pkg_config_exe"; then
 fi
 
 ##########################################
+# pkg-config multi-arch probe
+
+if $cc -print-multiarch >/dev/null 2>&1; then
+    mlibdir=$($cc -print-multiarch $QEMU_CFLAGS)
+fi
+if test -z "${mlibdir}"; then
+    mlibdir=$($cc --print-multi-os-directory $QEMU_CFLAGS)
+fi
+
+if [ -n "${mlibdir}" ] && [ -d "/usr/lib/${mlibdir}/pkgconfig" ]; then
+    export PKG_CONFIG_LIBDIR="/usr/lib/${mlibdir}/pkgconfig"
+fi
+
+##########################################
 # NPTL probe
 
 if test "$linux_user" = "yes"; then
-- 
2.1.0

Re: [Qemu-devel] [PATCH] configure: improve multiarch support for pkgconfig

[Paolo Bonzini]

On 09/04/2015 23:36, John Snow wrote:
> This will improve the multi-arch compilation for hosts using gcc.
> configurations using clang won't see an improvement, but also won't
> see a regression.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  configure | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/configure b/configure
> index 826d6fd..d27729a 100755
> --- a/configure
> +++ b/configure
> @@ -1759,6 +1759,20 @@ if ! has "$pkg_config_exe"; then
>  fi
>  
>  ##########################################
> +# pkg-config multi-arch probe
> +
> +if $cc -print-multiarch >/dev/null 2>&1; then
> +    mlibdir=$($cc -print-multiarch $QEMU_CFLAGS)
> +fi
> +if test -z "${mlibdir}"; then
> +    mlibdir=$($cc --print-multi-os-directory $QEMU_CFLAGS)
> +fi
> +
> +if [ -n "${mlibdir}" ] && [ -d "/usr/lib/${mlibdir}/pkgconfig" ]; then
> +    export PKG_CONFIG_LIBDIR="/usr/lib/${mlibdir}/pkgconfig"
> +fi

This is not enough.  It doesn't work unfortunately for cross compilers.

I would replace the third "if" with something like:

  orig_libdir=`$pkgconfig --variable pc_path pkg-config`
  if [ -n "$orig_libdir" ]; then
     export PKG_CONFIG_LIBDIR=$(echo $orig_libdir | sed \
         -e "s,???/pkgconfig:,???/${mlibdir}/pkgconfig:," -e t \
         -e "s,???/pkgconfig$,???/${mlibdir}/pkgconfig,g")
     export PKG_CONFIG_LIBDIR
  else
     echo Some warning about an old version of pkg-config?
     # proceed without setting PKG_CONFIG_LIBDIR then
  fi

The ??? are because for multiarch you probably should match
/lib/pkgconfig, but for multilib you cannot because it wouldn't match
/usr/lib64/pkgconfig.  So probably it needs two completely different sed
invocations.

Paolo

> +##########################################
>  # NPTL probe
>  
>  if test "$linux_user" = "yes"; then
> 

Re: [Qemu-devel] [PATCH] configure: improve multiarch support for pkgconfig

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1658 bytes --]

On Thu, Apr 09, 2015 at 05:36:12PM -0400, John Snow wrote:
> This will improve the multi-arch compilation for hosts using gcc.
> configurations using clang won't see an improvement, but also won't
> see a regression.

The commit description does not explain what this patch "improves".
Please add this explanation (or something equivalent) when merging:

32-bit compilation on 64-bit hosts is broken because pkgconfig isn't
multi-arch aware and selects the 64-bit glibconfig.h header file.  That
file assumes the LP64 data model so guint64 is defined as unsigned long.
This does not work for 32-bit builds where sizeof(unsigned long) == 4
bytes.

This patch makes "./configure --extra-cflags=-m32" just work by pointing
pkgconfig at the correct .pc files.

> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  configure | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/configure b/configure
> index 826d6fd..d27729a 100755
> --- a/configure
> +++ b/configure
> @@ -1759,6 +1759,20 @@ if ! has "$pkg_config_exe"; then
>  fi
>  
>  ##########################################
> +# pkg-config multi-arch probe
> +
> +if $cc -print-multiarch >/dev/null 2>&1; then
> +    mlibdir=$($cc -print-multiarch $QEMU_CFLAGS)
> +fi
> +if test -z "${mlibdir}"; then
> +    mlibdir=$($cc --print-multi-os-directory $QEMU_CFLAGS)
> +fi
> +
> +if [ -n "${mlibdir}" ] && [ -d "/usr/lib/${mlibdir}/pkgconfig" ]; then
> +    export PKG_CONFIG_LIBDIR="/usr/lib/${mlibdir}/pkgconfig"
> +fi
> +
> +##########################################
>  # NPTL probe

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH] configure: improve multiarch support for pkgconfig

[Peter Maydell]

On 10 April 2015 at 13:21, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> On Thu, Apr 09, 2015 at 05:36:12PM -0400, John Snow wrote:
>> This will improve the multi-arch compilation for hosts using gcc.
>> configurations using clang won't see an improvement, but also won't
>> see a regression.
>
> The commit description does not explain what this patch "improves".
> Please add this explanation (or something equivalent) when merging:
>
> 32-bit compilation on 64-bit hosts is broken because pkgconfig isn't
> multi-arch aware and selects the 64-bit glibconfig.h header file.  That
> file assumes the LP64 data model so guint64 is defined as unsigned long.
> This does not work for 32-bit builds where sizeof(unsigned long) == 4
> bytes.

Is this actually a fix for multiarch (which is the Debian/Ubuntu
approach where everything is split into /lib/i386-linux-gnu and
/lib/x86_64-linux-gnu and so on) or for the RedHat setup with /lib
and /lib64? (I forget the name of the latter, maybe multilib?)

-- PMM

Re: [Qemu-devel] [PATCH] configure: improve multiarch support for pkgconfig

[John Snow]

On 04/10/2015 08:55 AM, Peter Maydell wrote:
> On 10 April 2015 at 13:21, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>> On Thu, Apr 09, 2015 at 05:36:12PM -0400, John Snow wrote:
>>> This will improve the multi-arch compilation for hosts using gcc.
>>> configurations using clang won't see an improvement, but also won't
>>> see a regression.
>>
>> The commit description does not explain what this patch "improves".
>> Please add this explanation (or something equivalent) when merging:
>>
>> 32-bit compilation on 64-bit hosts is broken because pkgconfig isn't
>> multi-arch aware and selects the 64-bit glibconfig.h header file.  That
>> file assumes the LP64 data model so guint64 is defined as unsigned long.
>> This does not work for 32-bit builds where sizeof(unsigned long) == 4
>> bytes.
>
> Is this actually a fix for multiarch (which is the Debian/Ubuntu
> approach where everything is split into /lib/i386-linux-gnu and
> /lib/x86_64-linux-gnu and so on) or for the RedHat setup with /lib
> and /lib64? (I forget the name of the latter, maybe multilib?)
>
> -- PMM
>

It should apply to both, AFAIK. (Testing welcome ...!)

I wasn't very confident with the fix (I've been sitting on it for a 
while ...), but there's some good feedback, so I'll respin.

Thanks everyone.

--js

Re: [Qemu-devel] [PATCH] configure: improve multiarch support for pkgconfig

[Gerd Hoffmann]

  Hi,

> 32-bit compilation on 64-bit hosts is broken because pkgconfig isn't
> multi-arch aware and selects the 64-bit glibconfig.h header file.  That
> file assumes the LP64 data model so guint64 is defined as unsigned long.
> This does not work for 32-bit builds where sizeof(unsigned long) == 4
> bytes.

... there are more effects, like stuff being enabled because 64bit devel
lib is installed even when the 32bit devel lib isn't.

IMO it is fine to expect users set PKG_CONFIG_LIBDIR accordingly in that
case.  It would be very nice though to record this variable (in
config.status maybe?) so it doesn't get lost in case make figures it
should re-run configure because it was changed.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH] configure: improve multiarch support for pkgconfig

[John Snow]

On 04/10/2015 09:59 AM, Gerd Hoffmann wrote:
>    Hi,
>
>> 32-bit compilation on 64-bit hosts is broken because pkgconfig isn't
>> multi-arch aware and selects the 64-bit glibconfig.h header file.  That
>> file assumes the LP64 data model so guint64 is defined as unsigned long.
>> This does not work for 32-bit builds where sizeof(unsigned long) == 4
>> bytes.
>
> ... there are more effects, like stuff being enabled because 64bit devel
> lib is installed even when the 32bit devel lib isn't.
>
> IMO it is fine to expect users set PKG_CONFIG_LIBDIR accordingly in that
> case.  It would be very nice though to record this variable (in
> config.status maybe?) so it doesn't get lost in case make figures it
> should re-run configure because it was changed.
>

I'm not sure I follow you. What would be wrong with configure re-polling 
for the correct setting in that case?

Unless, perhaps, you are discussing the possibility of losing a user 
override from the first time they ran configure with PKG_CONFIG_LIBDIR 
already set to some custom value.

> cheers,
>    Gerd
>
>

Re: [Qemu-devel] [PATCH] configure: improve multiarch support for pkgconfig

[Gerd Hoffmann]

On Fr, 2015-04-10 at 10:15 -0400, John Snow wrote:
> 
> On 04/10/2015 09:59 AM, Gerd Hoffmann wrote:
> >    Hi,
> >
> >> 32-bit compilation on 64-bit hosts is broken because pkgconfig isn't
> >> multi-arch aware and selects the 64-bit glibconfig.h header file.  That
> >> file assumes the LP64 data model so guint64 is defined as unsigned long.
> >> This does not work for 32-bit builds where sizeof(unsigned long) == 4
> >> bytes.
> >
> > ... there are more effects, like stuff being enabled because 64bit devel
> > lib is installed even when the 32bit devel lib isn't.
> >
> > IMO it is fine to expect users set PKG_CONFIG_LIBDIR accordingly in that
> > case.  It would be very nice though to record this variable (in
> > config.status maybe?) so it doesn't get lost in case make figures it
> > should re-run configure because it was changed.
> >
> 
> I'm not sure I follow you. What would be wrong with configure re-polling 
> for the correct setting in that case?
> 
> Unless, perhaps, you are discussing the possibility of losing a user 
> override from the first time they ran configure with PKG_CONFIG_LIBDIR 
> already set to some custom value.

Yes, we lose a user override of PKG_CONFIG_LIBDIR today.  We should not.
Fixing that makes -m32 builds alot less painful already.

Trying to automatically set PKG_CONFIG_LIBDIR correctly for -m32 (but as
paolo already mentioned only in case it isn't set already) would be
sugar on top, no objections to that, but IMO we should get the basics
right first.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH] configure: improve multiarch support for pkgconfig

[Daniel P. Berrange]

On Thu, Apr 09, 2015 at 05:36:12PM -0400, John Snow wrote:
> This will improve the multi-arch compilation for hosts using gcc.
> configurations using clang won't see an improvement, but also won't
> see a regression.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  configure | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/configure b/configure
> index 826d6fd..d27729a 100755
> --- a/configure
> +++ b/configure
> @@ -1759,6 +1759,20 @@ if ! has "$pkg_config_exe"; then
>  fi
>  
>  ##########################################
> +# pkg-config multi-arch probe
> +
> +if $cc -print-multiarch >/dev/null 2>&1; then
> +    mlibdir=$($cc -print-multiarch $QEMU_CFLAGS)
> +fi
> +if test -z "${mlibdir}"; then
> +    mlibdir=$($cc --print-multi-os-directory $QEMU_CFLAGS)
> +fi
> +
> +if [ -n "${mlibdir}" ] && [ -d "/usr/lib/${mlibdir}/pkgconfig" ]; then
> +    export PKG_CONFIG_LIBDIR="/usr/lib/${mlibdir}/pkgconfig"
> +fi

IIUC, this overrides any setting of PKG_CONFIG_LIBDIR that the developer
may have set themselves before running configure. Please only set this,
if PKG_CONFIG_LIBDIR is not already present in the environment.


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|