[PATCH 2/2] tools/perf: Add perf binary dependent rule for shellcheck log in Makefile.perf

[Athira Rajeev]

> On 27-Sep-2023, at 9:55 AM, Athira Rajeev <atrajeev at linux.vnet.ibm.com> wrote:
> 
> 
> 
>> On 27-Sep-2023, at 5:25 AM, Namhyung Kim <namhyung at kernel.org> wrote:
>> 
>> On Thu, Sep 14, 2023 at 10:18 AM Athira Rajeev
>> <atrajeev at 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 <atrajeev at 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 <atrajeev at 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-atrajeev@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