Re: [PATCH v2] kbuild: tools: drop overridden CFLAGS from MAKEOVERRIDES

[Max Filippov <jcmvbkbc@xxxxxxxxx>]

Ping?

On Tue, Jan 30, 2024 at 8:25 PM Max Filippov <jcmvbkbc@xxxxxxxxx> wrote:
>
> Some Makefiles under tools/ use the 'override CFLAGS += ...' construct
> to add a few required options to CFLAGS passed by the user.
> Unfortunately that only works when user passes CFLAGS as an environment
> variable, i.e.
>   CFLAGS=... make ...
> and not in case when CFLAGS are passed as make command line arguments:
>   make ... CFLAGS=...
> It happens because in the latter case CFLAGS=... is recorded in the make
> variable MAKEOVERRIDES and this variable is passed in its original form
> to all $(MAKE) subcommands, taking precedence over modified CFLAGS value
> passed in the environment variable. E.g. this causes build failure for
> gpio and iio tools when the build is run with user CFLAGS because of
> missing _GNU_SOURCE definition needed for the asprintf().
>
> One way to fix it is by removing overridden variables from the
> MAKEOVERRIDES. Add macro 'drop-var-from-overrides' that removes a
> definition of a variable passed to it from the MAKEOVERRIDES and use it
> to fix CFLAGS passing for tools/gpio and tools/iio.
>
> This implementation tries to be precise in string processing and handle
> variables with embedded spaces and backslashes correctly. To achieve
> that it replaces every '\\' sequence with '\-' to make sure that every
> '\' in the resulting string is an escape character. It then replaces
> every '\ ' sequence with '\_' to turn string values with embedded spaces
> into single words. After filtering the overridden variable definition
> out of the resulting string these two transformations are reversed.
>
> Cc: stable@xxxxxxxxxxxxxxx
> Fixes: 4ccc98a48958 ("tools gpio: Allow overriding CFLAGS")
> Fixes: 572974610273 ("tools iio: Override CFLAGS assignments")
> Signed-off-by: Max Filippov <jcmvbkbc@xxxxxxxxx>
> ---
> Changes v1->v2:
> - make drop-var-from-overrides-code work correctly with arbitrary
>   variables, including thoses ending with '\'.
>
>  tools/gpio/Makefile            | 1 +
>  tools/iio/Makefile             | 1 +
>  tools/scripts/Makefile.include | 9 +++++++++
>  3 files changed, 11 insertions(+)
>
> diff --git a/tools/gpio/Makefile b/tools/gpio/Makefile
> index d29c9c49e251..46fc38d51639 100644
> --- a/tools/gpio/Makefile
> +++ b/tools/gpio/Makefile
> @@ -24,6 +24,7 @@ ALL_PROGRAMS := $(patsubst %,$(OUTPUT)%,$(ALL_TARGETS))
>  all: $(ALL_PROGRAMS)
>
>  export srctree OUTPUT CC LD CFLAGS
> +$(call drop-var-from-overrides,CFLAGS)
>  include $(srctree)/tools/build/Makefile.include
>
>  #
> diff --git a/tools/iio/Makefile b/tools/iio/Makefile
> index fa720f062229..04307588dd3f 100644
> --- a/tools/iio/Makefile
> +++ b/tools/iio/Makefile
> @@ -20,6 +20,7 @@ ALL_PROGRAMS := $(patsubst %,$(OUTPUT)%,$(ALL_TARGETS))
>  all: $(ALL_PROGRAMS)
>
>  export srctree OUTPUT CC LD CFLAGS
> +$(call drop-var-from-overrides,CFLAGS)
>  include $(srctree)/tools/build/Makefile.include
>
>  #
> diff --git a/tools/scripts/Makefile.include b/tools/scripts/Makefile.include
> index 6fba29f3222d..0f68b95cf55c 100644
> --- a/tools/scripts/Makefile.include
> +++ b/tools/scripts/Makefile.include
> @@ -51,6 +51,15 @@ define allow-override
>      $(eval $(1) = $(2)))
>  endef
>
> +# When a Makefile overrides a variable and exports it for the nested $(MAKE)
> +# invocations to use its modified value, it must remove that variable definition
> +# from the MAKEOVERRIDES variable, otherwise the original definition from the
> +# MAKEOVERRIDES takes precedence over the exported value.
> +drop-var-from-overrides = $(eval $(drop-var-from-overrides-code))
> +define drop-var-from-overrides-code
> +MAKEOVERRIDES := $(subst \-,\\,$(subst \_,\ ,$(filter-out $(1)=%,$(subst \ ,\_,$(subst \\,\-,$(MAKEOVERRIDES))))))
> +endef
> +
>  ifneq ($(LLVM),)
>  ifneq ($(filter %/,$(LLVM)),)
>  LLVM_PREFIX := $(LLVM)
> --
> 2.39.2
>


-- 
Thanks.
-- Max