perf: python: Fix build when PYTHON_CONFIG is user supplied

[PATCH] perf: python: Fix build when PYTHON_CONFIG is user supplied

Posted by James Clark 3 years, 8 months ago

The previous change to Python autodetection had a small mistake where
the auto value was used to determine the Python binary, rather than the
user supplied value. The Python binary is only used for one part of the
build process, rather than the final linking, so it was producing
correct builds in most scenarios, especially when the auto detected
value matched what the user wanted, or the system only had a valid set
of Pythons.

Change it so that the Python binary path is derived from either the
PYTHON_CONFIG value or PYTHON value, depending on what is specified by
the user. This was the original intention.

This error was spotted in a build failure an odd cross compilation
environment after commit 4c41cb46a732fe82 ("perf python: Prefer
python3") was merged.

Fixes: 630af16eee495f58 ("perf tools: Use Python devtools for version autodetection rather than runtime")
Signed-off-by: James Clark <james.clark@arm.com>
---
 tools/perf/Makefile.config | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index d3c254c0f5c6..a69da9f34486 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -265,7 +265,7 @@ endif
 # defined. get-executable-or-default fails with an error if the first argument is supplied but
 # doesn't exist.
 override PYTHON_CONFIG := $(call get-executable-or-default,PYTHON_CONFIG,$(PYTHON_AUTO))
-override PYTHON := $(call get-executable-or-default,PYTHON,$(subst -config,,$(PYTHON_AUTO)))
+override PYTHON := $(call get-executable-or-default,PYTHON,$(subst -config,,$(PYTHON_CONFIG)))
 
 grep-libs  = $(filter -l%,$(1))
 strip-libs  = $(filter-out -l%,$(1))
-- 
2.28.0

Re: [PATCH] perf: python: Fix build when PYTHON_CONFIG is user supplied

Posted by Ian Rogers 3 years, 8 months ago

On Thu, Jul 28, 2022 at 2:40 AM James Clark <james.clark@arm.com> wrote:
>
> The previous change to Python autodetection had a small mistake where
> the auto value was used to determine the Python binary, rather than the
> user supplied value. The Python binary is only used for one part of the
> build process, rather than the final linking, so it was producing
> correct builds in most scenarios, especially when the auto detected
> value matched what the user wanted, or the system only had a valid set
> of Pythons.
>
> Change it so that the Python binary path is derived from either the
> PYTHON_CONFIG value or PYTHON value, depending on what is specified by
> the user. This was the original intention.
>
> This error was spotted in a build failure an odd cross compilation
> environment after commit 4c41cb46a732fe82 ("perf python: Prefer
> python3") was merged.
>
> Fixes: 630af16eee495f58 ("perf tools: Use Python devtools for version autodetection rather than runtime")
> Signed-off-by: James Clark <james.clark@arm.com>

Acked-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
>  tools/perf/Makefile.config | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index d3c254c0f5c6..a69da9f34486 100644
> --- a/tools/perf/Makefile.config
> +++ b/tools/perf/Makefile.config
> @@ -265,7 +265,7 @@ endif
>  # defined. get-executable-or-default fails with an error if the first argument is supplied but
>  # doesn't exist.
>  override PYTHON_CONFIG := $(call get-executable-or-default,PYTHON_CONFIG,$(PYTHON_AUTO))
> -override PYTHON := $(call get-executable-or-default,PYTHON,$(subst -config,,$(PYTHON_AUTO)))
> +override PYTHON := $(call get-executable-or-default,PYTHON,$(subst -config,,$(PYTHON_CONFIG)))
>
>  grep-libs  = $(filter -l%,$(1))
>  strip-libs  = $(filter-out -l%,$(1))
> --
> 2.28.0
>

Re: [PATCH] perf: python: Fix build when PYTHON_CONFIG is user supplied

Posted by Arnaldo Carvalho de Melo 3 years, 8 months ago

Em Thu, Jul 28, 2022 at 09:37:32AM -0700, Ian Rogers escreveu:
> On Thu, Jul 28, 2022 at 2:40 AM James Clark <james.clark@arm.com> wrote:
> >
> > The previous change to Python autodetection had a small mistake where
> > the auto value was used to determine the Python binary, rather than the
> > user supplied value. The Python binary is only used for one part of the
> > build process, rather than the final linking, so it was producing
> > correct builds in most scenarios, especially when the auto detected
> > value matched what the user wanted, or the system only had a valid set
> > of Pythons.
> >
> > Change it so that the Python binary path is derived from either the
> > PYTHON_CONFIG value or PYTHON value, depending on what is specified by
> > the user. This was the original intention.
> >
> > This error was spotted in a build failure an odd cross compilation
> > environment after commit 4c41cb46a732fe82 ("perf python: Prefer
> > python3") was merged.
> >
> > Fixes: 630af16eee495f58 ("perf tools: Use Python devtools for version autodetection rather than runtime")
> > Signed-off-by: James Clark <james.clark@arm.com>
> 
> Acked-by: Ian Rogers <irogers@google.com>

Thanks, applied.

- Arnaldo

 
> Thanks,
> Ian
> 
> > ---
> >  tools/perf/Makefile.config | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> > index d3c254c0f5c6..a69da9f34486 100644
> > --- a/tools/perf/Makefile.config
> > +++ b/tools/perf/Makefile.config
> > @@ -265,7 +265,7 @@ endif
> >  # defined. get-executable-or-default fails with an error if the first argument is supplied but
> >  # doesn't exist.
> >  override PYTHON_CONFIG := $(call get-executable-or-default,PYTHON_CONFIG,$(PYTHON_AUTO))
> > -override PYTHON := $(call get-executable-or-default,PYTHON,$(subst -config,,$(PYTHON_AUTO)))
> > +override PYTHON := $(call get-executable-or-default,PYTHON,$(subst -config,,$(PYTHON_CONFIG)))
> >
> >  grep-libs  = $(filter -l%,$(1))
> >  strip-libs  = $(filter-out -l%,$(1))
> > --
> > 2.28.0
> >

-- 

- Arnaldo

Re: [PATCH] perf: python: Fix build when PYTHON_CONFIG is user supplied

Posted by James Clark 3 years, 7 months ago


On 28/07/2022 20:26, Arnaldo Carvalho de Melo wrote:
> Em Thu, Jul 28, 2022 at 09:37:32AM -0700, Ian Rogers escreveu:
>> On Thu, Jul 28, 2022 at 2:40 AM James Clark <james.clark@arm.com> wrote:
>>>
>>> The previous change to Python autodetection had a small mistake where
>>> the auto value was used to determine the Python binary, rather than the
>>> user supplied value. The Python binary is only used for one part of the
>>> build process, rather than the final linking, so it was producing
>>> correct builds in most scenarios, especially when the auto detected
>>> value matched what the user wanted, or the system only had a valid set
>>> of Pythons.
>>>
>>> Change it so that the Python binary path is derived from either the
>>> PYTHON_CONFIG value or PYTHON value, depending on what is specified by
>>> the user. This was the original intention.
>>>
>>> This error was spotted in a build failure an odd cross compilation
>>> environment after commit 4c41cb46a732fe82 ("perf python: Prefer
>>> python3") was merged.
>>>
>>> Fixes: 630af16eee495f58 ("perf tools: Use Python devtools for version autodetection rather than runtime")
>>> Signed-off-by: James Clark <james.clark@arm.com>
>>
>> Acked-by: Ian Rogers <irogers@google.com>
> 
> Thanks, applied.

Hi Arnaldo,

I couldn't find this change in any of your branches. Do you know if it
got dropped somehow or was there an issue with it?

Thanks
James

> 
> - Arnaldo
> 
>  
>> Thanks,
>> Ian
>>
>>> ---
>>>  tools/perf/Makefile.config | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
>>> index d3c254c0f5c6..a69da9f34486 100644
>>> --- a/tools/perf/Makefile.config
>>> +++ b/tools/perf/Makefile.config
>>> @@ -265,7 +265,7 @@ endif
>>>  # defined. get-executable-or-default fails with an error if the first argument is supplied but
>>>  # doesn't exist.
>>>  override PYTHON_CONFIG := $(call get-executable-or-default,PYTHON_CONFIG,$(PYTHON_AUTO))
>>> -override PYTHON := $(call get-executable-or-default,PYTHON,$(subst -config,,$(PYTHON_AUTO)))
>>> +override PYTHON := $(call get-executable-or-default,PYTHON,$(subst -config,,$(PYTHON_CONFIG)))
>>>
>>>  grep-libs  = $(filter -l%,$(1))
>>>  strip-libs  = $(filter-out -l%,$(1))
>>> --
>>> 2.28.0
>>>
>

Re: [PATCH] perf: python: Fix build when PYTHON_CONFIG is user supplied

Posted by Arnaldo Carvalho de Melo 3 years, 7 months ago

Em Mon, Aug 22, 2022 at 11:08:37AM +0100, James Clark escreveu:
> 
> 
> On 28/07/2022 20:26, Arnaldo Carvalho de Melo wrote:
> > Em Thu, Jul 28, 2022 at 09:37:32AM -0700, Ian Rogers escreveu:
> >> On Thu, Jul 28, 2022 at 2:40 AM James Clark <james.clark@arm.com> wrote:
> >>>
> >>> The previous change to Python autodetection had a small mistake where
> >>> the auto value was used to determine the Python binary, rather than the
> >>> user supplied value. The Python binary is only used for one part of the
> >>> build process, rather than the final linking, so it was producing
> >>> correct builds in most scenarios, especially when the auto detected
> >>> value matched what the user wanted, or the system only had a valid set
> >>> of Pythons.
> >>>
> >>> Change it so that the Python binary path is derived from either the
> >>> PYTHON_CONFIG value or PYTHON value, depending on what is specified by
> >>> the user. This was the original intention.
> >>>
> >>> This error was spotted in a build failure an odd cross compilation
> >>> environment after commit 4c41cb46a732fe82 ("perf python: Prefer
> >>> python3") was merged.
> >>>
> >>> Fixes: 630af16eee495f58 ("perf tools: Use Python devtools for version autodetection rather than runtime")
> >>> Signed-off-by: James Clark <james.clark@arm.com>
> >>
> >> Acked-by: Ian Rogers <irogers@google.com>
> > 
> > Thanks, applied.
> 
> Hi Arnaldo,
> 
> I couldn't find this change in any of your branches. Do you know if it
> got dropped somehow or was there an issue with it?

Applied it to my local perf/urgent branch, testing now. I thought I had
processed it, not really :-\

- Arnaldo