[Qemu-devel] [PATCH] configure: Add pkg-config handling for libgcrypt

zhe.he@windriver.com posted 1 patch 4 years, 7 months ago
Test docker-clang@ubuntu passed
Test FreeBSD passed
Test checkpatch passed
Test docker-mingw@fedora passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1567068782-371028-1-git-send-email-zhe.he@windriver.com
configure | 48 ++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 40 insertions(+), 8 deletions(-)
[Qemu-devel] [PATCH] configure: Add pkg-config handling for libgcrypt
Posted by zhe.he@windriver.com 4 years, 7 months ago
From: He Zhe <zhe.he@windriver.com>

libgcrypt may also be controlled by pkg-config, this patch adds pkg-config
handling for libgcrypt.

Signed-off-by: He Zhe <zhe.he@windriver.com>
---
 configure | 48 ++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 40 insertions(+), 8 deletions(-)

diff --git a/configure b/configure
index e44e454..0f362a7 100755
--- a/configure
+++ b/configure
@@ -2875,6 +2875,30 @@ has_libgcrypt() {
     return 0
 }
 
+has_libgcrypt_pkgconfig() {
+    if ! has $pkg_config ; then
+        return 1
+    fi
+
+    if ! $pkg_config --list-all | grep libgcrypt > /dev/null 2>&1 ; then
+        return 1
+    fi
+
+    if test -n "$cross_prefix" ; then
+        host=$($pkg_config --variable=host libgcrypt)
+        if test "${host%-gnu}-" != "${cross_prefix%-gnu}" ; then
+            print_error "host($host) does not match cross_prefix($cross_prefix)"
+            return 1
+        fi
+    fi
+
+    if ! $pkg_config --atleast-version=1.5.0 libgcrypt ; then
+        print_error "libgcrypt version is $($pkg_config --modversion libgcrypt)"
+        return 1
+    fi
+
+    return 0
+}
 
 if test "$nettle" != "no"; then
     pass="no"
@@ -2902,7 +2926,14 @@ fi
 
 if test "$gcrypt" != "no"; then
     pass="no"
-    if has_libgcrypt; then
+    if has_libgcrypt_pkgconfig; then
+        gcrypt_cflags=$($pkg_config --cflags libgcrypt)
+        if test "$static" = "yes" ; then
+            gcrypt_libs=$($pkg_config --libs --static libgcrypt)
+        else
+            gcrypt_libs=$($pkg_config --libs libgcrypt)
+        fi
+    elif has_libgcrypt; then
         gcrypt_cflags=$(libgcrypt-config --cflags)
         gcrypt_libs=$(libgcrypt-config --libs)
         # Debian has removed -lgpg-error from libgcrypt-config
@@ -2912,15 +2943,16 @@ if test "$gcrypt" != "no"; then
         then
             gcrypt_libs="$gcrypt_libs -lgpg-error"
         fi
+    fi
 
-        # Link test to make sure the given libraries work (e.g for static).
-        write_c_skeleton
-        if compile_prog "" "$gcrypt_libs" ; then
-            LIBS="$gcrypt_libs $LIBS"
-            QEMU_CFLAGS="$QEMU_CFLAGS $gcrypt_cflags"
-            pass="yes"
-        fi
+    # Link test to make sure the given libraries work (e.g for static).
+    write_c_skeleton
+    if compile_prog "" "$gcrypt_libs" ; then
+	    LIBS="$gcrypt_libs $LIBS"
+	    QEMU_CFLAGS="$QEMU_CFLAGS $gcrypt_cflags"
+	    pass="yes"
     fi
+
     if test "$pass" = "yes"; then
         gcrypt="yes"
         cat > $TMPC << EOF
-- 
2.7.4


Re: [Qemu-devel] [PATCH] configure: Add pkg-config handling for libgcrypt
Posted by Daniel P. Berrangé 4 years, 7 months ago
On Thu, Aug 29, 2019 at 04:53:02PM +0800, zhe.he@windriver.com wrote:
> From: He Zhe <zhe.he@windriver.com>
> 
> libgcrypt may also be controlled by pkg-config, this patch adds pkg-config
> handling for libgcrypt.

Where are you seeing pkg-config files for libgcrypt ?

The upstream project has (frustratingly) been hostile to any proposal to
add pkg-config support saying people should stick with their custom 
libgcrypt-config tool

   https://dev.gnupg.org/T2037

