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 <jo...@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 <tommyh...@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
