> On 27-Sep-2023, at 5:21 AM, Namhyung Kim <namhy...@kernel.org> wrote:
>
> Hello,
>
> On Thu, Sep 14, 2023 at 10:18 AM Athira Rajeev
> <atraj...@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 <atraj...@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
Hi Namhyung,
Thanks for the review. Sure, I will check on this
Athira
>
>
>> +
>> $(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
