Message-ID: <3a97df19-e376-412a-95be-37ffde765cc3-mfwitten@gmail.com>
Date:	Thu, 31 Mar 2011 15:37:40 +0000
From:	Michael Witten <mfwitten@...il.com>
To:	Arnaldo Carvalho de Melo <acme@...stprotocols.net>
Cc:	Raghavendra D Prabhu <rprabhu@...hang.net>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Use the environment variable PYTHON if defined

On Tue, 2011-03-29 17:40:25 -0300, Arnaldo Carvalho de Melo wrote:
>Em Tue, Mar 29, 2011 at 02:15:30PM -0500, Michael Witten escreveu:
>> On Tue, Mar 29, 2011 at 13:15, Raghavendra D Prabhu <rprabhu@...hang.net> wrote:
>>> 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.

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 <rprabhu@...hang.net>

for motivating this patch.

See:

  Message-ID: <e987828e-87ec-4973-95e7-47f10f5d9bab-mfwitten@...il.com>

Signed-off-by: Michael Witten <mfwitten@...il.com>
---
 tools/perf/Makefile          |   77 ++++++++++++++++++++++++++++++-----------
 tools/perf/feature-tests.mak |   46 +++++++++++++++++++++++++
 2 files changed, 102 insertions(+), 21 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 158c30e..3ef8a28 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,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
 
-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
@@ -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
diff --git a/tools/perf/feature-tests.mak b/tools/perf/feature-tests.mak
index b041ca6..01e1fb1 100644
--- a/tools/perf/feature-tests.mak
+++ b/tools/perf/feature-tests.mak
@@ -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
+# Usage: absolute-executable-path-or-empty = $(call get-executable-or-default,variable,default)
+#
+define get-executable-or-default
+$(if $($(1)),$(call _ge_attempt,$($(1)),$(1)),$(call _ge_attempt,$(2)))
+endef
+define _ge_attempt
+$(or $(call get-executable,$(1)),$(call _gea_warn,$(1)),$(call _gea_err,$(2)))
+endef
+_gea_warn = $(warning The path '$(1)' is not executable.)
+_gea_err  = $(if $(1),$(error Please set '$(1)' appropriately))
-- 
1.7.4.18.g68fe8

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists