Re: [PATCH 1/4] perf tools: pass PYTHON config to feature detection

[Namhyung Kim]

On Mon, Feb 06, 2017 at 10:19:30AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Wed, Feb 01, 2017 at 10:38:01PM -0800, David Carrillo-Cisneros escreveu:
> > Python's CC and link Makefile variables were not passed to feature
> > detection, causing feature detection to use system's Python rather than
> > PYTHON_CONFIG's one. This created a mismatch between the detected Python
> > support and the one actually used by perf when PYTHON_CONFIG is specified.
> > 
> > Fix it by moving Python's variable initialization to before feature
> > detection and pass FLAGS_PYTHON_EMBED to Python's feature detection's
> > build target.
> 
> Hi Namhyung, Thomas, since this was last touched in the cset below, can you
> please check this if possible and provide Acked-by tags?
> 
> Thanks,
> 
> commit 56c7d79e49776084b852e451bda5f59dc3bcf894
> Author: Namhyung Kim <namhyung@xxxxxxxxxx>
> Date:   Tue Jul 29 15:57:19 2014 +0900
> 
>     perf tools: Fix make PYTHON override
>     
>     Thomas reported that make PYTHON=python2 is not work on some systems.  I
>     can reproduce it on my ArchLinux box too.
>     
>     This is because it's overridden by config/feature-checks/Makefile
>     regardless of PYTHON setting.  I guess it's a bug slipped into during
>     the feature checking change.
>     
>     Actually, we don't need to check python-config in the feature-checks.
>     We can just pass appropriate FEATURE_CHECK_*FLAGS.
>     
>     Reported-by: Thomas Ilsche <thomas.ilsche@xxxxxxxxxxxxx>
>     Signed-off-by: Namhyung Kim <namhyung@xxxxxxxxxx>
>     Tested-by: Thomas Ilsche <thomas.ilsche@xxxxxxxxxxxxx>
> 
>  
> > Signed-off-by: David Carrillo-Cisneros <davidcc@xxxxxxxxxx>
> > ---
> >  tools/build/feature/Makefile |  2 +-
> >  tools/perf/Makefile.config   | 23 ++++++++++-------------
> >  2 files changed, 11 insertions(+), 14 deletions(-)
> > 
> > diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
> > index b564a2eea039..3dd2c600d250 100644
> > --- a/tools/build/feature/Makefile
> > +++ b/tools/build/feature/Makefile
> > @@ -171,7 +171,7 @@ $(OUTPUT)test-libperl.bin:
> >  	$(BUILD) $(FLAGS_PERL_EMBED)
> >  
> >  $(OUTPUT)test-libpython.bin:
> > -	$(BUILD)
> > +	$(BUILD) $(FLAGS_PYTHON_EMBED)
> >  
> >  $(OUTPUT)test-libpython-version.bin:
> >  	$(BUILD)
> > diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> > index 03cf947755b9..41f5c41c5660 100644
> > --- a/tools/perf/Makefile.config
> > +++ b/tools/perf/Makefile.config
> > @@ -166,10 +166,18 @@ PYTHON2_CONFIG := \
> >  override PYTHON_CONFIG := \
> >    $(call get-executable-or-default,PYTHON_CONFIG,$(PYTHON2_CONFIG))
> >  
> > +grep-libs  = $(filter -l%,$(1))
> > +strip-libs  = $(filter-out -l%,$(1))
> > +
> >  PYTHON_CONFIG_SQ := $(call shell-sq,$(PYTHON_CONFIG))
> >  
> > -PYTHON_EMBED_LDOPTS := $(shell $(PYTHON_CONFIG_SQ) --ldflags 2>/dev/null)
> > -PYTHON_EMBED_CCOPTS := $(shell $(PYTHON_CONFIG_SQ) --cflags 2>/dev/null)
> > +ifdef PYTHON_CONFIG
> > +  PYTHON_EMBED_LDOPTS := $(shell $(PYTHON_CONFIG_SQ) --ldflags 2>/dev/null)
> > +  PYTHON_EMBED_LDFLAGS := $(call strip-libs,$(PYTHON_EMBED_LDOPTS))
> > +  PYTHON_EMBED_LIBADD := $(call grep-libs,$(PYTHON_EMBED_LDOPTS)) -lutil
> > +  PYTHON_EMBED_CCOPTS := $(shell $(PYTHON_CONFIG_SQ) --cflags 2>/dev/null)
> > +  FLAGS_PYTHON_EMBED := $(PYTHON_EMBED_CCOPTS) $(PYTHON_EMBED_LDOPTS)
> > +endif
> >  
> >  FEATURE_CHECK_CFLAGS-libpython := $(PYTHON_EMBED_CCOPTS)
> >  FEATURE_CHECK_LDFLAGS-libpython := $(PYTHON_EMBED_LDOPTS)

Hmm.. I guess these feature check flags should be passed to the above
Makefile but it seems not.

Thanks,
Namhyung


> > @@ -542,8 +550,6 @@ ifndef NO_GTK2
> >    endif
> >  endif
> >  
> > -grep-libs  = $(filter -l%,$(1))
> > -strip-libs = $(filter-out -l%,$(1))
> >  
> >  ifdef NO_LIBPERL
> >    CFLAGS += -DNO_LIBPERL
> > @@ -591,18 +597,9 @@ else
> >        $(call disable-python,No 'python-config' tool was found: disables Python support - please install python-devel/python-dev)
> >      else
> >  
> > -      PYTHON_CONFIG_SQ := $(call shell-sq,$(PYTHON_CONFIG))
> > -
> > -      PYTHON_EMBED_LDOPTS := $(shell $(PYTHON_CONFIG_SQ) --ldflags 2>/dev/null)
> > -      PYTHON_EMBED_LDFLAGS := $(call strip-libs,$(PYTHON_EMBED_LDOPTS))
> > -      PYTHON_EMBED_LIBADD := $(call grep-libs,$(PYTHON_EMBED_LDOPTS)) -lutil
> > -      PYTHON_EMBED_CCOPTS := $(shell $(PYTHON_CONFIG_SQ) --cflags 2>/dev/null)
> > -      FLAGS_PYTHON_EMBED := $(PYTHON_EMBED_CCOPTS) $(PYTHON_EMBED_LDOPTS)
> > -
> >        ifneq ($(feature-libpython), 1)
> >          $(call disable-python,No 'Python.h' (for Python 2.x support) was found: disables Python support - please install python-devel/python-dev)
> >        else
> > -
> >          ifneq ($(feature-libpython-version), 1)
> >            $(warning Python 3 is not yet supported; please set)
> >            $(warning PYTHON and/or PYTHON_CONFIG appropriately.)
> > -- 
> > 2.11.0.483.g087da7b7c-goog