Skip to contentSkip to conversation

Perf: cannot overide PYTHON(_CONFIG)

PATCH Superseded Thomas Ilsche 3 participants 10 messages Jul 24, 2014 – Jul 29, 2014 perf events

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

Dear perf maintainers,

It is documented in Makefile.perf and the Python 3 error message to use make PYTHON=python2. However this does not work anymore since 9734163b6ee1425c6fa4b65d7e6ce34c9079420d moved the libpython feature check.

Further it would be more portable anyway to use "python2" as default instead of "python" as long as python3 is not supported. This follows the PEP 394 recommendation: https://www.python.org/dev/peps/pep-0394

I am not sure how to correctly fix the failure to respect PYTHON(_CONFIG). I would assume that early in perf/config/Makefile FEATURE_CHECK_[CFLAGS|LDFLAGS]-libpython(-version) needs to be set (before the feature checks are executed) and that the reconstruction of FLAGS_PYTHON_EMBED based on hardcoded python-config needs to be removed.

Best Regards,
Thomas

Signature · 11 lines
Dipl. Inf. Thomas Ilsche
Computer Scientist
Highly Adaptive Energy-Efficient Computing
CRC 912 HAEC: http://tu-dresden.de/sfb912
Technische Universität Dresden
Center for Information Services and High Performance Computing (ZIH)
01062 Dresden, Germany

Phone: +49 351 463-42168
Fax: +49 351 463-3773
E-Mail: [email protected]
maintainer
#1 lore raw

PATCH 1/2 perf tools: Fix make PYTHON override

Thomas reported that make PYTHON=python2 is not work on some systems. I can reproduce it on my ArchLinux box too.

This is because it's overridden by config/feature-checks/Makefile regardless of PYTHON setting. I guess it's a bug slipped into during the feature checking change.

Actually, we don't need to check python-config in the feature-checks. We can just pass appropriate FEATURE_CHECK_*FLAGS.

---
 tools/perf/config/Makefile                | 34 ++++++++++++++++++++-----------
 tools/perf/config/feature-checks/Makefile | 18 ++--------------
 2 files changed, 24 insertions(+), 28 deletions(-)
tools/perf/config/Makefile +22 -12
index 1f67aa02d240..e05d8f99424d 100644
@@ -120,6 +120,23 @@ ifdef PARSER_DEBUG
   CFLAGS             += -DPARSER_DEBUG
 endif
 
+ifndef NO_LIBPYTHON
+  override PYTHON := \
+    $(call get-executable-or-default,PYTHON,python)
+  override PYTHON_CONFIG := \
+    $(call get-executable-or-default,PYTHON_CONFIG,$(PYTHON)-config)
+
+  PYTHON_CONFIG_SQ := $(call shell-sq,$(PYTHON_CONFIG))
+
+  PYTHON_EMBED_LDOPTS := $(shell $(PYTHON_CONFIG_SQ) --ldflags 2>/dev/null)
+  PYTHON_EMBED_CCOPTS := $(shell $(PYTHON_CONFIG_SQ) --cflags 2>/dev/null)
+
+  FEATURE_CHECK_CFLAGS-libpython := $(PYTHON_EMBED_CCOPTS)
+  FEATURE_CHECK_LDFLAGS-libpython := $(PYTHON_EMBED_LDOPTS)
+  FEATURE_CHECK_CFLAGS-libpython-version := $(PYTHON_EMBED_CCOPTS)
+  FEATURE_CHECK_LDFLAGS-libpython-version := $(PYTHON_EMBED_LDOPTS)
+endif
+
 CFLAGS += -fno-omit-frame-pointer
 CFLAGS += -ggdb3
 CFLAGS += -funwind-tables
@@ -482,21 +499,14 @@ define disable-python_code
   NO_LIBPYTHON := 1
 endef
 
