On 17-Jan-2023, at 3:30 AM, Ian Rogers [off-list ref] wrote:

On Mon, Jan 16, 2023 at 4:00 AM Athira Rajeev
[off-list ref] wrote:

quoted

quoted

On 16-Jan-2023, at 11:47 AM, Ian Rogers [off-list ref] wrote:

On Sun, Jan 15, 2023 at 9:03 PM Athira Rajeev
[off-list ref] wrote:

quoted

quoted

On 28-Sep-2022, at 10:24 AM, Athira Rajeev [off-list ref] wrote:

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

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

Thanks Ian for the response.

quoted

I think some of what the patch is doing is good, some of it the

Ian, can I take this as an ack for the patch so that Arnaldo can pick this in acme git ?

Acked-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

Thanks Ian.

Hi Arnaldo,

I have re-posted the patch 2 ( in this series ) separately in new patchset here :
https://lore.kernel.org/linux-perf-users/20230116050131.17221-2-atrajeev@linux.vnet.ibm.com/ (local)

Ack from Ian is for Patch 1. Shall I rebase to upstream and post this patch 1 separately ?

Thanks
Athira

quoted

quoted

readability becomes a little harder by not being bash. I'm agnostic as
to which approach to take for the fix.

May be we can take this as separate mail thread to get everyone’s opinion on changing all tests in "tools/perf/tests/shell" except test_intel_pt.sh to use “bash" ?

quoted

An aside, I noticed that we do run some tests at build time:
https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/tree/lib/Makefile?h=perf/core#n390
So perhaps we can have a shellcheck build option, defaulted off but
enabled as part of Arnaldo's regular test scripts. The shellcheck
build option would run shellcheck to make sure that there weren't
errors in the shell code, which it is far too easy to introduce.

Sure, that is a good option to have. I will check on having “shellcheck" as a build option.

Thanks
Athira

quoted

Thanks,
Ian

quoted

quoted

quoted

On 23-Sep-2022, at 11:54 AM, Adrian Hunter [off-list ref] wrote:

On 22/09/22 22:15, Arnaldo Carvalho de Melo wrote:

quoted

Em Wed, Sep 21, 2022 at 10:38:38PM +0530, Athira Rajeev escreveu:

quoted

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.