> 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
Thanks
Athira
>
> Thanks,
> Namhyung
>
>
>> +$(DIRS):
>> + @mkdir -p $@
>> +
>> +clean:
>> + @rm -rf $(log_file) output
>> --
>> 2.31.1
