Re: [PATCH v1 55/58] perf Makefile: Update Python script installation path

sashiko-bot
2026-04-20T00:54:12Z

Sashiko AI review found 4 potential issue(s):
- [Medium] The `make_no_scripts` test target in `tools/perf/tests/make` is broken and fails to test building without the Python module.
- [Medium] Internal script resolution logic in `tools/perf/ui/browsers/scripts.c` was not updated to reflect the new `python` directory path.
- [Medium] The shell tests `script.sh` and `test_arm_coresight_disasm.sh` use hardcoded legacy paths to locate test scripts.
- [Low] Documentation files still explicitly reference the legacy `scripts/python` directories.
--

commit 5379ea21d984f43efaab5ea1317e05b8987445a4
Author: Ian Rogers <irogers@google.com>

perf Makefile: Update Python script installation path

Remove the libpython feature test since it is now a python-module feature
test, and update references accordingly. Remove references to the legacy
scripts/python directory and install scripts directly to the python
directory under libexec.

> Remove references to legacy 'scripts/python' directory and install
> scripts directly to 'python' directory under libexec. This aligns with
> the removal of embedded interpreter and move to standalone
> scripts.

Are there other areas that need to be updated to reflect this new path?

For example, find_scripts() in tools/perf/ui/browsers/scripts.c appears to
still search in "%s/scripts". Will this cause the UI script browser to fail
to locate the new Python scripts?

Similarly, do the shell tests in tools/perf/tests/shell/script.sh and
tools/perf/tests/shell/test_arm_coresight_disasm.sh need to be updated? They
use hardcoded paths like "../../scripts/python/parallel-perf.py" and
"../../scripts/python/arm-cs-trace-disasm.py" which seem like they will now
be skipped incorrectly or fail.

Also, does tools/perf/Documentation/db-export.txt need an update? It still
references legacy paths like tools/perf/scripts/python/export-to-sqlite.py.

> 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

[ ... ]

> @@ -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

Should NO_PYTHON_MODULE=1 be added here to maintain the minimal build test
coverage?

[ ... ]

> @@ -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

Since the make_no_scripts assignment was removed above, does leaving it in
the run loop evaluate to an empty string and cause the test harness to run a
default make command without any disabling flags?

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