Re: [PATCH 1/2] tools/perf/tests: Fix string substitutions in build id test

[Athira Rajeev <atrajeev@linux.vnet.ibm.com>]

Hi All,

Looking for what direction we can take here.
Do we want to change all tests in tools/perf/tests/shell except test_intel_pt.sh to use "bash" or go with
the approach in the patch ? Please share your comments

Thanks
Athira

> On 23-Sep-2022, at 11:54 AM, Adrian Hunter <adrian.hunter@intel.com> wrote:
> 
> On 22/09/22 22:15, Arnaldo Carvalho de Melo wrote:
>> Em Wed, Sep 21, 2022 at 10:38:38PM +0530, Athira Rajeev escreveu:
>>> The perf test named “build id cache operations” skips with below
>>> error on some distros:
>> 
>> I wonder if we shouldn't instead state that bash is needed?
>> 
>> ⬢[acme@toolbox perf-urgent]$ head -1 tools/perf/tests/shell/*.sh | grep ^#
>> #!/bin/sh
>> #!/bin/bash
>> #!/bin/sh
>> #!/bin/sh
>> #!/bin/sh
>> #!/bin/sh
>> #!/bin/sh
>> #!/bin/sh
>> #!/bin/sh
>> #!/bin/sh
>> #!/bin/bash
>> #!/bin/sh
>> #!/bin/sh
>> #!/bin/sh
>> #!/bin/bash
>> #!/bin/sh
>> #!/bin/bash
>> #!/bin/sh
>> #!/bin/sh
>> #!/bin/sh
>> #!/bin/sh
>> #!/bin/sh
>> #!/bin/sh
>> #!/bin/sh
>> #!/bin/sh
>> #!/bin/sh
>> ⬢[acme@toolbox perf-urgent]$
>> 
>> Opinions?
>> 
> 
> Please don't change tools/perf/tests/shell/test_intel_pt.sh
> 
> I started using shellcheck on that with the "perf test: 
> test_intel_pt.sh: Add per-thread test" patch set that I sent.
> 
> FYI, if you use shellcheck on buildid.sh, it shows up issues even
> after changing to bash:
> 
> *** Before ***
> 
> $ shellcheck -S warning tools/perf/tests/shell/buildid.sh 
> 
> In tools/perf/tests/shell/buildid.sh line 69:
>        link=${build_id_dir}/.build-id/${id:0:2}/${id:2}
>                                       ^-------^ SC2039: In POSIX sh, string indexing is undefined.
>                                                 ^-----^ SC2039: In POSIX sh, string indexing is undefined.
> 
> 
> In tools/perf/tests/shell/buildid.sh line 77:
>        file=${build_id_dir}/.build-id/${id:0:2}/`readlink ${link}`/elf
>                                       ^-------^ SC2039: In POSIX sh, string indexing is undefined.
> 
> 
> In tools/perf/tests/shell/buildid.sh line 123:
>        echo "running: perf record $@"
>                                   ^-- SC2145: Argument mixes string and array. Use * or separate argument.
> 
> 
> In tools/perf/tests/shell/buildid.sh line 124:
>        ${perf} record --buildid-all -o ${data} $@ &> ${log}
>                                                ^-- SC2068: Double quote array expansions to avoid re-splitting elements.
>                                                   ^-------^ SC2039: In POSIX sh, &> is undefined.
> 
> 
> In tools/perf/tests/shell/buildid.sh line 126:
>                echo "failed: record $@"
>                                     ^-- SC2145: Argument mixes string and array. Use * or separate argument.
> 
> 
> In tools/perf/tests/shell/buildid.sh line 131:
>        check ${@: -1}
>              ^------^ SC2068: Double quote array expansions to avoid re-splitting elements.
>              ^------^ SC2039: In POSIX sh, string indexing is undefined.
> 
> 
> In tools/perf/tests/shell/buildid.sh line 158:
> exit ${err}
>     ^----^ SC2154: err is referenced but not assigned.
> 
> For more information:
>  https://www.shellcheck.net/wiki/SC2068 -- Double quote array expansions to ...
>  https://www.shellcheck.net/wiki/SC2145 -- Argument mixes string and array. ...
>  https://www.shellcheck.net/wiki/SC2039 -- In POSIX sh, &> is undefined.
> 
> *** After ***
> 
> $ shellcheck -S warning tools/perf/tests/shell/buildid.sh 
> 
> In tools/perf/tests/shell/buildid.sh line 123:
>        echo "running: perf record $@"
>                                   ^-- SC2145: Argument mixes string and array. Use * or separate argument.
> 
> 
> In tools/perf/tests/shell/buildid.sh line 124:
>        ${perf} record --buildid-all -o ${data} $@ &> ${log}
>                                                ^-- SC2068: Double quote array expansions to avoid re-splitting elements.
> 
> 
> In tools/perf/tests/shell/buildid.sh line 126:
>                echo "failed: record $@"
>                                     ^-- SC2145: Argument mixes string and array. Use * or separate argument.
> 
> 
> In tools/perf/tests/shell/buildid.sh line 131:
>        check ${@: -1}
>              ^------^ SC2068: Double quote array expansions to avoid re-splitting elements.
> 
> 
> In tools/perf/tests/shell/buildid.sh line 158:
> exit ${err}
>     ^----^ SC2154: err is referenced but not assigned.
> 
> For more information:
>  https://www.shellcheck.net/wiki/SC2068 -- Double quote array expansions to ...
>  https://www.shellcheck.net/wiki/SC2145 -- Argument mixes string and array. ...
>  https://www.shellcheck.net/wiki/SC2154 -- err is referenced but not assigned.