Subject: Re: [PATCH/RFC 3/3] ci: run a pedantic build as part of the GitHub workflow

From: Phillip Wood <[email-masked]>
Date: Mon, 9 Aug 2021 15:56:01 +0100

Hi Carlo

On 09/08/2021 02:38, Carlo Marcelo Arenas Belón wrote:
> similar to the recently added sparse task, it is nice to know as early
> as possible.
> 
> add a dockerized build using fedora (that usually has the latest gcc)
> to be ahead of the curve and avoid older ISO C issues at the same time.

If we want to be able to compile with -Wpedantic then it might be better 
just to turn it on unconditionally in config.mak.dev. Then developers 
will see any errors before they push and the ci builds will all use it 
rather than having to run an extra job. I had a quick scan of the mail 
archive threads starting at [1,2] and it's not clear to me why 
-Wpedaintic was added as an optional extra.

Totally unrelated to this patch but while looking at the ci scripts I 
noticed that we only run the linux-gcc-4.8 job on travis, not on github.

Best Wishes

Phillip

[1] https://lore.kernel.org/git/[email-masked]/
[2] https://lore.kernel.org/git/[email-masked]/

> Signed-off-by: Carlo Marcelo Arenas Belón <[email-masked]>
> ---
>   .github/workflows/main.yml        |  2 ++
>   ci/install-docker-dependencies.sh |  4 ++++
>   ci/run-build-and-tests.sh         | 10 +++++++---
>   3 files changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
> index 47876a4f02..6b9427eff1 100644
> --- a/.github/workflows/main.yml
> +++ b/.github/workflows/main.yml
> @@ -259,6 +259,8 @@ jobs:
>             image: alpine
>           - jobname: Linux32
>             image: daald/ubuntu32:xenial
> +        - jobname: pedantic
> +          image: fedora
>       env:
>         jobname: ${{matrix.vector.jobname}}
>       runs-on: ubuntu-latest
> diff --git a/ci/install-docker-dependencies.sh b/ci/install-docker-dependencies.sh
> index 26a6689766..07a8c6b199 100755
> --- a/ci/install-docker-dependencies.sh
> +++ b/ci/install-docker-dependencies.sh
> @@ -15,4 +15,8 @@ linux-musl)
>   	apk add --update build-base curl-dev openssl-dev expat-dev gettext \
>   		pcre2-dev python3 musl-libintl perl-utils ncurses >/dev/null
>   	;;
> +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
> +	;;
>   esac
> diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh
> index 3ce81ffee9..f3aba5d6cb 100755
> --- a/ci/run-build-and-tests.sh
> +++ b/ci/run-build-and-tests.sh
> @@ -10,6 +10,11 @@ windows*) cmd //c mklink //j t\\.prove "$(cygpath -aw "$cache_dir/.prove")";;
>   *) ln -s "$cache_dir/.prove" t/.prove;;
>   esac
>   
> +if test "$jobname" = "pedantic"
> +then
> +	export DEVOPTS=pedantic
> +fi
> +
>   make
>   case "$jobname" in
>   linux-gcc)
> @@ -35,10 +40,9 @@ linux-clang)
>   	export GIT_TEST_DEFAULT_HASH=sha256
>   	make test
>   	;;
> -linux-gcc-4.8)
> +linux-gcc-4.8|pedantic)
>   	# Don't run the tests; we only care about whether Git can be
> -	# built with GCC 4.8, as it errors out on some undesired (C99)
> -	# constructs that newer compilers seem to quietly accept.
> +	# built with GCC 4.8 or with pedantic
>   	;;
>   *)
>   	make test
>