[PATCH V2 00/26] tools/perf: Fix shellcheck coding/formatting issues of perf tool shell scripts
Athira Rajeev
atrajeev at linux.vnet.ibm.com
Thu Jul 20 15:12:15 AEST 2023
> On 19-Jul-2023, at 11:16 PM, Ian Rogers <irogers at google.com> wrote:
>
> On Tue, Jul 18, 2023 at 11:17 PM kajoljain <kjain at linux.ibm.com> wrote:
>>
>> Hi,
>>
>> Looking for review comments on this patchset.
>>
>> Thanks,
>> Kajol Jain
>>
>>
>> On 7/9/23 23:57, Athira Rajeev wrote:
>>> Patchset covers a set of fixes for coding/formatting issues observed while
>>> running shellcheck tool on the perf shell scripts.
>>>
>>> This cleanup is a pre-requisite to include a build option for shellcheck
>>> discussed here: https://www.spinics.net/lists/linux-perf-users/msg25553.html
>>> First set of patches were posted here:
>>> https://lore.kernel.org/linux-perf-users/53B7D823-1570-4289-A632-2205EE2B522C@linux.vnet.ibm.com/T/#t
>>>
>>> This patchset covers remaining set of shell scripts which needs
>>> fix. Patch 1 is resubmission of patch 6 from the initial series.
>>> Patch 15, 16 and 22 touches code from tools/perf/trace/beauty.
>>> Other patches are fixes for scripts from tools/perf/tests.
>>>
>>> The shellcheck is run for severity level for errors and warnings.
>>> Command used:
>>>
>>> # for F in $(find tests/shell/ -perm -o=x -name '*.sh'); do shellcheck -S warning $F; done
>>> # echo $?
>>> 0
>>>
>
> I don't see anything objectionable in the changes so for the series:
> Acked-by: Ian Rogers <irogers at google.com>
>
> Some thoughts:
> - Adding "#!/bin/bash" to scripts in tools/perf/tests/lib - I think
> we didn't do this to avoid these being included as tests. There are
> now extra checks when finding shell tests, so I can imagine doing this
> isn't a regression but just a heads up.
> - I think James' comment was addressed:
> https://lore.kernel.org/linux-perf-users/334989bf-5501-494c-f246-81878fd2fed8@arm.com/
> - Why aren't these changes being mailed to LKML? The wider community
> on LKML have thoughts on shell scripts, plus it makes the changes miss
> my mail filters.
> - Can we automate this testing into the build? For example, following
> a similar kernel build pattern we run a python test and make the log
> output a requirement here:
> https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/pmu-events/Build?h=perf-tools-next#n30
> I think we can translate:
> for F in $(find tests/shell/ -perm -o=x -name '*.sh'); do shellcheck
> -S warning $F; done
> into a rule in make for log files that are then a dependency on the
> perf binary. We can then parallel shellcheck during the build and
> avoid regressions. We probably need a CONFIG_SHELLCHECK feature check
> in the build to avoid not having shellcheck breaking the build.
Hi Ian
Thanks for the comments.
Yes, next step after this is to include build option for shellcheck by updating Makefile.
We will surely get into that build option enablement patch once we have all these corrections in place.
Thanks
Athira
>
> Thanks,
> Ian
>
>>> Changelog:
>>> v1 -> v2:
>>> - Rebased on top of perf-tools-next from:
>>> https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/log/?h=perf-tools-next
>>>
>>> - Fixed shellcheck errors and warnings reported for newly
>>> added changes from perf-tools-next branch
>>>
>>> - Addressed review comment from James clark for patch
>>> number 13 from V1. The changes in patch 13 were not necessary
>>> since the file "tests/shell/lib/coresight.sh" is sourced from
>>> other test files.
>>>
>>> Akanksha J N (1):
>>> tools/perf/tests: Fix shellcheck warnings for
>>> trace+probe_vfs_getname.sh
>>>
>>> Athira Rajeev (14):
>>> tools/perf/tests: fix test_arm_spe_fork.sh signal case issues
>>> tools/perf/tests: Fix unused variable references in
>>> stat+csv_summary.sh testcase
>>> tools/perf/tests: fix shellcheck warning for
>>> test_perf_data_converter_json.sh testcase
>>> tools/perf/tests: Fix shellcheck issue for stat_bpf_counters.sh
>>> testcase
>>> tools/perf/tests: Fix shellcheck issues in
>>> tests/shell/stat+shadow_stat.sh tetscase
>>> tools/perf/tests: Fix shellcheck warnings for
>>> thread_loop_check_tid_10.sh
>>> tools/perf/tests: Fix shellcheck warnings for unroll_loop_thread_10.sh
>>> tools/perf/tests: Fix shellcheck warnings for lib/probe_vfs_getname.sh
>>> tools/perf/tests: Fix the shellcheck warnings in lib/waiting.sh
>>> tools/perf/trace: Fix x86_arch_prctl.sh to address shellcheck warnings
>>> tools/perf/arch/x86: Fix syscalltbl.sh to address shellcheck warnings
>>> tools/perf/tests/shell: Fix the shellcheck warnings in
>>> record+zstd_comp_decomp.sh
>>> tools/perf/tests/shell: Fix shellcheck warning for stat+std_output.sh
>>> testcase
>>> tools/perf/tests: Fix shellcheck warning for stat+std_output.sh
>>> testcase
>>>
>>> Kajol Jain (11):
>>> tools/perf/tests: Fix shellcheck warning for probe_vfs_getname.sh
>>> testcase
>>> tools/perf/tests: Fix shellcheck warning for record_offcpu.sh testcase
>>> tools/perf/tests: Fix shellcheck issue for lock_contention.sh testcase
>>> tools/perf/tests: Fix shellcheck issue for stat_bpf_counters_cgrp.sh
>>> testcase
>>> tools/perf/tests: Fix shellcheck warning for asm_pure_loop.sh shell
>>> script
>>> tools/perf/tests: Fix shellcheck warning for memcpy_thread_16k_10.sh
>>> shell script
>>> tools/perf/tests: Fix shellcheck warning for probe.sh shell script
>>> tools/perf/trace: Fix shellcheck issue for arch_errno_names.sh script
>>> tools/perf: Fix shellcheck issue for check-headers.sh script
>>> tools/shell/coresight: Fix shellcheck warning for
>>> thread_loop_check_tid_2.sh shell script
>>> tools/perf/tests/shell/lib: Fix shellcheck warning for stat_output.sh
>>> shell script
>>>
>>> .../arch/x86/entry/syscalls/syscalltbl.sh | 2 +-
>>> tools/perf/check-headers.sh | 6 ++--
>>> .../tests/shell/coresight/asm_pure_loop.sh | 2 +-
>>> .../shell/coresight/memcpy_thread_16k_10.sh | 2 +-
>>> .../coresight/thread_loop_check_tid_10.sh | 2 +-
>>> .../coresight/thread_loop_check_tid_2.sh | 2 +-
>>> .../shell/coresight/unroll_loop_thread_10.sh | 2 +-
>>> tools/perf/tests/shell/lib/probe.sh | 1 +
>>> .../perf/tests/shell/lib/probe_vfs_getname.sh | 5 ++--
>>> tools/perf/tests/shell/lib/stat_output.sh | 1 +
>>> tools/perf/tests/shell/lib/waiting.sh | 1 +
>>> tools/perf/tests/shell/lock_contention.sh | 12 ++++----
>>> tools/perf/tests/shell/probe_vfs_getname.sh | 4 +--
>>> .../tests/shell/record+zstd_comp_decomp.sh | 14 +++++-----
>>> tools/perf/tests/shell/record_offcpu.sh | 6 ++--
>>> tools/perf/tests/shell/stat+csv_output.sh | 2 +-
>>> tools/perf/tests/shell/stat+csv_summary.sh | 4 +--
>>> tools/perf/tests/shell/stat+shadow_stat.sh | 4 +--
>>> tools/perf/tests/shell/stat+std_output.sh | 3 +-
>>> tools/perf/tests/shell/stat_bpf_counters.sh | 4 +--
>>> .../tests/shell/stat_bpf_counters_cgrp.sh | 28 ++++++++-----------
>>> tools/perf/tests/shell/test_arm_spe_fork.sh | 2 +-
>>> .../shell/test_perf_data_converter_json.sh | 2 +-
>>> .../tests/shell/trace+probe_vfs_getname.sh | 6 ++--
>>> tools/perf/trace/beauty/arch_errno_names.sh | 15 ++++------
>>> tools/perf/trace/beauty/x86_arch_prctl.sh | 6 ++--
>>> 26 files changed, 67 insertions(+), 71 deletions(-)
More information about the Linuxppc-dev
mailing list