* [FYI][PATCH 1/1] tools build: Remove needless libpython-version feature check that breaks test-all fast path
@ 2021-11-30 13:26 Arnaldo Carvalho de Melo
2021-11-30 14:18 ` James Clark
0 siblings, 1 reply; 3+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-11-30 13:26 UTC (permalink / raw)
To: Jaroslav Škarvada
Cc: Adrian Hunter, Ian Rogers, Jiri Olsa, Namhyung Kim,
Linux Kernel Mailing List, linux-perf-users
Since 66dfdff03d196e51 ("perf tools: Add Python 3 support") we don't use
the tools/build/feature/test-libpython-version.c version in any Makefile
feature check:
$ find tools/ -type f | xargs grep feature-libpython-version
$
The only place where this was used was removed in 66dfdff03d196e51:
- ifneq ($(feature-libpython-version), 1)
- $(warning Python 3 is not yet supported; please set)
- $(warning PYTHON and/or PYTHON_CONFIG appropriately.)
- $(warning If you also have Python 2 installed, then)
- $(warning try something like:)
- $(warning $(and ,))
- $(warning $(and ,) make PYTHON=python2)
- $(warning $(and ,))
- $(warning Otherwise, disable Python support entirely:)
- $(warning $(and ,))
- $(warning $(and ,) make NO_LIBPYTHON=1)
- $(warning $(and ,))
- $(error $(and ,))
- else
- LDFLAGS += $(PYTHON_EMBED_LDFLAGS)
- EXTLIBS += $(PYTHON_EMBED_LIBADD)
- LANG_BINDINGS += $(obj-perf)python/perf.so
- $(call detected,CONFIG_LIBPYTHON)
- endif
And nowadays we either build with PYTHON=python3 or just install the
python3 devel packages and perf will build against it.
But the leftover feature-libpython-version check made the fast path
feature detection to break in all cases except when python2 devel files
were installed:
$ rpm -qa | grep python.*devel
python3-devel-3.9.7-1.fc34.x86_64
$ rm -rf /tmp/build/perf ; mkdir -p /tmp/build/perf ;
$ make -C tools/perf O=/tmp/build/perf install-bin
make: Entering directory '/var/home/acme/git/perf/tools/perf'
BUILD: Doing 'make -j32' parallel build
HOSTCC /tmp/build/perf/fixdep.o
<SNIP>
$ cat /tmp/build/perf/feature/test-all.make.output
In file included from test-all.c:18:
test-libpython-version.c:5:10: error: #error
5 | #error
| ^~~~~
$ ldd ~/bin/perf | grep python
libpython3.9.so.1.0 => /lib64/libpython3.9.so.1.0 (0x00007fda6dbcf000)
$
As python3 is the norm these days, fix this by just removing the unused
feature-libpython-version feature check, making the test-all fast path
to work with the common case.
With this:
$ rm -rf /tmp/build/perf ; mkdir -p /tmp/build/perf ;
$ make -C tools/perf O=/tmp/build/perf install-bin |& head
make: Entering directory '/var/home/acme/git/perf/tools/perf'
BUILD: Doing 'make -j32' parallel build
HOSTCC /tmp/build/perf/fixdep.o
HOSTLD /tmp/build/perf/fixdep-in.o
LINK /tmp/build/perf/fixdep
Auto-detecting system features:
... dwarf: [ on ]
... dwarf_getlocations: [ on ]
... glibc: [ on ]
$ ldd ~/bin/perf | grep python
libpython3.9.so.1.0 => /lib64/libpython3.9.so.1.0 (0x00007f58800b0000)
$ cat /tmp/build/perf/feature/test-all.make.output
$
Fixes: 66dfdff03d196e51 ("perf tools: Add Python 3 support")
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Ian Rogers <irogers@google.com>
Cc: Jaroslav Škarvada <jskarvad@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/build/Makefile.feature | 1 -
tools/build/feature/Makefile | 4 ----
tools/build/feature/test-all.c | 5 -----
tools/build/feature/test-libpython-version.c | 11 -----------
tools/perf/Makefile.config | 2 --
5 files changed, 23 deletions(-)
delete mode 100644 tools/build/feature/test-libpython-version.c
diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
index 45a9a59828c3c09d..ae61f464043a11fb 100644
--- a/tools/build/Makefile.feature
+++ b/tools/build/Makefile.feature
@@ -48,7 +48,6 @@ FEATURE_TESTS_BASIC := \
numa_num_possible_cpus \
libperl \
libpython \
- libpython-version \
libslang \
libslang-include-subdir \
libtraceevent \
diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
index 0a3244ad967307ce..1480910c792e2cb3 100644
--- a/tools/build/feature/Makefile
+++ b/tools/build/feature/Makefile
@@ -32,7 +32,6 @@ FILES= \
test-numa_num_possible_cpus.bin \
test-libperl.bin \
test-libpython.bin \
- test-libpython-version.bin \
test-libslang.bin \
test-libslang-include-subdir.bin \
test-libtraceevent.bin \
@@ -227,9 +226,6 @@ $(OUTPUT)test-libperl.bin:
$(OUTPUT)test-libpython.bin:
$(BUILD) $(FLAGS_PYTHON_EMBED)
-$(OUTPUT)test-libpython-version.bin:
- $(BUILD)
-
$(OUTPUT)test-libbfd.bin:
$(BUILD) -DPACKAGE='"perf"' -lbfd -ldl
diff --git a/tools/build/feature/test-all.c b/tools/build/feature/test-all.c
index 0b243ce842be3383..5ffafb967b6e4952 100644
--- a/tools/build/feature/test-all.c
+++ b/tools/build/feature/test-all.c
@@ -14,10 +14,6 @@
# include "test-libpython.c"
#undef main
-#define main main_test_libpython_version
-# include "test-libpython-version.c"
-#undef main
-
#define main main_test_libperl
# include "test-libperl.c"
#undef main
@@ -177,7 +173,6 @@
int main(int argc, char *argv[])
{
main_test_libpython();
- main_test_libpython_version();
main_test_libperl();
main_test_hello();
main_test_libelf();
diff --git a/tools/build/feature/test-libpython-version.c b/tools/build/feature/test-libpython-version.c
deleted file mode 100644
index 47714b942d4d35bf..0000000000000000
--- a/tools/build/feature/test-libpython-version.c
+++ /dev/null
@@ -1,11 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-#include <Python.h>
-
-#if PY_VERSION_HEX >= 0x03000000
- #error
-#endif
-
-int main(void)
-{
- return 0;
-}
diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index afd144725a0bf766..3df74cf5651af9dd 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -271,8 +271,6 @@ endif
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)
FEATURE_CHECK_LDFLAGS-libaio = -lrt
--
2.31.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [FYI][PATCH 1/1] tools build: Remove needless libpython-version feature check that breaks test-all fast path
2021-11-30 13:26 [FYI][PATCH 1/1] tools build: Remove needless libpython-version feature check that breaks test-all fast path Arnaldo Carvalho de Melo
@ 2021-11-30 14:18 ` James Clark
2021-11-30 14:33 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 3+ messages in thread
From: James Clark @ 2021-11-30 14:18 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jaroslav Škarvada
Cc: Adrian Hunter, Ian Rogers, Jiri Olsa, Namhyung Kim,
Linux Kernel Mailing List, linux-perf-users
On 30/11/2021 13:26, Arnaldo Carvalho de Melo wrote:
> Since 66dfdff03d196e51 ("perf tools: Add Python 3 support") we don't use
> the tools/build/feature/test-libpython-version.c version in any Makefile
> feature check:
>
> $ find tools/ -type f | xargs grep feature-libpython-version
> $
>
> The only place where this was used was removed in 66dfdff03d196e51:
>
> - ifneq ($(feature-libpython-version), 1)
> - $(warning Python 3 is not yet supported; please set)
> - $(warning PYTHON and/or PYTHON_CONFIG appropriately.)
> - $(warning If you also have Python 2 installed, then)
> - $(warning try something like:)
> - $(warning $(and ,))
> - $(warning $(and ,) make PYTHON=python2)
> - $(warning $(and ,))
> - $(warning Otherwise, disable Python support entirely:)
> - $(warning $(and ,))
> - $(warning $(and ,) make NO_LIBPYTHON=1)
> - $(warning $(and ,))
> - $(error $(and ,))
> - else
> - LDFLAGS += $(PYTHON_EMBED_LDFLAGS)
> - EXTLIBS += $(PYTHON_EMBED_LIBADD)
> - LANG_BINDINGS += $(obj-perf)python/perf.so
> - $(call detected,CONFIG_LIBPYTHON)
> - endif
>
> And nowadays we either build with PYTHON=python3 or just install the
> python3 devel packages and perf will build against it.
I just tried this and found a combo that doesn't work and fails with this
error (unrelated to this change):
Makefile.config:812: No 'python-config' tool was found: disables Python support - please install python-devel/python-dev
The combo is when the python2 runtime is installed, but the python3 devtools
are installed. I didn't realise this when I added the python 3 autodetection,
I only fixed the issue for a system that was solely python3.
Do you think I should fix this? Currently the workaround is PYTHON=python3,
maybe it's enough of an edge case that it's ok?
>
> But the leftover feature-libpython-version check made the fast path
> feature detection to break in all cases except when python2 devel files
> were installed:
>
> $ rpm -qa | grep python.*devel
> python3-devel-3.9.7-1.fc34.x86_64
> $ rm -rf /tmp/build/perf ; mkdir -p /tmp/build/perf ;
> $ make -C tools/perf O=/tmp/build/perf install-bin
> make: Entering directory '/var/home/acme/git/perf/tools/perf'
> BUILD: Doing 'make -j32' parallel build
> HOSTCC /tmp/build/perf/fixdep.o
> <SNIP>
> $ cat /tmp/build/perf/feature/test-all.make.output
> In file included from test-all.c:18:
> test-libpython-version.c:5:10: error: #error
> 5 | #error
> | ^~~~~
> $ ldd ~/bin/perf | grep python
> libpython3.9.so.1.0 => /lib64/libpython3.9.so.1.0 (0x00007fda6dbcf000)
> $
>
> As python3 is the norm these days, fix this by just removing the unused
> feature-libpython-version feature check, making the test-all fast path
> to work with the common case.
>
> With this:
>
> $ rm -rf /tmp/build/perf ; mkdir -p /tmp/build/perf ;
> $ make -C tools/perf O=/tmp/build/perf install-bin |& head
> make: Entering directory '/var/home/acme/git/perf/tools/perf'
> BUILD: Doing 'make -j32' parallel build
> HOSTCC /tmp/build/perf/fixdep.o
> HOSTLD /tmp/build/perf/fixdep-in.o
> LINK /tmp/build/perf/fixdep
>
> Auto-detecting system features:
> ... dwarf: [ on ]
> ... dwarf_getlocations: [ on ]
> ... glibc: [ on ]
> $ ldd ~/bin/perf | grep python
> libpython3.9.so.1.0 => /lib64/libpython3.9.so.1.0 (0x00007f58800b0000)
> $ cat /tmp/build/perf/feature/test-all.make.output
> $
>
> Fixes: 66dfdff03d196e51 ("perf tools: Add Python 3 support")
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: Ian Rogers <irogers@google.com>
> Cc: Jaroslav Škarvada <jskarvad@redhat.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> ---
> tools/build/Makefile.feature | 1 -
> tools/build/feature/Makefile | 4 ----
> tools/build/feature/test-all.c | 5 -----
> tools/build/feature/test-libpython-version.c | 11 -----------
> tools/perf/Makefile.config | 2 --
> 5 files changed, 23 deletions(-)
> delete mode 100644 tools/build/feature/test-libpython-version.c
>
Reviewed-by: James Clark <james.clark@arm.com>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [FYI][PATCH 1/1] tools build: Remove needless libpython-version feature check that breaks test-all fast path
2021-11-30 14:18 ` James Clark
@ 2021-11-30 14:33 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 3+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-11-30 14:33 UTC (permalink / raw)
To: James Clark
Cc: Jaroslav Škarvada, Adrian Hunter, Ian Rogers, Jiri Olsa,
Namhyung Kim, Linux Kernel Mailing List, linux-perf-users
Em Tue, Nov 30, 2021 at 02:18:54PM +0000, James Clark escreveu:
>
>
> On 30/11/2021 13:26, Arnaldo Carvalho de Melo wrote:
> > Since 66dfdff03d196e51 ("perf tools: Add Python 3 support") we don't use
> > the tools/build/feature/test-libpython-version.c version in any Makefile
> > feature check:
> >
> > $ find tools/ -type f | xargs grep feature-libpython-version
> > $
> >
> > The only place where this was used was removed in 66dfdff03d196e51:
> >
> > - ifneq ($(feature-libpython-version), 1)
> > - $(warning Python 3 is not yet supported; please set)
> > - $(warning PYTHON and/or PYTHON_CONFIG appropriately.)
> > - $(warning If you also have Python 2 installed, then)
> > - $(warning try something like:)
> > - $(warning $(and ,))
> > - $(warning $(and ,) make PYTHON=python2)
> > - $(warning $(and ,))
> > - $(warning Otherwise, disable Python support entirely:)
> > - $(warning $(and ,))
> > - $(warning $(and ,) make NO_LIBPYTHON=1)
> > - $(warning $(and ,))
> > - $(error $(and ,))
> > - else
> > - LDFLAGS += $(PYTHON_EMBED_LDFLAGS)
> > - EXTLIBS += $(PYTHON_EMBED_LIBADD)
> > - LANG_BINDINGS += $(obj-perf)python/perf.so
> > - $(call detected,CONFIG_LIBPYTHON)
> > - endif
> >
> > And nowadays we either build with PYTHON=python3 or just install the
> > python3 devel packages and perf will build against it.
>
> I just tried this and found a combo that doesn't work and fails with this
> error (unrelated to this change):
>
> Makefile.config:812: No 'python-config' tool was found: disables Python support - please install python-devel/python-dev
>
> The combo is when the python2 runtime is installed, but the python3 devtools
> are installed. I didn't realise this when I added the python 3 autodetection,
> I only fixed the issue for a system that was solely python3.
>
> Do you think I should fix this? Currently the workaround is PYTHON=python3,
> maybe it's enough of an edge case that it's ok?
We have a workaround, so perhaps you can just send a patch improving the
warning about python-config, suggesting to maybe using PYTHON=python3 on
the make command line?
But I won't complain if you want to provide something better :-)
> Reviewed-by: James Clark <james.clark@arm.com>
Thanks, adding it to the patch,
Regards,
- Arnaldo
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-11-30 14:33 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-11-30 13:26 [FYI][PATCH 1/1] tools build: Remove needless libpython-version feature check that breaks test-all fast path Arnaldo Carvalho de Melo
2021-11-30 14:18 ` James Clark
2021-11-30 14:33 ` Arnaldo Carvalho de Melo
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.