Re: [libvirt PATCH 09/12] ci: jobs.sh: run_cmd: Use eval to run commands

On Mon, Sep 18, 2023 at 11:31:53AM +0100, Daniel P. Berrangé wrote: > On Mon, Sep 18, 2023 at 12:22:45PM +0200, Erik Skultety wrote: > > We tried to evade usage of eval in commit 6214ae55f6a, but trying to > > use I/O redirections with a command doesn't have the desired effect, > > because when Shell eats the redirection it is applied to anything > > inside the run_cmd function, even the print command we use for > > debugging purposes. In order to print all commands and use the > > redirection only on the actual execution of a given command, let's > > adopt eval on "$@" and allow passing redirections as strings later on. > > Future patches will demonstrate this. > > > > Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; > > --- > > ci/jobs.sh | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/ci/jobs.sh b/ci/jobs.sh > > index f4e83dda2e..3a89cb1a69 100644 > > --- a/ci/jobs.sh > > +++ b/ci/jobs.sh > > @@ -15,7 +15,7 @@ fi > > GIT_ROOT="$(git rev-parse --show-toplevel)" > > run_cmd() { > > printf "\e[32m[RUN COMMAND]: '%s'\e[0m\n" "$*" > > - "$@" > > + eval "$@" > > } > > IMHO, we'd be better just creating a "run_cmd_quiet" variant which does > > "$@" 1>/dev/null 2>&1 I don't have a problem with ^this in principle, but eval is more flexible (and hence more dangerous) in what we can pass as parameters in the future.