-override PYTHON := \
-  $(call get-executable-or-default,PYTHON,python)
-
-ifndef PYTHON
-  $(call disable-python,python interpreter)
+ifdef NO_LIBPYTHON
+  $(call disable-python)
 else
 
-  PYTHON_WORD := $(call shell-wordify,$(PYTHON))
-
-  ifdef NO_LIBPYTHON
-    $(call disable-python)
+  ifndef PYTHON
+    $(call disable-python,python interpreter)
   else
-
-    override PYTHON_CONFIG := \
-      $(call get-executable-or-default,PYTHON_CONFIG,$(PYTHON)-config)
+    PYTHON_WORD := $(call shell-wordify,$(PYTHON))
 
     ifndef PYTHON_CONFIG
       $(call disable-python,python-config tool)
tools/perf/config/feature-checks/Makefile +2 -16
index 6088f8d8a434..72ab2984718e 100644
@@ -101,25 +101,11 @@ FLAGS_PERL_EMBED=$(PERL_EMBED_CCOPTS) $(PERL_EMBED_LDOPTS)
 test-libperl.bin:
 	$(BUILD) $(FLAGS_PERL_EMBED)
 
-override PYTHON := python
-override PYTHON_CONFIG := python-config
-
-escape-for-shell-sq =  $(subst ','\'',$(1))
-shell-sq = '$(escape-for-shell-sq)'
-
-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)
-
 test-libpython.bin:
-	$(BUILD) $(FLAGS_PYTHON_EMBED)
+	$(BUILD)
 
 test-libpython-version.bin:
-	$(BUILD) $(FLAGS_PYTHON_EMBED)
+	$(BUILD)
 
 test-libbfd.bin:
 	$(BUILD) -DPACKAGE='"perf"' -lbfd -lz -liberty -ldl
Signature · 1 line
2.0.0
maintainer
#2 lore raw

PATCH 2/2 perf tools: Default to python version 2

According to PEP 394 recommendation [1], it's more portable to use python2 rather than plain python to refer python binary version 2.

Since there're distros using python3 by default like Arch, and we don't support python3 (yet), it'd be better using python2 explicitly.

[1] https://www.python.org/dev/peps/pep-0394

---
 tools/perf/config/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/perf/config/Makefile +1 -1
index e05d8f99424d..0f4b7fbc4852 100644
@@ -122,7 +122,7 @@ endif
 
 ifndef NO_LIBPYTHON
   override PYTHON := \
-    $(call get-executable-or-default,PYTHON,python)
+    $(call get-executable-or-default,PYTHON,python2)
   override PYTHON_CONFIG := \
     $(call get-executable-or-default,PYTHON_CONFIG,$(PYTHON)-config)
 
Signature · 1 line
2.0.0
#3 lore raw

Re: [PATCH 2/2] perf tools: Default to python version 2

Hi Kim,

Quoted 2 lines · Namhyung Kim

According to PEP 394 recommendation [1], it's more portable to use python2 rather than plain python to refer python binary version 2.

I tried to find out how backwards-compatible this is. python2(-config) was first available in Python 2.7.3 (April 2012), but it is still not available in 2.6.9 (Oct. 2013). So it might be better to use python2 as default but fall back to python if python2 is not available.

Best,
Thomas

---

tools/perf/config/Makefile +2 -2
index f4f7f58..1b1bc9c 100644
@@ -117,8 +117,8 @@ ifdef PARSER_DEBUG
 endif
 
 ifndef NO_LIBPYTHON
-  override PYTHON := \
-    $(call get-executable-or-default,PYTHON,python)
+  PYTHON2 := $(call get-executable-or-default,PYTHON,python2)
+  override PYTHON := $(if $(PYTHON2),$(PYTHON2),$(call get-executable,python))
   override PYTHON_CONFIG := \
     $(call get-executable-or-default,PYTHON_CONFIG,$(PYTHON)-config)
maintainer
#4 lore raw

Re: [PATCH 2/2] perf tools: Default to python version 2

Hi Thomas,

