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

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Patrick

On 27/10/2023 12:19, 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:
>>> +  parallel:
>>> +    matrix:
>>> +      - jobname: linux-sha256
>>> +        image: ubuntu:latest
>>> +        CC: clang
>>> +      - jobname: linux-gcc
>>> +        image: ubuntu:20.04
>>> +        CC: gcc
>>> +        CC_PACKAGE: gcc-8
>>> +      - jobname: linux-TEST-vars
>>> +        image: ubuntu:20.04
>>> +        CC: gcc
>>> +        CC_PACKAGE: gcc-8
>>> +      - jobname: linux-gcc-default
>>> +        image: ubuntu:latest
>>> +        CC: gcc
>>> +      - jobname: linux-leaks
>>> +        image: ubuntu:latest
>>> +        CC: gcc
>>> +      - jobname: linux-asan-ubsan
>>> +        image: ubuntu:latest
>>> +        CC: clang
>>> +      - jobname: linux-musl
>>> +        image: alpine:latest

I assume you've chosen the configurations from the existing GitHub 
Actions that give the best coverage of the various options. One thing I 
noticed is that the is no equivalent of the "pedantic" job that builds 
git with "DEVELOPER=1 DEVOPTS=pedantic"

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

Oh I had a look at this and the docker based jobs on GitHub do not have 
a git repository so installing git means we now get a repository not 
found error from save_good_tree() instead. We should probably make 
save_good_tree() and check_unignored_build_artifacts() return early if 
there isn't a repository but that's orthogonal to this series.

Looking at the test output from the link in your cover letter we should 
we should also install apache2[1] and gnupg[2]

[1] https://gitlab.com/gitlab-org/git/-/jobs/5349205374#L1444
[2] https://gitlab.com/gitlab-org/git/-/jobs/5349205374#L1167

Best Wishes

Phillip