linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: German Gomez <german.gomez@arm.com>
To: James Clark <james.clark@arm.com>,
	acme@kernel.org, linux-perf-users@vger.kernel.org
Cc: Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@kernel.org>, Namhyung Kim <namhyung@kernel.org>,
	Arnaldo Carvalho de Melo <acme@redhat.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/1] perf tools: Use Python devtools for version autodetection rather than runtime
Date: Tue, 12 Apr 2022 16:30:18 +0100	[thread overview]
Message-ID: <1262ee1e-22dc-0cc6-3e60-257e17d9ed26@arm.com> (raw)
In-Reply-To: <20220309194313.3350126-2-james.clark@arm.com>

Hi James,

On 09/03/2022 19:43, James Clark wrote:
> This fixes the issue where the build will fail if only the Python2
> runtime is installed but the Python3 devtools are installed. Currently
> the workaround is 'make PYTHON=python3'.
>
> Fix it by autodetecting Python based on whether python[x]-config exists
> rather than just python[x] because both are needed for the build. Then
> -config is stripped to find the Python runtime.
>
> Testing
> =======
>
>  * Auto detect links with Python3 when the v3 devtools are installed
>    and only Python 2 runtime is installed
>  * Auto detect links with Python2 when both devtools are installed
>  * Sensible warning is printed if no Python devtools are installed
>  * 'make PYTHON=x' still automatically sets PYTHON_CONFIG=x-config
>  * 'make PYTHON=x' fails if x-config doesn't exist

If x is a valid python but no x-config is found in the system, the build
fails, instead of printing a warning like before. If we use this approach
I think [1] in the Makefile is never hit and needs to be cleaned up.

Thanks,
German

[1] https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/tree/tools/perf/Makefile.config?h=perf/core#n826:

>  * 'make PYTHON=python3' overrides Python2 devtools
>  * 'make PYTHON=python2' overrides Python3 devtools
>  * 'make PYTHON_CONFIG=x-config' works
>  * 'make PYTHON=x PYTHON_CONFIG=x' works
>  * 'make PYTHON=missing' reports an error
>  * 'make PYTHON_CONFIG=missing' reports an error
>
> Fixes: 79373082fa9d ("perf python: Autodetect python3 binary")
> Signed-off-by: James Clark <james.clark@arm.com>
> ---
>  tools/perf/Makefile.config | 39 ++++++++++++++++++++++++++------------
>  1 file changed, 27 insertions(+), 12 deletions(-)
>
> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index 96ad944ca6a8..b3fbb746e7f0 100644
> --- a/tools/perf/Makefile.config
> +++ b/tools/perf/Makefile.config
> @@ -239,18 +239,33 @@ ifdef PARSER_DEBUG
>  endif
>  
>  # Try different combinations to accommodate systems that only have
> -# python[2][-config] in weird combinations but always preferring
> -# python2 and python2-config as per pep-0394. If python2 or python
> -# aren't found, then python3 is used.
> -PYTHON_AUTO := python
> -PYTHON_AUTO := $(if $(call get-executable,python3),python3,$(PYTHON_AUTO))
> -PYTHON_AUTO := $(if $(call get-executable,python),python,$(PYTHON_AUTO))
> -PYTHON_AUTO := $(if $(call get-executable,python2),python2,$(PYTHON_AUTO))
> -override PYTHON := $(call get-executable-or-default,PYTHON,$(PYTHON_AUTO))
> -PYTHON_AUTO_CONFIG := \
> -  $(if $(call get-executable,$(PYTHON)-config),$(PYTHON)-config,python-config)
> -override PYTHON_CONFIG := \
> -  $(call get-executable-or-default,PYTHON_CONFIG,$(PYTHON_AUTO_CONFIG))
> +# python[2][3]-config in weird combinations in the following order of
> +# priority from lowest to highest:
> +#   * python3-config
> +#   * python-config
> +#   * python2-config as per pep-0394.
> +#   * $(PYTHON)-config (If PYTHON is user supplied but PYTHON_CONFIG isn't)
> +#
> +PYTHON_AUTO := python-config
> +PYTHON_AUTO := $(if $(call get-executable,python3-config),python3-config,$(PYTHON_AUTO))
> +PYTHON_AUTO := $(if $(call get-executable,python-config),python-config,$(PYTHON_AUTO))
> +PYTHON_AUTO := $(if $(call get-executable,python2-config),python2-config,$(PYTHON_AUTO))
> +
> +# If PYTHON is defined but PYTHON_CONFIG isn't, then take $(PYTHON)-config as if it was the user
> +# supplied value for PYTHON_CONFIG. Because it's "user supplied", error out if it doesn't exist.
> +ifdef PYTHON
> +  ifndef PYTHON_CONFIG
> +    PYTHON_CONFIG_AUTO := $(call get-executable,$(PYTHON)-config)
> +    PYTHON_CONFIG := $(if $(PYTHON_CONFIG_AUTO),$(PYTHON_CONFIG_AUTO),\
> +                          $(call $(error $(PYTHON)-config not found)))
> +  endif
> +endif
> +
> +# Select either auto detected python and python-config or use user supplied values if they are
> +# 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)))
>  
>  grep-libs  = $(filter -l%,$(1))
>  strip-libs  = $(filter-out -l%,$(1))

  reply	other threads:[~2022-04-12 15:30 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-09 19:43 [PATCH 0/1] perf tools: Use Python devtools for version autodetection James Clark
2022-03-09 19:43 ` [PATCH 1/1] perf tools: Use Python devtools for version autodetection rather than runtime James Clark
2022-04-12 15:30   ` German Gomez [this message]
2022-04-12 15:45     ` German Gomez

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1262ee1e-22dc-0cc6-3e60-257e17d9ed26@arm.com \
    --to=german.gomez@arm.com \
    --cc=acme@kernel.org \
    --cc=acme@redhat.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=james.clark@arm.com \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=namhyung@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).