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

sashiko-bot
2026-04-23T04:33:45Z

Sashiko AI review found 3 potential issue(s):
- [High] The TUI script browser fails to locate Python scripts due to hardcoded legacy paths.
- [High] Standalone Python scripts are installed with non-executable permissions (`-m 644`), breaking script execution.
- [Medium] Test targets `make_no_scripts` and `make_minimal` no longer disable Python module support.
--

commit 615391f35d543aa8e125cf128454ccde6c3b24ac
Author: Ian Rogers <irogers@google.com>

perf Makefile: Update Python script installation path

Remove libpython feature test that is now a python-module feature
test. Update references from libpython to python-module accordingly.

[ ... ]

> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index 2020532bab9cf..e50b1e8cf85df 100644
> --- 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

Since the commit message mentions a move to standalone scripts, will
installing these python files with non-executable permissions (-m 644)
prevent them from being run?

If scripts are transitioned to be executed directly via execvp(), it looks
like they will fail with a permission denied error because they lack execute
permissions.

This isn't a bug in the modified file, but does changing the installation
directory to no longer use 'scripts/' break script discovery in the TUI
script browser?

Looking at tools/perf/ui/browsers/scripts.c:find_scripts():

{
char scripts_path[PATH_MAX];

snprintf(scripts_path, sizeof(scripts_path), "%s/scripts", exec_path);
scripts_dir_fd = open(scripts_path, O_DIRECTORY);
pr_err("Failed to open directory '%s'", scripts_path);

Will the interactive TUI script browser fail to locate or execute any
installed python scripts because it still hardcodes the old lookup path?

[ ... ]

> diff --git a/tools/perf/tests/make b/tools/perf/tests/make
> index 31b064928cfca..f2c5f1c254a7c 100644
> --- 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

By removing NO_LIBPYTHON=1 from make_no_scripts without replacing it with
the new NO_PYTHON_MODULE=1 flag, does this target fail to disable Python
module support?

It seems make_no_scripts now evaluates to an empty configuration rather than
a script-less build test.

[ ... ]

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

Similarly, will make_minimal incorrectly include Python support if the
development headers are present on the host system? Does this invalidate the
minimal build test because it no longer represents a truly minimal
configuration?

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