* [PATCH 1/3] tools build feature: Use CC and CXX from parent
@ 2020-07-27 4:08 Thomas Hebb
2020-07-27 4:08 ` [PATCH 2/3] tools lib api: Get rid of useless conditional assignments Thomas Hebb
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Thomas Hebb @ 2020-07-27 4:08 UTC (permalink / raw)
To: linux-kernel
Cc: Thomas Hebb, Arnaldo Carvalho de Melo, David Carrillo-Cisneros,
Ian Rogers, Igor Lubashev, Jiri Olsa, Namhyung Kim,
Quentin Monnet, Song Liu, Stephane Eranian
commit c8c188679ccf ("tools build: Use the same CC for feature detection
and actual build") changed these assignments from unconditional (:=) to
conditional (?=) so that they wouldn't clobber values from the
environment. However, conditional assignment does not work properly for
variables that Make implicitly sets, among which are CC and CXX. To
quote tools/scripts/Makefile.include, which handles this properly:
# Makefiles suck: This macro sets a default value of $(2) for the
# variable named by $(1), unless the variable has been set by
# environment or command line. This is necessary for CC and AR
# because make sets default values, so the simpler ?= approach
# won't work as expected.
In other words, the conditional assignments will not run even if the
variables are not overridden in the environment; Make will set CC to
"cc" and CXX to "g++" when it starts[1], meaning the variables are not
empty by the time the conditional assignments are evaluated. This breaks
cross-compilation when CROSS_COMPILE is set but CC isn't, since "cc"
gets used for feature detection instead of the cross compiler (and
likewise for CXX).
To fix the issue, just pass down the values of CC and CXX computed by
the parent Makefile, which gets included by the Makefile that actually
builds whatever we're detecting features for and so is guaranteed to
have good values. This is a better solution anyway, since it means we
aren't trying to replicate the logic of the parent build system and so
don't risk it getting out of sync.
Leave PKG_CONFIG alone, since 1) there's no common logic to compute it
in Makefile.include, and 2) it's not an implicit variable, so
conditional assignment works properly.
[1] https://www.gnu.org/software/make/manual/html_node/Implicit-Variables.html
Fixes: c8c188679ccf ("tools build: Use the same CC for feature detection and actual build")
Signed-off-by: Thomas Hebb <tommyhebb@gmail.com>
---
tools/build/Makefile.feature | 2 +-
tools/build/feature/Makefile | 2 --
2 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
index cb152370fdef..774f0b0ca28a 100644
--- a/tools/build/Makefile.feature
+++ b/tools/build/Makefile.feature
@@ -8,7 +8,7 @@ endif
feature_check = $(eval $(feature_check_code))
define feature_check_code
- feature-$(1) := $(shell $(MAKE) OUTPUT=$(OUTPUT_FEATURES) CFLAGS="$(EXTRA_CFLAGS) $(FEATURE_CHECK_CFLAGS-$(1))" CXXFLAGS="$(EXTRA_CXXFLAGS) $(FEATURE_CHECK_CXXFLAGS-$(1))" LDFLAGS="$(LDFLAGS) $(FEATURE_CHECK_LDFLAGS-$(1))" -C $(feature_dir) $(OUTPUT_FEATURES)test-$1.bin >/dev/null 2>/dev/null && echo 1 || echo 0)
+ feature-$(1) := $(shell $(MAKE) OUTPUT=$(OUTPUT_FEATURES) CC=$(CC) CXX=$(CXX) CFLAGS="$(EXTRA_CFLAGS) $(FEATURE_CHECK_CFLAGS-$(1))" CXXFLAGS="$(EXTRA_CXXFLAGS) $(FEATURE_CHECK_CXXFLAGS-$(1))" LDFLAGS="$(LDFLAGS) $(FEATURE_CHECK_LDFLAGS-$(1))" -C $(feature_dir) $(OUTPUT_FEATURES)test-$1.bin >/dev/null 2>/dev/null && echo 1 || echo 0)
endef
feature_set = $(eval $(feature_set_code))
diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
index b1f0321180f5..93b590d81209 100644
--- a/tools/build/feature/Makefile
+++ b/tools/build/feature/Makefile
@@ -74,8 +74,6 @@ FILES= \
FILES := $(addprefix $(OUTPUT),$(FILES))
-CC ?= $(CROSS_COMPILE)gcc
-CXX ?= $(CROSS_COMPILE)g++
PKG_CONFIG ?= $(CROSS_COMPILE)pkg-config
LLVM_CONFIG ?= llvm-config
CLANG ?= clang
--
2.27.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/3] tools lib api: Get rid of useless conditional assignments
2020-07-27 4:08 [PATCH 1/3] tools build feature: Use CC and CXX from parent Thomas Hebb
@ 2020-07-27 4:08 ` Thomas Hebb
2020-07-27 4:08 ` [PATCH 3/3] libsubcmd: " Thomas Hebb
2020-07-28 14:42 ` [PATCH 1/3] tools build feature: Use CC and CXX from parent Jiri Olsa
2 siblings, 0 replies; 5+ messages in thread
From: Thomas Hebb @ 2020-07-27 4:08 UTC (permalink / raw)
To: linux-kernel; +Cc: Thomas Hebb
Conditional assignment does not work properly for variables that Make
implicitly sets, among which are CC and AR. To quote
tools/scripts/Makefile.include, which handles this properly:
# Makefiles suck: This macro sets a default value of $(2) for the
# variable named by $(1), unless the variable has been set by
# environment or command line. This is necessary for CC and AR
# because make sets default values, so the simpler ?= approach
# won't work as expected.
In other words, the conditional assignments will not run even if the
variables are not overridden in the environment; Make will set CC and AR
to default values when it starts[1], meaning they're not empty by the
time the conditional assignments are evaluated.
Since the assignments never run, we can just get rid of them. CC and AR
are already set properly by Makefile.include using the macro mentioned
in the quote above. In addition, we can get rid of the LD assignment,
because it's also set by Makefile.include.
[1] https://www.gnu.org/software/make/manual/html_node/Implicit-Variables.html
Signed-off-by: Thomas Hebb <tommyhebb@gmail.com>
---
tools/lib/api/Makefile | 4 ----
1 file changed, 4 deletions(-)
diff --git a/tools/lib/api/Makefile b/tools/lib/api/Makefile
index a13e9c7f1fc5..5f2e3f8acbd0 100644
--- a/tools/lib/api/Makefile
+++ b/tools/lib/api/Makefile
@@ -9,10 +9,6 @@ srctree := $(patsubst %/,%,$(dir $(srctree)))
#$(info Determined 'srctree' to be $(srctree))
endif
-CC ?= $(CROSS_COMPILE)gcc
-AR ?= $(CROSS_COMPILE)ar
-LD ?= $(CROSS_COMPILE)ld
-
MAKEFLAGS += --no-print-directory
LIBFILE = $(OUTPUT)libapi.a
--
2.27.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 3/3] libsubcmd: Get rid of useless conditional assignments
2020-07-27 4:08 [PATCH 1/3] tools build feature: Use CC and CXX from parent Thomas Hebb
2020-07-27 4:08 ` [PATCH 2/3] tools lib api: Get rid of useless conditional assignments Thomas Hebb
@ 2020-07-27 4:08 ` Thomas Hebb
2020-07-28 14:42 ` [PATCH 1/3] tools build feature: Use CC and CXX from parent Jiri Olsa
2 siblings, 0 replies; 5+ messages in thread
From: Thomas Hebb @ 2020-07-27 4:08 UTC (permalink / raw)
To: linux-kernel
Cc: Thomas Hebb, Arnaldo Carvalho de Melo, Ian Rogers, James Clark
Conditional assignment does not work properly for variables that Make
implicitly sets, among which are CC and AR. To quote
tools/scripts/Makefile.include, which handles this properly:
# Makefiles suck: This macro sets a default value of $(2) for the
# variable named by $(1), unless the variable has been set by
# environment or command line. This is necessary for CC and AR
# because make sets default values, so the simpler ?= approach
# won't work as expected.
In other words, the conditional assignments will not run even if the
variables are not overridden in the environment; Make will set CC and AR
to default values when it starts[1], meaning they're not empty by the
time the conditional assignments are evaluated.
Since the assignments never run, we can just get rid of them. CC and AR
are already set properly by Makefile.include using the macro mentioned
in the quote above. In addition, we can get rid of the LD assignment,
because it's also set by Makefile.include.
[1] https://www.gnu.org/software/make/manual/html_node/Implicit-Variables.html
Signed-off-by: Thomas Hebb <tommyhebb@gmail.com>
---
tools/lib/subcmd/Makefile | 4 ----
1 file changed, 4 deletions(-)
diff --git a/tools/lib/subcmd/Makefile b/tools/lib/subcmd/Makefile
index 1c777a72bb39..5f2058a6a1ce 100644
--- a/tools/lib/subcmd/Makefile
+++ b/tools/lib/subcmd/Makefile
@@ -9,10 +9,6 @@ srctree := $(patsubst %/,%,$(dir $(srctree)))
#$(info Determined 'srctree' to be $(srctree))
endif
-CC ?= $(CROSS_COMPILE)gcc
-LD ?= $(CROSS_COMPILE)ld
-AR ?= $(CROSS_COMPILE)ar
-
RM = rm -f
MAKEFLAGS += --no-print-directory
--
2.27.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/3] tools build feature: Use CC and CXX from parent
2020-07-27 4:08 [PATCH 1/3] tools build feature: Use CC and CXX from parent Thomas Hebb
2020-07-27 4:08 ` [PATCH 2/3] tools lib api: Get rid of useless conditional assignments Thomas Hebb
2020-07-27 4:08 ` [PATCH 3/3] libsubcmd: " Thomas Hebb
@ 2020-07-28 14:42 ` Jiri Olsa
2020-07-28 15:09 ` Arnaldo Carvalho de Melo
2 siblings, 1 reply; 5+ messages in thread
From: Jiri Olsa @ 2020-07-28 14:42 UTC (permalink / raw)
To: Thomas Hebb
Cc: linux-kernel, Arnaldo Carvalho de Melo, David Carrillo-Cisneros,
Ian Rogers, Igor Lubashev, Jiri Olsa, Namhyung Kim,
Quentin Monnet, Song Liu, Stephane Eranian
On Sun, Jul 26, 2020 at 09:08:14PM -0700, Thomas Hebb wrote:
> commit c8c188679ccf ("tools build: Use the same CC for feature detection
> and actual build") changed these assignments from unconditional (:=) to
> conditional (?=) so that they wouldn't clobber values from the
> environment. However, conditional assignment does not work properly for
> variables that Make implicitly sets, among which are CC and CXX. To
> quote tools/scripts/Makefile.include, which handles this properly:
>
> # Makefiles suck: This macro sets a default value of $(2) for the
> # variable named by $(1), unless the variable has been set by
> # environment or command line. This is necessary for CC and AR
> # because make sets default values, so the simpler ?= approach
> # won't work as expected.
>
> In other words, the conditional assignments will not run even if the
> variables are not overridden in the environment; Make will set CC to
> "cc" and CXX to "g++" when it starts[1], meaning the variables are not
> empty by the time the conditional assignments are evaluated. This breaks
> cross-compilation when CROSS_COMPILE is set but CC isn't, since "cc"
> gets used for feature detection instead of the cross compiler (and
> likewise for CXX).
>
> To fix the issue, just pass down the values of CC and CXX computed by
> the parent Makefile, which gets included by the Makefile that actually
> builds whatever we're detecting features for and so is guaranteed to
> have good values. This is a better solution anyway, since it means we
> aren't trying to replicate the logic of the parent build system and so
> don't risk it getting out of sync.
>
> Leave PKG_CONFIG alone, since 1) there's no common logic to compute it
> in Makefile.include, and 2) it's not an implicit variable, so
> conditional assignment works properly.
looks good, I wonder we might need it also for CLANG in the future
Acked-by: Jiri Olsa <jolsa@redhat.com>
thanks,
jirka
>
> [1] https://www.gnu.org/software/make/manual/html_node/Implicit-Variables.html
>
> Fixes: c8c188679ccf ("tools build: Use the same CC for feature detection and actual build")
> Signed-off-by: Thomas Hebb <tommyhebb@gmail.com>
> ---
>
> tools/build/Makefile.feature | 2 +-
> tools/build/feature/Makefile | 2 --
> 2 files changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
> index cb152370fdef..774f0b0ca28a 100644
> --- a/tools/build/Makefile.feature
> +++ b/tools/build/Makefile.feature
> @@ -8,7 +8,7 @@ endif
>
> feature_check = $(eval $(feature_check_code))
> define feature_check_code
> - feature-$(1) := $(shell $(MAKE) OUTPUT=$(OUTPUT_FEATURES) CFLAGS="$(EXTRA_CFLAGS) $(FEATURE_CHECK_CFLAGS-$(1))" CXXFLAGS="$(EXTRA_CXXFLAGS) $(FEATURE_CHECK_CXXFLAGS-$(1))" LDFLAGS="$(LDFLAGS) $(FEATURE_CHECK_LDFLAGS-$(1))" -C $(feature_dir) $(OUTPUT_FEATURES)test-$1.bin >/dev/null 2>/dev/null && echo 1 || echo 0)
> + feature-$(1) := $(shell $(MAKE) OUTPUT=$(OUTPUT_FEATURES) CC=$(CC) CXX=$(CXX) CFLAGS="$(EXTRA_CFLAGS) $(FEATURE_CHECK_CFLAGS-$(1))" CXXFLAGS="$(EXTRA_CXXFLAGS) $(FEATURE_CHECK_CXXFLAGS-$(1))" LDFLAGS="$(LDFLAGS) $(FEATURE_CHECK_LDFLAGS-$(1))" -C $(feature_dir) $(OUTPUT_FEATURES)test-$1.bin >/dev/null 2>/dev/null && echo 1 || echo 0)
> endef
>
> feature_set = $(eval $(feature_set_code))
> diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
> index b1f0321180f5..93b590d81209 100644
> --- a/tools/build/feature/Makefile
> +++ b/tools/build/feature/Makefile
> @@ -74,8 +74,6 @@ FILES= \
>
> FILES := $(addprefix $(OUTPUT),$(FILES))
>
> -CC ?= $(CROSS_COMPILE)gcc
> -CXX ?= $(CROSS_COMPILE)g++
> PKG_CONFIG ?= $(CROSS_COMPILE)pkg-config
> LLVM_CONFIG ?= llvm-config
> CLANG ?= clang
> --
> 2.27.0
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/3] tools build feature: Use CC and CXX from parent
2020-07-28 14:42 ` [PATCH 1/3] tools build feature: Use CC and CXX from parent Jiri Olsa
@ 2020-07-28 15:09 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 5+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-07-28 15:09 UTC (permalink / raw)
To: Jiri Olsa
Cc: Thomas Hebb, linux-kernel, Arnaldo Carvalho de Melo,
David Carrillo-Cisneros, Ian Rogers, Igor Lubashev, Jiri Olsa,
Namhyung Kim, Quentin Monnet, Song Liu, Stephane Eranian
Em Tue, Jul 28, 2020 at 04:42:02PM +0200, Jiri Olsa escreveu:
> On Sun, Jul 26, 2020 at 09:08:14PM -0700, Thomas Hebb wrote:
> > commit c8c188679ccf ("tools build: Use the same CC for feature detection
> > and actual build") changed these assignments from unconditional (:=) to
> > conditional (?=) so that they wouldn't clobber values from the
> > environment. However, conditional assignment does not work properly for
> > variables that Make implicitly sets, among which are CC and CXX. To
> > quote tools/scripts/Makefile.include, which handles this properly:
> >
> > # Makefiles suck: This macro sets a default value of $(2) for the
> > # variable named by $(1), unless the variable has been set by
> > # environment or command line. This is necessary for CC and AR
> > # because make sets default values, so the simpler ?= approach
> > # won't work as expected.
> >
> > In other words, the conditional assignments will not run even if the
> > variables are not overridden in the environment; Make will set CC to
> > "cc" and CXX to "g++" when it starts[1], meaning the variables are not
> > empty by the time the conditional assignments are evaluated. This breaks
> > cross-compilation when CROSS_COMPILE is set but CC isn't, since "cc"
> > gets used for feature detection instead of the cross compiler (and
> > likewise for CXX).
> >
> > To fix the issue, just pass down the values of CC and CXX computed by
> > the parent Makefile, which gets included by the Makefile that actually
> > builds whatever we're detecting features for and so is guaranteed to
> > have good values. This is a better solution anyway, since it means we
> > aren't trying to replicate the logic of the parent build system and so
> > don't risk it getting out of sync.
> >
> > Leave PKG_CONFIG alone, since 1) there's no common logic to compute it
> > in Makefile.include, and 2) it's not an implicit variable, so
> > conditional assignment works properly.
>
> looks good, I wonder we might need it also for CLANG in the future
>
> Acked-by: Jiri Olsa <jolsa@redhat.com>
Thanks for checking, I'll add it to the mix and see how it goes with the
container tests.
- Arnaldo
> thanks,
> jirka
>
> >
> > [1] https://www.gnu.org/software/make/manual/html_node/Implicit-Variables.html
> >
> > Fixes: c8c188679ccf ("tools build: Use the same CC for feature detection and actual build")
> > Signed-off-by: Thomas Hebb <tommyhebb@gmail.com>
> > ---
> >
> > tools/build/Makefile.feature | 2 +-
> > tools/build/feature/Makefile | 2 --
> > 2 files changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
> > index cb152370fdef..774f0b0ca28a 100644
> > --- a/tools/build/Makefile.feature
> > +++ b/tools/build/Makefile.feature
> > @@ -8,7 +8,7 @@ endif
> >
> > feature_check = $(eval $(feature_check_code))
> > define feature_check_code
> > - feature-$(1) := $(shell $(MAKE) OUTPUT=$(OUTPUT_FEATURES) CFLAGS="$(EXTRA_CFLAGS) $(FEATURE_CHECK_CFLAGS-$(1))" CXXFLAGS="$(EXTRA_CXXFLAGS) $(FEATURE_CHECK_CXXFLAGS-$(1))" LDFLAGS="$(LDFLAGS) $(FEATURE_CHECK_LDFLAGS-$(1))" -C $(feature_dir) $(OUTPUT_FEATURES)test-$1.bin >/dev/null 2>/dev/null && echo 1 || echo 0)
> > + feature-$(1) := $(shell $(MAKE) OUTPUT=$(OUTPUT_FEATURES) CC=$(CC) CXX=$(CXX) CFLAGS="$(EXTRA_CFLAGS) $(FEATURE_CHECK_CFLAGS-$(1))" CXXFLAGS="$(EXTRA_CXXFLAGS) $(FEATURE_CHECK_CXXFLAGS-$(1))" LDFLAGS="$(LDFLAGS) $(FEATURE_CHECK_LDFLAGS-$(1))" -C $(feature_dir) $(OUTPUT_FEATURES)test-$1.bin >/dev/null 2>/dev/null && echo 1 || echo 0)
> > endef
> >
> > feature_set = $(eval $(feature_set_code))
> > diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
> > index b1f0321180f5..93b590d81209 100644
> > --- a/tools/build/feature/Makefile
> > +++ b/tools/build/feature/Makefile
> > @@ -74,8 +74,6 @@ FILES= \
> >
> > FILES := $(addprefix $(OUTPUT),$(FILES))
> >
> > -CC ?= $(CROSS_COMPILE)gcc
> > -CXX ?= $(CROSS_COMPILE)g++
> > PKG_CONFIG ?= $(CROSS_COMPILE)pkg-config
> > LLVM_CONFIG ?= llvm-config
> > CLANG ?= clang
> > --
> > 2.27.0
> >
>
--
- Arnaldo
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-07-28 15:09 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-27 4:08 [PATCH 1/3] tools build feature: Use CC and CXX from parent Thomas Hebb
2020-07-27 4:08 ` [PATCH 2/3] tools lib api: Get rid of useless conditional assignments Thomas Hebb
2020-07-27 4:08 ` [PATCH 3/3] libsubcmd: " Thomas Hebb
2020-07-28 14:42 ` [PATCH 1/3] tools build feature: Use CC and CXX from parent Jiri Olsa
2020-07-28 15:09 ` Arnaldo Carvalho de Melo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).