2013-04-15 10:58:53

by Pekka Enberg

[permalink] [raw]
Subject: [PROBLEM] perf requires python-devel to compile

Hello,

I'm seeing this when I try to build perf in v3.9-rc7:

[penberg@golgotha perf]$ make
CHK -fstack-protector-all
CHK -Wstack-protector
CHK -Wvolatile-register-var
CHK -D_FORTIFY_SOURCE=2
CHK bionic
CHK libelf
CHK libdw
Makefile:584: No libdw.h found or old libdw.h found or elfutils is
older than 0.138, disables dwarf support. Please install new
elfutils-devel/libdw-dev
CHK libunwind
CHK -DLIBELF_MMAP
CHK libaudit
CHK libnewt
Makefile:673: newt not found, disables TUI support. Please install
newt-devel or libnewt-dev
CHK gtk2
CHK -DHAVE_GTK_INFO_BAR
CHK perl
Makefile:755: The path '/usr/bin/python-config' is not executable.
Makefile:755: *** Please set 'PYTHON_CONFIG' appropriately. Stop.

The problem is that I didn't have python-devel package installed and
get-executable-or-default decides to error out instead of letting the
Makefile disable Python support.

Pekka


2013-04-16 02:14:38

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PROBLEM] perf requires python-devel to compile

Hi Pekka,

On Mon, 15 Apr 2013 13:58:50 +0300, Pekka Enberg wrote:
> Hello,
>
> I'm seeing this when I try to build perf in v3.9-rc7:
>
> [penberg@golgotha perf]$ make
> CHK -fstack-protector-all
> CHK -Wstack-protector
> CHK -Wvolatile-register-var
> CHK -D_FORTIFY_SOURCE=2
> CHK bionic
> CHK libelf
> CHK libdw
> Makefile:584: No libdw.h found or old libdw.h found or elfutils is
> older than 0.138, disables dwarf support. Please install new
> elfutils-devel/libdw-dev
> CHK libunwind
> CHK -DLIBELF_MMAP
> CHK libaudit
> CHK libnewt
> Makefile:673: newt not found, disables TUI support. Please install
> newt-devel or libnewt-dev
> CHK gtk2
> CHK -DHAVE_GTK_INFO_BAR
> CHK perl
> Makefile:755: The path '/usr/bin/python-config' is not executable.
> Makefile:755: *** Please set 'PYTHON_CONFIG' appropriately. Stop.
>
> The problem is that I didn't have python-devel package installed and
> get-executable-or-default decides to error out instead of letting the
> Makefile disable Python support.

Right. I think the get-executable-or-default should not error out in
this case but just emit a warning and keep building.

Does following patch fix your problem?

-----------8<-------------8<-------------
diff --git a/tools/perf/config/utilities.mak b/tools/perf/config/utilities.mak
index 8ef3bd30a549..3b8036f8aca4 100644
--- a/tools/perf/config/utilities.mak
+++ b/tools/perf/config/utilities.mak
@@ -177,7 +177,7 @@ $(if $($(1)),$(call _ge_attempt,$($(1)),$(1)),$(call _ge_attempt,$(2),$(1)))
endef
_ge_attempt = $(if $(get-executable),$(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))
+_gea_err = $(if $(1),$(warning Please set '$(1)' appropriately))

# try-cc
# Usage: option = $(call try-cc, source-to-build, cc-options, msg)

2013-04-16 04:26:48

by David Ahern

[permalink] [raw]
Subject: Re: [PROBLEM] perf requires python-devel to compile

On 4/15/13 7:14 PM, Namhyung Kim wrote:
>> Makefile:755: The path '/usr/bin/python-config' is not executable.
>> Makefile:755: *** Please set 'PYTHON_CONFIG' appropriately. Stop.
>>
>> The problem is that I didn't have python-devel package installed and
>> get-executable-or-default decides to error out instead of letting the
>> Makefile disable Python support.
>
> Right. I think the get-executable-or-default should not error out in
> this case but just emit a warning and keep building.
>
> Does following patch fix your problem?
>
> -----------8<-------------8<-------------
> diff --git a/tools/perf/config/utilities.mak b/tools/perf/config/utilities.mak
> index 8ef3bd30a549..3b8036f8aca4 100644
> --- a/tools/perf/config/utilities.mak
> +++ b/tools/perf/config/utilities.mak
> @@ -177,7 +177,7 @@ $(if $($(1)),$(call _ge_attempt,$($(1)),$(1)),$(call _ge_attempt,$(2),$(1)))
> endef
> _ge_attempt = $(if $(get-executable),$(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))
> +_gea_err = $(if $(1),$(warning Please set '$(1)' appropriately))

