mbox series

[v2,0/2] crypto: fix build with gcrypt

Message ID 20200901133050.381844-1-berrange@redhat.com
Headers show
Series crypto: fix build with gcrypt | expand

Message

Daniel P. Berrangé Sept. 1, 2020, 1:30 p.m. UTC 
The build system failed to add gcrypt flags and also didn't link to
gnutls in all scenarios.  This was missed because of the lack of CI
coverage for various build scenarios

Changed in v2:

 - Change way we add library dependencies in meson rules
   to fix linux-user build with gcrypt/gnutls too.
 - Extend CI coverage to test 1 system and 1 linux-user
   build with each crypto combination, not merely tools.

Daniel P. Berrangé (2):
  crypto: fix build with gcrypt enabled
  gitlab: expand test coverage for crypto builds

 .gitlab-ci.yml                          | 69 +++++++++++++++++++++++++
 configure                               |  2 +
 crypto/meson.build                      | 42 +++++++++++----
 meson.build                             |  5 ++
 tests/docker/dockerfiles/centos7.docker |  2 +
 tests/docker/dockerfiles/centos8.docker |  1 +
 6 files changed, 110 insertions(+), 11 deletions(-)

-- 
2.26.2

Comments

Philippe Mathieu-Daudé Sept. 1, 2020, 3:10 p.m. UTC | #1 
On 9/1/20 3:30 PM, Daniel P. Berrangé wrote:
> Most jobs test the latest nettle library. This adds explicit coverage
> for latest gcrypt using Fedora, and old gcrypt and nettle using
> CentOS-7. The latter does a minimal tools-only build, as we only need to
> validate that the crypto code builds and unit tests pass. Finally a job
> disabling both nettle and gcrypt is provided to validate that gnutls
> still works.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  .gitlab-ci.yml                          | 69 +++++++++++++++++++++++++
>  tests/docker/dockerfiles/centos7.docker |  2 +
>  tests/docker/dockerfiles/centos8.docker |  1 +
>  3 files changed, 72 insertions(+)
> 
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index b7967b9a13..a74b16ff04 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -130,6 +130,7 @@ build-system-fedora:
>    <<: *native_build_job_definition
>    variables:
>      IMAGE: fedora
> +    CONFIGURE_ARGS: --disable-gcrypt --enable-nettle
>      TARGETS: tricore-softmmu unicore32-softmmu microblaze-softmmu mips-softmmu
>        xtensa-softmmu m68k-softmmu riscv32-softmmu ppc-softmmu sparc64-softmmu
>      MAKE_CHECK_ARGS: check-build
> @@ -160,6 +161,7 @@ build-system-centos:
>    <<: *native_build_job_definition
>    variables:
>      IMAGE: centos8
> +    CONFIGURE_ARGS: --disable-nettle --enable-gcrypt
>      TARGETS: ppc64-softmmu lm32-softmmu or1k-softmmu s390x-softmmu
>        x86_64-softmmu rx-softmmu sh4-softmmu nios2-softmmu
>      MAKE_CHECK_ARGS: check-build
> @@ -196,6 +198,7 @@ build-disabled:
>        --disable-guest-agent --disable-curses --disable-libxml2 --disable-tpm
>        --disable-qom-cast-debug --disable-spice --disable-vhost-vsock
>        --disable-vhost-net --disable-vhost-crypto --disable-vhost-user
> +      --disable-nettle --disable-gcrypt --disable-gnutls
>      TARGETS: i386-softmmu ppc64-softmmu mips64-softmmu i386-linux-user
>      MAKE_CHECK_ARGS: check-qtest SPEED=slow
>  
> @@ -271,3 +274,69 @@ build-tci:
>        done
>      - QTEST_QEMU_BINARY="./qemu-system-x86_64" ./tests/qtest/pxe-test
>      - QTEST_QEMU_BINARY="./qemu-system-s390x" ./tests/qtest/pxe-test -m slow
> +
> +# Most jobs test latest gcrypt or nettle builds
> +#
> +# These jobs test old gcrypt and nettle from RHEL7
> +# which had some API differences.
> +build-crypto-old-nettle:
> +  <<: *native_build_job_definition
> +  variables:
> +    IMAGE: centos7
> +    TARGETS: x86_64-softmmu x86_64-linux-user
> +    CONFIGURE_ARGS: --disable-gcrypt --enable-nettle
> +    MAKE_CHECK_ARGS: check-build
> +  artifacts:
> +    paths:
> +      - build
> +
> +check-crypto-old-nettle:
> +  <<: *native_test_job_definition
> +  needs:
> +    - job: build-crypto-old-nettle
> +      artifacts: true
> +  variables:
> +    IMAGE: centos7
> +    MAKE_CHECK_ARGS: check
> +
> +