Quoted 21 lines · Thomas Ilsche

Hi Kim,

Quoted 2 lines · Namhyung Kim

According to PEP 394 recommendation [1], it's more portable to use python2 rather than plain python to refer python binary version 2.

I tried to find out how backwards-compatible this is. python2(-config) was first available in Python 2.7.3 (April 2012), but it is still not available in 2.6.9 (Oct. 2013). So it might be better to use python2 as default but fall back to python if python2 is not available.

Best,
Thomas

---

tools/perf/config/Makefile +0 -0
index f4f7f58..1b1bc9c 100644

tools/perf/config/Makefile ifdef PARSER_DEBUG · line 120

Quoted 6 lines
tools/perf/config/Makefile +1 -2
@@ -117,8 +117,8 @@ ifdef PARSER_DEBUG
 endif
 
 ifndef NO_LIBPYTHON
-  override PYTHON := \
-    $(call get-executable-or-default,PYTHON,python)
+  PYTHON2 := $(call get-executable-or-default,PYTHON,python2)

But wouldn't it set PYTHON2 to python2 even if the system doesn't have python2 actually?

tools/perf/config/Makefile ifdef PARSER_DEBUG · line 121

Quoted 1 line
tools/perf/config/Makefile +1 -0
+  override PYTHON := $(if $(PYTHON2),$(PYTHON2),$(call get-executable,python))

And then it'll set PYTHON to python2, no?

tools/perf/config/Makefile ifdef PARSER_DEBUG · line 123

Quoted 2 lines
tools/perf/config/Makefile +0 -0
   override PYTHON_CONFIG := \
     $(call get-executable-or-default,PYTHON_CONFIG,$(PYTHON)-config)

I'm thinking about something like below.. but sadly it doesn't work for me.. hmm.

Thanks,
Namhyung

tools/perf/config/Makefile +2 -2
index 0f4b7fbc4852..60177278a357 100644
@@ -121,8 +121,8 @@ ifdef PARSER_DEBUG
 endif
 
 ifndef NO_LIBPYTHON
-  override PYTHON := \
-    $(call get-executable-or-default,PYTHON,python2)
+  PYTHON2 := $(if $(call get-executable,python2),python2,python)
+  override PYTHON := $(call get-executable-or-default,PYTHON,$(PYTHON2))
   override PYTHON_CONFIG := \
     $(call get-executable-or-default,PYTHON_CONFIG,$(PYTHON)-config)
 
#5 lore raw

Re: [PATCH 2/2] perf tools: Default to python version 2

Hi Namhyung,

apologies for mixing up your first name earlier.

Quoted 35 lines · Namhyung Kim

Hi Thomas,

Quoted 21 lines · Thomas Ilsche

Hi Kim,

Quoted 2 lines · Namhyung Kim

According to PEP 394 recommendation [1], it's more portable to use python2 rather than plain python to refer python binary version 2.

I tried to find out how backwards-compatible this is. python2(-config) was first available in Python 2.7.3 (April 2012), but it is still not available in 2.6.9 (Oct. 2013). So it might be better to use python2 as default but fall back to python if python2 is not available.

Best,
Thomas

---

tools/perf/config/Makefile +0 -0
index f4f7f58..1b1bc9c 100644

tools/perf/config/Makefile ifdef PARSER_DEBUG · line 120

Quoted 6 lines
tools/perf/config/Makefile +1 -2
@@ -117,8 +117,8 @@ ifdef PARSER_DEBUG
 endif
 
 ifndef NO_LIBPYTHON
-  override PYTHON := \
-    $(call get-executable-or-default,PYTHON,python)
+  PYTHON2 := $(call get-executable-or-default,PYTHON,python2)

But wouldn't it set PYTHON2 to python2 even if the system doesn't have python2 actually?

Quoted 4 lines

tools/perf/config/Makefile ifdef PARSER_DEBUG · line 121

