[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH v2 06/18] qemu-iotests: Improve portability by s

From: Eric Blake
Subject: Re: [Qemu-block] [PATCH v2 06/18] qemu-iotests: Improve portability by searching bash in the $PATH
Date: Tue, 29 Jan 2019 16:15:27 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0

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é <address@hidden>
> ---
> 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 <address@hidden>

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

Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]