Re: [PATCH] perf build: Specify that spellcheck should use the bash dialect.

[James Clark]

On 20/06/2025 6:40 pm, Collin Funk wrote:
> Hi James,
>
> James Clark <james.clark@xxxxxxxxxx> 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@xxxxxxxxxx>

And I'll send the bulk hashbang change separately.