Re: [PATCH 1/1] Makefile: only use compiler flags when supported

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3056 bytes --]

On Tue, Oct 08, 2024 at 02:41:32PM -0700, Markus Mayer wrote:
> We need to check if some compiler flags are supported as not all
> compilers may do so. For instance, "-Wsuggest-attribute=format" is
> supported by gcc but not by clang.
> 
> Using a compiler option that is not supported will most likely cause the
> build to fail, since we are building with "-Werror" and the compiler will
> issue a warning about the unknown option.
> 
> As a result, we only use the option if the compiler understands it.
> 
> Signed-off-by: Markus Mayer <mmayer@broadcom.com>

Hrm.  So, I can see the use, and the logic looks sound.  On the other
hand, I'm not especially keen to have a tiny little project like dtc
carry around a complex configure & build system with lots of compiler
knowledge.  This can be worked around by explicitly overriding CFLAGS,
and if using clang you're presumably already overriding CC.

This is perhaps another impetus to deprecate the Makefiles in favour
of the meson build system which we already support.  AIUI meson has a
fair bit of built in knowledge about compilers and their capabilities.

> ---
>  Makefile | 25 ++++++++++++++++++++++++-
>  1 file changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index f1f0ab368cd3..febdca5742c5 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -3,6 +3,26 @@
>  # Device Tree Compiler
>  #
>  
> +# try-run and cc-option were stolen from the Linux kernel
> +TMPOUT = .tmp_$$$$
> +# try-run
> +# Usage: option = $(call try-run, $(CC)...-o "$$TMP",option-ok,otherwise)
> +# Exit code chooses option. "$$TMP" serves as a temporary file and is
> +# automatically cleaned up.
> +try-run = $(shell set -e;		\
> +	TMP=$(TMPOUT)/tmp;		\
> +	mkdir -p $(TMPOUT);		\
> +	trap "rm -rf $(TMPOUT)" EXIT;	\
> +	if ($(1)) >/dev/null 2>&1;	\
> +	then echo "$(2)";		\
> +	else echo "$(3)";		\
> +	fi)
> +
> +# cc-option
> +# Usage: cflags-y += $(call cc-option,-march=winchip-c6,-march=i586)
> +cc-option = $(call try-run, \
> +	$(CC) -Werror $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2))
> +
>  #
>  # Version information will be constructed in this order:
>  # DTC_VERSION release version as MAJOR.MINOR.PATCH
> @@ -18,9 +38,12 @@ CONFIG_LOCALVERSION =
>  ASSUME_MASK ?= 0
>  
>  CPPFLAGS = -I libfdt -I . -DFDT_ASSUME_MASK=$(ASSUME_MASK)
> +# Warnings where we need to check they are supported
> +OPT_WARNINGS = $(call cc-option,-Wsuggest-attribute=format)
> +# Regular warnings
>  WARNINGS = -Wall -Wpointer-arith -Wcast-qual -Wnested-externs -Wsign-compare \
>  	-Wstrict-prototypes -Wmissing-prototypes -Wredundant-decls -Wshadow \
> -	-Wsuggest-attribute=format -Wwrite-strings
> +	-Wwrite-strings $(OPT_WARNINGS)
>  CFLAGS = -g -Os $(SHAREDLIB_CFLAGS) -Werror $(WARNINGS) $(EXTRA_CFLAGS)
>  
>  BISON = bison

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]