Subject: Re: [PATCH v2 5/5] ci: add support for GitLab CI

From: Patrick Steinhardt <[email-masked]>
Date: Fri, 27 Oct 2023 13:57:23 +0200

On Fri, Oct 27, 2023 at 01:19:35PM +0200, Patrick Steinhardt wrote:
> On Fri, Oct 27, 2023 at 11:19:04AM +0100, Phillip Wood wrote:
> > On 27/10/2023 10:25, Patrick Steinhardt wrote:
[snip]
> > > diff --git a/ci/install-docker-dependencies.sh b/ci/install-docker-dependencies.sh
> > > index d0bc19d3bb3..1cd92db1876 100755
> > > --- a/ci/install-docker-dependencies.sh
> > > +++ b/ci/install-docker-dependencies.sh
> > > @@ -7,6 +7,9 @@
> > >   begin_group "Install dependencies"
> > > +# Required so that apt doesn't wait for user input on certain packages.
> > > +export DEBIAN_FRONTEND=noninteractive
> > > +
> > >   case "$jobname" in
> > >   linux32)
> > >   	linux32 --32bit i386 sh -c '
> > > @@ -16,9 +19,13 @@ linux32)
> > >   	'
> > >   	;;
> > >   linux-musl)
> > > -	apk add --update build-base curl-dev openssl-dev expat-dev gettext \
> > > +	apk add --update git shadow sudo build-base curl-dev openssl-dev expat-dev gettext \
> > >   		pcre2-dev python3 musl-libintl perl-utils ncurses >/dev/null
> > 
> > It would be helpful to explain the new dependencies in the commit message. I
> > can see why you're adding sudo, but how were we getting away without
> > installing the other packages for GitHub Actions?
> 
> True, that part is missing.
> 
> - Both sudo and shadow are now required because of `useradd` that we use
>   to set up the unprivileged build.
> 
> - Git has been required all along, I think. `save_good_tree ()` is used
>   in our CI scripts, and Toon (fellow GitLabber from my team) has
>   noticed that the CI job warned about missing Git. The warning was
>   mostly benign as it seems, but still, doesn't hurt to fix it while at
>   it.
> 
> I'll have a look at whether I can add another patch on top that adjusts
> `ci/run-docker-build.sh` to do rootless builds, which would also make it
> more obvious why we now need to install sudo and shadow. And I'll make
> sure to document why we now need to have Git around.

Hum. After having a look at `ci/run-docker-build.sh` I don't feel like
it's sensible to update it. It's not even used anymore by our CI but
only by `ci/run-docker.sh`, which seems to be more of a developer-facing
script?

As you said, this smells like rotting bits that might rather be removed.
But in any case, as they don't relate to our current CI infrastructure
except for being in "ci/" I'll leave them be for now.

Patrick