From: Eric Blake <eblake@redhat.com>
To: "Maciej W. Rozycki" <macro@imgtec.com>, Stefan Weil <sw@weilnetz.de>
Cc: QEMU Trivial <qemu-trivial@nongnu.org>,
QEMU Developer <qemu-devel@nongnu.org>
Subject: Re: [Qemu-trivial] [Qemu-devel] [PATCH] configure: Use $(..) instead of deprecated `..`
Date: Wed, 8 Jun 2016 08:05:28 -0600 [thread overview]
Message-ID: <57582628.9060306@redhat.com> (raw)
In-Reply-To: <alpine.DEB.2.00.1606081305490.10382@tp.orcam.me.uk>
[-- Attachment #1: Type: text/plain, Size: 1715 bytes --]
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 <sw@weilnetz.de>
>> ---
>>
>> 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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
WARNING: multiple messages have this Message-ID (diff)
From: Eric Blake <eblake@redhat.com>
To: "Maciej W. Rozycki" <macro@imgtec.com>, Stefan Weil <sw@weilnetz.de>
Cc: QEMU Trivial <qemu-trivial@nongnu.org>,
QEMU Developer <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] [PATCH] configure: Use $(..) instead of deprecated `..`
Date: Wed, 8 Jun 2016 08:05:28 -0600 [thread overview]
Message-ID: <57582628.9060306@redhat.com> (raw)
In-Reply-To: <alpine.DEB.2.00.1606081305490.10382@tp.orcam.me.uk>
[-- Attachment #1: Type: text/plain, Size: 1715 bytes --]
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 <sw@weilnetz.de>
>> ---
>>
>> 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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
next prev parent reply other threads:[~2016-06-08 14:05 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-05-16 13:10 [Qemu-trivial] [PATCH] configure: Use $(..) instead of deprecated `..` Stefan Weil
2016-05-16 13:10 ` [Qemu-devel] " Stefan Weil
2016-05-29 8:19 ` [Qemu-trivial] " Michael Tokarev
2016-05-29 8:19 ` [Qemu-devel] " Michael Tokarev
2016-06-08 12:16 ` [Qemu-trivial] " Maciej W. Rozycki
2016-06-08 12:16 ` Maciej W. Rozycki
2016-06-08 12:48 ` [Qemu-trivial] " Daniel P. Berrange
2016-06-08 12:48 ` Daniel P. Berrange
2016-06-08 13:07 ` [Qemu-trivial] " Stefan Weil
2016-06-08 13:07 ` Stefan Weil
2016-06-08 13:46 ` [Qemu-trivial] " Maciej W. Rozycki
2016-06-08 13:46 ` Maciej W. Rozycki
2016-06-08 14:05 ` Eric Blake [this message]
2016-06-08 14:05 ` Eric Blake
2016-06-08 14:38 ` [Qemu-trivial] " Maciej W. Rozycki
2016-06-08 14:38 ` Maciej W. Rozycki
2016-06-08 15:07 ` [Qemu-trivial] " Peter Maydell
2016-06-08 15:07 ` Peter Maydell
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=57582628.9060306@redhat.com \
--to=eblake@redhat.com \
--cc=macro@imgtec.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-trivial@nongnu.org \
--cc=sw@weilnetz.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.