I'd copy the same comment for each library... In case
we add more jobs in the middle.

> +build-crypto-old-gcrypt:
> +  <<: *native_build_job_definition
> +  variables:
> +    IMAGE: centos7
> +    TARGETS: x86_64-softmmu x86_64-linux-user
> +    CONFIGURE_ARGS: --disable-nettle --enable-gcrypt
> +    MAKE_CHECK_ARGS: check-build
> +  artifacts:
> +    paths:
> +      - build
> +
> +check-crypto-old-gcrypt:
> +  <<: *native_test_job_definition
> +  needs:
> +    - job: build-crypto-old-gcrypt
> +      artifacts: true
> +  variables:
> +    IMAGE: centos7
> +    MAKE_CHECK_ARGS: check
> +
> +
> +build-crypto-only-gnutls:

Aren't these 'old' jobs too (centos 7, not 8)?

> +  <<: *native_build_job_definition
> +  variables:
> +    IMAGE: centos7
> +    TARGETS: x86_64-softmmu x86_64-linux-user
> +    CONFIGURE_ARGS: --disable-nettle --disable-gcrypt --enable-gnutls
> +    MAKE_CHECK_ARGS: check-build
> +  artifacts:
> +    paths:
> +      - build
> +
> +check-crypto-only-gnutls:
> +  <<: *native_test_job_definition
> +  needs:
> +    - job: build-crypto-only-gnutls
> +      artifacts: true
> +  variables:
> +    IMAGE: centos7
> +    MAKE_CHECK_ARGS: check
> diff --git a/tests/docker/dockerfiles/centos7.docker b/tests/docker/dockerfiles/centos7.docker
> index e197acdc3c..46277773bf 100644
> --- a/tests/docker/dockerfiles/centos7.docker
> +++ b/tests/docker/dockerfiles/centos7.docker
> @@ -15,9 +15,11 @@ ENV PACKAGES \
>      gettext \
>      git \
>      glib2-devel \
> +    gnutls-devel \
>      libaio-devel \
>      libepoxy-devel \
>      libfdt-devel \
> +    libgcrypt-devel \
>      librdmacm-devel \
>      libzstd-devel \
>      lzo-devel \

