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

[Athira Rajeev]

> On 13-Oct-2023, at 1:06 PM, 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
> 
> Condition for shellcheck is added in Makefile.perf 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.
> 
> Example build failure with present scripts in tests/shell:
> 
> INSTALL libsubcmd_headers
> INSTALL libperf_headers
> INSTALL libapi_headers
> INSTALL libsymbol_headers
> INSTALL libbpf_headers
> make[3]: *** [/root/athira/namhyung/perf-tools-next/tools/perf/tests/Makefile.tests:17: output/tests/shell/record_sideband.dep] Error 1
> make[3]: *** Waiting for unfinished jobs....
> make[3]: *** [/root/athira/namhyung/perf-tools-next/tools/perf/tests/Makefile.tests:17: output/tests/shell/test_arm_coresight.dep] Error 1
> make[3]: *** [/root/athira/namhyung/perf-tools-next/tools/perf/tests/Makefile.tests:17: output/tests/shell/lock_contention.dep] Error 1
> make[2]: *** [Makefile.perf:675: SHELLCHECK_TEST] Error 2
> make[1]: *** [Makefile.perf:238: sub-make] Error 2
> make: *** [Makefile:70: all] Error 2
> 
> After this, for testing, changed "tests/shell/record.sh" to
> break shellcheck format. In the next make run, it is
> also captured:
> 
> make[3]: *** [/root/athira/namhyung/perf-tools-next/tools/perf/tests/Makefile.tests:17: output/tests/shell/record_sideband.dep] Error 1
> make[3]: *** Waiting for unfinished jobs....
> make[3]: *** [/root/athira/namhyung/perf-tools-next/tools/perf/tests/Makefile.tests:17: output/tests/shell/record.dep] Error 1
> make[3]: *** [/root/athira/namhyung/perf-tools-next/tools/perf/tests/Makefile.tests:17: output/tests/shell/test_arm_coresight.dep] Error 1
> make[3]: *** [/root/athira/namhyung/perf-tools-next/tools/perf/tests/Makefile.tests:17: output/tests/shell/lock_contention.dep] Error 1
> make[2]: *** [Makefile.perf:675: SHELLCHECK_TEST] Error 2
> make[1]: *** [Makefile.perf:238: sub-make] Error 2
> make: *** [Makefile:70: all] Error 2
> 
> The exact failure log can be found in:
> output/tests/shell/record.dep.log file
> 
> This is reported at build time. To be able to go ahead with
> the build or disable shellcheck even though it is known that
> some test is broken, add a "NO_SHELLCHECK" option. Example:
> 
>  make NO_LIBTRACEEVENT=1 NO_SHELLCHECK=1
> 
>  INSTALL libsubcmd_headers
>  INSTALL libsymbol_headers
>  INSTALL libperf_headers
>  INSTALL libapi_headers
>  INSTALL libbpf_headers
>  PERF_VERSION = 6.6.rc1.g7108a40e02ae
>  GEN     perf-iostat
>  GEN     perf-archive
>  CC      util/header.o
>  LD      util/perf-in.o
>  LD      perf-in.o
>  LINK    perf
> 
> Signed-off-by: Athira Rajeev <atrajeev at linux.vnet.ibm.com>
> ---
> changelog:
> v2 -> v3:
> Add option "NO_SHELLCHECK". This will allow to go ahead
> with the build or disable shellcheck even though it is
> known that some test is broken
> 
> v1 -> v2:
> Version1 had shellcheck in feature check which is not
> required since shellcheck is already a binary. Presence
> of binary can be checked using:
> $(shell command -v shellcheck)
> Addressed these changes as suggested by Namhyung in V2
> Feature test logic is removed in V2. Also added example
> for build breakage when shellcheck fails in commit message

Hi All,

Looking for review comments on this patch

Thanks
Athira
> 
> tools/perf/Makefile.perf        | 20 +++++++++++++++++++-
> tools/perf/tests/Makefile.tests | 24 ++++++++++++++++++++++++
> 2 files changed, 43 insertions(+), 1 deletion(-)
> create mode 100644 tools/perf/tests/Makefile.tests
> 
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index 456872ac410d..bb49eb8b0d43 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -134,6 +134,8 @@ include ../scripts/utilities.mak
> # x86 instruction decoder - new instructions test
> #
> # Define GEN_VMLINUX_H to generate vmlinux.h from the BTF.
> +#
> +# Define NO_SHELLCHECK if you do not want to run shellcheck during build
> 
> # As per kernel Makefile, avoid funny character set dependencies
> unexport LC_ALL
> @@ -671,7 +673,22 @@ $(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
> +
> +SHELLCHECK := $(shell command -v shellcheck)
> +ifeq ($(NO_SHELLCHECK),1)
> +SHELLCHECK :=
> +endif
> +
> +ifneq ($(SHELLCHECK),)
> +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 $@
> 
> @@ -1134,6 +1151,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..8011e99768a3
> --- /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
> +
> +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)
> + $(eval input_file := $(subst output/,./,$(patsubst %.dep, %.sh, $@)))
> + $(Q)$(call frecho-cmd,test)@shellcheck -S warning ${input_file} 1> $@.log && ([[ ! -s $@.log ]])
> + $(Q)$(call frecho-cmd,test)@touch $@
> + $(Q)$(call frecho-cmd,test)@rm -rf $@.log
> +$(DIRS):
> + @mkdir -p $@
> +
> +clean:
> + @rm -rf output
> -- 
> 2.31.1
>