Re: [XEN PATCH v1 1/6] CI: Refresh the Debian 12 x86_64 container

[Javi Merino <javi.merino@xxxxxxxxx>]

On Thu, Oct 24, 2024 at 03:04:10PM +0100, Andrew Cooper wrote:
> On 24/10/2024 11:04 am, Javi Merino wrote:
> > Rework the container to use heredocs for readability, and use
> > apt-get --no-install-recommends to keep the size down.
> >
> > This reduces the size of the (uncompressed) container from 3.44GB to 1.67GB.
> 
> !!
> 
> >
> > Signed-off-by: Javi Merino <javi.merino@xxxxxxxxx>
> > ---
> >  automation/build/debian/12-x86_64.dockerfile | 68 ++++++++++++++++++++
> >  automation/build/debian/bookworm.dockerfile  | 57 ----------------
> >  automation/gitlab-ci/build.yaml              | 20 +++---
> >  automation/gitlab-ci/test.yaml               | 14 ++--
> >  automation/scripts/containerize              |  2 +-
> >  5 files changed, 86 insertions(+), 75 deletions(-)
> >  create mode 100644 automation/build/debian/12-x86_64.dockerfile
> >  delete mode 100644 automation/build/debian/bookworm.dockerfile
> >
> > diff --git a/automation/build/debian/12-x86_64.dockerfile 
> > b/automation/build/debian/12-x86_64.dockerfile
> > new file mode 100644
> > index 000000000000..e0ca8b7e9c91
> > --- /dev/null
> > +++ b/automation/build/debian/12-x86_64.dockerfile
> > @@ -0,0 +1,68 @@
> > +# syntax=docker/dockerfile:1
> > +FROM --platform=linux/amd64 debian:bookworm
> > +LABEL maintainer.name="The Xen Project" \
> > +      maintainer.email="xen-devel@xxxxxxxxxxxxxxxxxxxx"
> 
> This wants to become two LABEL lines.

Yes, Anthony pointed it out in another patch.  I have fixed all the
dockerfiles in these series.

> > +
> > +ENV DEBIAN_FRONTEND=noninteractive
> > +
> > +# build depends
> > +RUN <<EOF
> > +#!/bin/bash
> > +    set -eu
> 
> Doesn't this need a `useradd --create-home user` here?
> 
> [Edit] Oh, no, because of the script change.  In which case can you note
> this in the commit message and says a root container for now, until some
> other CI scripts can be adjusted.

I put it in the cover letter. I'll add it to the commit message as
well.

> > +
> > +    apt-get update
> 
> apt-get -y

apt-get update refreshes the package lists.  -y doesn't do anything
here.  It is needed for "apt-get install" below but not for
apt-get update.  It would be needed for "apt-get upgrade", but
we don't.

> > +    DEPS=(
> > +        # Xen
> > +        bison
> > +        build-essential
> > +        checkpolicy
> > +        clang
> > +        flex
> > +
> > +        # Tools (general)
> > +        ca-certificates
> 
> Interestingly, we've gained ca-certificates and dropped apt-transport-https.

ca-certificates is needed for curl, wget or anything that tries to
validate tls certificates.  It is a Recommends of libcurl, as
curl by default validates the ca certificate of https servers.

> ca-certificates is a side effect of --no-install-recommends, so is
> fine.  I recall there being a specific reason why we needed
> apt-transport-https, but I can't recall why exactly.  Something about
> the LetsEncrypt Cert used by xenbits IIRC.

I dropped apt-transport-https because it doesn't make sense to have
it.  apt-transport-https allows apt to access package repositories over https,
but we were installing alongside all the other packages.  apt is never
used again, so giving it the ability to install packages over https is
pointless.

> Anthony - do you remember?
> 
> 
> > +        expect
> 
> Expect is only for the test phase, so should move later.

I put it here because ./configure checks for it.

> > +        git-core
> > +        libnl-3-dev
> 
> libnl-3-dev should be down in the #libxl section.  It's only for COLO
> support.

Moved.

> > +        pkg-config
> > +        wget
> > +        # libxenguest dombuilder
> > +        liblzma-dev
> > +        zlib1g-dev
> 
> This is also fun.  In Ubuntu, I've got:
> 
>     libbz2-dev
>     libzstd-dev
>     liblzo2-dev
>     liblzma-dev
>     zlib1g-dev
> 
> which I think is all the algorithms we support in libxenguest.

I did this in the arm64v8 container and forgot to do it here.  Fixed now.

> Any decompressor which we don't find a suitable devel package gets the
> hypervisor form instead.
> 
> > +        # To build the documentation
> > +        pandoc
> 
> I know we had pandoc before, but I'd like to drop it.
> 
> I'm intending to turn off docs generally, and do them separately in a
> single job that has *all* the docs build dependencies, not a misc subset
> that the build system happens not to complain at.

I had the "build the docs as its own job" in my TODO list and was
going to drop pandoc from this dockerfile then.  I can remove pandoc
in this commit if you prefer.

> I'm on the fence about the Qemu build things.  It's off by default now,
> but the container never previously had meson/ninja so it wouldn't have
> built either.  Perhaps leave them out until someone complains.

I thought I had removed them.  Is there anything else that needs to
go?

> One thing you did drop which probably wants to stay is golang.  We have
> golang bindings for libxl which (like Ocaml) are built conditionally on
> finding the toolchain.

Gah.  Another one that I did in the arm64 container that I forgot to
move here.  I will add golang-go in the next version of the series.

Thanks,
Javi