We should try to keep the same set of packages installed (if possible)
in the older distrib supported and in the more recent one. Not sure
what the best way to do that though.

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> diff --git a/tests/docker/dockerfiles/centos8.docker b/tests/docker/dockerfiles/centos8.docker
> index 9852c5b9ee..f435616d6a 100644
> --- a/tests/docker/dockerfiles/centos8.docker
> +++ b/tests/docker/dockerfiles/centos8.docker
> @@ -13,6 +13,7 @@ ENV PACKAGES \
>      glib2-devel \
>      libaio-devel \
>      libepoxy-devel \
> +    libgcrypt-devel \
>      lzo-devel \
>      make \
>      mesa-libEGL-devel \
>
Daniel P. Berrangé Sept. 1, 2020, 3:27 p.m. UTC | #2 
On Tue, Sep 01, 2020 at 05:10:20PM +0200, Philippe Mathieu-Daudé wrote:
> On 9/1/20 3:30 PM, Daniel P. Berrangé wrote:
> > Most jobs test the latest nettle library. This adds explicit coverage
> > for latest gcrypt using Fedora, and old gcrypt and nettle using
> > CentOS-7. The latter does a minimal tools-only build, as we only need to
> > validate that the crypto code builds and unit tests pass. Finally a job
> > disabling both nettle and gcrypt is provided to validate that gnutls
> > still works.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  .gitlab-ci.yml                          | 69 +++++++++++++++++++++++++
> >  tests/docker/dockerfiles/centos7.docker |  2 +
> >  tests/docker/dockerfiles/centos8.docker |  1 +
> >  3 files changed, 72 insertions(+)
> > 
> > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> > index b7967b9a13..a74b16ff04 100644
> > --- a/.gitlab-ci.yml
> > +++ b/.gitlab-ci.yml
> > @@ -130,6 +130,7 @@ build-system-fedora:
> >    <<: *native_build_job_definition
> >    variables:
> >      IMAGE: fedora
> > +    CONFIGURE_ARGS: --disable-gcrypt --enable-nettle
> >      TARGETS: tricore-softmmu unicore32-softmmu microblaze-softmmu mips-softmmu
> >        xtensa-softmmu m68k-softmmu riscv32-softmmu ppc-softmmu sparc64-softmmu
> >      MAKE_CHECK_ARGS: check-build
> > @@ -160,6 +161,7 @@ build-system-centos:
> >    <<: *native_build_job_definition
> >    variables:
> >      IMAGE: centos8
> > +    CONFIGURE_ARGS: --disable-nettle --enable-gcrypt
> >      TARGETS: ppc64-softmmu lm32-softmmu or1k-softmmu s390x-softmmu
> >        x86_64-softmmu rx-softmmu sh4-softmmu nios2-softmmu
> >      MAKE_CHECK_ARGS: check-build
> > @@ -196,6 +198,7 @@ build-disabled:
> >        --disable-guest-agent --disable-curses --disable-libxml2 --disable-tpm
> >        --disable-qom-cast-debug --disable-spice --disable-vhost-vsock
> >        --disable-vhost-net --disable-vhost-crypto --disable-vhost-user
> > +      --disable-nettle --disable-gcrypt --disable-gnutls
> >      TARGETS: i386-softmmu ppc64-softmmu mips64-softmmu i386-linux-user
> >      MAKE_CHECK_ARGS: check-qtest SPEED=slow
> >  
> > @@ -271,3 +274,69 @@ build-tci:
> >        done
> >      - QTEST_QEMU_BINARY="./qemu-system-x86_64" ./tests/qtest/pxe-test
> >      - QTEST_QEMU_BINARY="./qemu-system-s390x" ./tests/qtest/pxe-test -m slow
> > +
> > +# Most jobs test latest gcrypt or nettle builds
> > +#
> > +# These jobs test old gcrypt and nettle from RHEL7
> > +# which had some API differences.
> > +build-crypto-old-nettle:
> > +  <<: *native_build_job_definition
> > +  variables:
> > +    IMAGE: centos7
> > +    TARGETS: x86_64-softmmu x86_64-linux-user
> > +    CONFIGURE_ARGS: --disable-gcrypt --enable-nettle
> > +    MAKE_CHECK_ARGS: check-build
> > +  artifacts:
> > +    paths:
> > +      - build
> > +
> > +check-crypto-old-nettle:
> > +  <<: *native_test_job_definition
> > +  needs:
> > +    - job: build-crypto-old-nettle
> > +      artifacts: true
> > +  variables:
> > +    IMAGE: centos7
> > +    MAKE_CHECK_ARGS: check
> > +
> > +
> 
> I'd copy the same comment for each library... In case
> we add more jobs in the middle.
> 
> > +build-crypto-old-gcrypt:
> > +  <<: *native_build_job_definition
> > +  variables:
> > +    IMAGE: centos7
> > +    TARGETS: x86_64-softmmu x86_64-linux-user
> > +    CONFIGURE_ARGS: --disable-nettle --enable-gcrypt
> > +    MAKE_CHECK_ARGS: check-build
> > +  artifacts:
> > +    paths:
> > +      - build
> > +
> > +check-crypto-old-gcrypt:
> > +  <<: *native_test_job_definition
> > +  needs:
> > +    - job: build-crypto-old-gcrypt
> > +      artifacts: true
> > +  variables:
> > +    IMAGE: centos7
> > +    MAKE_CHECK_ARGS: check
> > +
> > +
> > +build-crypto-only-gnutls:
> 
> Aren't these 'old' jobs too (centos 7, not 8)?

It doesn't matter what distro this job builds on - centos 7 was
essentially just a cut+paste choice. The key point is this is only
enabling GNUTLS - the age of gnutls/gcrypt/nettle doesn't matter.

> 
> > +  <<: *native_build_job_definition
> > +  variables:
> > +    IMAGE: centos7
> > +    TARGETS: x86_64-softmmu x86_64-linux-user
> > +    CONFIGURE_ARGS: --disable-nettle --disable-gcrypt --enable-gnutls
> > +    MAKE_CHECK_ARGS: check-build
> > +  artifacts:
> > +    paths:
> > +      - build
> > +
> > +check-crypto-only-gnutls:
> > +  <<: *native_test_job_definition
> > +  needs:
> > +    - job: build-crypto-only-gnutls
> > +      artifacts: true
> > +  variables:
> > +    IMAGE: centos7
> > +    MAKE_CHECK_ARGS: check

Regards,
Daniel