qemu-trivial
[Top][All Lists]
Advanced

[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

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

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