Messages in this thread Patch in this message | | | Date | Tue, 29 Mar 2011 23:45:24 +0530 | From | Raghavendra D Prabhu <> | Subject | Re: [PATCH] Use the environment variable PYTHON if defined |
| |
* On Mon, Mar 28, 2011 at 10:36:02PM +0000, Michael Witten <mfwitten@gmail.com> wrote: >On Sat, Mar 26, 2011 at 17:44, Raghavendra D Prabhu <rprabhu@wnohang.net> wrote: >> On systems with python{2,3} installed, perf build can break >> which can be fixed by exporting PYTHON to the right value. >> Added support for PYTHON in the Makefile. > >Yes, I too have run into this problem; my distribution, Arch Linux, >installs python3 as the main python: > > /usr/bin/python{,-config} > >and python2 as an ancillary, versioned python: > > /usr/bin/python2{,-config} > >The real problem here is not so much that perf's Makefile >hardcodes the program names `python' and `python-config'; >rather, the problem is that perf's python code (both .c >and probably .py) is incompatible with python3, thereby >causing the build to break. > >The Correct Solution (for the most part) is to make perf's >python code compatible with both versions of python >(I was hoping somebody who knows these things better would >do it, which is why I never submitted a patch for >changing the hardcoded values). > >In any case, Raghavendra's patch is a pretty darn good >bandage that will allow people to keep working; however, >$(PYTHON) can be used in a few other places, and the >assignment of its value can be error-checked more thoroughly. > >The following is a patch that does a bit more; you can save >this email to `path/to/email' and then apply the patch >by running: > > git am --scissors path/to/email > >8<--------8<--------8<--------8<--------8<--------8<--------8< >Subject: [PATCH] perf tools: Makefile: PYTHON{,_CONFIG} to bandage python3 incompatibility >Currently, python3 is not supported by perf's code; this >can cause the build to fail for systems that have python3 >installed as the default python: > > python{,-config} > >The Correct Solution is to write compatibility code so that >python3 works out-of-the-box. > >However, users often have an ancillary python2 installed: > > python2{,-config} > >Therefore, a quick fix is to allow the user to specify those >ancillary paths as the python binaries that Makefile should >use, thereby avoiding python3 altogether. > >This commit adds the ability to set PYTHON and/or PYTHON_CONFIG >either as environment variables or as make variables on the >command line; the paths may be relative, and usually only >PYTHON is necessary in order for PYTHON_CONFIG to be defined >implicitly. > >Thanks to: > > Raghavendra D Prabhu <rprabhu@wnohang.net> > >for motivating this patch. > >See: > > Message-ID: <e987828e-87ec-4973-95e7-47f10f5d9bab-mfwitten@gmail.com> > >Signed-off-by: Michael Witten <mfwitten@gmail.com> >--- > tools/perf/Makefile | 22 +++++++++++++++++--- > tools/perf/feature-tests.mak | 44 ++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 62 insertions(+), 4 deletions(-) > >diff --git a/tools/perf/Makefile b/tools/perf/Makefile >index 158c30e..b468383 100644 >--- a/tools/perf/Makefile >+++ b/tools/perf/Makefile >@@ -13,6 +13,12 @@ endif > > # Define V to have a more verbose compile.
>+# Define PYTHON to point to the python binary if the default >+# `python' is not correct; for example: PYTHON=python2 >+# >+# Define PYTHON_CONFIG to point to the python-config binary if >+# the default `$(PYTHON)-config' is not correct. >+# > # Define ASCIIDOC8 if you want to format documentation with AsciiDoc 8
> # Define DOCBOOK_XSL_172 if you want to format man pages with DocBook XSL v1.72. >@@ -165,7 +171,7 @@ grep-libs = $(filter -l%,$(1)) > strip-libs = $(filter-out -l%,$(1)) > > $(OUTPUT)python/perf.so: $(PYRF_OBJS) >- $(QUIET_GEN)python util/setup.py --quiet build_ext --build-lib='$(OUTPUT)python' \ >+ $(QUIET_GEN)$(PYTHON) util/setup.py --quiet build_ext --build-lib='$(OUTPUT)python' \ > --build-temp='$(OUTPUT)python/temp'
> # No Perl scripts right now: >@@ -474,10 +480,18 @@ endif > ifdef NO_LIBPYTHON > BASIC_CFLAGS += -DNO_LIBPYTHON > else >+ override PYTHON := \ >+ $(or $(call get-supplied-or-default-executable,$(PYTHON),python),\ >+ $(error Please set PYTHON appropriately)) >+ >+ override PYTHON_CONFIG := \ >+ $(or $(call get-supplied-or-default-executable,$(PYTHON_CONFIG),$(PYTHON)-config),\ >+ $(error Please set PYTHON_CONFIG appropriately)) >+ >- PYTHON_EMBED_LDOPTS = $(shell python-config --ldflags 2>/dev/null) >+ PYTHON_EMBED_LDOPTS = $(shell $(PYTHON_CONFIG) --ldflags 2>/dev/null) > PYTHON_EMBED_LDFLAGS = $(call strip-libs,$(PYTHON_EMBED_LDOPTS)) > PYTHON_EMBED_LIBADD = $(call grep-libs,$(PYTHON_EMBED_LDOPTS)) >- PYTHON_EMBED_CCOPTS = `python-config --cflags 2>/dev/null` >+ PYTHON_EMBED_CCOPTS = $(shell $(PYTHON_CONFIG) --cflags 2>/dev/null) > FLAGS_PYTHON_EMBED=$(PYTHON_EMBED_CCOPTS) $(PYTHON_EMBED_LDOPTS) > ifneq ($(call try-cc,$(SOURCE_PYTHON_EMBED),$(FLAGS_PYTHON_EMBED)),y) > msg := $(warning No Python.h found, install python-dev[el] to have python support in 'perf script' and to build the python bindings) >@@ -829,7 +843,7 @@ clean: > $(RM) *.spec *.pyc *.pyo */*.pyc */*.pyo $(OUTPUT)common-cmds.h TAGS tags cscope* > $(MAKE) -C Documentation/ clean > $(RM) $(OUTPUT)PERF-VERSION-FILE $(OUTPUT)PERF-CFLAGS >- @python util/setup.py clean --build-lib='$(OUTPUT)python' \ >+ @$(PYTHON) util/setup.py clean --build-lib='$(OUTPUT)python' \ > --build-temp='$(OUTPUT)python/temp' > > .PHONY: all install clean strip >diff --git a/tools/perf/feature-tests.mak b/tools/perf/feature-tests.mak >index b041ca6..8d677f4 100644 >--- a/tools/perf/feature-tests.mak >+++ b/tools/perf/feature-tests.mak >@@ -128,3 +128,47 @@ try-cc = $(shell sh -c \ > echo "$(1)" | \ > $(CC) -x c - $(2) -o "$$TMP" > /dev/null 2>&1 && echo y; \ > rm -f "$$TMP"') >+ >+# is-absolute >+# Usage: bool-value = $(call is-absolute,path) >+# >+define is-absolute >+$(shell sh -c "echo '$(1)' | grep ^/") >+endef >+ >+# lookup >+# Usage: absolute-executable-path-or-empty = $(call lookup,path) >+# >+define lookup >+$(shell sh -c "command -v '$(1)'") >+endef >+ >+# is-executable >+# Usage: bool-value = $(call is-executable,path) >+# >+define is-executable >+$(shell sh -c "test -f '$(1)' -a -x '$(1)' && echo y") >+endef >+ >+# get-executable >+# Usage: absolute-executable-path-or-empty = $(call get-executable,path) >+# >+# The goal is to get an absolute path for an executable; >+# the `command -v' is defined by POSIX, but it's not >+# necessarily very portable, so it's only used if >+# relative path resolution is requested, as determined >+# by the presence of a leading `/'. >+# >+define get-executable >+$(if $(1),$(if $(call is-absolute,$(1)),$(if $(call is-executable,$(1)),$(1)),$(call lookup,$(1)))) >+endef >+ >+# get-supplied-or-default-executable >+# Usage: absolute-executable-path-or-empty = $(call get-supplied-or-default-executable,path,default) >+# >+define get-supplied-or-default-executable >+$(if $(1),$(call _attempt,$(1)),$(call _attempt,$(2))) >+endef >+define _attempt >+$(or $(call get-executable,$(1)),$(warning The path '$(1)' is not executable.)) >+endef Hi,
@Michael, yeah, I use Arch linux too and noticed this issue when building it. Thanks.
@Arnaldo, the patch submitted by Michael seems to be taking care of far more cases than mine, so that is much better. I will check my encoding and/or the signed-off issue next time I submit anything (I had created it with git format-patch --stdout followed by mutt -H <file>, not sure whether this messed it up, or Vim did it). Thanks again. [unhandled content-type:application/pgp-signature] | |