Re: [PATCH v6 55/59] perf Makefile: Update Python script installation path

sashiko-bot
2026-04-25T18:26:09Z

Sashiko AI review found 2 potential issue(s):
- [Medium] Python scripts will silently disappear from the 'perf report' interactive script browser.
- [Medium] The `make_no_scripts` and `make_minimal` test targets in `tools/perf/tests/make` are silently broken, neutering build test coverage.
--

commit 097e9f6a5c8c0be76822e797df41c8183e50dce1
Author: Ian Rogers <irogers@google.com>

perf Makefile: Update Python script installation path

This commit replaces the libpython feature test with a python-module feature
test and updates the associated build configuration. It also changes the
installation directory for Python scripts from the legacy scripts/python path
to a direct python directory under libexec.

> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
[ ... ]
> @@ -1099,10 +1097,10 @@ endif
> $(call QUIET_INSTALL, perf-iostat) \
> $(INSTALL) $(OUTPUT)perf-iostat -t '$(DESTDIR_SQ)$(perfexec_instdir_SQ)'
>
> -ifndef NO_LIBPYTHON
> +ifndef NO_PYTHON_MODULE
> $(call QUIET_INSTALL, python-scripts) \
> - $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(perfexec_instdir_SQ)/scripts/python'; \
> - $(INSTALL) python/*.py -m 644 -t '$(DESTDIR_SQ)$(perfexec_instdir_SQ)/scripts/python'
> + $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(perfexec_instdir_SQ)/python'; \
> + $(INSTALL) python/*.py -m 644 -t '$(DESTDIR_SQ)$(perfexec_instdir_SQ)/python'
> endif

Does this regression silently drop Python scripts from the perf report
interactive script browser?

Looking at tools/perf/ui/browsers/scripts.c, find_scripts() still hardcodes
its search path to scripts/. Because the scripts are now installed directly
under python/, the readdir loop will fail to discover them.

Additionally, the same browser code contains:

tools/perf/ui/browsers/scripts.c:find_scripts() {
...
#ifndef HAVE_LIBPYTHON_SUPPORT
if (strstr(lang_dirent->d_name, "python"))
continue;
#endif
...
}

Since the libpython feature name was removed and HAVE_LIBPYTHON_SUPPORT
is no longer generated, won't this check always trigger, causing the browser
to unconditionally ignore Python scripts even if the directory search path
were updated?

> diff --git a/tools/perf/tests/make b/tools/perf/tests/make
> --- a/tools/perf/tests/make
> +++ b/tools/perf/tests/make
[ ... ]
> @@ -75,8 +75,6 @@ make_jevents_all := JEVENTS_ARCH=all
> make_no_bpf_skel := BUILD_BPF_SKEL=0
> make_gen_vmlinux_h := GEN_VMLINUX_H=1
>
> -make_no_libpython := NO_LIBPYTHON=1
> -make_no_scripts := NO_LIBPYTHON=1
> make_no_slang := NO_SLANG=1
> make_no_gtk2 := NO_GTK2=1
> make_no_ui := NO_SLANG=1 NO_GTK2=1
> @@ -118,7 +116,7 @@ make_install_prefix_slash := install prefix=/tmp/krava/
> make_static := LDFLAGS=-static NO_PERF_READ_VDSO32=1 NO_PERF_READ_VDSOX32=1 NO_JVMTI=1 NO_LIBTRACEEVENT=1 NO_LIBELF=1
>
> # all the NO_* variable combined
> -make_minimal := NO_LIBPYTHON=1 NO_GTK2=1
> +make_minimal := NO_GTK2=1
> make_minimal += NO_DEMANGLE=1 NO_LIBELF=1 NO_BACKTRACE=1
> make_minimal += NO_LIBNUMA=1 NO_LIBBIONIC=1 NO_LIBDW=1
> make_minimal += NO_LIBBPF=1
> @@ -150,7 +148,6 @@ run += make_jevents_all
> run += make_no_bpf_skel
> run += make_gen_vmlinux_h
>
> -run += make_no_libpython
> run += make_no_scripts

Because the definition for make_no_scripts was removed, run += make_no_scripts
will evaluate to an empty target. Does this regression cause the test
harness to silently run a default build without passing NO_PYTHON_MODULE=1?

Similarly, should make_minimal have been updated to include NO_PYTHON_MODULE=1
instead of just dropping NO_LIBPYTHON=1? Without it, the test suite no
longer explicitly verifies that perf builds successfully without Python support.

--
Sashiko AI review · https://sashiko.dev/#/patchset/20260425174858.3922152-1-irogers@google.com?part=55