[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-trivial] [Qemu-devel] [PATCH] configure: Use $(..) instead of
From: |
Eric Blake |
Subject: |
Re: [Qemu-trivial] [Qemu-devel] [PATCH] configure: Use $(..) instead of deprecated `..` |
Date: |
Wed, 8 Jun 2016 08:05:28 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
On 06/08/2016 06:16 AM, Maciej W. Rozycki wrote:
> On Mon, 16 May 2016, Stefan Weil wrote:
>
>> This fixes these warnings from shellcheck:
>>
>> ^-- SC2006: Use $(..) instead of deprecated `..`
>>
>> Signed-off-by: Stefan Weil <address@hidden>
>> ---
>>
>> More warnings from shellcheck for configure and other files
>> will be handled by later patches.
>
> Unlike `..` the $(..) Bourne shell construct is not fully portable, some
> implementations do not recognise it.
All POSIX implementations support it. The only shell that doesn't is
from Solaris /bin/sh, and that pre-dates modern OpenSolaris which has
(finally) modernized their shell. And I don't think we have anyone
actively trying to build qemu for Solaris (we have bug reports for
mingw, BSD, and Mac OS, but I haven't seen anyone complaining about a
failed Solaris build).
> So what's the technical justification -- beyond shutting up some random
> checker tool -- for making this change? Does the benefit recognised in
> making this change outweigh the limitation introduced? How about handling
> failures properly?
We already have places where we assume a POSIX shell, and $() is cleaner
to use than `` when it comes to nesting quotes.
> NB given the above, and especially because of the introduced functional
> regression mentioned, I don't think this change qualifies as trivial.
I agree that calling this trivial may be a stretch, but I see no problem
with the patch itself if the commit message is beefed up to provide more
justification than just "silence a lint-like tool".
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature