* [PATCH] perf build: Specify that spellcheck should use the bash dialect.
@ 2025-06-13 3:36 Collin Funk
2025-06-19 10:28 ` James Clark
` (2 more replies)
0 siblings, 3 replies; 18+ messages in thread
From: Collin Funk @ 2025-06-13 3:36 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Alexander Shishkin, Jiri Olsa, Ian Rogers,
Adrian Hunter, Liang, Kan, James Clark
Cc: linux-perf-users, linux-kernel, Collin Funk
When someone has a global shellcheckrc file, for example at
~/.config/shellcheckrc, with the directive 'shell=sh', building perf
will fail with many shellcheck errors like:
In tests/shell/base_probe/test_adding_kernel.sh line 294:
(( TEST_RESULT += $? ))
^---------------------^ SC3006 (warning): In POSIX sh, standalone ((..)) is undefined.
For more information:
https://www.shellcheck.net/wiki/SC3006 -- In POSIX sh, standalone ((..)) is...
make[5]: *** [tests/Build:91: tests/shell/base_probe/test_adding_kernel.sh.shellcheck_log] Error 1
Passing the '-s bash' option ensures that it runs correctly regardless
of a developers global configuration.
Signed-off-by: Collin Funk <collin.funk1@gmail.com>
---
tools/perf/tests/Build | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
index 2181f5a92148..26efc5d20f6c 100644
--- a/tools/perf/tests/Build
+++ b/tools/perf/tests/Build
@@ -89,7 +89,7 @@ endif
$(OUTPUT)%.shellcheck_log: %
$(call rule_mkdir)
- $(Q)$(call echo-cmd,test)shellcheck -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
+ $(Q)$(call echo-cmd,test)shellcheck -s bash -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
perf-test-y += $(SHELL_TEST_LOGS)
--
2.49.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] perf build: Specify that spellcheck should use the bash dialect.
2025-06-13 3:36 [PATCH] perf build: Specify that spellcheck should use the bash dialect Collin Funk
@ 2025-06-19 10:28 ` James Clark
2025-06-20 17:40 ` Collin Funk
2025-06-20 19:49 ` Namhyung Kim
2025-06-24 2:05 ` [PATCH v2] [PATCH] perf build: Specify that shellcheck " Collin Funk
2025-06-24 5:44 ` [PATCH v3] " Collin Funk
2 siblings, 2 replies; 18+ messages in thread
From: James Clark @ 2025-06-19 10:28 UTC (permalink / raw)
To: Collin Funk
Cc: linux-perf-users, linux-kernel, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Alexander Shishkin,
Jiri Olsa, Ian Rogers, Adrian Hunter, Liang, Kan
On 13/06/2025 4:36 am, Collin Funk wrote:
> When someone has a global shellcheckrc file, for example at
> ~/.config/shellcheckrc, with the directive 'shell=sh', building perf
> will fail with many shellcheck errors like:
>
> In tests/shell/base_probe/test_adding_kernel.sh line 294:
> (( TEST_RESULT += $? ))
> ^---------------------^ SC3006 (warning): In POSIX sh, standalone ((..)) is undefined.
>
> For more information:
> https://www.shellcheck.net/wiki/SC3006 -- In POSIX sh, standalone ((..)) is...
> make[5]: *** [tests/Build:91: tests/shell/base_probe/test_adding_kernel.sh.shellcheck_log] Error 1
>
> Passing the '-s bash' option ensures that it runs correctly regardless
> of a developers global configuration.
>
> Signed-off-by: Collin Funk <collin.funk1@gmail.com>
> ---
> tools/perf/tests/Build | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
> index 2181f5a92148..26efc5d20f6c 100644
> --- a/tools/perf/tests/Build
> +++ b/tools/perf/tests/Build
> @@ -89,7 +89,7 @@ endif
>
> $(OUTPUT)%.shellcheck_log: %
> $(call rule_mkdir)
> - $(Q)$(call echo-cmd,test)shellcheck -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
> + $(Q)$(call echo-cmd,test)shellcheck -s bash -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
>
> perf-test-y += $(SHELL_TEST_LOGS)
>
If we're enforcing bash style with static analysis shouldn't we also
change all the hashbangs to bash? Recently there have been changes to
change sh to bash in some of the tests so presumably the hard rule for
sh is no more?
In the past I've had to replace bashisms that didn't work in sh but it
would be nice to have only one language to write tests in. I doubt
anyone running the tests today is running somewhere without bash, or
that changing it will break anything. If anything it will fix more
bashisms that have already been written.
Just for reference there are 34 #!/bin/bash and 42 #!/bin/sh in
tools/perf/tests
Thanks
James
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] perf build: Specify that spellcheck should use the bash dialect.
2025-06-19 10:28 ` James Clark
@ 2025-06-20 17:40 ` Collin Funk
2025-06-23 8:10 ` James Clark
2025-06-20 19:49 ` Namhyung Kim
1 sibling, 1 reply; 18+ messages in thread
From: Collin Funk @ 2025-06-20 17:40 UTC (permalink / raw)
To: James Clark
Cc: linux-perf-users, linux-kernel, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Alexander Shishkin,
Jiri Olsa, Ian Rogers, Adrian Hunter, Liang, Kan
Hi James,
James Clark <james.clark@linaro.org> writes:
> If we're enforcing bash style with static analysis shouldn't we also
> change all the hashbangs to bash? Recently there have been changes to
> change sh to bash in some of the tests so presumably the hard rule for
> sh is no more?
>
> In the past I've had to replace bashisms that didn't work in sh but it
> would be nice to have only one language to write tests in. I doubt
> anyone running the tests today is running somewhere without bash, or
> that changing it will break anything. If anything it will fix more
> bashisms that have already been written.
>
> Just for reference there are 34 #!/bin/bash and 42 #!/bin/sh in
> tools/perf/tests
That sounds reasonable to me. Writing portable shell is a hassle and if
we already assume a working /bin/bash in some places, I don't see a
reason not to use it for the others.
Regarding this patch, shellcheck will use the file extension or shebang
only if it does not find a 'shell' directive in a .shellcheckrc. So that
change will still require this patch.
I saw it was used in other places, so I assumed this patch was fine:
$ find tools/perf -name Build | xargs grep bash
tools/perf/Build: $(Q)$(call echo-cmd,test)shellcheck -s bash -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
tools/perf/trace/beauty/Build: $(Q)$(call echo-cmd,test)shellcheck -s bash -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
Collin
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] perf build: Specify that spellcheck should use the bash dialect.
2025-06-19 10:28 ` James Clark
2025-06-20 17:40 ` Collin Funk
@ 2025-06-20 19:49 ` Namhyung Kim
2025-06-23 8:08 ` James Clark
1 sibling, 1 reply; 18+ messages in thread
From: Namhyung Kim @ 2025-06-20 19:49 UTC (permalink / raw)
To: James Clark
Cc: Collin Funk, linux-perf-users, linux-kernel, Peter Zijlstra,
Ingo Molnar, Arnaldo Carvalho de Melo, Alexander Shishkin,
Jiri Olsa, Ian Rogers, Adrian Hunter, Liang, Kan
Hello,
On Thu, Jun 19, 2025 at 11:28:46AM +0100, James Clark wrote:
>
>
> On 13/06/2025 4:36 am, Collin Funk wrote:
> > When someone has a global shellcheckrc file, for example at
> > ~/.config/shellcheckrc, with the directive 'shell=sh', building perf
> > will fail with many shellcheck errors like:
> >
> > In tests/shell/base_probe/test_adding_kernel.sh line 294:
> > (( TEST_RESULT += $? ))
> > ^---------------------^ SC3006 (warning): In POSIX sh, standalone ((..)) is undefined.
> >
> > For more information:
> > https://www.shellcheck.net/wiki/SC3006 -- In POSIX sh, standalone ((..)) is...
> > make[5]: *** [tests/Build:91: tests/shell/base_probe/test_adding_kernel.sh.shellcheck_log] Error 1
> >
> > Passing the '-s bash' option ensures that it runs correctly regardless
> > of a developers global configuration.
> >
> > Signed-off-by: Collin Funk <collin.funk1@gmail.com>
> > ---
> > tools/perf/tests/Build | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
> > index 2181f5a92148..26efc5d20f6c 100644
> > --- a/tools/perf/tests/Build
> > +++ b/tools/perf/tests/Build
> > @@ -89,7 +89,7 @@ endif
> > $(OUTPUT)%.shellcheck_log: %
> > $(call rule_mkdir)
> > - $(Q)$(call echo-cmd,test)shellcheck -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
> > + $(Q)$(call echo-cmd,test)shellcheck -s bash -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
> > perf-test-y += $(SHELL_TEST_LOGS)
>
> If we're enforcing bash style with static analysis shouldn't we also change
> all the hashbangs to bash? Recently there have been changes to change sh to
> bash in some of the tests so presumably the hard rule for sh is no more?
>
> In the past I've had to replace bashisms that didn't work in sh but it would
> be nice to have only one language to write tests in. I doubt anyone running
> the tests today is running somewhere without bash, or that changing it will
> break anything. If anything it will fix more bashisms that have already been
> written.
>
> Just for reference there are 34 #!/bin/bash and 42 #!/bin/sh in
> tools/perf/tests
Thanks for raising the concern. I agree that having one standard is a
way to go but I really don't have preference between those shells.
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] perf build: Specify that spellcheck should use the bash dialect.
2025-06-20 19:49 ` Namhyung Kim
@ 2025-06-23 8:08 ` James Clark
0 siblings, 0 replies; 18+ messages in thread
From: James Clark @ 2025-06-23 8:08 UTC (permalink / raw)
To: Namhyung Kim
Cc: Collin Funk, linux-perf-users, linux-kernel, Peter Zijlstra,
Ingo Molnar, Arnaldo Carvalho de Melo, Alexander Shishkin,
Jiri Olsa, Ian Rogers, Adrian Hunter, Liang, Kan
On 20/06/2025 8:49 pm, Namhyung Kim wrote:
> Hello,
>
> On Thu, Jun 19, 2025 at 11:28:46AM +0100, James Clark wrote:
>>
>>
>> On 13/06/2025 4:36 am, Collin Funk wrote:
>>> When someone has a global shellcheckrc file, for example at
>>> ~/.config/shellcheckrc, with the directive 'shell=sh', building perf
>>> will fail with many shellcheck errors like:
>>>
>>> In tests/shell/base_probe/test_adding_kernel.sh line 294:
>>> (( TEST_RESULT += $? ))
>>> ^---------------------^ SC3006 (warning): In POSIX sh, standalone ((..)) is undefined.
>>>
>>> For more information:
>>> https://www.shellcheck.net/wiki/SC3006 -- In POSIX sh, standalone ((..)) is...
>>> make[5]: *** [tests/Build:91: tests/shell/base_probe/test_adding_kernel.sh.shellcheck_log] Error 1
>>>
>>> Passing the '-s bash' option ensures that it runs correctly regardless
>>> of a developers global configuration.
>>>
>>> Signed-off-by: Collin Funk <collin.funk1@gmail.com>
>>> ---
>>> tools/perf/tests/Build | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
>>> index 2181f5a92148..26efc5d20f6c 100644
>>> --- a/tools/perf/tests/Build
>>> +++ b/tools/perf/tests/Build
>>> @@ -89,7 +89,7 @@ endif
>>> $(OUTPUT)%.shellcheck_log: %
>>> $(call rule_mkdir)
>>> - $(Q)$(call echo-cmd,test)shellcheck -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
>>> + $(Q)$(call echo-cmd,test)shellcheck -s bash -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
>>> perf-test-y += $(SHELL_TEST_LOGS)
>>
>> If we're enforcing bash style with static analysis shouldn't we also change
>> all the hashbangs to bash? Recently there have been changes to change sh to
>> bash in some of the tests so presumably the hard rule for sh is no more?
>>
>> In the past I've had to replace bashisms that didn't work in sh but it would
>> be nice to have only one language to write tests in. I doubt anyone running
>> the tests today is running somewhere without bash, or that changing it will
>> break anything. If anything it will fix more bashisms that have already been
>> written.
>>
>> Just for reference there are 34 #!/bin/bash and 42 #!/bin/sh in
>> tools/perf/tests
>
> Thanks for raising the concern. I agree that having one standard is a
> way to go but I really don't have preference between those shells.
>
> Thanks,
> Namhyung
>
I would vote for bash then, just because it has a few extra builtins
that are sometimes useful. I can send a patch that does the change to
see if anyone objects.
Thanks
James
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] perf build: Specify that spellcheck should use the bash dialect.
2025-06-20 17:40 ` Collin Funk
@ 2025-06-23 8:10 ` James Clark
2025-06-23 16:37 ` Ian Rogers
0 siblings, 1 reply; 18+ messages in thread
From: James Clark @ 2025-06-23 8:10 UTC (permalink / raw)
To: Collin Funk
Cc: linux-perf-users, linux-kernel, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Alexander Shishkin,
Jiri Olsa, Ian Rogers, Adrian Hunter, Liang, Kan
On 20/06/2025 6:40 pm, Collin Funk wrote:
> Hi James,
>
> James Clark <james.clark@linaro.org> writes:
>
>> If we're enforcing bash style with static analysis shouldn't we also
>> change all the hashbangs to bash? Recently there have been changes to
>> change sh to bash in some of the tests so presumably the hard rule for
>> sh is no more?
>>
>> In the past I've had to replace bashisms that didn't work in sh but it
>> would be nice to have only one language to write tests in. I doubt
>> anyone running the tests today is running somewhere without bash, or
>> that changing it will break anything. If anything it will fix more
>> bashisms that have already been written.
>>
>> Just for reference there are 34 #!/bin/bash and 42 #!/bin/sh in
>> tools/perf/tests
>
> That sounds reasonable to me. Writing portable shell is a hassle and if
> we already assume a working /bin/bash in some places, I don't see a
> reason not to use it for the others.
>
> Regarding this patch, shellcheck will use the file extension or shebang
> only if it does not find a 'shell' directive in a .shellcheckrc. So that
> change will still require this patch.
>
> I saw it was used in other places, so I assumed this patch was fine:
>
> $ find tools/perf -name Build | xargs grep bash
> tools/perf/Build: $(Q)$(call echo-cmd,test)shellcheck -s bash -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
> tools/perf/trace/beauty/Build: $(Q)$(call echo-cmd,test)shellcheck -s bash -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
>
> Collin
In that case:
Reviewed-by: James Clark <james.clark@linaro.org>
And I'll send the bulk hashbang change separately.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] perf build: Specify that spellcheck should use the bash dialect.
2025-06-23 8:10 ` James Clark
@ 2025-06-23 16:37 ` Ian Rogers
2025-06-24 2:08 ` Collin Funk
0 siblings, 1 reply; 18+ messages in thread
From: Ian Rogers @ 2025-06-23 16:37 UTC (permalink / raw)
To: James Clark
Cc: Collin Funk, linux-perf-users, linux-kernel, Peter Zijlstra,
Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim,
Alexander Shishkin, Jiri Olsa, Adrian Hunter, Liang, Kan
On Mon, Jun 23, 2025 at 1:10 AM James Clark <james.clark@linaro.org> wrote:
>
>
>
> On 20/06/2025 6:40 pm, Collin Funk wrote:
> > Hi James,
> >
> > James Clark <james.clark@linaro.org> writes:
> >
> >> If we're enforcing bash style with static analysis shouldn't we also
> >> change all the hashbangs to bash? Recently there have been changes to
> >> change sh to bash in some of the tests so presumably the hard rule for
> >> sh is no more?
> >>
> >> In the past I've had to replace bashisms that didn't work in sh but it
> >> would be nice to have only one language to write tests in. I doubt
> >> anyone running the tests today is running somewhere without bash, or
> >> that changing it will break anything. If anything it will fix more
> >> bashisms that have already been written.
> >>
> >> Just for reference there are 34 #!/bin/bash and 42 #!/bin/sh in
> >> tools/perf/tests
> >
> > That sounds reasonable to me. Writing portable shell is a hassle and if
> > we already assume a working /bin/bash in some places, I don't see a
> > reason not to use it for the others.
> >
> > Regarding this patch, shellcheck will use the file extension or shebang
> > only if it does not find a 'shell' directive in a .shellcheckrc. So that
> > change will still require this patch.
> >
> > I saw it was used in other places, so I assumed this patch was fine:
> >
> > $ find tools/perf -name Build | xargs grep bash
> > tools/perf/Build: $(Q)$(call echo-cmd,test)shellcheck -s bash -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
> > tools/perf/trace/beauty/Build: $(Q)$(call echo-cmd,test)shellcheck -s bash -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
> >
> > Collin
>
> In that case:
>
> Reviewed-by: James Clark <james.clark@linaro.org>
>
> And I'll send the bulk hashbang change separately.
I've no objection to switching to using bash globally. It seems
sub-optimal that we've copy-pasted the shellcheck command across many
different Build files and that this patch will cause the
tools/perf/tests/Build one to differ. My preference would be to have a
global definition probably in Makefile.perf, then use it consistently.
Alternative all shellcheck invocations can pass "-s bash" for the sake
of consistency. Fwiw, I think the 'tools/arch/x86/tools/gen-insn-*'
which is to some extent taken from the kernel's 'arch/x86/tools' is
okay with the change too.
Thanks,
Ian
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2] [PATCH] perf build: Specify that shellcheck should use the bash dialect.
2025-06-13 3:36 [PATCH] perf build: Specify that spellcheck should use the bash dialect Collin Funk
2025-06-19 10:28 ` James Clark
@ 2025-06-24 2:05 ` Collin Funk
2025-06-24 5:21 ` Ian Rogers
2025-06-24 5:44 ` [PATCH v3] " Collin Funk
2 siblings, 1 reply; 18+ messages in thread
From: Collin Funk @ 2025-06-24 2:05 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Alexander Shishkin, Jiri Olsa, Ian Rogers,
Adrian Hunter, Liang, Kan, James Clark, Charlie Jenkins,
Ravi Bangoria, Masami Hiramatsu (Google)
Cc: linux-perf-users, linux-kernel, Collin Funk
When someone has a global shellcheckrc file, for example at
~/.config/shellcheckrc, with the directive 'shell=sh', building perf
will fail with many shellcheck errors like:
In tests/shell/base_probe/test_adding_kernel.sh line 294:
(( TEST_RESULT += $? ))
^---------------------^ SC3006 (warning): In POSIX sh, standalone ((..)) is undefined.
For more information:
https://www.shellcheck.net/wiki/SC3006 -- In POSIX sh, standalone ((..)) is...
make[5]: *** [tests/Build:91: tests/shell/base_probe/test_adding_kernel.sh.shellcheck_log] Error 1
Passing the '-s bash' option ensures that it runs correctly regardless
of a developers global configuration.
This patch adds '-s bash' to the SHELLCHECK variable in Makefile.perf
and makes use of the variable consistently.
Signed-off-by: Collin Funk <collin.funk1@gmail.com>
---
tools/perf/Build | 2 +-
tools/perf/Makefile.perf | 2 +-
tools/perf/arch/x86/Build | 2 +-
tools/perf/arch/x86/tests/Build | 2 +-
tools/perf/tests/Build | 2 +-
tools/perf/trace/beauty/Build | 2 +-
tools/perf/util/Build | 2 +-
7 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/tools/perf/Build b/tools/perf/Build
index 06107f1e1d42..e69665bf9dce 100644
--- a/tools/perf/Build
+++ b/tools/perf/Build
@@ -73,7 +73,7 @@ endif
$(OUTPUT)%.shellcheck_log: %
$(call rule_mkdir)
- $(Q)$(call echo-cmd,test)shellcheck -s bash -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
+ $(Q)$(call echo-cmd,test)$(SHELLCHECK) -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
perf-y += $(SHELL_TEST_LOGS)
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index d4c7031b01a7..6810d321ff73 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -252,7 +252,7 @@ endif
ifeq ($(NO_SHELLCHECK),1)
SHELLCHECK :=
else
- SHELLCHECK := $(shell which shellcheck 2> /dev/null)
+ SHELLCHECK := $(shell which shellcheck 2> /dev/null) -s bash
endif
# shellcheck is using in tools/perf/tests/Build with option -a/--check-sourced (
diff --git a/tools/perf/arch/x86/Build b/tools/perf/arch/x86/Build
index afae7b8f6bd6..71e2553e5af1 100644
--- a/tools/perf/arch/x86/Build
+++ b/tools/perf/arch/x86/Build
@@ -10,6 +10,6 @@ endif
$(OUTPUT)%.shellcheck_log: %
$(call rule_mkdir)
- $(Q)$(call echo-cmd,test)shellcheck -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
+ $(Q)$(call echo-cmd,test)$(SHELLCHECK) -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
perf-test-y += $(SHELL_TEST_LOGS)
diff --git a/tools/perf/arch/x86/tests/Build b/tools/perf/arch/x86/tests/Build
index 5e00cbfd2d56..fd3af16f63bb 100644
--- a/tools/perf/arch/x86/tests/Build
+++ b/tools/perf/arch/x86/tests/Build
@@ -22,6 +22,6 @@ endif
$(OUTPUT)%.shellcheck_log: %
$(call rule_mkdir)
- $(Q)$(call echo-cmd,test)shellcheck -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
+ $(Q)$(call echo-cmd,test)$(SHELLCHECK) -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
perf-test-y += $(SHELL_TEST_LOGS)
diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
index 2181f5a92148..4a27fde30eb6 100644
--- a/tools/perf/tests/Build
+++ b/tools/perf/tests/Build
@@ -89,7 +89,7 @@ endif
$(OUTPUT)%.shellcheck_log: %
$(call rule_mkdir)
- $(Q)$(call echo-cmd,test)shellcheck -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
+ $(Q)$(call echo-cmd,test)$(SHELLCHECK) -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
perf-test-y += $(SHELL_TEST_LOGS)
diff --git a/tools/perf/trace/beauty/Build b/tools/perf/trace/beauty/Build
index f50ebdc445b8..727ce0a5c30a 100644
--- a/tools/perf/trace/beauty/Build
+++ b/tools/perf/trace/beauty/Build
@@ -31,6 +31,6 @@ endif
$(OUTPUT)%.shellcheck_log: %
$(call rule_mkdir)
- $(Q)$(call echo-cmd,test)shellcheck -s bash -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
+ $(Q)$(call echo-cmd,test)$(SHELLCHECK) -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
perf-y += $(SHELL_TEST_LOGS)
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 7910d908c814..626a359fee1e 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -421,7 +421,7 @@ endif
$(OUTPUT)%.shellcheck_log: %
$(call rule_mkdir)
- $(Q)$(call echo-cmd,test)shellcheck -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
+ $(Q)$(call echo-cmd,test)$(SHELLCHECK) -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
perf-util-y += $(SHELL_TEST_LOGS)
--
2.49.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] perf build: Specify that spellcheck should use the bash dialect.
2025-06-23 16:37 ` Ian Rogers
@ 2025-06-24 2:08 ` Collin Funk
0 siblings, 0 replies; 18+ messages in thread
From: Collin Funk @ 2025-06-24 2:08 UTC (permalink / raw)
To: Ian Rogers
Cc: James Clark, linux-perf-users, linux-kernel, Peter Zijlstra,
Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim,
Alexander Shishkin, Jiri Olsa, Adrian Hunter, Liang, Kan
Hi Ian,
Ian Rogers <irogers@google.com> writes:
>> > $ find tools/perf -name Build | xargs grep bash
>> > tools/perf/Build: $(Q)$(call echo-cmd,test)shellcheck -s bash -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
>> > tools/perf/trace/beauty/Build: $(Q)$(call echo-cmd,test)shellcheck -s bash -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
>> >
>> > Collin
>>
>> In that case:
>>
>> Reviewed-by: James Clark <james.clark@linaro.org>
>>
>> And I'll send the bulk hashbang change separately.
>
> I've no objection to switching to using bash globally. It seems
> sub-optimal that we've copy-pasted the shellcheck command across many
> different Build files and that this patch will cause the
> tools/perf/tests/Build one to differ. My preference would be to have a
> global definition probably in Makefile.perf, then use it consistently.
> Alternative all shellcheck invocations can pass "-s bash" for the sake
> of consistency. Fwiw, I think the 'tools/arch/x86/tools/gen-insn-*'
> which is to some extent taken from the kernel's 'arch/x86/tools' is
> okay with the change too.
Good point. It turns out there was a SHELLCHECK variable in
Makefile.perf, but it was not used consistently.
I have submitted V2 that adds the '-s bash' option to the definition and
actually uses the variable [1]
Collin
[1] https://lore.kernel.org/all/f7ea3a430dc2bd77656c50f93283547d1245e2fe.1750730589.git.collin.funk1@gmail.com/
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] [PATCH] perf build: Specify that shellcheck should use the bash dialect.
2025-06-24 2:05 ` [PATCH v2] [PATCH] perf build: Specify that shellcheck " Collin Funk
@ 2025-06-24 5:21 ` Ian Rogers
2025-06-24 5:51 ` Collin Funk
0 siblings, 1 reply; 18+ messages in thread
From: Ian Rogers @ 2025-06-24 5:21 UTC (permalink / raw)
To: Collin Funk
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
Liang, Kan, James Clark, Charlie Jenkins, Ravi Bangoria,
Masami Hiramatsu (Google), linux-perf-users, linux-kernel
On Mon, Jun 23, 2025 at 7:06 PM Collin Funk <collin.funk1@gmail.com> wrote:
>
> When someone has a global shellcheckrc file, for example at
> ~/.config/shellcheckrc, with the directive 'shell=sh', building perf
> will fail with many shellcheck errors like:
>
> In tests/shell/base_probe/test_adding_kernel.sh line 294:
> (( TEST_RESULT += $? ))
> ^---------------------^ SC3006 (warning): In POSIX sh, standalone ((..)) is undefined.
>
> For more information:
> https://www.shellcheck.net/wiki/SC3006 -- In POSIX sh, standalone ((..)) is...
> make[5]: *** [tests/Build:91: tests/shell/base_probe/test_adding_kernel.sh.shellcheck_log] Error 1
>
> Passing the '-s bash' option ensures that it runs correctly regardless
> of a developers global configuration.
>
> This patch adds '-s bash' to the SHELLCHECK variable in Makefile.perf
> and makes use of the variable consistently.
>
> Signed-off-by: Collin Funk <collin.funk1@gmail.com>
> ---
> tools/perf/Build | 2 +-
> tools/perf/Makefile.perf | 2 +-
> tools/perf/arch/x86/Build | 2 +-
> tools/perf/arch/x86/tests/Build | 2 +-
> tools/perf/tests/Build | 2 +-
> tools/perf/trace/beauty/Build | 2 +-
> tools/perf/util/Build | 2 +-
> 7 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/tools/perf/Build b/tools/perf/Build
> index 06107f1e1d42..e69665bf9dce 100644
> --- a/tools/perf/Build
> +++ b/tools/perf/Build
> @@ -73,7 +73,7 @@ endif
>
> $(OUTPUT)%.shellcheck_log: %
> $(call rule_mkdir)
> - $(Q)$(call echo-cmd,test)shellcheck -s bash -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
> + $(Q)$(call echo-cmd,test)$(SHELLCHECK) -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
>
> perf-y += $(SHELL_TEST_LOGS)
>
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index d4c7031b01a7..6810d321ff73 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -252,7 +252,7 @@ endif
> ifeq ($(NO_SHELLCHECK),1)
> SHELLCHECK :=
> else
> - SHELLCHECK := $(shell which shellcheck 2> /dev/null)
> + SHELLCHECK := $(shell which shellcheck 2> /dev/null) -s bash
Could we also place the "-a -S warning" warning here too?
Thanks,
Ian
> endif
>
> # shellcheck is using in tools/perf/tests/Build with option -a/--check-sourced (
> diff --git a/tools/perf/arch/x86/Build b/tools/perf/arch/x86/Build
> index afae7b8f6bd6..71e2553e5af1 100644
> --- a/tools/perf/arch/x86/Build
> +++ b/tools/perf/arch/x86/Build
> @@ -10,6 +10,6 @@ endif
>
> $(OUTPUT)%.shellcheck_log: %
> $(call rule_mkdir)
> - $(Q)$(call echo-cmd,test)shellcheck -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
> + $(Q)$(call echo-cmd,test)$(SHELLCHECK) -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
>
> perf-test-y += $(SHELL_TEST_LOGS)
> diff --git a/tools/perf/arch/x86/tests/Build b/tools/perf/arch/x86/tests/Build
> index 5e00cbfd2d56..fd3af16f63bb 100644
> --- a/tools/perf/arch/x86/tests/Build
> +++ b/tools/perf/arch/x86/tests/Build
> @@ -22,6 +22,6 @@ endif
>
> $(OUTPUT)%.shellcheck_log: %
> $(call rule_mkdir)
> - $(Q)$(call echo-cmd,test)shellcheck -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
> + $(Q)$(call echo-cmd,test)$(SHELLCHECK) -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
>
> perf-test-y += $(SHELL_TEST_LOGS)
> diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
> index 2181f5a92148..4a27fde30eb6 100644
> --- a/tools/perf/tests/Build
> +++ b/tools/perf/tests/Build
> @@ -89,7 +89,7 @@ endif
>
> $(OUTPUT)%.shellcheck_log: %
> $(call rule_mkdir)
> - $(Q)$(call echo-cmd,test)shellcheck -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
> + $(Q)$(call echo-cmd,test)$(SHELLCHECK) -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
>
> perf-test-y += $(SHELL_TEST_LOGS)
>
> diff --git a/tools/perf/trace/beauty/Build b/tools/perf/trace/beauty/Build
> index f50ebdc445b8..727ce0a5c30a 100644
> --- a/tools/perf/trace/beauty/Build
> +++ b/tools/perf/trace/beauty/Build
> @@ -31,6 +31,6 @@ endif
>
> $(OUTPUT)%.shellcheck_log: %
> $(call rule_mkdir)
> - $(Q)$(call echo-cmd,test)shellcheck -s bash -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
> + $(Q)$(call echo-cmd,test)$(SHELLCHECK) -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
>
> perf-y += $(SHELL_TEST_LOGS)
> diff --git a/tools/perf/util/Build b/tools/perf/util/Build
> index 7910d908c814..626a359fee1e 100644
> --- a/tools/perf/util/Build
> +++ b/tools/perf/util/Build
> @@ -421,7 +421,7 @@ endif
>
> $(OUTPUT)%.shellcheck_log: %
> $(call rule_mkdir)
> - $(Q)$(call echo-cmd,test)shellcheck -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
> + $(Q)$(call echo-cmd,test)$(SHELLCHECK) -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
>
> perf-util-y += $(SHELL_TEST_LOGS)
>
> --
> 2.49.0
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3] perf build: Specify that shellcheck should use the bash dialect.
2025-06-13 3:36 [PATCH] perf build: Specify that spellcheck should use the bash dialect Collin Funk
2025-06-19 10:28 ` James Clark
2025-06-24 2:05 ` [PATCH v2] [PATCH] perf build: Specify that shellcheck " Collin Funk
@ 2025-06-24 5:44 ` Collin Funk
2025-06-24 9:37 ` James Clark
` (2 more replies)
2 siblings, 3 replies; 18+ messages in thread
From: Collin Funk @ 2025-06-24 5:44 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Alexander Shishkin, Jiri Olsa, Ian Rogers,
Adrian Hunter, Liang, Kan, James Clark, Charlie Jenkins,
Ravi Bangoria, Masami Hiramatsu (Google)
Cc: linux-perf-users, linux-kernel, Collin Funk
When someone has a global shellcheckrc file, for example at
~/.config/shellcheckrc, with the directive 'shell=sh', building perf
will fail with many shellcheck errors like:
In tests/shell/base_probe/test_adding_kernel.sh line 294:
(( TEST_RESULT += $? ))
^---------------------^ SC3006 (warning): In POSIX sh, standalone ((..)) is undefined.
For more information:
https://www.shellcheck.net/wiki/SC3006 -- In POSIX sh, standalone ((..)) is...
make[5]: *** [tests/Build:91: tests/shell/base_probe/test_adding_kernel.sh.shellcheck_log] Error 1
Passing the '-s bash' option ensures that it runs correctly regardless
of a developers global configuration.
This patch adds '-s bash' and other options to the SHELLCHECK variable
in Makefile.perf and makes use of the variable consistently.
Signed-off-by: Collin Funk <collin.funk1@gmail.com>
---
tools/perf/Build | 2 +-
tools/perf/Makefile.perf | 2 +-
tools/perf/arch/x86/Build | 2 +-
tools/perf/arch/x86/tests/Build | 2 +-
tools/perf/tests/Build | 2 +-
tools/perf/trace/beauty/Build | 2 +-
tools/perf/util/Build | 2 +-
7 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/tools/perf/Build b/tools/perf/Build
index 06107f1e1d42..b03cc59dabf8 100644
--- a/tools/perf/Build
+++ b/tools/perf/Build
@@ -73,7 +73,7 @@ endif
$(OUTPUT)%.shellcheck_log: %
$(call rule_mkdir)
- $(Q)$(call echo-cmd,test)shellcheck -s bash -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
+ $(Q)$(call echo-cmd,test)$(SHELLCHECK) "$<" > $@ || (cat $@ && rm $@ && false)
perf-y += $(SHELL_TEST_LOGS)
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index d4c7031b01a7..e0cf8db5462b 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -252,7 +252,7 @@ endif
ifeq ($(NO_SHELLCHECK),1)
SHELLCHECK :=
else
- SHELLCHECK := $(shell which shellcheck 2> /dev/null)
+ SHELLCHECK := $(shell which shellcheck 2> /dev/null) -s bash -a -S warning
endif
# shellcheck is using in tools/perf/tests/Build with option -a/--check-sourced (
diff --git a/tools/perf/arch/x86/Build b/tools/perf/arch/x86/Build
index afae7b8f6bd6..d31a1168757c 100644
--- a/tools/perf/arch/x86/Build
+++ b/tools/perf/arch/x86/Build
@@ -10,6 +10,6 @@ endif
$(OUTPUT)%.shellcheck_log: %
$(call rule_mkdir)
- $(Q)$(call echo-cmd,test)shellcheck -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
+ $(Q)$(call echo-cmd,test)$(SHELLCHECK) "$<" > $@ || (cat $@ && rm $@ && false)
perf-test-y += $(SHELL_TEST_LOGS)
diff --git a/tools/perf/arch/x86/tests/Build b/tools/perf/arch/x86/tests/Build
index 5e00cbfd2d56..01d5527f38c7 100644
--- a/tools/perf/arch/x86/tests/Build
+++ b/tools/perf/arch/x86/tests/Build
@@ -22,6 +22,6 @@ endif
$(OUTPUT)%.shellcheck_log: %
$(call rule_mkdir)
- $(Q)$(call echo-cmd,test)shellcheck -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
+ $(Q)$(call echo-cmd,test)$(SHELLCHECK) "$<" > $@ || (cat $@ && rm $@ && false)
perf-test-y += $(SHELL_TEST_LOGS)
diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
index 2181f5a92148..d6c35dd0de3b 100644
--- a/tools/perf/tests/Build
+++ b/tools/perf/tests/Build
@@ -89,7 +89,7 @@ endif
$(OUTPUT)%.shellcheck_log: %
$(call rule_mkdir)
- $(Q)$(call echo-cmd,test)shellcheck -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
+ $(Q)$(call echo-cmd,test)$(SHELLCHECK) "$<" > $@ || (cat $@ && rm $@ && false)
perf-test-y += $(SHELL_TEST_LOGS)
diff --git a/tools/perf/trace/beauty/Build b/tools/perf/trace/beauty/Build
index f50ebdc445b8..561590ee8cda 100644
--- a/tools/perf/trace/beauty/Build
+++ b/tools/perf/trace/beauty/Build
@@ -31,6 +31,6 @@ endif
$(OUTPUT)%.shellcheck_log: %
$(call rule_mkdir)
- $(Q)$(call echo-cmd,test)shellcheck -s bash -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
+ $(Q)$(call echo-cmd,test)$(SHELLCHECK) "$<" > $@ || (cat $@ && rm $@ && false)
perf-y += $(SHELL_TEST_LOGS)
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 7910d908c814..2dfa09a6f27d 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -421,7 +421,7 @@ endif
$(OUTPUT)%.shellcheck_log: %
$(call rule_mkdir)
- $(Q)$(call echo-cmd,test)shellcheck -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
+ $(Q)$(call echo-cmd,test)$(SHELLCHECK) "$<" > $@ || (cat $@ && rm $@ && false)
perf-util-y += $(SHELL_TEST_LOGS)
--
2.49.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v2] [PATCH] perf build: Specify that shellcheck should use the bash dialect.
2025-06-24 5:21 ` Ian Rogers
@ 2025-06-24 5:51 ` Collin Funk
0 siblings, 0 replies; 18+ messages in thread
From: Collin Funk @ 2025-06-24 5:51 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
Liang, Kan, James Clark, Charlie Jenkins, Ravi Bangoria,
Masami Hiramatsu (Google), linux-perf-users, linux-kernel
Ian Rogers <irogers@google.com> writes:
> Could we also place the "-a -S warning" warning here too?
>
> Thanks,
> Ian
Sure, done in V3 here [1].
Collin
[1] https://lore.kernel.org/linux-perf-users/f8415e57c938482668717d918ab566ff5082f281.1750743784.git.collin.funk1@gmail.com/T/#u
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3] perf build: Specify that shellcheck should use the bash dialect.
2025-06-24 5:44 ` [PATCH v3] " Collin Funk
@ 2025-06-24 9:37 ` James Clark
2025-06-26 17:39 ` Namhyung Kim
2025-06-28 3:41 ` [PATCH v4] " Collin Funk
2 siblings, 0 replies; 18+ messages in thread
From: James Clark @ 2025-06-24 9:37 UTC (permalink / raw)
To: Collin Funk
Cc: linux-perf-users, linux-kernel, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Alexander Shishkin,
Jiri Olsa, Ian Rogers, Adrian Hunter, Liang, Kan, Charlie Jenkins,
Ravi Bangoria, Masami Hiramatsu (Google)
On 24/06/2025 6:44 am, Collin Funk wrote:
> When someone has a global shellcheckrc file, for example at
> ~/.config/shellcheckrc, with the directive 'shell=sh', building perf
> will fail with many shellcheck errors like:
>
> In tests/shell/base_probe/test_adding_kernel.sh line 294:
> (( TEST_RESULT += $? ))
> ^---------------------^ SC3006 (warning): In POSIX sh, standalone ((..)) is undefined.
>
> For more information:
> https://www.shellcheck.net/wiki/SC3006 -- In POSIX sh, standalone ((..)) is...
> make[5]: *** [tests/Build:91: tests/shell/base_probe/test_adding_kernel.sh.shellcheck_log] Error 1
>
> Passing the '-s bash' option ensures that it runs correctly regardless
> of a developers global configuration.
>
> This patch adds '-s bash' and other options to the SHELLCHECK variable
> in Makefile.perf and makes use of the variable consistently.
>
Reviewed-by: James Clark <james.clark@linaro.org>
> Signed-off-by: Collin Funk <collin.funk1@gmail.com>
> ---
> tools/perf/Build | 2 +-
> tools/perf/Makefile.perf | 2 +-
> tools/perf/arch/x86/Build | 2 +-
> tools/perf/arch/x86/tests/Build | 2 +-
> tools/perf/tests/Build | 2 +-
> tools/perf/trace/beauty/Build | 2 +-
> tools/perf/util/Build | 2 +-
> 7 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/tools/perf/Build b/tools/perf/Build
> index 06107f1e1d42..b03cc59dabf8 100644
> --- a/tools/perf/Build
> +++ b/tools/perf/Build
> @@ -73,7 +73,7 @@ endif
>
> $(OUTPUT)%.shellcheck_log: %
> $(call rule_mkdir)
> - $(Q)$(call echo-cmd,test)shellcheck -s bash -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
> + $(Q)$(call echo-cmd,test)$(SHELLCHECK) "$<" > $@ || (cat $@ && rm $@ && false)
>
> perf-y += $(SHELL_TEST_LOGS)
>
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index d4c7031b01a7..e0cf8db5462b 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -252,7 +252,7 @@ endif
> ifeq ($(NO_SHELLCHECK),1)
> SHELLCHECK :=
> else
> - SHELLCHECK := $(shell which shellcheck 2> /dev/null)
> + SHELLCHECK := $(shell which shellcheck 2> /dev/null) -s bash -a -S warning
> endif
>
> # shellcheck is using in tools/perf/tests/Build with option -a/--check-sourced (
> diff --git a/tools/perf/arch/x86/Build b/tools/perf/arch/x86/Build
> index afae7b8f6bd6..d31a1168757c 100644
> --- a/tools/perf/arch/x86/Build
> +++ b/tools/perf/arch/x86/Build
> @@ -10,6 +10,6 @@ endif
>
> $(OUTPUT)%.shellcheck_log: %
> $(call rule_mkdir)
> - $(Q)$(call echo-cmd,test)shellcheck -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
> + $(Q)$(call echo-cmd,test)$(SHELLCHECK) "$<" > $@ || (cat $@ && rm $@ && false)
>
> perf-test-y += $(SHELL_TEST_LOGS)
> diff --git a/tools/perf/arch/x86/tests/Build b/tools/perf/arch/x86/tests/Build
> index 5e00cbfd2d56..01d5527f38c7 100644
> --- a/tools/perf/arch/x86/tests/Build
> +++ b/tools/perf/arch/x86/tests/Build
> @@ -22,6 +22,6 @@ endif
>
> $(OUTPUT)%.shellcheck_log: %
> $(call rule_mkdir)
> - $(Q)$(call echo-cmd,test)shellcheck -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
> + $(Q)$(call echo-cmd,test)$(SHELLCHECK) "$<" > $@ || (cat $@ && rm $@ && false)
>
> perf-test-y += $(SHELL_TEST_LOGS)
> diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
> index 2181f5a92148..d6c35dd0de3b 100644
> --- a/tools/perf/tests/Build
> +++ b/tools/perf/tests/Build
> @@ -89,7 +89,7 @@ endif
>
> $(OUTPUT)%.shellcheck_log: %
> $(call rule_mkdir)
> - $(Q)$(call echo-cmd,test)shellcheck -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
> + $(Q)$(call echo-cmd,test)$(SHELLCHECK) "$<" > $@ || (cat $@ && rm $@ && false)
>
> perf-test-y += $(SHELL_TEST_LOGS)
>
> diff --git a/tools/perf/trace/beauty/Build b/tools/perf/trace/beauty/Build
> index f50ebdc445b8..561590ee8cda 100644
> --- a/tools/perf/trace/beauty/Build
> +++ b/tools/perf/trace/beauty/Build
> @@ -31,6 +31,6 @@ endif
>
> $(OUTPUT)%.shellcheck_log: %
> $(call rule_mkdir)
> - $(Q)$(call echo-cmd,test)shellcheck -s bash -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
> + $(Q)$(call echo-cmd,test)$(SHELLCHECK) "$<" > $@ || (cat $@ && rm $@ && false)
>
> perf-y += $(SHELL_TEST_LOGS)
> diff --git a/tools/perf/util/Build b/tools/perf/util/Build
> index 7910d908c814..2dfa09a6f27d 100644
> --- a/tools/perf/util/Build
> +++ b/tools/perf/util/Build
> @@ -421,7 +421,7 @@ endif
>
> $(OUTPUT)%.shellcheck_log: %
> $(call rule_mkdir)
> - $(Q)$(call echo-cmd,test)shellcheck -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
> + $(Q)$(call echo-cmd,test)$(SHELLCHECK) "$<" > $@ || (cat $@ && rm $@ && false)
>
> perf-util-y += $(SHELL_TEST_LOGS)
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3] perf build: Specify that shellcheck should use the bash dialect.
2025-06-24 5:44 ` [PATCH v3] " Collin Funk
2025-06-24 9:37 ` James Clark
@ 2025-06-26 17:39 ` Namhyung Kim
2025-06-28 3:35 ` Collin Funk
2025-06-28 3:41 ` [PATCH v4] " Collin Funk
2 siblings, 1 reply; 18+ messages in thread
From: Namhyung Kim @ 2025-06-26 17:39 UTC (permalink / raw)
To: Collin Funk
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
Liang, Kan, James Clark, Charlie Jenkins, Ravi Bangoria,
Masami Hiramatsu (Google), linux-perf-users, linux-kernel
Hello,
On Mon, Jun 23, 2025 at 10:44:59PM -0700, Collin Funk wrote:
> When someone has a global shellcheckrc file, for example at
> ~/.config/shellcheckrc, with the directive 'shell=sh', building perf
> will fail with many shellcheck errors like:
>
> In tests/shell/base_probe/test_adding_kernel.sh line 294:
> (( TEST_RESULT += $? ))
> ^---------------------^ SC3006 (warning): In POSIX sh, standalone ((..)) is undefined.
>
> For more information:
> https://www.shellcheck.net/wiki/SC3006 -- In POSIX sh, standalone ((..)) is...
> make[5]: *** [tests/Build:91: tests/shell/base_probe/test_adding_kernel.sh.shellcheck_log] Error 1
>
> Passing the '-s bash' option ensures that it runs correctly regardless
> of a developers global configuration.
>
> This patch adds '-s bash' and other options to the SHELLCHECK variable
> in Makefile.perf and makes use of the variable consistently.
>
> Signed-off-by: Collin Funk <collin.funk1@gmail.com>
> ---
> tools/perf/Build | 2 +-
> tools/perf/Makefile.perf | 2 +-
> tools/perf/arch/x86/Build | 2 +-
> tools/perf/arch/x86/tests/Build | 2 +-
> tools/perf/tests/Build | 2 +-
> tools/perf/trace/beauty/Build | 2 +-
> tools/perf/util/Build | 2 +-
> 7 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/tools/perf/Build b/tools/perf/Build
> index 06107f1e1d42..b03cc59dabf8 100644
> --- a/tools/perf/Build
> +++ b/tools/perf/Build
> @@ -73,7 +73,7 @@ endif
>
> $(OUTPUT)%.shellcheck_log: %
> $(call rule_mkdir)
> - $(Q)$(call echo-cmd,test)shellcheck -s bash -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
> + $(Q)$(call echo-cmd,test)$(SHELLCHECK) "$<" > $@ || (cat $@ && rm $@ && false)
>
> perf-y += $(SHELL_TEST_LOGS)
>
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index d4c7031b01a7..e0cf8db5462b 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -252,7 +252,7 @@ endif
> ifeq ($(NO_SHELLCHECK),1)
> SHELLCHECK :=
> else
> - SHELLCHECK := $(shell which shellcheck 2> /dev/null)
> + SHELLCHECK := $(shell which shellcheck 2> /dev/null) -s bash -a -S warning
This caused a trouble on a test environment where 'which' (and
'shellcheck' as well) is not available. Now it makes SHELLCHECK
non-empty unconditionally.
So the version check below failed like below:
make[1]: which: No such file or directory
/bin/sh: - : invalid option
Usage: /bin/sh [GNU long option] [option] ...
/bin/sh [GNU long option] [option] script-file ...
GNU long options:
--debug
--debugger
--dump-po-strings
--dump-strings
--help
--init-file
--login
--noediting
--noprofile
--norc
--posix
--pretty-print
--rcfile
--rpm-requires
--restricted
--verbose
--version
Shell options:
-ilrsD or -c command or -O shopt_option (invocation only)
-abefhkmnptuvxBCEHPT or -o option
expr: syntax error: unexpected argument ‘060’
And it failed to build later on shellchecks.
TEST /build/arch/x86/tests/gen-insn-x86-dat.sh.shellcheck_log
/bin/sh: line 1: -s: command not found
make[6]: *** [arch/x86/tests/Build:25: /build/arch/x86/tests/gen-insn-x86-dat.sh.shellcheck_log] Error 1
make[6]: *** Waiting for unfinished jobs....
I think it's better to convert 'which' to 'command -v' (in other places
too) and add the options after the version check.
Thanks,
Namhyung
> endif
>
> # shellcheck is using in tools/perf/tests/Build with option -a/--check-sourced (
> diff --git a/tools/perf/arch/x86/Build b/tools/perf/arch/x86/Build
> index afae7b8f6bd6..d31a1168757c 100644
> --- a/tools/perf/arch/x86/Build
> +++ b/tools/perf/arch/x86/Build
> @@ -10,6 +10,6 @@ endif
>
> $(OUTPUT)%.shellcheck_log: %
> $(call rule_mkdir)
> - $(Q)$(call echo-cmd,test)shellcheck -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
> + $(Q)$(call echo-cmd,test)$(SHELLCHECK) "$<" > $@ || (cat $@ && rm $@ && false)
>
> perf-test-y += $(SHELL_TEST_LOGS)
> diff --git a/tools/perf/arch/x86/tests/Build b/tools/perf/arch/x86/tests/Build
> index 5e00cbfd2d56..01d5527f38c7 100644
> --- a/tools/perf/arch/x86/tests/Build
> +++ b/tools/perf/arch/x86/tests/Build
> @@ -22,6 +22,6 @@ endif
>
> $(OUTPUT)%.shellcheck_log: %
> $(call rule_mkdir)
> - $(Q)$(call echo-cmd,test)shellcheck -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
> + $(Q)$(call echo-cmd,test)$(SHELLCHECK) "$<" > $@ || (cat $@ && rm $@ && false)
>
> perf-test-y += $(SHELL_TEST_LOGS)
> diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
> index 2181f5a92148..d6c35dd0de3b 100644
> --- a/tools/perf/tests/Build
> +++ b/tools/perf/tests/Build
> @@ -89,7 +89,7 @@ endif
>
> $(OUTPUT)%.shellcheck_log: %
> $(call rule_mkdir)
> - $(Q)$(call echo-cmd,test)shellcheck -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
> + $(Q)$(call echo-cmd,test)$(SHELLCHECK) "$<" > $@ || (cat $@ && rm $@ && false)
>
> perf-test-y += $(SHELL_TEST_LOGS)
>
> diff --git a/tools/perf/trace/beauty/Build b/tools/perf/trace/beauty/Build
> index f50ebdc445b8..561590ee8cda 100644
> --- a/tools/perf/trace/beauty/Build
> +++ b/tools/perf/trace/beauty/Build
> @@ -31,6 +31,6 @@ endif
>
> $(OUTPUT)%.shellcheck_log: %
> $(call rule_mkdir)
> - $(Q)$(call echo-cmd,test)shellcheck -s bash -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
> + $(Q)$(call echo-cmd,test)$(SHELLCHECK) "$<" > $@ || (cat $@ && rm $@ && false)
>
> perf-y += $(SHELL_TEST_LOGS)
> diff --git a/tools/perf/util/Build b/tools/perf/util/Build
> index 7910d908c814..2dfa09a6f27d 100644
> --- a/tools/perf/util/Build
> +++ b/tools/perf/util/Build
> @@ -421,7 +421,7 @@ endif
>
> $(OUTPUT)%.shellcheck_log: %
> $(call rule_mkdir)
> - $(Q)$(call echo-cmd,test)shellcheck -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
> + $(Q)$(call echo-cmd,test)$(SHELLCHECK) "$<" > $@ || (cat $@ && rm $@ && false)
>
> perf-util-y += $(SHELL_TEST_LOGS)
>
> --
> 2.49.0
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3] perf build: Specify that shellcheck should use the bash dialect.
2025-06-26 17:39 ` Namhyung Kim
@ 2025-06-28 3:35 ` Collin Funk
2025-06-28 3:49 ` Collin Funk
0 siblings, 1 reply; 18+ messages in thread
From: Collin Funk @ 2025-06-28 3:35 UTC (permalink / raw)
To: Namhyung Kim
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
Liang, Kan, James Clark, Charlie Jenkins, Ravi Bangoria,
Masami Hiramatsu (Google), linux-perf-users, linux-kernel
Hi Namhyung,
Namhyung Kim <namhyung@kernel.org> writes:
> This caused a trouble on a test environment where 'which' (and
> 'shellcheck' as well) is not available. Now it makes SHELLCHECK
> non-empty unconditionally.
>
> So the version check below failed like below:
>
> make[1]: which: No such file or directory
> /bin/sh: - : invalid option
> Usage: /bin/sh [GNU long option] [option] ...
> /bin/sh [GNU long option] [option] script-file ...
> GNU long options:
> --debug
> --debugger
> --dump-po-strings
> --dump-strings
> --help
> --init-file
> --login
> --noediting
> --noprofile
> --norc
> --posix
> --pretty-print
> --rcfile
> --rpm-requires
> --restricted
> --verbose
> --version
> Shell options:
> -ilrsD or -c command or -O shopt_option (invocation only)
> -abefhkmnptuvxBCEHPT or -o option
> expr: syntax error: unexpected argument ‘060’
>
> And it failed to build later on shellchecks.
>
> TEST /build/arch/x86/tests/gen-insn-x86-dat.sh.shellcheck_log
> /bin/sh: line 1: -s: command not found
> make[6]: *** [arch/x86/tests/Build:25: /build/arch/x86/tests/gen-insn-x86-dat.sh.shellcheck_log] Error 1
> make[6]: *** Waiting for unfinished jobs....
>
> I think it's better to convert 'which' to 'command -v' (in other places
> too) and add the options after the version check.
Oops, I assumed that on a system without shellcheck NO_SHELLCHECK would
be defined. Let me write another version.
I think the 'command -v' change is best left for a separate patch(s).
Since it is used in many other places, and maybe others will raise
objections.
Collin
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v4] perf build: Specify that shellcheck should use the bash dialect.
2025-06-24 5:44 ` [PATCH v3] " Collin Funk
2025-06-24 9:37 ` James Clark
2025-06-26 17:39 ` Namhyung Kim
@ 2025-06-28 3:41 ` Collin Funk
2025-07-01 17:55 ` Namhyung Kim
2 siblings, 1 reply; 18+ messages in thread
From: Collin Funk @ 2025-06-28 3:41 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Alexander Shishkin, Jiri Olsa, Ian Rogers,
Adrian Hunter, Liang, Kan, James Clark, Charlie Jenkins,
Ravi Bangoria, Masami Hiramatsu (Google)
Cc: linux-perf-users, linux-kernel, Collin Funk
When someone has a global shellcheckrc file, for example at
~/.config/shellcheckrc, with the directive 'shell=sh', building perf
will fail with many shellcheck errors like:
In tests/shell/base_probe/test_adding_kernel.sh line 294:
(( TEST_RESULT += $? ))
^---------------------^ SC3006 (warning): In POSIX sh, standalone ((..)) is undefined.
For more information:
https://www.shellcheck.net/wiki/SC3006 -- In POSIX sh, standalone ((..)) is...
make[5]: *** [tests/Build:91: tests/shell/base_probe/test_adding_kernel.sh.shellcheck_log] Error 1
Passing the '-s bash' option ensures that it runs correctly regardless
of a developers global configuration.
This patch adds '-s bash' and other options to the SHELLCHECK variable
in Makefile.perf and makes use of the variable consistently.
Signed-off-by: Collin Funk <collin.funk1@gmail.com>
---
tools/perf/Build | 2 +-
tools/perf/Makefile.perf | 2 ++
tools/perf/arch/x86/Build | 2 +-
tools/perf/arch/x86/tests/Build | 2 +-
tools/perf/tests/Build | 2 +-
tools/perf/trace/beauty/Build | 2 +-
tools/perf/util/Build | 2 +-
7 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/tools/perf/Build b/tools/perf/Build
index 06107f1e1d42..b03cc59dabf8 100644
--- a/tools/perf/Build
+++ b/tools/perf/Build
@@ -73,7 +73,7 @@ endif
$(OUTPUT)%.shellcheck_log: %
$(call rule_mkdir)
- $(Q)$(call echo-cmd,test)shellcheck -s bash -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
+ $(Q)$(call echo-cmd,test)$(SHELLCHECK) "$<" > $@ || (cat $@ && rm $@ && false)
perf-y += $(SHELL_TEST_LOGS)
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index d4c7031b01a7..5f76c82e0aec 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -262,6 +262,8 @@ ifneq ($(SHELLCHECK),)
ifeq ($(shell expr $(shell $(SHELLCHECK) --version | grep version: | \
sed -e 's/.\+ \([0-9]\+\).\([0-9]\+\).\([0-9]\+\)/\1\2\3/g') \< 060), 1)
SHELLCHECK :=
+ else
+ SHELLCHECK := $(SHELLCHECK) -s bash -a -S warning
endif
endif
diff --git a/tools/perf/arch/x86/Build b/tools/perf/arch/x86/Build
index afae7b8f6bd6..d31a1168757c 100644
--- a/tools/perf/arch/x86/Build
+++ b/tools/perf/arch/x86/Build
@@ -10,6 +10,6 @@ endif
$(OUTPUT)%.shellcheck_log: %
$(call rule_mkdir)
- $(Q)$(call echo-cmd,test)shellcheck -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
+ $(Q)$(call echo-cmd,test)$(SHELLCHECK) "$<" > $@ || (cat $@ && rm $@ && false)
perf-test-y += $(SHELL_TEST_LOGS)
diff --git a/tools/perf/arch/x86/tests/Build b/tools/perf/arch/x86/tests/Build
index 5e00cbfd2d56..01d5527f38c7 100644
--- a/tools/perf/arch/x86/tests/Build
+++ b/tools/perf/arch/x86/tests/Build
@@ -22,6 +22,6 @@ endif
$(OUTPUT)%.shellcheck_log: %
$(call rule_mkdir)
- $(Q)$(call echo-cmd,test)shellcheck -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
+ $(Q)$(call echo-cmd,test)$(SHELLCHECK) "$<" > $@ || (cat $@ && rm $@ && false)
perf-test-y += $(SHELL_TEST_LOGS)
diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
index 2181f5a92148..d6c35dd0de3b 100644
--- a/tools/perf/tests/Build
+++ b/tools/perf/tests/Build
@@ -89,7 +89,7 @@ endif
$(OUTPUT)%.shellcheck_log: %
$(call rule_mkdir)
- $(Q)$(call echo-cmd,test)shellcheck -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
+ $(Q)$(call echo-cmd,test)$(SHELLCHECK) "$<" > $@ || (cat $@ && rm $@ && false)
perf-test-y += $(SHELL_TEST_LOGS)
diff --git a/tools/perf/trace/beauty/Build b/tools/perf/trace/beauty/Build
index f50ebdc445b8..561590ee8cda 100644
--- a/tools/perf/trace/beauty/Build
+++ b/tools/perf/trace/beauty/Build
@@ -31,6 +31,6 @@ endif
$(OUTPUT)%.shellcheck_log: %
$(call rule_mkdir)
- $(Q)$(call echo-cmd,test)shellcheck -s bash -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
+ $(Q)$(call echo-cmd,test)$(SHELLCHECK) "$<" > $@ || (cat $@ && rm $@ && false)
perf-y += $(SHELL_TEST_LOGS)
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 7910d908c814..2dfa09a6f27d 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -421,7 +421,7 @@ endif
$(OUTPUT)%.shellcheck_log: %
$(call rule_mkdir)
- $(Q)$(call echo-cmd,test)shellcheck -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
+ $(Q)$(call echo-cmd,test)$(SHELLCHECK) "$<" > $@ || (cat $@ && rm $@ && false)
perf-util-y += $(SHELL_TEST_LOGS)
--
2.50.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v3] perf build: Specify that shellcheck should use the bash dialect.
2025-06-28 3:35 ` Collin Funk
@ 2025-06-28 3:49 ` Collin Funk
0 siblings, 0 replies; 18+ messages in thread
From: Collin Funk @ 2025-06-28 3:49 UTC (permalink / raw)
To: Namhyung Kim
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
Liang, Kan, James Clark, Charlie Jenkins, Ravi Bangoria,
Masami Hiramatsu (Google), linux-perf-users, linux-kernel
Collin Funk <collin.funk1@gmail.com> writes:
> Oops, I assumed that on a system without shellcheck NO_SHELLCHECK would
> be defined. Let me write another version.
>
> I think the 'command -v' change is best left for a separate patch(s).
> Since it is used in many other places, and maybe others will raise
> objections.
Okay, I sent V4 which makes sure SHELLCHECK is empty when the program
doesn't exist [1]. If it does exist, we then check the version. If it
does exit and is a supported version, we add the options, or else we set
it to empty.
Collin
[1] https://lore.kernel.org/linux-perf-users/63491dbc8439edf2e949d80e264b9d22332fea61.1751082075.git.collin.funk1@gmail.com/T/#u
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v4] perf build: Specify that shellcheck should use the bash dialect.
2025-06-28 3:41 ` [PATCH v4] " Collin Funk
@ 2025-07-01 17:55 ` Namhyung Kim
0 siblings, 0 replies; 18+ messages in thread
From: Namhyung Kim @ 2025-07-01 17:55 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
Liang, Kan, James Clark, Charlie Jenkins, Ravi Bangoria,
Masami Hiramatsu (Google), Collin Funk
Cc: linux-perf-users, linux-kernel
On Fri, 27 Jun 2025 20:41:25 -0700, Collin Funk wrote:
> When someone has a global shellcheckrc file, for example at
> ~/.config/shellcheckrc, with the directive 'shell=sh', building perf
> will fail with many shellcheck errors like:
>
> In tests/shell/base_probe/test_adding_kernel.sh line 294:
> (( TEST_RESULT += $? ))
> ^---------------------^ SC3006 (warning): In POSIX sh, standalone ((..)) is undefined.
>
> [...]
Applied to perf-tools-next, thanks!
Best regards,
Namhyung
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2025-07-01 17:55 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-13 3:36 [PATCH] perf build: Specify that spellcheck should use the bash dialect Collin Funk
2025-06-19 10:28 ` James Clark
2025-06-20 17:40 ` Collin Funk
2025-06-23 8:10 ` James Clark
2025-06-23 16:37 ` Ian Rogers
2025-06-24 2:08 ` Collin Funk
2025-06-20 19:49 ` Namhyung Kim
2025-06-23 8:08 ` James Clark
2025-06-24 2:05 ` [PATCH v2] [PATCH] perf build: Specify that shellcheck " Collin Funk
2025-06-24 5:21 ` Ian Rogers
2025-06-24 5:51 ` Collin Funk
2025-06-24 5:44 ` [PATCH v3] " Collin Funk
2025-06-24 9:37 ` James Clark
2025-06-26 17:39 ` Namhyung Kim
2025-06-28 3:35 ` Collin Funk
2025-06-28 3:49 ` Collin Funk
2025-06-28 3:41 ` [PATCH v4] " Collin Funk
2025-07-01 17:55 ` Namhyung Kim
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).