Re: [PATCH v3 1/5] build: make cc-option properly deal with unrecognized sub-options

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Wed, Aug 16, 2023 at 08:06:52AM +0200, Jan Beulich wrote:
> On 11.08.2023 15:48, Anthony PERARD wrote:
> > But isn't it doing doing pattern matching on an error message going to
> > lead sometime to false positive? Linux's build system seems to works
> > fine by just using the exit value. They've got a few trick to deal with
> > -Wno-* and with clang.
> > 
> > For -Wno-$(warning), they test -W$(warning) instead. For clang, they've
> > enable additional warnings:
> >     -Werror=unknown-warning-option
> >     -Werror=ignored-optimization-argument
> >     -Werror=option-ignored
> >     -Werror=unused-command-line-argument
> 
> I think using just -Werror is going to be enough. The completely changed

Yes, looks like -Werror is enough. I'm not sure why Linux has them
because they tests flags with -Werror in most cases.

> patch below appears to be doing fine, but of course requires me to drop
> ...
> 
> > In any case, the patch is fine.
> > Reviewed-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> 
> ... this.
> 
> Jan
> 
> --- a/Config.mk
> +++ b/Config.mk
> @@ -81,17 +81,17 @@ PYTHON_PREFIX_ARG ?= --prefix="$(prefix)
>  
>  # cc-option: Check if compiler supports first option, else fall back to 
> second.
>  #
> -# This is complicated by the fact that unrecognised -Wno-* options:
> +# This is complicated by the fact that with most gcc versions unrecognised
> +# -Wno-* options:
>  #   (a) are ignored unless the compilation emits a warning; and
>  #   (b) even then produce a warning rather than an error
> -# To handle this we do a test compile, passing the option-under-test, on a 
> code
> -# fragment that will always produce a warning (integer assigned to pointer).
> -# We then grep for the option-under-test in the compiler's output, the 
> presence
> -# of which would indicate an "unrecognized command-line option" 
> warning/error.
> +# Further Clang also only warns for unrecognised -W* options.  To handle this
> +# we do a test compile, substituting -Wno-* by -W* and adding -Werror.  This
> +# way all unrecognised options are diagnosed uniformly, allowing us to merely
> +# check exit status.
>  #
>  # Usage: cflags-y += $(call cc-option,$(CC),-march=winchip-c6,-march=i586)
> -cc-option = $(shell if test -z "`echo 'void*p=1;' | \
> -              $(1) $(2) -c -o /dev/null -x c - 2>&1 | grep -- 
> $(2:-Wa$(comma)%=%) -`"; \
> +cc-option = $(shell if $(1) $(2:-Wno-%=-W%) -Werror -c -o /dev/null -x c 
> /dev/null >/dev/null 2>&1; \
>                then echo "$(2)"; else echo "$(3)"; fi ;)

I've try to compare the result of cc-option with and without this change
in the gitlab CI, and it seems that the result is the same for the flags
we tests. So this change looks fine:
Reviewed-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>

Thanks,

-- 
Anthony PERARD