Use the environment variable PYTHON if defined

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.

---
 tools/perf/Makefile |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

tools/perf/Makefile +6 -2

index 7141c42..d998221 100644
@@ -149,6 +149,9 @@ endif
 #
 # Define NO_PERL if you do not want Perl scripts or libraries at all.
 #
+# Define PYTHON to point to python binary if it is not
+# /usr/bin/python, it will be used/called as $PYTHON-config.
+#
 # Define INTERNAL_QSORT to use Git's implementation of qsort(), which
 # is a simplified version of the merge sort used in glibc. This is
 # recommended if Git triggers O(n^2) behavior in your platform's qsort().

@@ -629,10 +632,11 @@ endif
 ifdef NO_LIBPYTHON
 	BASIC_CFLAGS += -DNO_LIBPYTHON
 else
-       PYTHON_EMBED_LDOPTS = $(shell python-config --ldflags 2>/dev/null)
+       PYTHON ?= /usr/bin/python
+       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 = ` $(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)
 		BASIC_CFLAGS += -DNO_LIBPYTHON

Signature · 5 lines

1.7.4.1

--------------------------
Raghavendra Prabhu
GPG ID:D72BE977

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.

Quoted 3 lines · Raghavendra D Prabhu

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 <[email protected]>

for motivating this patch.

See:

Message-ID: <[email protected]>

Signed-off-by Michael Witten <[email protected]>

---

tools/perf/Makefile          |   22 +++++++++++++++++---
tools/perf/feature-tests.mak |   44 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 62 insertions(+), 4 deletions(-)

tools/perf/Makefile +7 -1

index 158c30e..b468383 100644
@@ -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

tools/perf/Makefile +10 -2

+        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)

tools/perf/Makefile +1 -1

@@ -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

tools/perf/feature-tests.mak +34 -0

index b041ca6..8d677f4 100644
@@ -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

the patch submitted by Michael seems to be taking care of far more cases than mine, so that is much better.

Quoted 2 lines

the patch submitted by Michael seems to be taking care of far more cases than mine, so that is much better.

One major issue with my current patch is that I opted to stop the build with an error message even when using the default python command names; is this undesirable? Is it better to fail silently as before (via the somewhat cryptic Python.h message), or is it better to force the user to specify that no python support should be built?

I think that the best course of action is to emit a warning and go

Quoted 1 line · Arnaldo Carvalho de Melo

I think that the best course of action is to emit a warning and go

Good.

I'll make it so!

Em Tue, Mar 29, 2011 at 04:02:07PM -0500, Michael Witten escreveu:

Quoted 7 lines

Quoted 1 line · Arnaldo Carvalho de Melo

I think that the best course of action is to emit a warning and go

Good.

I'll make it so!

Thanks.

Hi,
   Regarding the point where it warns about python-devel not existing, it looks like that existing feature test will pass even when python3 is chosen and fail at a later stage. So I made this little change to feature test so that it fails when python3 is chosen and perf build succeeds with the warning printed rather than failing later.

Quoted 6 lines · Raghavendra D Prabhu

Hi,
   Regarding the point where it warns about python-devel not existing, it looks like that existing feature test will pass even when python3 is chosen and fail at a later stage. So I made this little change to feature test so that it fails when python3 is chosen and perf build succeeds with the war ning printed rather than failing later.

Good point.

However, try-cc does not run the resulting program after compiling it, so the test still succeeds.

What you want to do is make the compilation fail;
to do so, move your test here:

#include <Python.h>

#if PY_VERSION_HEX >= 0x03000000
#error "Python 3 is not yet supported; set PYTHON{,_CONFIG}"

int main(void)
...

P.S.
Put your replies below the email quote.

Em Tue, Mar 29, 2011 at 02:15:30PM -0500, Michael Witten escreveu:

Quoted 9 lines · Raghavendra D Prabhu

Quoted 2 lines

the patch submitted by Michael seems to be taking care of far more cases than mine, so that is much better.

One major issue with my current patch is that I opted to stop the build with an error message even when using the default python command names; is this undesirable? Is it better to fail silently as before (via the somewhat cryptic Python.h message), or is it better to force the user to specify that no python support should be built?

I think that the best course of action is to emit a warning and go, i.e. we don't have to make it harder for people that don't want $FOO support to make that clear.

Quoted 11 lines · Arnaldo Carvalho de Melo

Em Tue, Mar 29, 2011 at 02:15:30PM -0500, Michael Witten escreveu:

Quoted 9 lines · Raghavendra D Prabhu

Quoted 2 lines

the patch submitted by Michael seems to be taking care of far more cases than mine, so that is much better.

One major issue with my current patch is that I opted to stop the build with an error message even when using the default python command names; is this undesirable? Is it better to fail silently as before (via the somewhat cryptic Python.h message), or is it better to force the user to specify that no python support should be built?

Quoted 3 lines

I think that the best course of action is to emit a warning and go, i.e. we don't have to make it harder for people that don't want $FOO support to make that clear.

The following patch implements the above requested behavior, and it is also a bit more robust than the previous patch.

However, consider running this command:

make clean NO_LIBPYTHON=1

and the resulting output:

Python support won't be built
PERF_VERSION = 2.6.38.8823.gf4ad7d
Python support won't be built
rm -f {*.o,*/*.o,*/*/*.o,*/*/*/*.o,libperf.a,perf-archive}
rm -f perf perf-archive perf
rm -f *.spec *.pyc *.pyo */*.pyc */*.pyo common-cmds.h TAGS tags cscope*
make -C Documentation/ clean
make[1]: Entering directory `/usr/src/vanilla/git/linux-2.6/repo/tools/perf/Documentation'
Python support won't be built
make[2]: Entering directory `/usr/src/vanilla/git/linux-2.6/repo/tools/perf'
make[2]: `PERF-VERSION-FILE' is up to date.
make[2]: Leaving directory `/usr/src/vanilla/git/linux-2.6/repo/tools/perf'
rm -f *.xml *.xml+ *.html *.html+ *.1 *.5 *.7
rm -f *.texi *.texi+ *.texi++ perf.info perfman.info
rm -f howto-index.txt howto/*.html doc.dep
rm -f technical/api-*.html technical/api-index.txt
rm -f cmds-ancillaryinterrogators.txt cmds-ancillarymanipulators.txt cmds-mainporcelain.txt cmds-plumbinginterrogators.txt cmds-plumbingmanipulators.txt cmds-synchingrepositories.txt cmds-synchelpers.txt cmds-purehelpers.txt cmds-foreignscminterface.txt *.made make[1]: Leaving directory `/usr/src/vanilla/git/linux-2.6/repo/tools/perf/Documentation'
rm -f PERF-VERSION-FILE PERF-CFLAGS
'/usr/bin/python' util/setup.py clean --build-lib='python' --build-temp='python/temp'
running clean

