[PATCH] Use the environment variable PYTHON if defined

[PATCH] Use the environment variable PYTHON if defined

[Raghavendra D Prabhu]

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

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

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 7141c42..d998221 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -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
-- 
1.7.4.1

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


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

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

[Arnaldo Carvalho de Melo]

Em Sun, Mar 27, 2011 at 04:14:08AM +0530, Raghavendra D Prabhu escreveu:
> 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(-)

Can you please add your Signed-off-by line? Please check
Documentation/SubmittingPatches, "12) Sign your work".

Also please check the encoding, I'm getting things like:

@@ -629,10 +632,11 @@ endif
  ifdef NO_LIBPYTHON
        BASIC_CFLAGS +=3D -DNO_LIBPYTHON
  else
-       PYTHON_EMBED_LDOPTS =3D $(shell python-config --ldflags 2>/dev/null)
+       PYTHON ?=3D /usr/bin/python
+       PYTHON_EMBED_LDOPTS =3D $(shell $(PYTHON)-config --ldflags 2>/dev/n=
ull)

See the /dev/null line wrapping? Also the +=3D, perhaps sending as an
attachment helps.

For more info please read Documentation/email-clients.txt .

The patch looks otherwise a fix, so please take care of the avove issues and
resubmit,

Michael, if you could double check and give me your Acked-by, that would be
appreciated :-)

Thanks,

- Arnaldo

 
> diff --git a/tools/perf/Makefile b/tools/perf/Makefile
> index 7141c42..d998221 100644
> --- a/tools/perf/Makefile
> +++ b/tools/perf/Makefile
> @@ -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
> -- 
> 1.7.4.1
> 
> --------------------------
> Raghavendra Prabhu
> GPG ID:D72BE977
> 

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

[Michael Witten]

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
-- 
1.7.4.18.g68fe8

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

[Raghavendra D Prabhu]

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

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

[Michael Witten]

On Tue, Mar 29, 2011 at 13:15, Raghavendra D Prabhu <rprabhu@wnohang.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?

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

[Arnaldo Carvalho de Melo]

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

Just not having python-dev{el} installed should be enough to disable the
feature. For people that want the feature the warning is enough to tell
them to install the missing package.

- Arnaldo

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

[Michael Witten]

On Tue, Mar 29, 2011 at 15:40, Arnaldo Carvalho de Melo
<acme@ghostprotocols.net> wrote:
> I think that the best course of action is to emit a warning and go

Good.

I'll make it so!

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

[Arnaldo Carvalho de Melo]

Em Tue, Mar 29, 2011 at 04:02:07PM -0500, Michael Witten escreveu:
> On Tue, Mar 29, 2011 at 15:40, Arnaldo Carvalho de Melo
> <acme@ghostprotocols.net> wrote:
> > I think that the best course of action is to emit a warning and go
> 
> Good.
> 
> I'll make it so!

Thanks.

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

[Raghavendra D Prabhu]

