Re: [PATCH v4 0/5] meson: add corresponding target for Makefile's hdr-check

From: Karthik Nayak <karthik.188@gmail.com>
Date: Apr 22, 2025, 18:56 UTC
Message-ID: <CAOLa=ZSi5FuhpaAiGn5OydLUXhkWC9VrkOvXKwTeP=9S2uhKgQ@mail.gmail.com>
In-Reply-To: <17c2eb4f-e291-4189-9846-0f42bdead01d@gmail.com>

Phillip Wood <phillip.wood123@gmail.com> writes:

Show 50 quoted lines

> Hi Karthik
>
> On 21/04/2025 16:33, Karthik Nayak wrote:
>
> Thanks for putting this together, I've left a couple of code comments below.
>
>>      Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
>>
>> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
>> index 37541f3d10..a09fcf4d72 100644
>> --- a/.github/workflows/main.yml
>> +++ b/.github/workflows/main.yml
>> @@ -414,6 +414,16 @@ jobs:
>>       - name: prepare libc6 for actions
>>         if: matrix.vector.jobname == 'linux32'
>>         run: apt -q update && apt -q -y install libc6-amd64 lib64stdc++6
>> +    - name: install git in container
>> +      run: |
>> +        if [ -f /etc/alpine-release ]; then
>> +          apk update && apk add --no-cache git
>> +        elif [ -f /etc/almalinux-release ] || [ -f /etc/redhat-release ]; then
>> +           dnf -y install git
>> +        else
>> +          apt -q update && apt -q -y install git
>> +        fi
>> +        git config --global --add safe.directory "$GITHUB_WORKSPACE"
>
> I'd be tempted to check for which package manager to use by using
> `command -v`. That way the only distribution specific knowledge we need
> is the package manager and we don't have to worry about the names of the
> various release files in /etc.
>
> 	if command -v git
> 	then
> 		: nothing to do
> 	elif command -v apk
> 	then
> 		apk add git
> 	elif command -v dnf
> 	then
> 		dnf -y install git
> 	else
> 		apt-get -q -y install git
> 	fi
>
> The commands above omit anything that updates the package cache as we do
> that anyway in install-dependencies.sh and we only really care about
> getting some version of git installed here. It also uses apt-get to
> match what we do in install-dependencies.sh
>

Seems like this is a no-go, since apt-get fails [1] without first updating the package cache. So I'm going to do that for all the commands, which should also ensure that the package cache update in 'install-dependencies.sh' is mostly a no-op.

[1]: https://github.com/gitgitgadget/git/actions/runs/14598683520/job/40951070359?pr=1905

Show 10 quoted lines

> I also wonder if we should ditch the checkout action and use something like
>
>      git clone --depth=1 --single-branch ${GITHUB_REF_NAME} \
> 	${GITHUB_SERVER_URL}/${GITHUB_REPOSITORY}.git
>
> so that we know we will be building from a git repository.
>
> Best Wishes
>
> Phillip