Re: [PATCH] Use the environment variable PYTHON if defined

[Raghavendra D Prabhu <rprabhu@wnohang.net>]

[-- Attachment #1: Type: text/plain, Size: 7746 bytes --]

* 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.

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]