Quoted 1 line
tools/perf/config/Makefile +1 -0
+  override PYTHON := $(if $(PYTHON2),$(PYTHON2),$(call get-executable,python))

And then it'll set PYTHON to python2, no?

No, get-executable-or-default only returns anything if it is an actual executable (asserted by a 'command -v' and 'test -f $ -a -x $'). At least this is how I understand the Makefile code. I also tested it with a Python 2.6.9 (no python2 in path) and it worked (after fixing some totally unrelated issues in python headers). It will however complain that "The path 'python2' is not executable." before using python.

Quoted 7 lines

tools/perf/config/Makefile ifdef PARSER_DEBUG · line 127

Quoted 3 lines
tools/perf/config/Makefile +0 -0
   override PYTHON_CONFIG := \
     $(call get-executable-or-default,PYTHON_CONFIG,$(PYTHON)-config)

I'm thinking about something like below.. but sadly it doesn't work for me.. hmm.

Actually this appears to work for me (with 2.6.9 & 2.7.6) and I find this solution more elegant.

Best,
Thomas

Quoted 22 lines

Thanks,
Namhyung

tools/perf/config/Makefile +2 -2
index 0f4b7fbc4852..60177278a357 100644
@@ -121,8 +121,8 @@ ifdef PARSER_DEBUG
 endif
 
 ifndef NO_LIBPYTHON
-  override PYTHON := \
-    $(call get-executable-or-default,PYTHON,python2)
+  PYTHON2 := $(if $(call get-executable,python2),python2,python)
+  override PYTHON := $(call get-executable-or-default,PYTHON,$(PYTHON2))
   override PYTHON_CONFIG := \
     $(call get-executable-or-default,PYTHON_CONFIG,$(PYTHON)-config)
 
Signature · 11 lines
Dipl. Inf. Thomas Ilsche
Computer Scientist
Highly Adaptive Energy-Efficient Computing
CRC 912 HAEC: http://tu-dresden.de/sfb912
Technische Universität Dresden
Center for Information Services and High Performance Computing (ZIH)
01062 Dresden, Germany

Phone: +49 351 463-42168
Fax: +49 351 463-3773
E-Mail: [email protected]
maintainer
#6 lore raw

Re: [PATCH 2/2] perf tools: Default to python version 2

Hi Thomas,

Quoted 3 lines · Thomas Ilsche

Hi Namhyung,

apologies for mixing up your first name earlier.

It's okay, nevermind. :)

Quoted 13 lines · Namhyung Kim
Quoted 12 lines · Thomas Ilsche

Review

Quoted 4 lines

ifndef NO_LIBPYTHON

(quoted diff) +1 -2
-  override PYTHON := \
-    $(call get-executable-or-default,PYTHON,python)
+  PYTHON2 := $(call get-executable-or-default,PYTHON,python2)

But wouldn't it set PYTHON2 to python2 even if the system doesn't have python2 actually?

Quoted 1 line

+ override PYTHON := $(if $(PYTHON2),$(PYTHON2),$(call get-executable,python))

Quoted 8 lines
Quoted 2 lines

And then it'll set PYTHON to python2, no?

No, get-executable-or-default only returns anything if it is an actual executable (asserted by a 'command -v' and 'test -f $ -a -x $'). At least this is how I understand the Makefile code. I also tested it with a Python 2.6.9 (no python2 in path) and it worked (after fixing some totally unrelated issues in python headers). It will however complain that "The path 'python2' is not executable." before using python.

Yeah, that's annoying.

Quoted 11 lines
Quoted 8 lines
Quoted 4 lines

   override PYTHON_CONFIG := \
     $(call get-executable-or-default,PYTHON_CONFIG,$(PYTHON)-config)

I'm thinking about something like below.. but sadly it doesn't work for me.. hmm.

Actually this appears to work for me (with 2.6.9 & 2.7.6) and I find this solution more elegant.

Thanks for testing. It was my fault on setting symlink to a non-existing file for testing. It now works well for me too.

