From: Phillip Wood <phillip.wood123@gmail.com>
Date: Tue, 22 Apr 2025 13:24:18 GMT
Subject: Re: [PATCH v4 0/5] meson: add corresponding target for Makefile's hdr-check
Message-ID: <17c2eb4f-e291-4189-9846-0f42bdead01d@gmail.com>
In-Reply-To: <CAOLa=ZSR=7TEWLHa-wzBB4x+4+-BH3UC3G7s24Bc26JH63QKOA@mail.gmail.com>

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

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