[PROBLEM] perf requires python-devel to compile

[PROBLEM] perf requires python-devel to compile

[Pekka Enberg]

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

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

[Namhyung Kim]

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)

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

[David Ahern]

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

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

[Michael Witten]

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

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

[Pekka Enberg]

Hello,

On Tue, Apr 16, 2013 at 5:14 AM, Namhyung Kim <namhyung@kernel.org> 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

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

[David Ahern]

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

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

[Michael Witten]

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

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

[Michael Witten]

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 <penberg@kernel.org>
Link: http://lkml.kernel.org/r/CAOJsxLHv17Ys3M7P5q25imkUxQW6LE_vABxh1N3Tt7Mv6Ho4iw@mail.gmail.com
Signed-off-by: Michael Witten <mfwitten@gmail.com>

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

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

[tip-bot for Michael Witten]

Commit-ID:  a363a9da65d253fa7354ce5fd630f4f94df934cc
Gitweb:     http://git.kernel.org/tip/a363a9da65d253fa7354ce5fd630f4f94df934cc
Author:     Michael Witten <mfwitten@gmail.com>
AuthorDate: Wed, 17 Apr 2013 02:23:16 +0000
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
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 <penberg@kernel.org>
Link: http://lkml.kernel.org/r/CAOJsxLHv17Ys3M7P5q25imkUxQW6LE_vABxh1N3Tt7Mv6Ho4iw@mail.gmail.com
Signed-off-by: Michael Witten <mfwitten@gmail.com>
---
 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.)