You'll notice that the line:

Python support won't be built

is repeated thrice. This is annoying, but I think it could be fixed by restructuring the Makefile to avoid unnecessary re-processing by GNU Make; as a boon, if such a restructuring is possible, then it would also improve the efficiency of running `make'.

Thus, I wouldn't worry about it now.

Sincerely,
Michael Witten

8<--------8<--------8<--------8<--------8<--------8<--------8<--------8<------
Subject: [PATCH] perf tools: Makefile: PYTHON{,_CONFIG} to bandage python3 incompatibility
Date: Thu, 31 Mar 2011 13:52:07 +0000
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; as an added benefit, python may be installed in non-standard locations without the need for updating any PATH variable.

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. Some rudimentary error checking is performed when the user explicitly specifies a value for any of these variables.

Thanks to:

Raghavendra D Prabhu <[email protected]>

for motivating this patch.

See:

Message-ID: <[email protected]>

Signed-off-by Michael Witten <[email protected]>

---

tools/perf/Makefile          |   77 ++++++++++++++++++++++++++++++-----------
tools/perf/feature-tests.mak |   46 +++++++++++++++++++++++++
2 files changed, 102 insertions(+), 21 deletions(-)

tools/perf/Makefile +9 -2

index 158c30e..3ef8a28 100644
@@ -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,8 +171,9 @@ 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' \
-						--build-temp='$(OUTPUT)python/temp'
+	$(QUIET_GEN)'$(PYTHON)' util/setup.py --quiet  build_ext            \
+	                                      --build-lib='$(OUTPUT)python' \
+	                                      --build-temp='$(OUTPUT)python/temp'
 
 # No Perl scripts right now:
 

@@ -471,24 +478,53 @@ else
 	endif

endif

tools/perf/Makefile +48 -19

-ifdef NO_LIBPYTHON
-	BASIC_CFLAGS += -DNO_LIBPYTHON
-else
-       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`
-	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)
-		BASIC_CFLAGS += -DNO_LIBPYTHON
-	else
-               ALL_LDFLAGS += $(PYTHON_EMBED_LDFLAGS)
-               EXTLIBS += $(PYTHON_EMBED_LIBADD)
-		LIB_OBJS += $(OUTPUT)util/scripting-engines/trace-event-python.o
-		LIB_OBJS += $(OUTPUT)scripts/python/Perf-Trace-Util/Context.o
-		LANG_BINDINGS += $(OUTPUT)python/perf.so
-	endif
-endif
+disable-python = $(eval $(call disable-python_code,$(1)))
+define disable-python_code
+  BASIC_CFLAGS += -DNO_LIBPYTHON
+  $(if $(1),$(warning No $(1) was found))
+  $(info Python support won't be built)
+endef
+
+override PYTHON := \
+  $(call get-executable-or-default,PYTHON,python)
+
+ifndef PYTHON
+  $(call disable-python,python interpreter)
+  python-clean=
+else
+
+  python-clean = '$(PYTHON)' util/setup.py clean \
+    --build-lib='$(OUTPUT)python'                \
+    --build-temp='$(OUTPUT)python/temp'
+
+  ifdef NO_LIBPYTHON
+    $(call disable-python)
+  else
+
+    override PYTHON_CONFIG := \
+      $(call get-executable-or-default,PYTHON_CONFIG,$(PYTHON)-config)
+
+    ifndef PYTHON_CONFIG
+      $(call disable-python,python-config tool)
+    else
+
+      PYTHON_EMBED_LDOPTS  = $(shell sh -c "'$(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  = $(shell sh -c "'$(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)
+        $(call disable-python,Python.h)
+      else
+        ALL_LDFLAGS += $(PYTHON_EMBED_LDFLAGS)
+        EXTLIBS += $(PYTHON_EMBED_LIBADD)
+        LIB_OBJS += $(OUTPUT)util/scripting-engines/trace-event-python.o
+        LIB_OBJS += $(OUTPUT)scripts/python/Perf-Trace-Util/Context.o
+        LANG_BINDINGS += $(OUTPUT)python/perf.so
+      endif
+    endif
+  endif
+endif

ifdef NO_DEMANGLE

tools/perf/Makefile +1 -2

@@ -829,8 +865,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' \
-				   --build-temp='$(OUTPUT)python/temp'
+	$(python-clean)
 

.PHONY: all install clean strip
.PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell

tools/perf/feature-tests.mak +36 -0

index b041ca6..01e1fb1 100644
@@ -128,3 +128,49 @@ 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

Apologies for the delay. I am getting a merge conflict with master now, it may need to be rebased after the 1b7155f7de119870f0d3fad89f125de2ff6c16be commit.

Quoted 3 lines · Raghavendra D Prabhu

Apologies for the delay. I am getting a merge conflict with master now, it may need to be rebased after the 1b7155f7de119870f0d3fad89f125de2ff6c16be commit.

Erg... git am' isn't really any help there, as it doesn't leave information in the files about where there are conflicts; I had to resort to running git merge' by hand.

Anyway, I've sent along (as replies to this email) *2* patches; the first one:

perf tools: Makefile: Clean up `python/perf.so' rule

is just a minor tweak of the offending conflict, and the second one is the updated patch proper:

perf tools: Makefile: PYTHON{,_CONFIG} to bandage Python 3 incompatibility

They should apply cleanly on Arnaldo's `perf/urgent' branch from his repository here:

git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux-2.6.git

the tip of which contains the commit you mentioned:

1b7155f7de119870f0d3fad89f125de2ff6c16be

Arnaldo, is it generally a good idea to branch from `perf/urgent' in order to keep things up to date?

Sincerely,
Michael Witten

I have a question regarding the message that is displayed. It says "No Python.h found" when variable PYTHON is not set correctly, it would be better if it said "No Python.h for Python 2.x found" since the header file supplied by Py3k is also Python.h and in the rare case of only Py3k installed on the box, it may be confusing.