diff mbox series

configure: Add pkg-config handling for libgcrypt

Message ID 1567068782-371028-1-git-send-email-zhe.he@windriver.com
State New
Headers show
Series configure: Add pkg-config handling for libgcrypt | expand

Commit Message

He Zhe Aug. 29, 2019, 8:53 a.m. UTC 
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(-)

Comments

He Zhe Aug. 29, 2019, 9:26 a.m. UTC | #1 
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
Andrea Bolognani Jan. 7, 2022, 11:43 a.m. UTC | #2 
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.
Thomas Huth Jan. 7, 2022, 11:55 a.m. UTC | #3 
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
Daniel P. Berrangé Jan. 7, 2022, 12:06 p.m. UTC | #4 
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
Andrea Bolognani Jan. 7, 2022, 1:39 p.m. UTC | #5 
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.
diff mbox series

Patch

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