Even if this is something added by some distro downstream, what is the
benefit in using it, compared with libgcrypt-confg which should already
work & is portable.

> 
> Signed-off-by: He Zhe <zhe.he@windriver.com>
> ---
>  configure | 48 ++++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 40 insertions(+), 8 deletions(-)
> 
> diff --git a/configure b/configure
> index e44e454..0f362a7 100755
> --- a/configure
> +++ b/configure
> @@ -2875,6 +2875,30 @@ has_libgcrypt() {
>      return 0
>  }
>  
> +has_libgcrypt_pkgconfig() {
> +    if ! has $pkg_config ; then
> +        return 1
> +    fi
> +
> +    if ! $pkg_config --list-all | grep libgcrypt > /dev/null 2>&1 ; then
> +        return 1
> +    fi
> +
> +    if test -n "$cross_prefix" ; then
> +        host=$($pkg_config --variable=host libgcrypt)
> +        if test "${host%-gnu}-" != "${cross_prefix%-gnu}" ; then
> +            print_error "host($host) does not match cross_prefix($cross_prefix)"
> +            return 1
> +        fi
> +    fi
> +
> +    if ! $pkg_config --atleast-version=1.5.0 libgcrypt ; then
> +        print_error "libgcrypt version is $($pkg_config --modversion libgcrypt)"
> +        return 1
> +    fi
> +
> +    return 0
> +}
>  
>  if test "$nettle" != "no"; then
>      pass="no"
> @@ -2902,7 +2926,14 @@ fi
>  
>  if test "$gcrypt" != "no"; then
>      pass="no"
> -    if has_libgcrypt; then
> +    if has_libgcrypt_pkgconfig; then
> +        gcrypt_cflags=$($pkg_config --cflags libgcrypt)
> +        if test "$static" = "yes" ; then
> +            gcrypt_libs=$($pkg_config --libs --static libgcrypt)
> +        else
> +            gcrypt_libs=$($pkg_config --libs libgcrypt)
> +        fi
> +    elif has_libgcrypt; then
>          gcrypt_cflags=$(libgcrypt-config --cflags)
>          gcrypt_libs=$(libgcrypt-config --libs)
>          # Debian has removed -lgpg-error from libgcrypt-config
> @@ -2912,15 +2943,16 @@ if test "$gcrypt" != "no"; then
>          then
>              gcrypt_libs="$gcrypt_libs -lgpg-error"
>          fi
> +    fi
>  
> -        # Link test to make sure the given libraries work (e.g for static).
> -        write_c_skeleton
> -        if compile_prog "" "$gcrypt_libs" ; then
> -            LIBS="$gcrypt_libs $LIBS"
> -            QEMU_CFLAGS="$QEMU_CFLAGS $gcrypt_cflags"
> -            pass="yes"
> -        fi
> +    # Link test to make sure the given libraries work (e.g for static).
> +    write_c_skeleton
> +    if compile_prog "" "$gcrypt_libs" ; then
> +	    LIBS="$gcrypt_libs $LIBS"
> +	    QEMU_CFLAGS="$QEMU_CFLAGS $gcrypt_cflags"
> +	    pass="yes"
>      fi
> +
>      if test "$pass" = "yes"; then
>          gcrypt="yes"
>          cat > $TMPC << EOF
> -- 
> 2.7.4
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH] configure: Add pkg-config handling for libgcrypt
Posted by He Zhe 4 years, 7 months ago

On 8/29/19 5:15 PM, Daniel P. Berrangé wrote:
> On Thu, Aug 29, 2019 at 04:53:02PM +0800, zhe.he@windriver.com wrote:
>> From: He Zhe <zhe.he@windriver.com>
>>
>> libgcrypt may also be controlled by pkg-config, this patch adds pkg-config
>> handling for libgcrypt.
> Where are you seeing pkg-config files for libgcrypt ?
>
> The upstream project has (frustratingly) been hostile to any proposal to
> add pkg-config support saying people should stick with their custom 
> libgcrypt-config tool
>
>    https://dev.gnupg.org/T2037
>
> Even if this is something added by some distro downstream, what is the
> benefit in using it, compared with libgcrypt-confg which should already
> work & is portable.

IMHO, it could be easy for people to use pkg-config as a center to control
configurations for many different packages.

This is just an addition for qemu to be able to work in both cases. It does not
remove libgcrypt-confg and can fall back to libgcrypt-confg when pkg-config does
not work.

Zhe

>
>> Signed-off-by: He Zhe <zhe.he@windriver.com>
>> ---
>>  configure | 48 ++++++++++++++++++++++++++++++++++++++++--------
>>  1 file changed, 40 insertions(+), 8 deletions(-)
>>
>> diff --git a/configure b/configure
>> index e44e454..0f362a7 100755
>> --- a/configure
>> +++ b/configure
>> @@ -2875,6 +2875,30 @@ has_libgcrypt() {
>>      return 0
>>  }
>>  
>> +has_libgcrypt_pkgconfig() {
>> +    if ! has $pkg_config ; then
>> +        return 1
>> +    fi
>> +
>> +    if ! $pkg_config --list-all | grep libgcrypt > /dev/null 2>&1 ; then
>> +        return 1
>> +    fi
>> +
>> +    if test -n "$cross_prefix" ; then
>> +        host=$($pkg_config --variable=host libgcrypt)
>> +        if test "${host%-gnu}-" != "${cross_prefix%-gnu}" ; then
>> +            print_error "host($host) does not match cross_prefix($cross_prefix)"
>> +            return 1
>> +        fi
>> +    fi
>> +
>> +    if ! $pkg_config --atleast-version=1.5.0 libgcrypt ; then
>> +        print_error "libgcrypt version is $($pkg_config --modversion libgcrypt)"
>> +        return 1
>> +    fi
>> +
>> +    return 0
>> +}
>>  
>>  if test "$nettle" != "no"; then
>>      pass="no"
>> @@ -2902,7 +2926,14 @@ fi
>>  
>>  if test "$gcrypt" != "no"; then
>>      pass="no"
>> -    if has_libgcrypt; then
>> +    if has_libgcrypt_pkgconfig; then
>> +        gcrypt_cflags=$($pkg_config --cflags libgcrypt)
>> +        if test "$static" = "yes" ; then
>> +            gcrypt_libs=$($pkg_config --libs --static libgcrypt)
>> +        else
>> +            gcrypt_libs=$($pkg_config --libs libgcrypt)
>> +        fi
>> +    elif has_libgcrypt; then
>>          gcrypt_cflags=$(libgcrypt-config --cflags)
>>          gcrypt_libs=$(libgcrypt-config --libs)
>>          # Debian has removed -lgpg-error from libgcrypt-config
>> @@ -2912,15 +2943,16 @@ if test "$gcrypt" != "no"; then
>>          then
>>              gcrypt_libs="$gcrypt_libs -lgpg-error"
>>          fi
>> +    fi
>>  
>> -        # Link test to make sure the given libraries work (e.g for static).
>> -        write_c_skeleton
>> -        if compile_prog "" "$gcrypt_libs" ; then
>> -            LIBS="$gcrypt_libs $LIBS"
>> -            QEMU_CFLAGS="$QEMU_CFLAGS $gcrypt_cflags"
>> -            pass="yes"
>> -        fi
>> +    # Link test to make sure the given libraries work (e.g for static).
>> +    write_c_skeleton
>> +    if compile_prog "" "$gcrypt_libs" ; then
>> +	    LIBS="$gcrypt_libs $LIBS"
>> +	    QEMU_CFLAGS="$QEMU_CFLAGS $gcrypt_cflags"
>> +	    pass="yes"
>>      fi
>> +
>>      if test "$pass" = "yes"; then
>>          gcrypt="yes"
>>          cat > $TMPC << EOF
>> -- 
>> 2.7.4
>>
> Regards,
> Daniel


Re: [Qemu-devel] [PATCH] configure: Add pkg-config handling for libgcrypt
Posted by Daniel P. Berrangé 4 years, 7 months ago
On Thu, Aug 29, 2019 at 05:26:49PM +0800, He Zhe wrote:
> 
> 
> On 8/29/19 5:15 PM, Daniel P. Berrangé wrote:
> > On Thu, Aug 29, 2019 at 04:53:02PM +0800, zhe.he@windriver.com wrote:
> >> From: He Zhe <zhe.he@windriver.com>
> >>
> >> libgcrypt may also be controlled by pkg-config, this patch adds pkg-config
> >> handling for libgcrypt.
> > Where are you seeing pkg-config files for libgcrypt ?
> >
> > The upstream project has (frustratingly) been hostile to any proposal to
> > add pkg-config support saying people should stick with their custom 
> > libgcrypt-config tool
> >
> >    https://dev.gnupg.org/T2037
> >
> > Even if this is something added by some distro downstream, what is the
> > benefit in using it, compared with libgcrypt-confg which should already
> > work & is portable.
> 
> IMHO, it could be easy for people to use pkg-config as a center to control
> configurations for many different packages.
> 
> This is just an addition for qemu to be able to work in both cases. It does not
> remove libgcrypt-confg and can fall back to libgcrypt-confg when pkg-config does
> not work.

