> On 27-Sep-2023, at 9:55 AM, Athira Rajeev <atraj...@linux.vnet.ibm.com> wrote:
>
>
>
>> On 27-Sep-2023, at 5:25 AM, Namhyung Kim <namhy...@kernel.org> wrote:
>>
>> On Thu, Sep 14, 2023 at 10:18 AM Athira Rajeev
>> <atraj...@linux.vnet.ibm.com> wrote:
>>>
>>> Add rule in new Makefile "tests/Makefile.tests" for running
>>> shellcheck on shell test scripts. This automates below shellcheck
>>> into the build.
>>>
>>> $ for F in $(find tests/shell/ -perm -o=x -name '*.sh'); do
>>> shellcheck -S warning $F; done
>>
>> I think you can do it if $(shell command -v shellcheck) returns
>> non-empty string (the path to the shellcheck). Then the feature
>> test logic can be gone.
>
> Ok, I will try this.
>>
>>>
>>> CONFIG_SHELLCHECK check is added to avoid build breakage in
>>> the absence of shellcheck binary. Update Makefile.perf to contain
>>> new rule for "SHELLCHECK_TEST" which is for making shellcheck
>>> test as a dependency on perf binary. Added "tests/Makefile.tests"
>>> to run shellcheck on shellscripts in tests/shell. The make rule
>>> "SHLLCHECK_RUN" ensures that, every time during make, shellcheck
>>> will be run only on modified files during subsequent invocations.
>>> By this, if any newly added shell scripts or fixes in existing
>>> scripts breaks coding/formatting style, it will get captured
>>> during the perf build.
>>
>> Can you show me the example output?
>
> Sure, I will add it.
>>
>>>
>>> Signed-off-by: Athira Rajeev <atraj...@linux.vnet.ibm.com>
>>> ---
>>> tools/perf/Makefile.perf | 12 +++++++++++-
>>> tools/perf/tests/Makefile.tests | 24 ++++++++++++++++++++++++
>>> 2 files changed, 35 insertions(+), 1 deletion(-)
>>> create mode 100644 tools/perf/tests/Makefile.tests
>>>
>>> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
>>> index f6fdc2d5a92f..c27f54771e90 100644
>>> --- a/tools/perf/Makefile.perf
>>> +++ b/tools/perf/Makefile.perf
>>> @@ -667,7 +667,16 @@ $(PERF_IN): prepare FORCE
>>> $(PMU_EVENTS_IN): FORCE prepare
>>> $(Q)$(MAKE) -f $(srctree)/tools/build/Makefile.build dir=pmu-events
>>> obj=pmu-events
>>>
>>> -$(OUTPUT)perf: $(PERFLIBS) $(PERF_IN) $(PMU_EVENTS_IN)
>>> +# Runs shellcheck on perf test shell scripts
>>> +ifeq ($(CONFIG_SHELLCHECK),y)
>>> +SHELLCHECK_TEST: FORCE prepare
>>> + $(Q)$(MAKE) -f $(srctree)/tools/perf/tests/Makefile.tests
>>> +else
>>> +SHELLCHECK_TEST:
>>> + @:
>>> +endif
>>> +
>>> +$(OUTPUT)perf: $(PERFLIBS) $(PERF_IN) $(PMU_EVENTS_IN) SHELLCHECK_TEST
>>> $(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) \
>>> $(PERF_IN) $(PMU_EVENTS_IN) $(LIBS) -o $@
>>>
>>> @@ -1129,6 +1138,7 @@ bpf-skel-clean:
>>> $(call QUIET_CLEAN, bpf-skel) $(RM) -r $(SKEL_TMP_OUT) $(SKELETONS)
>>>
>>> clean:: $(LIBAPI)-clean $(LIBBPF)-clean $(LIBSUBCMD)-clean
>>> $(LIBSYMBOL)-clean $(LIBPERF)-clean fixdep-clean python-clean
>>> bpf-skel-clean tests-coresight-targets-clean
>>> + $(Q)$(MAKE) -f $(srctree)/tools/perf/tests/Makefile.tests clean
>>> $(call QUIET_CLEAN, core-objs) $(RM) $(LIBPERF_A)
>>> $(OUTPUT)perf-archive $(OUTPUT)perf-iostat $(LANG_BINDINGS)
>>> $(Q)find $(or $(OUTPUT),.) -name '*.o' -delete -o -name '\.*.cmd'
>>> -delete -o -name '\.*.d' -delete
>>> $(Q)$(RM) $(OUTPUT).config-detected
>>> diff --git a/tools/perf/tests/Makefile.tests
>>> b/tools/perf/tests/Makefile.tests
>>> new file mode 100644
>>> index 000000000000..e74575559e83
>>> --- /dev/null
>>> +++ b/tools/perf/tests/Makefile.tests
>>> @@ -0,0 +1,24 @@
>>> +# SPDX-License-Identifier: GPL-2.0
>>> +# Athira Rajeev <atraj...@linux.vnet.ibm.com>, 2023
>>> +-include $(OUTPUT).config-detected
>>> +
>>> +log_file = $(OUTPUT)shellcheck_test.log
>>> +PROGS = $(subst ./,,$(shell find tests/shell -perm -o=x -type f -name
>>> '*.sh'))
>>> +DEPS = $(addprefix output/,$(addsuffix .dep,$(basename $(PROGS))))
>>> +DIRS = $(shell echo $(dir $(DEPS)) | xargs -n1 | sort -u | xargs)
>>> +
>>> +.PHONY: all
>>> +all: SHELLCHECK_RUN
>>> + @:
>>> +
>>> +SHELLCHECK_RUN: $(DEPS) $(DIRS)
>>> +
>>> +output/%.dep: %.sh | $(DIRS)
>>> + $(call rule_mkdir)
>>> + $(Q)$(call frecho-cmd,test)@touch $@
>>> + $(Q)$(call frecho-cmd,test)@shellcheck -S warning $(subst
>>> output/,./,$(patsubst %.dep, %.sh, $@)) 1> ${log_file} && ([[ ! -s
>>> ${log_file} ]])
>>
>> This line is too long, please wrap it with some backslashes.
> Ok
>
> I will address all the comments in next version
Hi Namhyung,
While working on V2 for the Makefile changes and testing, came across three
issues with latest scripts in perf-tools-next.
I have addressed those in below patchset:
https://lore.kernel.org/linux-perf-users/20230929041133.95355-1-atraj...@linux.vnet.ibm.com/T/#m7b3dc8a96467058e1b392183190baed47ae0eb75
[PATCH 0/3] Fix for shellcheck issues with latest scripts in tests/shell
For the Makefile.perf changes, I will send V2 separately addressing review
comments
Thanks
Athira
>
> Thanks
> Athira
>>
>> Thanks,
>> Namhyung
>>
>>
>>> +$(DIRS):
>>> + @mkdir -p $@
>>> +
>>> +clean:
>>> + @rm -rf $(log_file) output
>>> --
>>> 2.31.1