[-- Attachment #1.1: Type: text/plain, Size: 742 bytes --]

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.
* On Tue, Mar 29, 2011 at 06:14:37PM -0300, Arnaldo Carvalho de Melo <acme@ghostprotocols.net> wrote:
>Em Tue, Mar 29, 2011 at 04:02:07PM -0500, Michael Witten escreveu:
>> On Tue, Mar 29, 2011 at 15:40, Arnaldo Carvalho de Melo
>> <acme@ghostprotocols.net> wrote:
>> > I think that the best course of action is to emit a warning and go

>> Good.

>> I'll make it so!
>
>Thanks.

[-- Attachment #1.2: patch --]
[-- Type: text/plain, Size: 767 bytes --]

From: Raghavendra D Prabhu <rprabhu@wnohang.net>
Date: Fri, 1 Apr 2011 00:17:36 +0530
Subject: [PATCH] Added a py3k test to feature tests.

Currently, make fails at a later stage when python-devel test succeeds
incorrectly for Py3k. This fixes that.

Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net>
---
 tools/perf/feature-tests.mak |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/tools/perf/feature-tests.mak b/tools/perf/feature-tests.mak
index b041ca6..ba287c9 100644
--- a/tools/perf/feature-tests.mak
+++ b/tools/perf/feature-tests.mak
@@ -84,6 +84,8 @@ define SOURCE_PYTHON_EMBED
 
 int main(void)
 {
+	if (PY_VERSION_HEX >= 0x03000000)
+		return 1;
 	Py_Initialize();
 	return 0;
 }
-- 
1.7.4.2


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

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

[Michael Witten]

On Thu, Mar 31, 2011 at 14:15, Raghavendra D Prabhu <rprabhu@wnohang.net> wrote:
> 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.

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.

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

[Raghavendra D Prabhu]

[-- Attachment #1.1: Type: text/plain, Size: 1102 bytes --]

* On Thu, Mar 31, 2011 at 03:29:12PM -0500, Michael Witten <mfwitten@gmail.com> wrote:
>On Thu, Mar 31, 2011 at 14:15, Raghavendra D Prabhu <rprabhu@wnohang.net> wrote:
>> 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.
Thanks. I have attached the corrected patch and also have tested it.
--------------------------
Raghavendra Prabhu
GPG ID:D72BE977


[-- Attachment #1.2: 0001-Added-a-py3k-test-to-feature-tests.patch --]
[-- Type: text/plain, Size: 929 bytes --]

From 8c52bc1f2acb16d3bc12ea43c82665ebef93285a Mon Sep 17 00:00:00 2001
From: Raghavendra D Prabhu <rprabhu@wnohang.net>
Date: Sun, 3 Apr 2011 01:59:12 +0530
Subject: [PATCH] Added a py3k test to feature tests.

Currently, make fails at a later stage when python-devel test succeeds
incorrectly for Py3k. This fixes that.

Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net>
---
 tools/perf/feature-tests.mak |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/tools/perf/feature-tests.mak b/tools/perf/feature-tests.mak
index b041ca6..8e5d16a 100644
--- a/tools/perf/feature-tests.mak
+++ b/tools/perf/feature-tests.mak
@@ -82,6 +82,10 @@ ifndef NO_LIBPYTHON
 define SOURCE_PYTHON_EMBED
 #include <Python.h>
 
+#if PY_VERSION_HEX >= 0x03000000
+	#error "Python 3 is not yet supported; set PYTHON{,_CONFIG}"
+#endif
+
 int main(void)
 {
 	Py_Initialize();
-- 
1.7.4.2


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

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

[Michael Witten]

On Sun, 3 Apr 2011 02:16:31 +0530, Raghavendra D Prabhu wrote:

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

Actually, it still wouldn't work because the user-defined
make function:

  try-cc

handles its input somewhat naively (in particular, naive shell quoting
causes the embedded double quotes `"' to cause a compilation error);
I'll send in a small patch to fix that up later, but for now I've
incorporated the above check into the PYTHON{,_CONFIG} patch in a
more informational form that obviates the problems with try-cc.

I think this patch should pretty much be final; Raghavendra,
could you please check it over and give your OK so that
Arnaldo could possibly commit it?

Sincerely,
Michael Witten

8<-----------8<-----------8<-----------8<-----------8<-----------8<-----------
Subject: [PATCH] perf tools: Makefile: PYTHON{,_CONFIG} to bandage Python 3 incompatibility
Date: Sat, 2 Apr 2011 21:46:09 +0000
Currently, Python 3 is not supported by perf's code; this
can cause the build to fail for systems that have Python 3
installed as the default python:

  python{,-config}

The Correct Solution is to write compatibility code so that
Python 3 works out-of-the-box.

However, users often have an ancillary Python 2 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 Python 3 altogether; as an added benefit,
the Python binaries 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.

In addition, this commit introduces significantly robust makefile
infrastructure for working with paths and communicating with the
shell; it's currently only used for handling Python, but I hope
it will prove useful in refactoring the makefiles.

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             |  100 +++++++++++++++++-----
 tools/perf/config/utilities.mak |  180 +++++++++++++++++++++++++++++++++++++++
 tools/perf/feature-tests.mak    |    8 ++-
 3 files changed, 266 insertions(+), 22 deletions(-)
 create mode 100644 tools/perf/config/utilities.mak

diff --git a/tools/perf/feature-tests.mak b/tools/perf/feature-tests.mak
index b041ca6..1b33420 100644
--- a/tools/perf/feature-tests.mak
+++ b/tools/perf/feature-tests.mak
@@ -79,9 +79,15 @@ endef
 endif
 
 ifndef NO_LIBPYTHON
+define SOURCE_PYTHON_VERSION
+#include <Python.h>
+#if PY_VERSION_HEX >= 0x03000000
+	#error
+#endif
+int main(void){}
+endef
 define SOURCE_PYTHON_EMBED
 #include <Python.h>
-
 int main(void)
 {
 	Py_Initialize();
diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 158c30e..ee90b22 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -5,6 +5,8 @@ endif
 # The default target of this Makefile is...
 all:
 
+include config/utilities.mak
+
 ifneq ($(OUTPUT),)
 # check that the output directory actually exists
 OUTDIR := $(shell cd $(OUTPUT) && /bin/pwd)
@@ -13,6 +15,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 +173,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_WORD) util/setup.py --quiet build_ext \
+	  --build-lib='$(OUTPUT)python' \
+	  --build-temp='$(OUTPUT)python/temp'
 #
 # No Perl scripts right now:
 #
@@ -471,24 +480,74 @@ 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 $(disable-python_code))
+define disable-python_code
+  BASIC_CFLAGS += -DNO_LIBPYTHON
+  $(if $(1),$(warning No $(1) was found))
+  $(warning 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_WORD := $(call shell-wordify,$(PYTHON))
+
+  python-clean := $(PYTHON_WORD) 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_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))
+      PYTHON_EMBED_CCOPTS := $(shell $(PYTHON_CONFIG_SQ) --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
+
+        ifneq ($(call try-cc,$(SOURCE_PYTHON_VERSION),$(FLAGS_PYTHON_EMBED)),y)
+          $(warning Python 3 is not yet supported; please set)
+          $(warning PYTHON and/or PYTHON_CONFIG appropriately.)
+          $(warning If you also have Python 2 installed, then)
+          $(warning try something like:)
+          $(warning $(and ,))
+          $(warning $(and ,)  make PYTHON=python2)
+          $(warning $(and ,))
+          $(warning Otherwise, disable Python support entirely:)
+          $(warning $(and ,))
+          $(warning $(and ,)  make NO_LIBPYTHON=1)
+          $(warning $(and ,))
+          $(error   $(and ,))
+        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
+endif
 
 ifdef NO_DEMANGLE
@@ -829,8 +888,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/config/utilities.mak b/tools/perf/config/utilities.mak
new file mode 100644
index 0000000..b229fe9
--- /dev/null
+++ b/tools/perf/config/utilities.mak
@@ -0,0 +1,180 @@
+# This allows us to work with the newline character:
+define newline
+
+
+endef
+newline := $(newline)
+
+# nl-escape
+#
+# Usage: escape = $(call nl-escape[,escape])
+#
+# This is used as the common way to specify
+# what should replace a newline when escaping
+# newlines; the default is a bizarre string.
+#
+nl-escape = $(or $(1),m822df3020w6a44id34bt574ctac44eb9f4n)
+
+# escape-nl
+#
+# Usage: escaped-text = $(call escape-nl,text[,escape])
+#
+# GNU make's $(shell ...) function converts to a
+# single space each newline character in the output
+# produced during the expansion; this may not be
+# desirable.
+#
+# The only solution is to change each newline into
+# something that won't be converted, so that the
+# information can be recovered later with
+# $(call unescape-nl...)
+#
+escape-nl = $(subst $(newline),$(call nl-escape,$(2)),$(1))
+
+# unescape-nl
+#
+# Usage: text = $(call unescape-nl,escaped-text[,escape])
+#
+# See escape-nl.
+#
+unescape-nl = $(subst $(call nl-escape,$(2)),$(newline),$(1))
+
+# shell-escape-nl
+#
+# Usage: $(shell some-command | $(call shell-escape-nl[,escape]))
+#
+# Use this to escape newlines from within a shell call;
+# the default escape is a bizarre string.
+#
+# NOTE: The escape is used directly as a string constant
+#       in an `awk' program that is delimited by shell
+#       single-quotes, so be wary of the characters
+#       that are chosen.
+#
+define shell-escape-nl
+awk 'NR==1 {t=$$0} NR>1 {t=t "$(nl-escape)" $$0} END {printf t}'
+endef
+
+# shell-unescape-nl
+#
+# Usage: $(shell some-command | $(call shell-unescape-nl[,escape]))
+#
+# Use this to unescape newlines from within a shell call;
+# the default escape is a bizarre string.
+#
+# NOTE: The escape is used directly as an extended regular
+#       expression constant in an `awk' program that is
+#       delimited by shell single-quotes, so be wary
+#       of the characters that are chosen.
+#
+# (The bash shell has a bug where `{gsub(...),...}' is
+#  misinterpreted as a brace expansion; this can be
+#  overcome by putting a space between `{' and `gsub').
+#
+define shell-unescape-nl
+awk 'NR==1 {t=$$0} NR>1 {t=t "\n" $$0} END { gsub(/$(nl-escape)/,"\n",t); printf t }'
+endef
+
+# escape-for-shell-sq
+#
+# Usage: embeddable-text = $(call escape-for-shell-sq,text)
+#
+# This function produces text that is suitable for
+# embedding in a shell string that is delimited by
+# single-quotes.
+#
+escape-for-shell-sq =  $(subst ','\'',$(1))
+
+# shell-sq
+#
+# Usage: single-quoted-and-escaped-text = $(call shell-sq,text)
+#
+shell-sq = '$(escape-for-shell-sq)'
+
+# shell-wordify
+#
+# Usage: wordified-text = $(call shell-wordify,text)
+#
+# For instance:
+#
+#  |define text
+#  |hello
+#  |world
+#  |endef
+#  |
+#  |target:
+#  |	echo $(call shell-wordify,$(text))
+#
+# At least GNU make gets confused by expanding a newline
+# within the context of a command line of a makefile rule
+# (this is in constrast to a `$(shell ...)' function call,
+# which can handle it just fine).
+#
+# This function avoids the problem by producing a string
+# that works as a shell word, regardless of whether or
+# not it contains a newline.
+#
+# If the text to be wordified contains a newline, then
+# an intrictate shell command substitution is constructed
+# to render the text as a single line; when the shell
+# processes the resulting escaped text, it transforms
+# it into the original unescaped text.
+#
+# If the text does not contain a newline, then this function
+# produces the same results as the `$(shell-sq)' function.
+#
+shell-wordify = $(if $(findstring $(newline),$(1)),$(_sw-esc-nl),$(shell-sq))
+define _sw-esc-nl
+"$$(echo $(call escape-nl,$(shell-sq),$(2)) | $(call shell-unescape-nl,$(2)))"
+endef
+
+# is-absolute
+#
+# Usage: bool-value = $(call is-absolute,path)
+#
+is-absolute = $(shell echo $(shell-sq) | grep ^/ -q && echo y)
+
+# lookup
+#
+# Usage: absolute-executable-path-or-empty = $(call lookup,path)
+#
+# (It's necessary to use `sh -c' because GNU make messes up by
+#  trying too hard and getting things wrong).
+#
+lookup = $(call unescape-nl,$(shell sh -c $(_l-sh)))
+_l-sh = $(call shell-sq,command -v $(shell-sq) | $(call shell-escape-nl,))
+
+# is-executable
+#
+# Usage: bool-value = $(call is-executable,path)
+#
+# (It's necessary to use `sh -c' because GNU make messes up by
+#  trying too hard and getting things wrong).
+#
+is-executable = $(call _is-executable-helper,$(shell-sq))
+_is-executable-helper = $(shell sh -c $(_is-executable-sh))
+_is-executable-sh = $(call shell-sq,test -f $(1) -a -x $(1) && echo y)
+
+# 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 `/'.
+#
+get-executable = $(if $(1),$(if $(is-absolute),$(_ge-abspath),$(lookup)))
+_ge-abspath = $(if $(is-executable),$(1))
+
+# 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
+_ge_attempt = $(or $(get-executable),$(_gea_warn),$(call _gea_err,$(2)))
+_gea_warn = $(warning The path '$(1)' is not executable.)
+_gea_err  = $(if $(1),$(error Please set '$(1)' appropriately))
-- 
1.7.4.18.g68fe8

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

[Michael Witten]

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

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

[Raghavendra D Prabhu]

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

* On Thu, Mar 31, 2011 at 03:37:40PM +0000, Michael Witten <mfwitten@gmail.com> wrote:
>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@wnohang.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@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          |   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))

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

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

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

[Michael Witten]

On Sat, 9 Apr 2011 02:47:09 +0530, Raghavendra D Prabhu wrote:

> 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

[PATCH 2/2] perf tools: Makefile: PYTHON{,_CONFIG} to bandage Python 3 incompatibility

[Michael Witten]

Currently, Python 3 is not supported by perf's code; this
can cause the build to fail for systems that have Python 3
installed as the default python:

  python{,-config}

The Correct Solution is to write compatibility code so that
Python 3 works out-of-the-box.

However, users often have an ancillary Python 2 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 Python 3 altogether; as an added benefit,
the Python binaries 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.

In addition, this commit introduces significantly robust makefile
infrastructure for working with paths and communicating with the
shell; it's currently only used for handling Python, but I hope
it will prove useful in refactoring the makefiles.

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             |   97 +++++++++++++++++-----
 tools/perf/config/utilities.mak |  180 +++++++++++++++++++++++++++++++++++++++
 tools/perf/feature-tests.mak    |    8 ++-
 3 files changed, 264 insertions(+), 21 deletions(-)
 create mode 100644 tools/perf/config/utilities.mak

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index aaf4dd3..1fd46bf 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -5,6 +5,8 @@ endif
 # The default target of this Makefile is...
 all:
 
+include config/utilities.mak
+
 ifneq ($(OUTPUT),)
 # check that the output directory actually exists
 OUTDIR := $(shell cd $(OUTPUT) && /bin/pwd)
@@ -13,6 +15,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 +173,7 @@ grep-libs = $(filter -l%,$(1))
 strip-libs = $(filter-out -l%,$(1))
 
 $(OUTPUT)python/perf.so: $(PYRF_OBJS)
-	$(QUIET_GEN)CFLAGS='$(BASIC_CFLAGS)' python util/setup.py \
+	$(QUIET_GEN)CFLAGS='$(BASIC_CFLAGS)' $(PYTHON_WORD) util/setup.py \
 	  --quiet build_ext \
 	  --build-lib='$(OUTPUT)python' \
 	  --build-temp='$(OUTPUT)python/temp'
@@ -473,24 +481,74 @@ else
 	endif
 endif
 
-ifdef NO_LIBPYTHON
-	BASIC_CFLAGS += -DNO_LIBPYTHON
+disable-python = $(eval $(disable-python_code))
+define disable-python_code
+  BASIC_CFLAGS += -DNO_LIBPYTHON
+  $(if $(1),$(warning No $(1) was found))
+  $(warning 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_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
+
+  PYTHON_WORD := $(call shell-wordify,$(PYTHON))
+
+  python-clean := $(PYTHON_WORD) 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_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))
+      PYTHON_EMBED_CCOPTS := $(shell $(PYTHON_CONFIG_SQ) --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
+
+        ifneq ($(call try-cc,$(SOURCE_PYTHON_VERSION),$(FLAGS_PYTHON_EMBED)),y)
+          $(warning Python 3 is not yet supported; please set)
+          $(warning PYTHON and/or PYTHON_CONFIG appropriately.)
+          $(warning If you also have Python 2 installed, then)
+          $(warning try something like:)
+          $(warning $(and ,))
+          $(warning $(and ,)  make PYTHON=python2)
+          $(warning $(and ,))
+          $(warning Otherwise, disable Python support entirely:)
+          $(warning $(and ,))
+          $(warning $(and ,)  make NO_LIBPYTHON=1)
+          $(warning $(and ,))
+          $(error   $(and ,))
+        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
 endif
 
 ifdef NO_DEMANGLE
@@ -831,8 +889,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/config/utilities.mak b/tools/perf/config/utilities.mak
new file mode 100644
index 0000000..6d8ff88
--- /dev/null
+++ b/tools/perf/config/utilities.mak
@@ -0,0 +1,180 @@
+# This allows us to work with the newline character:
+define newline
+
+
+endef
+newline := $(newline)
+
+# nl-escape
+#
+# Usage: escape = $(call nl-escape[,escape])
+#
+# This is used as the common way to specify
+# what should replace a newline when escaping
+# newlines; the default is a bizarre string.
+#
+nl-escape = $(or $(1),m822df3020w6a44id34bt574ctac44eb9f4n)
+
+# escape-nl
+#
+# Usage: escaped-text = $(call escape-nl,text[,escape])
+#
+# GNU make's $(shell ...) function converts to a
+# single space each newline character in the output
+# produced during the expansion; this may not be
+# desirable.
+#
+# The only solution is to change each newline into
+# something that won't be converted, so that the
+# information can be recovered later with
+# $(call unescape-nl...)
+#
+escape-nl = $(subst $(newline),$(call nl-escape,$(2)),$(1))
+
+# unescape-nl
+#
+# Usage: text = $(call unescape-nl,escaped-text[,escape])
+#
+# See escape-nl.
+#
+unescape-nl = $(subst $(call nl-escape,$(2)),$(newline),$(1))
+
+# shell-escape-nl
+#
+# Usage: $(shell some-command | $(call shell-escape-nl[,escape]))
+#
+# Use this to escape newlines from within a shell call;
+# the default escape is a bizarre string.
+#
+# NOTE: The escape is used directly as a string constant
+#       in an `awk' program that is delimited by shell
+#       single-quotes, so be wary of the characters
+#       that are chosen.
+#
+define shell-escape-nl
+awk 'NR==1 {t=$$0} NR>1 {t=t "$(nl-escape)" $$0} END {printf t}'
+endef
+
+# shell-unescape-nl
+#
+# Usage: $(shell some-command | $(call shell-unescape-nl[,escape]))
+#
+# Use this to unescape newlines from within a shell call;
+# the default escape is a bizarre string.
+#
+# NOTE: The escape is used directly as an extended regular
+#       expression constant in an `awk' program that is
+#       delimited by shell single-quotes, so be wary
+#       of the characters that are chosen.
+#
+# (The bash shell has a bug where `{gsub(...),...}' is
+#  misinterpreted as a brace expansion; this can be
+#  overcome by putting a space between `{' and `gsub').
+#
+define shell-unescape-nl
+awk 'NR==1 {t=$$0} NR>1 {t=t "\n" $$0} END { gsub(/$(nl-escape)/,"\n",t); printf t }'
+endef
+
+# escape-for-shell-sq
+#
+# Usage: embeddable-text = $(call escape-for-shell-sq,text)
+#
+# This function produces text that is suitable for
+# embedding in a shell string that is delimited by
+# single-quotes.
+#
+escape-for-shell-sq =  $(subst ','\'',$(1))
+
+# shell-sq
+#
+# Usage: single-quoted-and-escaped-text = $(call shell-sq,text)
+#
+shell-sq = '$(escape-for-shell-sq)'
+
+# shell-wordify
+#
+# Usage: wordified-text = $(call shell-wordify,text)
+#
+# For instance:
+#
+#  |define text
+#  |hello
+#  |world
+#  |endef
+#  |
+#  |target:
+#  |	echo $(call shell-wordify,$(text))
+#
+# At least GNU make gets confused by expanding a newline
+# within the context of a command line of a makefile rule
+# (this is in constrast to a `$(shell ...)' function call,
+# which can handle it just fine).
+#
+# This function avoids the problem by producing a string
+# that works as a shell word, regardless of whether or
+# not it contains a newline.
+#
+# If the text to be wordified contains a newline, then
+# an intrictate shell command substitution is constructed
+# to render the text as a single line; when the shell
+# processes the resulting escaped text, it transforms
+# it into the original unescaped text.
+#
+# If the text does not contain a newline, then this function
+# produces the same results as the `$(shell-sq)' function.
+#
+shell-wordify = $(if $(findstring $(newline),$(1)),$(_sw-esc-nl),$(shell-sq))
+define _sw-esc-nl
+"$$(echo $(call escape-nl,$(shell-sq),$(2)) | $(call shell-unescape-nl,$(2)))"
+endef
+
+# is-absolute
+#
+# Usage: bool-value = $(call is-absolute,path)
+#
+is-absolute = $(shell echo $(shell-sq) | grep ^/ -q && echo y)
+
+# lookup
+#
+# Usage: absolute-executable-path-or-empty = $(call lookup,path)
+#
+# (It's necessary to use `sh -c' because GNU make messes up by
+#  trying too hard and getting things wrong).
+#
+lookup = $(call unescape-nl,$(shell sh -c $(_l-sh)))
+_l-sh = $(call shell-sq,command -v $(shell-sq) | $(call shell-escape-nl,))
+
+# is-executable
+#
+# Usage: bool-value = $(call is-executable,path)
+#
+# (It's necessary to use `sh -c' because GNU make messes up by
+#  trying too hard and getting things wrong).
+#
+is-executable = $(call _is-executable-helper,$(shell-sq))
+_is-executable-helper = $(shell sh -c $(_is-executable-sh))
+_is-executable-sh = $(call shell-sq,test -f $(1) -a -x $(1) && echo y)
+
+# 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 `/'.
+#
+get-executable = $(if $(1),$(if $(is-absolute),$(_ge-abspath),$(lookup)))
+_ge-abspath = $(if $(is-executable),$(1))
+
+# 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
+_ge_attempt = $(or $(get-executable),$(_gea_warn),$(call _gea_err,$(2)))
+_gea_warn = $(warning The path '$(1)' is not executable.)
+_gea_err  = $(if $(1),$(error Please set '$(1)' appropriately))
diff --git a/tools/perf/feature-tests.mak b/tools/perf/feature-tests.mak
index b041ca6..1b33420 100644
--- a/tools/perf/feature-tests.mak
+++ b/tools/perf/feature-tests.mak
@@ -79,9 +79,15 @@ endef
 endif
 
 ifndef NO_LIBPYTHON
+define SOURCE_PYTHON_VERSION
+#include <Python.h>
+#if PY_VERSION_HEX >= 0x03000000
+	#error
+#endif
+int main(void){}
+endef
 define SOURCE_PYTHON_EMBED
 #include <Python.h>
-
 int main(void)
 {
 	Py_Initialize();
-- 
1.7.4.18.g68fe8

[PATCH 1/2] perf tools: Makefile: Clean up `python/perf.so' rule

[Michael Witten]

There is no need for a subshell or an explicit `export';
as per the POSIX Shell Command Language specification:

  http://pubs.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html#tag_02_09_01
  http://pubs.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html#tag_02_10_02

It is only necessary to include the environment variable
assignment just before the command to be run.

Also, it is better to use single-quotes, because GNU make
might expand `$(BASIC_CFLAGS)' into something that the shell
could interpret within double-quotes.

Signed-off-by: Michael Witten <mfwitten@gmail.com>
---
 tools/perf/Makefile |   10 ++++------
 1 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 207dee5..aaf4dd3 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -165,12 +165,10 @@ grep-libs = $(filter -l%,$(1))
 strip-libs = $(filter-out -l%,$(1))
 
 $(OUTPUT)python/perf.so: $(PYRF_OBJS)
-	$(QUIET_GEN)( \
-		export CFLAGS="$(BASIC_CFLAGS)"; \
-		python util/setup.py --quiet  build_ext --build-lib='$(OUTPUT)python' \
-			--build-temp='$(OUTPUT)python/temp' \
-	)
-
+	$(QUIET_GEN)CFLAGS='$(BASIC_CFLAGS)' python util/setup.py \
+	  --quiet build_ext \
+	  --build-lib='$(OUTPUT)python' \
+	  --build-temp='$(OUTPUT)python/temp'
 #
 # No Perl scripts right now:
 #
-- 
1.7.4.18.g68fe8

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

[Raghavendra D Prabhu]

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

* On Sat, Apr 09, 2011 at 01:24:57AM +0000, Michael Witten <mfwitten@gmail.com> wrote:
>On Sat, 9 Apr 2011 02:47:09 +0530, Raghavendra D Prabhu wrote:
>
>> 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
Tested it on the perf/urgent branch. Works great. 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.

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

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

[Michael Witten]

On Sun, 10 Apr 2011 00:12:45 +0530, Raghavendra D Prabhu wrote:

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

Well...

If PYTHON or PYTHON_CONFIG don't point to an executable, then
there is a complaint about no executable being found.

If PYTHON (or, more specifically, PYTHON_CONFIG) points to the
Python *3* binary, then the message will be about Python 3 being
incompatible.

If PYTHON or PYTHON_CONFIG point to an executable that does not
provide a suitable `Python.h', then it says `No Python.h found'.

Thus, I think all of the cases are covered pretty well.
In particular, let's say that somebody triggers the third case,
but is confused because he does indeed have Python 3 with a valid
Python.h installed. That `No Python.h found' message will cause
him to look at how he configured PYTHON{,_CONFIG}, and he'll
see that he used the wrong executable (which is probably a
fairly unlikely scenario, anyway). Thus, he will configure the
right variable to point to the Python 3 executable, only to
get a message that Python 3 is incompatible. Thus, he will then
know that Python 2 is required.

I suppose, though, we could simplify the experience for the user
by giving a hint earlier on that Python 2 is required.

The patch that follows fulfills your request. My advice is to
save this email to /tmp/email and then apply it over the last
couple of patches like this:

  $ git apply /tmp/email
  $ git commit -a --amend -C HEAD

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 1fd46bf..b5276c7 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -522,7 +522,7 @@ else
       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)
+        $(call disable-python,Python.h (for Python 2.x))
       else
 
         ifneq ($(call try-cc,$(SOURCE_PYTHON_VERSION),$(FLAGS_PYTHON_EMBED)),y)

[PULL] Improve Python 3 handling

[Michael Witten]

Let's get this finished up.

Arnaldo, pull from here:

  git://github.com/mfwitten/linux-2.6.git perf/makefile/config

to get the Python stuff plus a little clean up (for the sake of
consistency and in preparation for improving the process of
configuring the build).

  [0] perf tools: Makefile: Clean up `python/perf.so' rule
  [1] perf tools: Makefile: PYTHON{,_CONFIG} to bandage Python 3 incompatibility
  [2] perf tools: Move `try-cc'
  [3] perf tools: git mv tools/perf/{features-tests.mak,config/}

Stats:

  tools/perf/Makefile                       |  107 ++++++++++++----
  tools/perf/{ => config}/feature-tests.mak |   16 +--
  tools/perf/config/utilities.mak           |  188 +++++++++++++++++++++++++++++
  3 files changed, 276 insertions(+), 35 deletions(-)
  rename tools/perf/{ => config}/feature-tests.mak (86%)
  create mode 100644 tools/perf/config/utilities.mak

Sincerely,
Michael Witten

[tip:perf/core] perf tools: Makefile: PYTHON{,_CONFIG} to bandage Python 3 incompatibility

[tip-bot for Michael Witten]

Commit-ID:  ced465c400b23656ef2c4fbfb4add0e5b92e3d97
Gitweb:     http://git.kernel.org/tip/ced465c400b23656ef2c4fbfb4add0e5b92e3d97
Author:     Michael Witten <mfwitten@gmail.com>
AuthorDate: Sat, 2 Apr 2011 21:46:09 +0000
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 19 Apr 2011 08:18:36 -0300

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

Currently, Python 3 is not supported by perf's code; this
can cause the build to fail for systems that have Python 3
installed as the default python:

  python{,-config}

The Correct Solution is to write compatibility code so that
Python 3 works out-of-the-box.

However, users often have an ancillary Python 2 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 Python 3 altogether; as an added benefit,
the Python binaries 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.

In addition, this commit introduces significantly robust makefile
infrastructure for working with paths and communicating with the
shell; it's currently only used for handling Python, but I hope
it will prove useful in refactoring the makefiles.

Thanks to:

  Raghavendra D Prabhu <rprabhu@wnohang.net>

for motivating this patch.

Acked-by: Raghavendra D Prabhu <rprabhu@wnohang.net>
Link: http://lkml.kernel.org/r/e987828e-87ec-4973-95e7-47f10f5d9bab-mfwitten@gmail.com
Signed-off-by: Michael Witten <mfwitten@gmail.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Makefile             |   97 +++++++++++++++++-----
 tools/perf/config/utilities.mak |  180 +++++++++++++++++++++++++++++++++++++++
 tools/perf/feature-tests.mak    |    8 ++-
 3 files changed, 264 insertions(+), 21 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index aaf4dd3..b5276c7 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -5,6 +5,8 @@ endif
 # The default target of this Makefile is...
 all:
 
+include config/utilities.mak
+
 ifneq ($(OUTPUT),)
 # check that the output directory actually exists
 OUTDIR := $(shell cd $(OUTPUT) && /bin/pwd)
@@ -13,6 +15,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 +173,7 @@ grep-libs = $(filter -l%,$(1))
 strip-libs = $(filter-out -l%,$(1))
 
 $(OUTPUT)python/perf.so: $(PYRF_OBJS)
-	$(QUIET_GEN)CFLAGS='$(BASIC_CFLAGS)' python util/setup.py \
+	$(QUIET_GEN)CFLAGS='$(BASIC_CFLAGS)' $(PYTHON_WORD) util/setup.py \
 	  --quiet build_ext \
 	  --build-lib='$(OUTPUT)python' \
 	  --build-temp='$(OUTPUT)python/temp'
@@ -473,24 +481,74 @@ else
 	endif
 endif
 
-ifdef NO_LIBPYTHON
-	BASIC_CFLAGS += -DNO_LIBPYTHON
+disable-python = $(eval $(disable-python_code))
+define disable-python_code
+  BASIC_CFLAGS += -DNO_LIBPYTHON
+  $(if $(1),$(warning No $(1) was found))
+  $(warning 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_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
+
+  PYTHON_WORD := $(call shell-wordify,$(PYTHON))
+
+  python-clean := $(PYTHON_WORD) 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_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))
+      PYTHON_EMBED_CCOPTS := $(shell $(PYTHON_CONFIG_SQ) --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 (for Python 2.x))
+      else
+
+        ifneq ($(call try-cc,$(SOURCE_PYTHON_VERSION),$(FLAGS_PYTHON_EMBED)),y)
+          $(warning Python 3 is not yet supported; please set)
+          $(warning PYTHON and/or PYTHON_CONFIG appropriately.)
+          $(warning If you also have Python 2 installed, then)
+          $(warning try something like:)
+          $(warning $(and ,))
+          $(warning $(and ,)  make PYTHON=python2)
+          $(warning $(and ,))
+          $(warning Otherwise, disable Python support entirely:)
+          $(warning $(and ,))
+          $(warning $(and ,)  make NO_LIBPYTHON=1)
+          $(warning $(and ,))
+          $(error   $(and ,))
+        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
 endif
 
 ifdef NO_DEMANGLE
@@ -831,8 +889,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/config/utilities.mak b/tools/perf/config/utilities.mak
new file mode 100644
index 0000000..6d8ff88
--- /dev/null
+++ b/tools/perf/config/utilities.mak
@@ -0,0 +1,180 @@
+# This allows us to work with the newline character:
+define newline
+
+
+endef
+newline := $(newline)
+
+# nl-escape
+#
+# Usage: escape = $(call nl-escape[,escape])
+#
+# This is used as the common way to specify
+# what should replace a newline when escaping
+# newlines; the default is a bizarre string.
+#
+nl-escape = $(or $(1),m822df3020w6a44id34bt574ctac44eb9f4n)
+
+# escape-nl
+#
+# Usage: escaped-text = $(call escape-nl,text[,escape])
+#
+# GNU make's $(shell ...) function converts to a
+# single space each newline character in the output
+# produced during the expansion; this may not be
+# desirable.
+#
+# The only solution is to change each newline into
+# something that won't be converted, so that the
+# information can be recovered later with
+# $(call unescape-nl...)
+#
+escape-nl = $(subst $(newline),$(call nl-escape,$(2)),$(1))
+
+# unescape-nl
+#
+# Usage: text = $(call unescape-nl,escaped-text[,escape])
+#
+# See escape-nl.
+#
+unescape-nl = $(subst $(call nl-escape,$(2)),$(newline),$(1))
+
+# shell-escape-nl
+#
+# Usage: $(shell some-command | $(call shell-escape-nl[,escape]))
+#
+# Use this to escape newlines from within a shell call;
+# the default escape is a bizarre string.
+#
+# NOTE: The escape is used directly as a string constant
+#       in an `awk' program that is delimited by shell
+#       single-quotes, so be wary of the characters
+#       that are chosen.
+#
+define shell-escape-nl
+awk 'NR==1 {t=$$0} NR>1 {t=t "$(nl-escape)" $$0} END {printf t}'
+endef
+
+# shell-unescape-nl
+#
+# Usage: $(shell some-command | $(call shell-unescape-nl[,escape]))
+#
+# Use this to unescape newlines from within a shell call;
+# the default escape is a bizarre string.
+#
+# NOTE: The escape is used directly as an extended regular
+#       expression constant in an `awk' program that is
+#       delimited by shell single-quotes, so be wary
+#       of the characters that are chosen.
+#
+# (The bash shell has a bug where `{gsub(...),...}' is
+#  misinterpreted as a brace expansion; this can be
+#  overcome by putting a space between `{' and `gsub').
+#
+define shell-unescape-nl
+awk 'NR==1 {t=$$0} NR>1 {t=t "\n" $$0} END { gsub(/$(nl-escape)/,"\n",t); printf t }'
+endef
+
+# escape-for-shell-sq
+#
+# Usage: embeddable-text = $(call escape-for-shell-sq,text)
+#
+# This function produces text that is suitable for
+# embedding in a shell string that is delimited by
+# single-quotes.
+#
+escape-for-shell-sq =  $(subst ','\'',$(1))
+
+# shell-sq
+#
+# Usage: single-quoted-and-escaped-text = $(call shell-sq,text)
+#
+shell-sq = '$(escape-for-shell-sq)'
+
+# shell-wordify
+#
+# Usage: wordified-text = $(call shell-wordify,text)
+#
+# For instance:
+#
+#  |define text
+#  |hello
+#  |world
+#  |endef
+#  |
+#  |target:
+#  |	echo $(call shell-wordify,$(text))
+#
+# At least GNU make gets confused by expanding a newline
+# within the context of a command line of a makefile rule
+# (this is in constrast to a `$(shell ...)' function call,
+# which can handle it just fine).
+#
+# This function avoids the problem by producing a string
+# that works as a shell word, regardless of whether or
+# not it contains a newline.
+#
+# If the text to be wordified contains a newline, then
+# an intrictate shell command substitution is constructed
+# to render the text as a single line; when the shell
+# processes the resulting escaped text, it transforms
+# it into the original unescaped text.
+#
+# If the text does not contain a newline, then this function
+# produces the same results as the `$(shell-sq)' function.
+#
+shell-wordify = $(if $(findstring $(newline),$(1)),$(_sw-esc-nl),$(shell-sq))
+define _sw-esc-nl
+"$$(echo $(call escape-nl,$(shell-sq),$(2)) | $(call shell-unescape-nl,$(2)))"
+endef
+
+# is-absolute
+#
+# Usage: bool-value = $(call is-absolute,path)
+#
+is-absolute = $(shell echo $(shell-sq) | grep ^/ -q && echo y)
+
+# lookup
+#
+# Usage: absolute-executable-path-or-empty = $(call lookup,path)
+#
+# (It's necessary to use `sh -c' because GNU make messes up by
+#  trying too hard and getting things wrong).
+#
+lookup = $(call unescape-nl,$(shell sh -c $(_l-sh)))
+_l-sh = $(call shell-sq,command -v $(shell-sq) | $(call shell-escape-nl,))
+
+# is-executable
+#
+# Usage: bool-value = $(call is-executable,path)
+#
+# (It's necessary to use `sh -c' because GNU make messes up by
+#  trying too hard and getting things wrong).
+#
+is-executable = $(call _is-executable-helper,$(shell-sq))
+_is-executable-helper = $(shell sh -c $(_is-executable-sh))
+_is-executable-sh = $(call shell-sq,test -f $(1) -a -x $(1) && echo y)
+
+# 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 `/'.
+#
+get-executable = $(if $(1),$(if $(is-absolute),$(_ge-abspath),$(lookup)))
+_ge-abspath = $(if $(is-executable),$(1))
+
+# 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
+_ge_attempt = $(or $(get-executable),$(_gea_warn),$(call _gea_err,$(2)))
+_gea_warn = $(warning The path '$(1)' is not executable.)
+_gea_err  = $(if $(1),$(error Please set '$(1)' appropriately))
diff --git a/tools/perf/feature-tests.mak b/tools/perf/feature-tests.mak
index b041ca6..1b33420 100644
--- a/tools/perf/feature-tests.mak
+++ b/tools/perf/feature-tests.mak
@@ -79,9 +79,15 @@ endef
 endif
 
 ifndef NO_LIBPYTHON
+define SOURCE_PYTHON_VERSION
+#include <Python.h>
+#if PY_VERSION_HEX >= 0x03000000
+	#error
+#endif
+int main(void){}
+endef
 define SOURCE_PYTHON_EMBED
 #include <Python.h>
-
 int main(void)
 {
 	Py_Initialize();