Can I add your Tested-by then?

Thanks,
Namhyung

author Tested-by
#7 lore raw

Re: [PATCH 2/2] perf tools: Default to python version 2

Hi Namhyung,

Quoted 11 lines
Quoted 6 lines
Quoted 2 lines

I'm thinking about something like below.. but sadly it doesn't work for me.. hmm.

Actually this appears to work for me (with 2.6.9 & 2.7.6) and I find this solution more
elegant.

Thanks for testing. It was my fault on setting symlink to a non-existing file for testing. It now works well for me too.

Can I add your Tested-by then?

Yes

Plase note that your patch requires the previous patch that just set the default to python2.

I tested in an Arch Linux VM with:
A) no system install of python, source installation 2.6.9 in PATH
B) no system install of python, source installation 2.7.6 in PATH
C) python3 (as python) and python2 system installation

In all cases it reports to build with libpython and perf script with a dummy Python script works.

If no python whatsoever is in PATH it reports:

config/Makefile:121: The path 'python' is not executable.
sh: line 0: command: -c: invalid option
command: usage: command [-pVv] command [arg ...] config/Makefile:122: The path '-config' is not executable.

And the created perf executable does not work with the dummy script.

If only pthon3 is in PATH I get the expected version error message (build stops.)

Note that for the manual old installations of python I had an issue with linking order and a -Werror issue that I resolved manually. But this is fixed with the latest system install of python.

Best,
Thomas

Quoted 3 lines

Thanks,
Namhyung

#8 lore raw

Re: [PATCH 2/2] perf tools: Default to python version 2

Hi Thomas,

Quoted 12 lines · Thomas Ilsche

Hi Namhyung,

Quoted 10 lines
Quoted 6 lines
Quoted 2 lines

I'm thinking about something like below.. but sadly it doesn't work for me.. hmm.

Actually this appears to work for me (with 2.6.9 & 2.7.6) and I find this solution more
elegant.

Thanks for testing. It was my fault on setting symlink to a non-existing file for testing. It now works well for me too.

Quoted 7 lines
Quoted 1 line

Can I add your Tested-by then?

Yes

Plase note that your patch requires the previous patch that just set the default to python2.

Ah, I'll fix it when sending v2.

Quoted 16 lines

I tested in an Arch Linux VM with:
A) no system install of python, source installation 2.6.9 in PATH
B) no system install of python, source installation 2.7.6 in PATH
C) python3 (as python) and python2 system installation

In all cases it reports to build with libpython and perf script with a dummy Python script works.

If no python whatsoever is in PATH it reports:

config/Makefile:121: The path 'python' is not executable.
sh: line 0: command: -c: invalid option
command: usage: command [-pVv] command [arg ...] config/Makefile:122: The path '-config' is not executable.

Quoted 7 lines

And the created perf executable does not work with the dummy script.

If only pthon3 is in PATH I get the expected version error message (build stops.)

Note that for the manual old installations of python I had an issue with linking order and a -Werror issue that I resolved manually. But this is fixed with the latest system install of python.

Thank you a lot for testing and explaining various cases!

Namhyung

#9 lore raw

Re: [PATCH 1/2] perf tools: Fix make PYTHON override

In reply to Namhyung Kim

Hi Namhyung,

Quoted 9 lines · Namhyung Kim

Thomas reported that make PYTHON=python2 is not work on some systems. I can reproduce it on my ArchLinux box too.

This is because it's overridden by config/feature-checks/Makefile regardless of PYTHON setting. I guess it's a bug slipped into during the feature checking change.

Actually, we don't need to check python-config in the feature-checks. We can just pass appropriate FEATURE_CHECK_*FLAGS.

The patch works fine for me.

Thanks,
Thomas

Quoted 96 lines
---
 tools/perf/config/Makefile                | 34 ++++++++++++++++++++-----------
 tools/perf/config/feature-checks/Makefile | 18 ++--------------
 2 files changed, 24 insertions(+), 28 deletions(-)
