* [PATCH 1/2] tools/perf: Add new CONFIG_SHELLCHECK for detecting shellcheck binary
@ 2023-09-14 17:18 Athira Rajeev
2023-09-14 17:18 ` [PATCH 2/2] tools/perf: Add perf binary dependent rule for shellcheck log in Makefile.perf Athira Rajeev
2023-09-26 23:51 ` [PATCH 1/2] tools/perf: Add new CONFIG_SHELLCHECK for detecting shellcheck binary Namhyung Kim
0 siblings, 2 replies; 7+ messages in thread
From: Athira Rajeev @ 2023-09-14 17:18 UTC (permalink / raw)
To: acme, jolsa, adrian.hunter, irogers, namhyung
Cc: linux-perf-users, linuxppc-dev, maddy, atrajeev, kjain, disgoel
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@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"
+
$(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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] tools/perf: Add perf binary dependent rule for shellcheck log in Makefile.perf
2023-09-14 17:18 [PATCH 1/2] tools/perf: Add new CONFIG_SHELLCHECK for detecting shellcheck binary Athira Rajeev
@ 2023-09-14 17:18 ` Athira Rajeev
2023-09-26 23:55 ` Namhyung Kim
2023-09-26 23:51 ` [PATCH 1/2] tools/perf: Add new CONFIG_SHELLCHECK for detecting shellcheck binary Namhyung Kim
1 sibling, 1 reply; 7+ messages in thread
From: Athira Rajeev @ 2023-09-14 17:18 UTC (permalink / raw)
To: acme, jolsa, adrian.hunter, irogers, namhyung
Cc: linux-perf-users, linuxppc-dev, maddy, atrajeev, kjain, disgoel
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
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.
Signed-off-by: Athira Rajeev <atrajeev@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@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} ]])
+$(DIRS):
+ @mkdir -p $@
+
+clean:
+ @rm -rf $(log_file) output
--
2.31.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] tools/perf: Add new CONFIG_SHELLCHECK for detecting shellcheck binary
2023-09-14 17:18 [PATCH 1/2] tools/perf: Add new CONFIG_SHELLCHECK for detecting shellcheck binary Athira Rajeev
2023-09-14 17:18 ` [PATCH 2/2] tools/perf: Add perf binary dependent rule for shellcheck log in Makefile.perf Athira Rajeev
@ 2023-09-26 23:51 ` Namhyung Kim
2023-09-27 4:23 ` Athira Rajeev
1 sibling, 1 reply; 7+ messages in thread
From: Namhyung Kim @ 2023-09-26 23:51 UTC (permalink / raw)
To: Athira Rajeev
Cc: acme, jolsa, adrian.hunter, irogers, linux-perf-users,
linuxppc-dev, maddy, kjain, disgoel
Hello,
On Thu, Sep 14, 2023 at 10:18 AM Athira Rajeev
<atrajeev@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@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
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] tools/perf: Add perf binary dependent rule for shellcheck log in Makefile.perf
2023-09-14 17:18 ` [PATCH 2/2] tools/perf: Add perf binary dependent rule for shellcheck log in Makefile.perf Athira Rajeev
@ 2023-09-26 23:55 ` Namhyung Kim
2023-09-27 4:25 ` Athira Rajeev
0 siblings, 1 reply; 7+ messages in thread
From: Namhyung Kim @ 2023-09-26 23:55 UTC (permalink / raw)
To: Athira Rajeev
Cc: acme, jolsa, adrian.hunter, irogers, linux-perf-users,
linuxppc-dev, maddy, kjain, disgoel
On Thu, Sep 14, 2023 at 10:18 AM Athira Rajeev
<atrajeev@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.
>
> 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?
>
> Signed-off-by: Athira Rajeev <atrajeev@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@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.
Thanks,
Namhyung
> +$(DIRS):
> + @mkdir -p $@
> +
> +clean:
> + @rm -rf $(log_file) output
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] tools/perf: Add new CONFIG_SHELLCHECK for detecting shellcheck binary
2023-09-26 23:51 ` [PATCH 1/2] tools/perf: Add new CONFIG_SHELLCHECK for detecting shellcheck binary Namhyung Kim
@ 2023-09-27 4:23 ` Athira Rajeev
0 siblings, 0 replies; 7+ messages in thread
From: Athira Rajeev @ 2023-09-27 4:23 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Adrian Hunter, Ian Rogers,
linux-perf-users, linuxppc-dev, Madhavan Srinivasan, Kajol Jain,
Disha Goel
> On 27-Sep-2023, at 5:21 AM, Namhyung Kim <namhyung@kernel.org> wrote:
>
> Hello,
>
> On Thu, Sep 14, 2023 at 10:18 AM Athira Rajeev
> <atrajeev@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@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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] tools/perf: Add perf binary dependent rule for shellcheck log in Makefile.perf
2023-09-26 23:55 ` Namhyung Kim
@ 2023-09-27 4:25 ` Athira Rajeev
2023-09-29 4:18 ` Athira Rajeev
0 siblings, 1 reply; 7+ messages in thread
From: Athira Rajeev @ 2023-09-27 4:25 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Adrian Hunter, irogers,
linux-perf-users, linuxppc-dev, maddy, kjain, disgoel
> On 27-Sep-2023, at 5:25 AM, Namhyung Kim <namhyung@kernel.org> wrote:
>
> On Thu, Sep 14, 2023 at 10:18 AM Athira Rajeev
> <atrajeev@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@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@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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] tools/perf: Add perf binary dependent rule for shellcheck log in Makefile.perf
2023-09-27 4:25 ` Athira Rajeev
@ 2023-09-29 4:18 ` Athira Rajeev
0 siblings, 0 replies; 7+ messages in thread
From: Athira Rajeev @ 2023-09-29 4:18 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Adrian Hunter, Ian Rogers,
linux-perf-users, linuxppc-dev, Madhavan Srinivasan, Kajol Jain,
disgoel
> On 27-Sep-2023, at 9:55 AM, Athira Rajeev <atrajeev@linux.vnet.ibm.com> wrote:
>
>
>
>> On 27-Sep-2023, at 5:25 AM, Namhyung Kim <namhyung@kernel.org> wrote:
>>
>> On Thu, Sep 14, 2023 at 10:18 AM Athira Rajeev
>> <atrajeev@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@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@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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-09-29 4:18 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-14 17:18 [PATCH 1/2] tools/perf: Add new CONFIG_SHELLCHECK for detecting shellcheck binary Athira Rajeev
2023-09-14 17:18 ` [PATCH 2/2] tools/perf: Add perf binary dependent rule for shellcheck log in Makefile.perf Athira Rajeev
2023-09-26 23:55 ` Namhyung Kim
2023-09-27 4:25 ` Athira Rajeev
2023-09-29 4:18 ` Athira Rajeev
2023-09-26 23:51 ` [PATCH 1/2] tools/perf: Add new CONFIG_SHELLCHECK for detecting shellcheck binary Namhyung Kim
2023-09-27 4:23 ` Athira Rajeev
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).