In this case you don't want a warning, you just want python support
disabled and move on. I've been getting around this in a minimal install
of F18 in a VM using PYTHON_CONFIG=/bin/false; haven't had time to
search for the proper solution.

David

2013-04-16 17:11:43

by Michael Witten

[permalink] [raw]
Subject: Re: [PROBLEM] perf requires python-devel to compile

On Mon, 15 Apr 2013 21:26:40 -0700, David Ahern wrote:

> On 4/15/13 7:14 PM, Namhyung Kim wrote:
>>> Makefile:755: The path '/usr/bin/python-config' is not executable.
>>> Makefile:755: *** Please set 'PYTHON_CONFIG' appropriately. Stop.
>>>
>>> The problem is that I didn't have python-devel package installed and
>>> get-executable-or-default decides to error out instead of letting the
>>> Makefile disable Python support.
>>
>> Right. I think the get-executable-or-default should not error out in
>> this case but just emit a warning and keep building.
>>
>> Does following patch fix your problem?
>>
>> -----------8<-------------8<-------------
>> diff --git a/tools/perf/config/utilities.mak b/tools/perf/config/utilities.mak
>> index 8ef3bd30a549..3b8036f8aca4 100644
>> --- a/tools/perf/config/utilities.mak
>> +++ b/tools/perf/config/utilities.mak
>> @@ -177,7 +177,7 @@ $(if $($(1)),$(call _ge_attempt,$($(1)),$(1)),$(call _ge_attempt,$(2),$(1)))
>> endef
>> _ge_attempt = $(if $(get-executable),$(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))
>> +_gea_err = $(if $(1),$(warning Please set '$(1)' appropriately))
>
> In this case you don't want a warning, you just want python support
> disabled and move on. I've been getting around this in a minimal install
> of F18 in a VM using PYTHON_CONFIG=/bin/false; haven't had time to
> search for the proper solution.

You should probably disable python support more directly:

make NO_LIBPYTHON=1

That being said, this issue was introduced with the following commit:

31160d7feab786c991780d7f0ce2755a469e0e5e

namely due to:

... Also fix an issue where _get_attempt was called with only
one argument. This prevented the error message from printing
the name of the variable that can be used to fix the problem.

specifically:

-$(if $($(1)),$(call _ge_attempt,$($(1)),$(1)),$(call _ge_attempt,$(2)))
+$(if $($(1)),$(call _ge_attempt,$($(1)),$(1)),$(call _ge_attempt,$(2),$(1)))

The "missing" argument was in fact missing on purpose; it's a signal
that the error message should be skipped, because the failure was due
to the default value, not the user-supplied value.

That being said, I think there's room for improvement; for instance,
the error handling should perhaps not belong there. I will look into
it presently.

Sincerely,
Michael Witten

2013-04-16 17:16:43

by Pekka Enberg

[permalink] [raw]
Subject: Re: [PROBLEM] perf requires python-devel to compile

Hello,

On Tue, Apr 16, 2013 at 5:14 AM, Namhyung Kim <[email protected]> wrote:
>> The problem is that I didn't have python-devel package installed and
>> get-executable-or-default decides to error out instead of letting the
>> Makefile disable Python support.
>
> Right. I think the get-executable-or-default should not error out in
> this case but just emit a warning and keep building.
>
> Does following patch fix your problem?
>
> -----------8<-------------8<-------------
> diff --git a/tools/perf/config/utilities.mak b/tools/perf/config/utilities.mak
> index 8ef3bd30a549..3b8036f8aca4 100644
> --- a/tools/perf/config/utilities.mak
> +++ b/tools/perf/config/utilities.mak
> @@ -177,7 +177,7 @@ $(if $($(1)),$(call _ge_attempt,$($(1)),$(1)),$(call _ge_attempt,$(2),$(1)))
> endef
> _ge_attempt = $(if $(get-executable),$(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))
> +_gea_err = $(if $(1),$(warning Please set '$(1)' appropriately))
>
> # try-cc
> # Usage: option = $(call try-cc, source-to-build, cc-options, msg)

I considered this myself but I think this needs to fixed at a higher
level and keep the error out.

Pekka

2013-04-16 20:32:18

by David Ahern

[permalink] [raw]
Subject: Re: [PROBLEM] perf requires python-devel to compile

