kbuild: tools: drop overridden CFLAGS from MAKEOVERRIDES

PATCH In review Max Filippov 2 participants 3 messages Jan 31, 2024 – Feb 24, 2024

Subsystems (4)

Stable branch GPIO IIO GPIO uapi

lore

Share & tools

lore

Max Filippov

author

2024-01-31 04:25 UTC #0 lore raw

PATCH v2 1/1 kbuild: tools: drop overridden CFLAGS from MAKEOVERRIDES

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 [email protected]

Fixes 4ccc98a48958 ("tools gpio: Allow overriding CFLAGS")

Fixes 572974610273 ("tools iio: Override CFLAGS assignments")

Signed-off-by Max Filippov <[email protected]>

---
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(+)

tools/gpio/Makefile +1 -0

index d29c9c49e251..46fc38d51639 100644
@@ -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
 
 #

tools/iio/Makefile +1 -0

index fa720f062229..04307588dd3f 100644
@@ -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
 
 #

tools/scripts/Makefile.include +9 -0

index 6fba29f3222d..0f68b95cf55c 100644
@@ -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)

Signature · 1 line

2.39.2

Max Filippov

author

2024-02-22 04:47 UTC #1 lore raw

Ping?

Quoted 89 lines · Max Filippov

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 [email protected]
Fixes 4ccc98a48958 ("tools gpio: Allow overriding CFLAGS")
Fixes 572974610273 ("tools iio: Override CFLAGS assignments")
Signed-off-by Max Filippov <[email protected]>
---
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(+)
tools/gpio/Makefile +1 -0
index d29c9c49e251..46fc38d51639 100644
@@ -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
 
 #
tools/iio/Makefile +1 -0
index fa720f062229..04307588dd3f 100644
@@ -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
 
 #
tools/scripts/Makefile.include +9 -1
index 6fba29f3222d..0f68b95cf55c 100644
@@ -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

Signature · 2 lines

Thanks.
-- Max

Jonathan Cameron

maintainer Acked-by

2024-02-24 11:11 UTC #2 lore raw

Quoted 35 lines · On Wed, 21 Feb 2024 20:47:33 -0800 Max Filippov <[email protected]> wrote:

Ping?
Quoted 32 lines · Max Filippov
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 [email protected]
Fixes 4ccc98a48958 ("tools gpio: Allow overriding CFLAGS")
Fixes 572974610273 ("tools iio: Override CFLAGS assignments")
Signed-off-by Max Filippov <[email protected]>

I'm fine with this, but it's out of my area of expertise - rarely write Makefiles etc! So I'd like some more inputs from others.

With that in mind,

Acked-by Jonathan Cameron <[email protected]>

Quoted 59 lines

Quoted 57 lines

---
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(+)

tools/gpio/Makefile +1 -0

index d29c9c49e251..46fc38d51639 100644
@@ -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
 
 #

tools/iio/Makefile +1 -0

index fa720f062229..04307588dd3f 100644
@@ -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
 
 #

tools/scripts/Makefile.include +9 -1

index 6fba29f3222d..0f68b95cf55c 100644
@@ -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