The addition has a maint cost associated with it, since we have have two
different ways to achieve the same thing. When only one of the approaches
is provided by upstream, the other is not going to be widely tested. In
maintaining packages in Fedora which rely on pkg-config files that are
not upstream, we've seen frequent breakage when. So my preference is
stick with what we have that is supported by upstream gcrypt.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH] configure: Add pkg-config handling for libgcrypt
Posted by Andrea Bolognani 2 years, 3 months ago
On Thu, Aug 29, 2019 at 10:15:05AM +0100, Daniel P. Berrangé wrote:
> On Thu, Aug 29, 2019 at 04:53:02PM +0800, zhe.he@windriver.com wrote:
> > libgcrypt may also be controlled by pkg-config, this patch adds pkg-config
> > handling for libgcrypt.
>
> Where are you seeing pkg-config files for libgcrypt ?
>
> The upstream project has (frustratingly) been hostile to any proposal to
> add pkg-config support saying people should stick with their custom
> libgcrypt-config tool
>
>    https://dev.gnupg.org/T2037
>
> Even if this is something added by some distro downstream, what is the
> benefit in using it, compared with libgcrypt-confg which should already
> work & is portable.

Resurrecting an old thread to point out that the upstream stance
seems to have changed since that discussion:

  https://git.gnupg.org/cgi-bin/gitweb.cgi?p=libgcrypt.git;a=commit;h=97194b422bc89a6137f4e218d4cdee118c63e96e

libgcrypt 1.9.0, released almost exactly a year ago, comes with a
pkg-config file out of the box. With that in mind, I think it would
make sense to re-evaluate this patch for inclusion.

-- 
Andrea Bolognani / Red Hat / Virtualization


Re: [Qemu-devel] [PATCH] configure: Add pkg-config handling for libgcrypt
Posted by Thomas Huth 2 years, 3 months ago
On 07/01/2022 12.43, Andrea Bolognani wrote:
> On Thu, Aug 29, 2019 at 10:15:05AM +0100, Daniel P. Berrangé wrote:
>> On Thu, Aug 29, 2019 at 04:53:02PM +0800, zhe.he@windriver.com wrote:
>>> libgcrypt may also be controlled by pkg-config, this patch adds pkg-config
>>> handling for libgcrypt.
>>
>> Where are you seeing pkg-config files for libgcrypt ?
>>
>> The upstream project has (frustratingly) been hostile to any proposal to
>> add pkg-config support saying people should stick with their custom
>> libgcrypt-config tool
>>
>>     https://dev.gnupg.org/T2037
>>
>> Even if this is something added by some distro downstream, what is the
>> benefit in using it, compared with libgcrypt-confg which should already
>> work & is portable.
> 
> Resurrecting an old thread to point out that the upstream stance
> seems to have changed since that discussion:
> 
>    https://git.gnupg.org/cgi-bin/gitweb.cgi?p=libgcrypt.git;a=commit;h=97194b422bc89a6137f4e218d4cdee118c63e96e
> 
> libgcrypt 1.9.0, released almost exactly a year ago, comes with a
> pkg-config file out of the box. With that in mind, I think it would
> make sense to re-evaluate this patch for inclusion.

Maybe ... but we switched to Meson in between, so the patch needs to be 
rewritten to use meson.build instead, I think. Also it seems like version 
1.9 is not available in all distros yet, so someone needs to do an 
assessment whether the distros that use an older version of libgrypt provide 
a .pc file or not...

  Thomas