On 4/16/13 10:08 AM, Michael Witten wrote:
> You should probably disable python support more directly:
>
> make NO_LIBPYTHON=1

sure, but I should not have to do anything. The intent of the existing
auto-probing code is to figure out what is installed and build a binary
with those capabilities. In this case not having python installed causes
it to blow up.

>
> That being said, this issue was introduced with the following commit:
>
> 31160d7feab786c991780d7f0ce2755a469e0e5e
>
> namely due to:
>
> ... Also fix an issue where _get_attempt was called with only
> one argument. This prevented the error message from printing
> the name of the variable that can be used to fix the problem.
>
> specifically:
>
> -$(if $($(1)),$(call _ge_attempt,$($(1)),$(1)),$(call _ge_attempt,$(2)))
> +$(if $($(1)),$(call _ge_attempt,$($(1)),$(1)),$(call _ge_attempt,$(2),$(1)))
>
> The "missing" argument was in fact missing on purpose; it's a signal
> that the error message should be skipped, because the failure was due
> to the default value, not the user-supplied value.
>
> That being said, I think there's room for improvement; for instance,
> the error handling should perhaps not belong there. I will look into
> it presently.

Thanks for the pointer. I'll take a look when I get some time.

David

2013-04-16 20:54:18

by Michael Witten

[permalink] [raw]
Subject: Re: [PROBLEM] perf requires python-devel to compile

On Tue, 16 Apr 2013 13:32:08 -0700, David Ahern wrote:

> On 4/16/13 10:08 AM, Michael Witten wrote:
>> You should probably disable python support more directly:
>>
>> make NO_LIBPYTHON=1
>
> sure, but I should not have to do anything. The intent of the existing
> auto-probing code is to figure out what is installed and build a binary
> with those capabilities. In this case not having python installed causes
> it to blow up.

That's certainly how it behaved up until the regression.

To make matters worse, the NO_LIBPYTHON variable is checked only *after*
probing for an executable `python'; in the case that no python is installed
at all, the workaround is to double up on your current trick:

make PYTHON=false PYTHON_CONFIG=false

2013-04-17 03:53:05

by Michael Witten

[permalink] [raw]
Subject: [PATCH] perf tools: Revert regression in configuration of Python support

On Tue, 16 Apr 2013 20:41:59 -0000, Michael Witten wrote:

> On Tue, 16 Apr 2013 13:32:08 -0700, David Ahern wrote:
>
>> On 4/16/13 10:08 AM, Michael Witten wrote:
>>> You should probably disable python support more directly:
>>>
>>> make NO_LIBPYTHON=1
>>
>> sure, but I should not have to do anything. The intent of the existing
>> auto-probing code is to figure out what is installed and build a binary
>> with those capabilities. In this case not having python installed causes
>> it to blow up.
>
> That's certainly how it behaved up until the regression.
>
> To make matters worse, the NO_LIBPYTHON variable is checked only *after*
> probing for an executable `python'; in the case that no python is installed
> at all, the workaround is to double up on your current trick:
>
> make PYTHON=false PYTHON_CONFIG=false

I'm probably going to submit a small patch series to improve this
configuration code in general, but there's no reason to wait for
me to do this.

Thus, I think the simplest thing to do right away is just to revert
the one-line change that led to the regression, thereby restoring
the old behavior which has hitherto worked well enough.

The following patch applies to at least the following commit:

bb33db7a076f4719dc68c235e187dd4bfb16b621

To apply this patch, save this email to:

/path/to/email

and then run:

git am --scissors /path/to/email

Sincerely,
Michael Witten

8<-----------8<-----------8<-----------8<-----------8<-----------8<-----------
Among other things, the following:

commit 31160d7feab786c991780d7f0ce2755a469e0e5e
Date: Tue Jan 8 16:22:36 2013 -0500
perf tools: Fix GNU make v3.80 compatibility issue

attempts to aid the user by tapping into an existing error message,
as described in the commit message:

... Also fix an issue where _get_attempt was called with only
one argument. This prevented the error message from printing
the name of the variable that can be used to fix the problem.

or more precisely:

-$(if $($(1)),$(call _ge_attempt,$($(1)),$(1)),$(call _ge_attempt,$(2)))
+$(if $($(1)),$(call _ge_attempt,$($(1)),$(1)),$(call _ge_attempt,$(2),$(1)))

However, The "missing" argument was in fact missing on purpose; it's
absence is a signal that the error message should be skipped, because
the failure would be due to the default value, not any user-supplied
value. This can be seen in how `_ge_attempt' uses `gea_err' (in the
config/utilities.mak file):

_ge_attempt = $(if $(get-executable),$(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))

That is, because the argument is no longer missing, the value `$(1)'
(associated with `_gea_err') always evaluates to true, thus always
triggering the error condition that is meant to be reserved for
only the case when a user explicitly supplies an invalid value.

Concretely, the result is a regression in the Makefile's configuration
of python support; rather than gracefully disable support when the
relevant executables cannot be found according to default values, the
build process halts in error as though the user explicitly supplied
the values.

This new commit simply reverts the offending one-line change.

Reported-by: Pekka Enberg <[email protected]>
Link: http://lkml.kernel.org/r/CAOJsxLHv17Ys3M7P5q25imkUxQW6LE_vABxh1N3Tt7Mv6Ho4iw@mail.gmail.com
Signed-off-by: Michael Witten <[email protected]>

---
tools/perf/config/utilities.mak | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/config/utilities.mak b/tools/perf/config/utilities.mak
index 8ef3bd3..3e89719 100644
--- a/tools/perf/config/utilities.mak
+++ b/tools/perf/config/utilities.mak
@@ -173,7 +173,7 @@ _ge-abspath = $(if $(is-executable),$(1))
# 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),$(1)))
+$(if $($(1)),$(call _ge_attempt,$($(1)),$(1)),$(call _ge_attempt,$(2)))
endef
_ge_attempt = $(if $(get-executable),$(get-executable),$(_gea_warn)$(call _gea_err,$(2)))
_gea_warn = $(warning The path '$(1)' is not executable.)

Subject: [tip:perf/urgent] perf tools: Revert regression in configuration of Python support

Commit-ID: a363a9da65d253fa7354ce5fd630f4f94df934cc
Gitweb: http://git.kernel.org/tip/a363a9da65d253fa7354ce5fd630f4f94df934cc
Author: Michael Witten <[email protected]>
AuthorDate: Wed, 17 Apr 2013 02:23:16 +0000
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 9 Jul 2013 17:29:01 -0300

perf tools: Revert regression in configuration of Python support

Among other things, the following:

commit 31160d7feab786c991780d7f0ce2755a469e0e5e
Date: Tue Jan 8 16:22:36 2013 -0500
perf tools: Fix GNU make v3.80 compatibility issue

attempts to aid the user by tapping into an existing error message,
as described in the commit message:

... Also fix an issue where _get_attempt was called with only
one argument. This prevented the error message from printing
the name of the variable that can be used to fix the problem.

or more precisely:

-$(if $($(1)),$(call _ge_attempt,$($(1)),$(1)),$(call _ge_attempt,$(2)))
+$(if $($(1)),$(call _ge_attempt,$($(1)),$(1)),$(call _ge_attempt,$(2),$(1)))

However, The "missing" argument was in fact missing on purpose; it's
absence is a signal that the error message should be skipped, because
the failure would be due to the default value, not any user-supplied
value. This can be seen in how `_ge_attempt' uses `gea_err' (in the
config/utilities.mak file):

_ge_attempt = $(if $(get-executable),$(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))

That is, because the argument is no longer missing, the value `$(1)'
(associated with `_gea_err') always evaluates to true, thus always
triggering the error condition that is meant to be reserved for
only the case when a user explicitly supplies an invalid value.

Concretely, the result is a regression in the Makefile's configuration
of python support; rather than gracefully disable support when the
relevant executables cannot be found according to default values, the
build process halts in error as though the user explicitly supplied
the values.

This new commit simply reverts the offending one-line change.

Reported-by: Pekka Enberg <[email protected]>
Link: http://lkml.kernel.org/r/CAOJsxLHv17Ys3M7P5q25imkUxQW6LE_vABxh1N3Tt7Mv6Ho4iw@mail.gmail.com
Signed-off-by: Michael Witten <[email protected]>
---
tools/perf/config/utilities.mak | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/config/utilities.mak b/tools/perf/config/utilities.mak
index faffb52..94d2d4f 100644
--- a/tools/perf/config/utilities.mak
+++ b/tools/perf/config/utilities.mak
@@ -173,7 +173,7 @@ _ge-abspath = $(if $(is-executable),$(1))
# 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),$(1)))
+$(if $($(1)),$(call _ge_attempt,$($(1)),$(1)),$(call _ge_attempt,$(2)))
endef
_ge_attempt = $(if $(get-executable),$(get-executable),$(_gea_warn)$(call _gea_err,$(2)))
_gea_warn = $(warning The path '$(1)' is not executable.)