Perf: cannot overide PYTHON(_CONFIG)
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]
Namhyung Kim
maintainerPATCH 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
@@ -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
@@ -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
Namhyung Kim
maintainerPATCH 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
@@ -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
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
@@ -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)
Namhyung Kim
maintainerRe: [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,
ThomasSigned-off-by Thomas Ilsche <[email protected]>---
tools/perf/config/Makefile +0
-0
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
-0override 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
@@ -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)
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,
ThomasSigned-off-by Thomas Ilsche <[email protected]>---
tools/perf/config/Makefile +0
-0tools/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
-0override 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,
Namhyungtools/perf/config/Makefile +2
-2@@ -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]
Namhyung Kim
maintainerRe: [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
Makefilecode. 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
Thomas Ilsche
Tested-byRe: [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
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
Tested-by Thomas Ilsche <[email protected]>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 installationIn 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
Re: [PATCH 1/2] perf tools: Fix make PYTHON override
In reply to Namhyung KimHi 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
Reported-by Thomas Ilsche <[email protected]>Signed-off-by Namhyung Kim <[email protected]>--- 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@@ -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@@ -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]