Re: [Qemu-devel] [PATCH] configure: Add pkg-config handling for libgcrypt
Posted by Daniel P. Berrangé 2 years, 3 months ago
On Fri, Jan 07, 2022 at 12:55:42PM +0100, Thomas Huth wrote:
> On 07/01/2022 12.43, Andrea Bolognani wrote:
> > On Thu, Aug 29, 2019 at 10:15:05AM +0100, Daniel P. Berrangé wrote:
> > > On Thu, Aug 29, 2019 at 04:53:02PM +0800, zhe.he@windriver.com wrote:
> > > > libgcrypt may also be controlled by pkg-config, this patch adds pkg-config
> > > > handling for libgcrypt.
> > > 
> > > Where are you seeing pkg-config files for libgcrypt ?
> > > 
> > > The upstream project has (frustratingly) been hostile to any proposal to
> > > add pkg-config support saying people should stick with their custom
> > > libgcrypt-config tool
> > > 
> > >     https://dev.gnupg.org/T2037
> > > 
> > > Even if this is something added by some distro downstream, what is the
> > > benefit in using it, compared with libgcrypt-confg which should already
> > > work & is portable.
> > 
> > Resurrecting an old thread to point out that the upstream stance
> > seems to have changed since that discussion:
> > 
> >    https://git.gnupg.org/cgi-bin/gitweb.cgi?p=libgcrypt.git;a=commit;h=97194b422bc89a6137f4e218d4cdee118c63e96e
> > 
> > libgcrypt 1.9.0, released almost exactly a year ago, comes with a
> > pkg-config file out of the box. With that in mind, I think it would
> > make sense to re-evaluate this patch for inclusion.
> 
> Maybe ... but we switched to Meson in between, so the patch needs to be
> rewritten to use meson.build instead, I think. Also it seems like version
> 1.9 is not available in all distros yet, so someone needs to do an
> assessment whether the distros that use an older version of libgrypt provide
> a .pc file or not...

Comes back to my question of what is the benefit of using the .pc file
when what we have already works and is known to be portable.

When using meson there is essentially zero burden to using the
libgcrypt-config approach, because that's all handled transparently
by meson.  This is different from the situation with configure,
where libgcrypt-config required extra work on our side.

Overall I don't see any need to change it.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


Re: [Qemu-devel] [PATCH] configure: Add pkg-config handling for libgcrypt
Posted by Andrea Bolognani 2 years, 3 months ago
On Fri, Jan 07, 2022 at 12:06:47PM +0000, Daniel P. Berrangé wrote:
> On Fri, Jan 07, 2022 at 12:55:42PM +0100, Thomas Huth wrote:
> > On 07/01/2022 12.43, Andrea Bolognani wrote:
> > > On Thu, Aug 29, 2019 at 10:15:05AM +0100, Daniel P. Berrangé wrote:
> > > > Where are you seeing pkg-config files for libgcrypt ?
> > > >
> > > > The upstream project has (frustratingly) been hostile to any proposal to
> > > > add pkg-config support saying people should stick with their custom
> > > > libgcrypt-config tool
> > > >
> > > >     https://dev.gnupg.org/T2037
> > >
> > > Resurrecting an old thread to point out that the upstream stance
> > > seems to have changed since that discussion:
> > >
> > >    https://git.gnupg.org/cgi-bin/gitweb.cgi?p=libgcrypt.git;a=commit;h=97194b422bc89a6137f4e218d4cdee118c63e96e
> > >
> > > libgcrypt 1.9.0, released almost exactly a year ago, comes with a
> > > pkg-config file out of the box. With that in mind, I think it would
> > > make sense to re-evaluate this patch for inclusion.
> >
> > Maybe ... but we switched to Meson in between, so the patch needs to be
> > rewritten to use meson.build instead, I think.

Right. I didn't mean that the patch should be merged as-is, and I
wouldn't expect it to even apply two years later :)

> > Also it seems like version
> > 1.9 is not available in all distros yet, so someone needs to do an
> > assessment whether the distros that use an older version of libgrypt provide
> > a .pc file or not...

The original approach used the .pc file where present and used
libgcrypt-config as a fallback. Once QEMU stopped targeting all
operating system that have libgcrypt < 1.9, the fallback path could
have been dropped.

> Comes back to my question of what is the benefit of using the .pc file
> when what we have already works and is known to be portable.
>
> When using meson there is essentially zero burden to using the
> libgcrypt-config approach, because that's all handled transparently
> by meson.  This is different from the situation with configure,
> where libgcrypt-config required extra work on our side.

I didn't realize that was the case! I see now that Meson implements
special handling for libgcrypt, which is very nice indeed :)

> Overall I don't see any need to change it.

IIUC, the fact that the call currently looks like

  gcrypt = dependency('libgcrypt', version: '>=1.8',
                      method: 'config-tool', ...)

means that Meson will not use the .pc file even when it's present. I
think changing method to "auto" would cause it to use the .pc file
when it's available, which is likely a better behavior.

-- 
Andrea Bolognani / Red Hat / Virtualization