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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 24/10/2024 4:10 pm, Javi Merino wrote:
> On Thu, Oct 24, 2024 at 03:04:10PM +0100, Andrew Cooper wrote:
>> On 24/10/2024 11:04 am, Javi Merino wrote:
>>> +
>>> +    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.

Hmm ok.  We might want to adjust the others to match then.

>
>>> +    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.

That is, as they say, an assumption.

fe746c26c0d2 ("automation/gitlab: add https transport support to Debian
images")

Although, subsequently the use of apt.llvm.org was removed:

a6b1e2b80fe2 ("automation: Remove clang-8 from Debian unstable container")
7a2983757216 ("CI: Remove llvm-8 from the Debian Stretch container")

So I guess we're back to being ok without it.
>>> +        expect
>> Expect is only for the test phase, so should move later.
> I put it here because ./configure checks for it.

It does?

That's not necessary/expected.

>> 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.

Dropping packages from existing containers is complicated, because the
container (name) is shared with prior branches.  You have to wait until
the oldest version of Xen which still uses the package leaves testing
(== leaves security support, == 3y), or we've backported changes to all
branches to drop the dependency.

The rename here gives us leeway because this change won't clobber any
older branches in Xen, but I don't want to set the precedent.

>
>> 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?

These containers are both for CI and human use, so "what happens in CI"
isn't the only consideration.

But, given that Qemu didn't build in the old container anyway, I'm not
overly fussed about keeping it working in the new container.

So yes, please keep the deps removed.  We can always add them back in later.

~Andrew