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
--
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]
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
Suggested-by: Thomas Ilsche <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/config/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index e05d8f99424d..0f4b7fbc4852 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -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)
--
2.0.0
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.
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(-)
diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index 1f67aa02d240..e05d8f99424d 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -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)
diff --git a/tools/perf/config/feature-checks/Makefile b/tools/perf/config/feature-checks/Makefile
index 6088f8d8a434..72ab2984718e 100644
--- a/tools/perf/config/feature-checks/Makefile
+++ b/tools/perf/config/feature-checks/Makefile
@@ -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
--
2.0.0
Hi Namhyung,
On 2014-07-25 05:14, Namhyung Kim wrote:
> 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
>
> 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(-)
>
> diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
> index 1f67aa02d240..e05d8f99424d 100644
> --- a/tools/perf/config/Makefile
> +++ b/tools/perf/config/Makefile
> @@ -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)
> diff --git a/tools/perf/config/feature-checks/Makefile b/tools/perf/config/feature-checks/Makefile
> index 6088f8d8a434..72ab2984718e 100644
> --- a/tools/perf/config/feature-checks/Makefile
> +++ b/tools/perf/config/feature-checks/Makefile
> @@ -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
>
--
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]
Hi Kim,
On 2014-07-25 05:14, Namhyung Kim wrote:
> 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
Signed-off-by: Thomas Ilsche <[email protected]>
---
diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index f4f7f58..1b1bc9c 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -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)
Hi Thomas,
On Fri, 25 Jul 2014 11:28:55 +0200, Thomas Ilsche wrote:
> Hi Kim,
>
> On 2014-07-25 05:14, Namhyung Kim wrote:
>> 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
>
> Signed-off-by: Thomas Ilsche <[email protected]>
> ---
> diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
> index f4f7f58..1b1bc9c 100644
> --- a/tools/perf/config/Makefile
> +++ b/tools/perf/config/Makefile
> @@ -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?
> + override PYTHON := $(if $(PYTHON2),$(PYTHON2),$(call get-executable,python))
And then it'll set PYTHON to python2, no?
> 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
diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index 0f4b7fbc4852..60177278a357 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -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)
Hi Namhyung,
apologies for mixing up your first name earlier.
On 2014-07-25 12:24, Namhyung Kim wrote:
> Hi Thomas,
>
> On Fri, 25 Jul 2014 11:28:55 +0200, Thomas Ilsche wrote:
>> Hi Kim,
>>
>> On 2014-07-25 05:14, Namhyung Kim wrote:
>>> 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
>>
>> Signed-off-by: Thomas Ilsche <[email protected]>
>> ---
>> diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
>> index f4f7f58..1b1bc9c 100644
>> --- a/tools/perf/config/Makefile
>> +++ b/tools/perf/config/Makefile
>> @@ -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?
>
>
>> + 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.
>
>> 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
>
> Thanks,
> Namhyung
>
>
>
> diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
> index 0f4b7fbc4852..60177278a357 100644
> --- a/tools/perf/config/Makefile
> +++ b/tools/perf/config/Makefile
> @@ -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)
>
>
--
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]
Hi Thomas,
On Fri, 25 Jul 2014 13:43:24 +0200, Thomas Ilsche wrote:
> Hi Namhyung,
>
> apologies for mixing up your first name earlier.
It's okay, nevermind. :)
> On 2014-07-25 12:24, Namhyung Kim wrote:
>> On Fri, 25 Jul 2014 11:28:55 +0200, Thomas Ilsche wrote:
>>> 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?
>>
>>
>>> + 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.
Yeah, that's annoying.
>
>>
>>> 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
Hi Namhyung,
>>> 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
Tested-by: Thomas Ilsche <[email protected]>
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
>
> Thanks,
> Namhyung
Hi Thomas,
On Mon, 28 Jul 2014 13:11:05 +0200, Thomas Ilsche wrote:
> Hi Namhyung,
>
>>>> 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
> 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.
>
> 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.
Thank you a lot for testing and explaining various cases!
Namhyung