[PATCH 1/2] tools/perf: Add new CONFIG_SHELLCHECK for detecting shellcheck binary

[Namhyung Kim]

Hello,

On Thu, Sep 14, 2023 at 10:18 AM Athira Rajeev
<atrajeev at linux.vnet.ibm.com> wrote:
>
> shellcheck tool can detect coding/formatting issues on
> shell scripts. In perf directory "tests/shell", there are lot
> of shell test scripts and this tool can detect coding/formatting
> issues on these scripts.
>
> Example to use shellcheck for severity level for
> errors and warnings, below command is used:
>
>    # for F in $(find tests/shell/ -perm -o=x -name '*.sh'); do shellcheck -S warning $F; done
>    # echo $?
>      0
>
> This testing needs to be automated into the build so that it
> can avoid regressions and also run the check for newly added
> during build test itself. Add a new feature check to detect
> presence of shellcheck. Add CONFIG_SHELLCHECK feature check in
> the build to avoid not having shellcheck breaking the build.
>
> Signed-off-by: Athira Rajeev <atrajeev at linux.vnet.ibm.com>
> ---
>  tools/build/Makefile.feature |  6 ++++--
>  tools/build/feature/Makefile |  8 +++++++-
>  tools/perf/Makefile.config   | 10 ++++++++++
>  3 files changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
> index 934e2777a2db..23f56b95babf 100644
> --- a/tools/build/Makefile.feature
> +++ b/tools/build/Makefile.feature
> @@ -72,7 +72,8 @@ FEATURE_TESTS_BASIC :=                  \
>          libzstd                                \
>          disassembler-four-args         \
>          disassembler-init-styled       \
> -        file-handle
> +        file-handle                    \
> +        shellcheck
>
>  # FEATURE_TESTS_BASIC + FEATURE_TESTS_EXTRA is the complete list
>  # of all feature tests
> @@ -138,7 +139,8 @@ FEATURE_DISPLAY ?=              \
>           get_cpuid              \
>           bpf                   \
>           libaio                        \
> -         libzstd
> +         libzstd               \
> +         shellcheck
>
>  #
>  # Declare group members of a feature to display the logical OR of the detection
> diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
> index 3184f387990a..44ba6d0c98d0 100644
> --- a/tools/build/feature/Makefile
> +++ b/tools/build/feature/Makefile
> @@ -76,7 +76,8 @@ FILES=                                          \
>           test-libzstd.bin                      \
>           test-clang-bpf-co-re.bin              \
>           test-file-handle.bin                  \
> -         test-libpfm4.bin
> +         test-libpfm4.bin                      \
> +         test-shellcheck.bin
>
>  FILES := $(addprefix $(OUTPUT),$(FILES))
>
> @@ -92,6 +93,8 @@ __BUILD = $(CC) $(CFLAGS) -MD -Wall -Werror -o $@ $(patsubst %.bin,%.c,$(@F)) $(
>  __BUILDXX = $(CXX) $(CXXFLAGS) -MD -Wall -Werror -o $@ $(patsubst %.bin,%.cpp,$(@F)) $(LDFLAGS)
>    BUILDXX = $(__BUILDXX) > $(@:.bin=.make.output) 2>&1
>
> +  BUILD_BINARY = sh -c $1 > $(@:.bin=.make.output) 2>&1
> +
>  ###############################
>
>  $(OUTPUT)test-all.bin:
> @@ -207,6 +210,9 @@ $(OUTPUT)test-libslang-include-subdir.bin:
>  $(OUTPUT)test-libtraceevent.bin:
>         $(BUILD) -ltraceevent
>
> +$(OUTPUT)test-shellcheck.bin:
> +       $(BUILD_BINARY) "shellcheck --version"

I don't think it'd generate the .bin file.

Anyway, it's a binary file already.  Can we check it with
`command -v` and get rid of the feature test?

Thanks,
Namhyung


> +
>  $(OUTPUT)test-libtracefs.bin:
>          $(BUILD) $(shell $(PKG_CONFIG) --cflags libtraceevent 2>/dev/null) -ltracefs
>
> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index d66b52407e19..e71fe95ad865 100644
> --- a/tools/perf/Makefile.config
> +++ b/tools/perf/Makefile.config
> @@ -779,6 +779,16 @@ ifndef NO_SLANG
>    endif
>  endif
>
> +ifneq ($(NO_SHELLCHECK),1)
> +  $(call feature_check,shellcheck)
> +  ifneq ($(feature-shellcheck), 1)
> +    msg := $(warning No shellcheck found. please install ShellCheck);
> +  else
> +    $(call detected,CONFIG_SHELLCHECK)
> +    NO_SHELLCHECK := 0
> +  endif
> +endif
> +
>  ifdef GTK2
>    FLAGS_GTK2=$(CFLAGS) $(LDFLAGS) $(EXTLIBS) $(shell $(PKG_CONFIG) --libs --cflags gtk+-2.0 2>/dev/null)
>    $(call feature_check,gtk2)
> --
> 2.31.1
>