Re: [Qemu-devel] [PATCH v2 06/18] qemu-iotests: Improve portability by searching bash in the $PATH

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2361 bytes --]

On 1/29/19 11:53 AM, Philippe Mathieu-Daudé wrote:
> Bash is not always installed as /bin/bash. In particular on OpenBSD,
> the package installs it in /usr/local/bin.
> Use the 'env' shebang to search bash in the $PATH.
> 
> Patch created mechanically by running:
> 
>   $ git grep -lE '#! ?/bin/bash' -- tests/qemu-iotests | \
>     while read f; do \
>       sed -i 's_#!.\?/bin/bash_#! /usr/bin/env bash_' $f; \

I prefer | over _ when writing a sed expression around something with
embedded / (why? Because | has to be quoted in shell, but _ does not,
and it makes it more likely that I'll notice if I botched quoting or the
intended sed command).  But for a commit message, pasting what you
actually ran is fine.

Why a space between #! and /usr/bin/env? That's not our prevailing style:

$ git grep '^#! /' | wc
     13      30     541
$ git grep '^#!/' | wc
    337     448   13442

Mishandles instances of:

#!/bin/bash -x

or any other case where there is something after /bin/bash (since some
platforms permit at most 1 argument to the interpreter, and your
conversion would be trying to pass 2 arguments as 1).  BSD env (and more
recently GNU Coreutils env) added:

#!/usr/bin/env -S /bin/bash -x

to work around platforms with odd shebang limitations (the lone argument
"-S /bin/bash -x" is sensibly re-split by env before exec'ing bash with
an argument of -x) - but that's not portable yet.  Thankfully, it looks
like none of your conversions encountered that problem; but I might used
an explict end-of-line anchor to make sure, as in:

sed -i 's|^#!.\?/bin/bash$|#!/usr/bin/env bash|' $f

>     done
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> Can 'env' be located elsewhere than /usr/bin/env?

Possibly, but #! requires absolute paths on some platforms, so you have
a chicken-and-egg problem if it is not, so in practice /usr/bin/env is
reliable; and we're already using it elsewhere, so any platform where it
does not exist will have a lot more than just this patch to fix up.

Comments are tied to the commit message, but the mechanical conversion
itself looks sound, therefore:

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]