tools/perf/config/Makefile +22 -12
index 1f67aa02d240..e05d8f99424d 100644
@@ -120,6 +120,23 @@ ifdef PARSER_DEBUG
   CFLAGS             += -DPARSER_DEBUG
 endif
 
+ifndef NO_LIBPYTHON
+  override PYTHON := \
+    $(call get-executable-or-default,PYTHON,python)
+  override PYTHON_CONFIG := \
+    $(call get-executable-or-default,PYTHON_CONFIG,$(PYTHON)-config)
+
+  PYTHON_CONFIG_SQ := $(call shell-sq,$(PYTHON_CONFIG))
+
+  PYTHON_EMBED_LDOPTS := $(shell $(PYTHON_CONFIG_SQ) --ldflags 2>/dev/null)
+  PYTHON_EMBED_CCOPTS := $(shell $(PYTHON_CONFIG_SQ) --cflags 2>/dev/null)
+
+  FEATURE_CHECK_CFLAGS-libpython := $(PYTHON_EMBED_CCOPTS)
+  FEATURE_CHECK_LDFLAGS-libpython := $(PYTHON_EMBED_LDOPTS)
+  FEATURE_CHECK_CFLAGS-libpython-version := $(PYTHON_EMBED_CCOPTS)
+  FEATURE_CHECK_LDFLAGS-libpython-version := $(PYTHON_EMBED_LDOPTS)
+endif
+
 CFLAGS += -fno-omit-frame-pointer
 CFLAGS += -ggdb3
 CFLAGS += -funwind-tables
@@ -482,21 +499,14 @@ define disable-python_code
   NO_LIBPYTHON := 1
 endef
 
-override PYTHON := \
-  $(call get-executable-or-default,PYTHON,python)
-
-ifndef PYTHON
-  $(call disable-python,python interpreter)
+ifdef NO_LIBPYTHON
+  $(call disable-python)
 else
 
-  PYTHON_WORD := $(call shell-wordify,$(PYTHON))
-
-  ifdef NO_LIBPYTHON
-    $(call disable-python)
+  ifndef PYTHON
+    $(call disable-python,python interpreter)
   else
-
-    override PYTHON_CONFIG := \
-      $(call get-executable-or-default,PYTHON_CONFIG,$(PYTHON)-config)
+    PYTHON_WORD := $(call shell-wordify,$(PYTHON))
 
     ifndef PYTHON_CONFIG
       $(call disable-python,python-config tool)
tools/perf/config/feature-checks/Makefile +2 -16
index 6088f8d8a434..72ab2984718e 100644
@@ -101,25 +101,11 @@ FLAGS_PERL_EMBED=$(PERL_EMBED_CCOPTS) $(PERL_EMBED_LDOPTS)
 test-libperl.bin:
 	$(BUILD) $(FLAGS_PERL_EMBED)
 
-override PYTHON := python
-override PYTHON_CONFIG := python-config
-
-escape-for-shell-sq =  $(subst ','\'',$(1))
-shell-sq = '$(escape-for-shell-sq)'
-
-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)
-
 test-libpython.bin:
-	$(BUILD) $(FLAGS_PYTHON_EMBED)
+	$(BUILD)
 
 test-libpython-version.bin:
-	$(BUILD) $(FLAGS_PYTHON_EMBED)
+	$(BUILD)
 
 test-libbfd.bin:
 	$(BUILD) -DPACKAGE='"perf"' -lbfd -lz -liberty -ldl
Signature · 11 lines
Dipl. Inf. Thomas Ilsche
Computer Scientist
Highly Adaptive Energy-Efficient Computing
CRC 912 HAEC: http://tu-dresden.de/sfb912
Technische Universität Dresden
Center for Information Services and High Performance Computing (ZIH)
01062 Dresden, Germany

Phone: +49 351 463-42168
Fax: +49 351 463-3773
E-Mail: [email protected]