On Tue, Oct 31, 2023 at 10:47:44AM -0700, Victoria Dye wrote:
> Patrick Steinhardt wrote:> diff --git a/ci/install-docker-dependencies.sh b/ci/install-docker-dependencies.sh
> > index 6e845283680..48cb2e735b5 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,11 +19,19 @@ linux32)
> > '
> > ;;
> > linux-musl)
> > - apk add --update build-base curl-dev openssl-dev expat-dev gettext \
> > + apk add --update shadow sudo build-base curl-dev openssl-dev expat-dev gettext \
> > pcre2-dev python3 musl-libintl perl-utils ncurses \
> > apache2 apache2-http2 apache2-proxy apache2-ssl apache2-webdav apr-util-dbd_sqlite3 \
> > bash cvs gnupg perl-cgi perl-dbd-sqlite >/dev/null
> > ;;
> > +linux-*)
> > + apt update -q &&
> > + apt install -q -y sudo git make language-pack-is libsvn-perl apache2 libssl-dev \
> > + libcurl4-openssl-dev libexpat-dev tcl tk gettext zlib1g-dev \
> > + perl-modules liberror-perl libauthen-sasl-perl libemail-valid-perl \
> > + libdbd-sqlite3-perl libio-socket-ssl-perl libnet-smtp-ssl-perl ${CC_PACKAGE:-${CC:-gcc}} \
> > + apache2 cvs cvsps gnupg libcgi-pm-perl subversion
> > + ;;
> > pedantic)
> > dnf -yq update >/dev/null &&
> > dnf -yq install make gcc findutils diffutils perl python3 gettext zlib-devel expat-devel openssl-devel curl-devel pcre2-devel >/dev/null
>
> ...
>
> > diff --git a/ci/lib.sh b/ci/lib.sh
> > index e14b1029fad..6e3d64004ec 100755
> > --- a/ci/lib.sh
> > +++ b/ci/lib.sh
> > @@ -208,6 +224,7 @@ then
> > cache_dir="$HOME/test-cache/$SYSTEM_PHASENAME"
> >
> > GIT_TEST_OPTS="--write-junit-xml"
> > + JOBS=10
> > elif test true = "$GITHUB_ACTIONS"
> > then
> > CI_TYPE=github-actions
>
> ...
>
> > -MAKEFLAGS="$MAKEFLAGS --jobs=10"
> > -GIT_PROVE_OPTS="--timer --jobs 10 --state=failed,slow,save"
> > +MAKEFLAGS="$MAKEFLAGS --jobs=${JOBS}"
> > +GIT_PROVE_OPTS="--timer --jobs ${JOBS} --state=failed,slow,save"
> >
>
> Organizationally, this commit seems to be doing two things at once:
>
> - Adding GitLab-specific CI setup (either in the new .gitlab-ci.yml or in
> conditions gated on "gitlab-ci").
> - Updating the common CI scripts with things that are needed for GitLab CI,
> but aren't conditioned on it (i.e. the patch excerpts I've included
> above).
>
> I'd prefer these being separated into two patches, mainly to isolate "things
> that affect all CI" from "things that affect only GitLab CI". This is
> ultimately a pretty minor nit, though; if you're not planning on re-rolling
> (or just disagree with what I'm suggesting :) ), I'm okay with leaving it
> as-is.
Yeah, the JOBS refactoring can certainly be split out into a preparatory
commit where we unify the envvars (currently patch 5). But for the other
changes it makes a bit less sense to do so, in my opinion:
- The DEBIAN_FRONTEND variable isn't needed before as the there are
no Docker-based CI jobs that use apt.
- Adding the shadow and sudo packages to the linux-musl job wouldn't
be needed either as there are no cases yet where we run
unprivileged CI builds via Docker.
- Adding the apt packages as a preparatory step doesn't make much
sense either as there is no Docker job using it.
But anyway. I will:
- Move around the JOBS variable refactoring to a preparatory patch,
which feels sensible to me.
- Move the `DEBIAN_FRONTEND` varible into the "linux-*" case, which
should further clarify that this only impacts the newly added and
thus GitLab-specific infrastructure.
With these changes, the only thing left in this commit that is not
guarded by a GitLab CI specific condition is the change to the
"linux-musl" case where we install shadow and sudo now. But I don't feel
like it makes sense to move them into a standalone preparatory commit.
Thanks!
Patrick
> Otherwise, I can't comment on the correctness of the GitLab CI definition (I
> assume you've tested it anyway), but AFAICT the changes above shouldn't break
> GitHub CI.
Attachment:
signature.asc
Description: PGP signature
