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

[Ian Rogers]

On Thu, Sep 22, 2022 at 12:15 PM Arnaldo Carvalho de Melo
<acme at kernel.org> 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 at 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 at toolbox perf-urgent]$
>
> Opinions?

+1 to bash. Perhaps python longer term?  The XML test output generated
by things like kunit is possible to generate from either bash or
python, but in my experience the python stuff feels better built.

Thanks,
Ian

> - Arnaldo
>
> > <<>>
> >  78: build id cache operations                                       :
> > test child forked, pid 111101
> > WARNING: wine not found. PE binaries will not be run.
> > test binaries: /tmp/perf.ex.SHA1.PKz /tmp/perf.ex.MD5.Gt3 ./tests/shell/../pe-file.exe
> > DEBUGINFOD_URLS=
> > Adding 4abd406f041feb4f10ecde3fc30fd0639e1a91cb /tmp/perf.ex.SHA1.PKz: Ok
> > build id: 4abd406f041feb4f10ecde3fc30fd0639e1a91cb
> > ./tests/shell/buildid.sh: 69: ./tests/shell/buildid.sh: Bad substitution
> > test child finished with -2
> > build id cache operations: Skip
> > <<>>
> >
> > The test script "tests/shell/buildid.sh" uses some of the
> > string substitution ways which are supported in bash, but not in
> > "sh" or other shells. Above error on line number 69 that reports
> > "Bad substitution" is:
> >
> > <<>>
> > link=${build_id_dir}/.build-id/${id:0:2}/${id:2}
> > <<>>
> >
> > Here the way of getting first two characters from id ie,
> > ${id:0:2} and similarly expressions like ${id:2} is not
> > recognised in "sh". So the line errors and instead of
> > hitting failure, the test gets skipped as shown in logs.
> > So the syntax issue causes test not to be executed in
> > such cases. Similarly usage : "${@: -1}" [ to pick last
> > argument passed to a function] in “test_record” doesn’t
> > work in all distros.
> >
> > Fix this by using alternative way with "cut" command
> > to pick "n" characters from the string. Also fix the usage
> > of “${@: -1}” to work in all cases.
> >
> > Another usage in “test_record” is:
> > <<>>
> > ${perf} record --buildid-all -o ${data} $@ &> ${log}
> > <<>>
> >
> > This causes the perf record to start in background and
> > Results in the data file not being created by the time
> > "check" function is invoked. Below log shows perf record
> > result getting displayed after the call to "check" function.
> >
> > <<>>
> > running: perf record /tmp/perf.ex.SHA1.EAU
> > build id: 4abd406f041feb4f10ecde3fc30fd0639e1a91cb
> > link: /tmp/perf.debug.mLT/.build-id/4a/bd406f041feb4f10ecde3fc30fd0639e1a91cb
> > failed: link /tmp/perf.debug.mLT/.build-id/4a/bd406f041feb4f10ecde3fc30fd0639e1a91cb does not exist
> > test child finished with -1
> > build id cache operations: FAILED!
> > root at machine:~/athira/linux/tools/perf# Couldn't synthesize bpf events.
> > [ perf record: Woken up 1 times to write data ]
> > [ perf record: Captured and wrote 0.010 MB /tmp/perf.data.bFF ]
> > <<>>
> >
> > Fix this by redirecting output instead of using “&” which
> > starts the command in background.
> >
> > Signed-off-by: Athira Rajeev <atrajeev at linux.vnet.ibm.com>
> > ---
> >  tools/perf/tests/shell/buildid.sh | 16 +++++++++-------
> >  1 file changed, 9 insertions(+), 7 deletions(-)
> >
> > diff --git a/tools/perf/tests/shell/buildid.sh b/tools/perf/tests/shell/buildid.sh
> > index f05670d1e39e..3512c4423d48 100755
> > --- a/tools/perf/tests/shell/buildid.sh
> > +++ b/tools/perf/tests/shell/buildid.sh
> > @@ -66,7 +66,7 @@ check()
> >       esac
> >       echo "build id: ${id}"
> >
> > -     link=${build_id_dir}/.build-id/${id:0:2}/${id:2}
> > +     link=${build_id_dir}/.build-id/$(echo ${id}|cut -c 1-2)/$(echo ${id}|cut -c 3-)
> >       echo "link: ${link}"
> >
> >       if [ ! -h $link ]; then
> > @@ -74,7 +74,7 @@ check()
> >               exit 1
> >       fi
> >
> > -     file=${build_id_dir}/.build-id/${id:0:2}/`readlink ${link}`/elf
> > +     file=${build_id_dir}/.build-id/$(echo ${id}|cut -c 1-2)/`readlink ${link}`/elf
> >       echo "file: ${file}"
> >
> >       if [ ! -x $file ]; then
> > @@ -117,20 +117,22 @@ test_record()
> >  {
> >       data=$(mktemp /tmp/perf.data.XXX)
> >       build_id_dir=$(mktemp -d /tmp/perf.debug.XXX)
> > -     log=$(mktemp /tmp/perf.log.XXX)
> > +     log_out=$(mktemp /tmp/perf.log.out.XXX)
> > +     log_err=$(mktemp /tmp/perf.log.err.XXX)
> >       perf="perf --buildid-dir ${build_id_dir}"
> > +     eval last=\${$#}
> >
> >       echo "running: perf record $@"
> > -     ${perf} record --buildid-all -o ${data} $@ &> ${log}
> > +     ${perf} record --buildid-all -o ${data} $@ 1>${log_out} 2>${log_err}
> >       if [ $? -ne 0 ]; then
> >               echo "failed: record $@"
> > -             echo "see log: ${log}"
> > +             echo "see log: ${log_err}"
> >               exit 1
> >       fi
> >
> > -     check ${@: -1}
> > +     check $last
> >
> > -     rm -f ${log}
> > +     rm -f ${log_out} ${log_err}
> >       rm -rf ${build_id_dir}
> >       rm -rf ${data}
> >  }
> > --
> > 2.17.1
>
> --
>
> - Arnaldo