Re: [PATCH v4 8/8] ci: add support for GitLab CI

From: Victoria Dye <vdye@github.com>
Date: Oct 31, 2023, 17:47 UTC
Message-ID: <4c8c2f19-1a7e-4524-81e7-c74091e88edf@github.com>
In-Reply-To: <f3f2c98a0dc6042b7ed5eab9c10bee4f64858f02.1698742590.git.ps@pks.im>

Patrick Steinhardt wrote:> diff --git a/ci/install-docker-dependencies.sh b/ci/install-docker-dependencies.sh

Show 34 quoted lines

> 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

...

Show 12 quoted lines

> 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

...

Show 5 quoted lines

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

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.