Re: [PATCH v2 05/12] crypto: bump min gcrypt to 1.8.0, dropping RHEL-7 support

[Thomas Huth <thuth@redhat.com>]

On 14/05/2021 14.04, Daniel P. Berrangé wrote:
> It has been over two years since RHEL-8 was released, and thus per the
> platform build policy, we no longer need to support RHEL-7 as a build
> target. This lets us increment the minimum required gcrypt version and
> assume that HMAC is always supported
> 
> Per repology, current shipping versions are:
> 
>               RHEL-8: 1.8.5
>        Debian Buster: 1.8.4
>   openSUSE Leap 15.2: 1.8.2
>     Ubuntu LTS 18.04: 1.8.1
>     Ubuntu LTS 20.04: 1.8.5
>              FreeBSD: 1.9.2
>            Fedora 33: 1.8.6
>            Fedora 34: 1.9.3
>              OpenBSD: 1.9.3
>       macOS HomeBrew: 1.9.3
> 
> Ubuntu LTS 18.04 has the oldest version and so 1.8.0 is the new minimum.

Maybe add a note why 1.8.0 and not 1.8.1 ? (if I see it correctly, the 
configure script does not check for the third digit?)

> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   .gitlab-ci.yml     | 10 ----------
>   configure          | 18 +-----------------
>   crypto/meson.build |  6 +-----
>   3 files changed, 2 insertions(+), 32 deletions(-)
> 
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index f012b16b79..f44c5b08ef 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -707,16 +707,6 @@ build-coroutine-sigaltstack:
>   #
>   # These jobs test old gcrypt and nettle from RHEL7
>   # which had some API differences.
> -crypto-old-gcrypt:
> -  <<: *native_build_job_definition
> -  needs:
> -    job: amd64-centos7-container
> -  variables:
> -    IMAGE: centos7
> -    TARGETS: x86_64-softmmu x86_64-linux-user
> -    CONFIGURE_ARGS: --disable-nettle --enable-gcrypt
> -    MAKE_CHECK_ARGS: check
> -
>   crypto-only-gnutls:
>     <<: *native_build_job_definition
>     needs:
> diff --git a/configure b/configure
> index 050299290d..f077cdb9c3 100755
> --- a/configure
> +++ b/configure
> @@ -426,7 +426,6 @@ gnutls="$default_feature"
>   nettle="$default_feature"
>   nettle_xts="no"
>   gcrypt="$default_feature"
> -gcrypt_hmac="no"
>   gcrypt_xts="no"
>   qemu_private_xts="yes"
>   auth_pam="$default_feature"
> @@ -2849,7 +2848,7 @@ has_libgcrypt() {
>       maj=`libgcrypt-config --version | awk -F . '{print $1}'`
>       min=`libgcrypt-config --version | awk -F . '{print $2}'`
>   
> -    if test $maj != 1 || test $min -lt 5
> +    if test $maj != 1 || test $min -lt 8
>       then
>          return 1
>       fi
> @@ -2915,18 +2914,6 @@ if test "$gcrypt" != "no"; then
>           gcrypt="yes"
>           cat > $TMPC << EOF
>   #include <gcrypt.h>
> -int main(void) {
> -  gcry_mac_hd_t handle;
> -  gcry_mac_open(&handle, GCRY_MAC_HMAC_MD5,
> -                GCRY_MAC_FLAG_SECURE, NULL);
> -  return 0;
> -}
> -EOF
> -        if compile_prog "$gcrypt_cflags" "$gcrypt_libs" ; then
> -            gcrypt_hmac=yes
> -        fi
> -        cat > $TMPC << EOF
> -#include <gcrypt.h>
>   int main(void) {
>     gcry_cipher_hd_t handle;
>     gcry_cipher_open(&handle, GCRY_CIPHER_AES, GCRY_CIPHER_MODE_XTS, 0);
> @@ -5722,9 +5709,6 @@ if test "$gnutls" = "yes" ; then
>   fi
>   if test "$gcrypt" = "yes" ; then
>     echo "CONFIG_GCRYPT=y" >> $config_host_mak
> -  if test "$gcrypt_hmac" = "yes" ; then
> -    echo "CONFIG_GCRYPT_HMAC=y" >> $config_host_mak
> -  fi
>     echo "GCRYPT_CFLAGS=$gcrypt_cflags" >> $config_host_mak
>     echo "GCRYPT_LIBS=$gcrypt_libs" >> $config_host_mak
>   fi
> diff --git a/crypto/meson.build b/crypto/meson.build
> index 7f37b5d335..af7e80c6f6 100644
> --- a/crypto/meson.build
> +++ b/crypto/meson.build
> @@ -26,11 +26,7 @@ if 'CONFIG_NETTLE' in config_host
>     crypto_ss.add(files('hash-nettle.c', 'hmac-nettle.c', 'pbkdf-nettle.c'))
>   elif 'CONFIG_GCRYPT' in config_host
>     crypto_ss.add(files('hash-gcrypt.c', 'pbkdf-gcrypt.c'))
> -  if 'CONFIG_GCRYPT_HMAC' in config_host
> -    crypto_ss.add(files('hmac-gcrypt.c'))
> -  else
> -    crypto_ss.add(files('hmac-glib.c'))
> -  endif
> +  crypto_ss.add(files('hmac-gcrypt.c'))
> 

I think you could also add it to the previous line (as it is done with 
nettle) instead of adding it in a separate line. But well, that's just 
cosmetics, so either way:

Reviewed-by: Thomas Huth <thuth@redhat.com>