From: Junio C Hamano <gitster@pobox.com>
To: Eric Sunshine <sunshine@sunshineco.com>
Cc: Armin Kunaschik <megabreit@googlemail.com>,
Git List <git@vger.kernel.org>
Subject: Re: t4151 missing quotes
Date: Mon, 09 May 2016 13:45:10 -0700 [thread overview]
Message-ID: <xmqqmvnzt0yx.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <CAPig+cS4Bj4N8d1a29z8=f30owOec1pB=yF32ZUPmDH2Tu2kXA@mail.gmail.com> (Eric Sunshine's message of "Mon, 9 May 2016 16:16:03 -0400")
Eric Sunshine <sunshine@sunshineco.com> writes:
> On Mon, May 9, 2016 at 2:56 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Something like this follows Documentation/SubmittingPatches [...]
>>
>> -- >8 --
>> From: Armin Kunaschik <megabreit@googlemail.com>
>> Subject: t4151: make sure argument to 'test -z' is given
>>
>> 88d50724 (am --skip: revert changes introduced by failed 3way merge,
>> 2015-06-06), unlike all the other patches in the series, forgot to
>> quote the output from "$(git ls-files -u)" when using it as the
>> argument to "test -z", leading to a syntax error.
>
> To make it clear that this was not a syntax error in the typical case,
> it might make sense to say:
>
> ...potentially leading to a syntax error if some earlier tests failed.
Hmph, do we have a broken &&-chain?
If an earlier test fails and leaves an unmerged path, "ls-files -u"
would give some output, so "test -z" would get one or more non-empty
strings; if we feed multiple, this would fail. But we would not have
even run "test -z" as long as we properly &&-chain these tests.
I think the real issue is when the earlier step succeeds and does
not leave any unmerged path. In that case, we would run "test -z"
without anything else on the command line, which would lead to an
syntax error.
Side Note: /usr/bin/test and test (built into bash and dash)
seem not to care about the lack of string in "test -z <string>"
and "test -n <string>". It appears to me that they just take
"-z" and "-n" without "<string>" as a special case of "test
<string>" that is fed "-z" or "-n" as <string>. Apparently, the
platform Armin is working on doesn't.
Perhaps
... leading to a syntax error on some platforms whose "test"
does not interpret "test -z" (no other arguments) as testing if
a string "-z" is the null string (which GNU test and test that
is built into bash and dash seem to do).
would be an improvement?
next prev parent reply other threads:[~2016-05-09 20:45 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-05-09 16:09 t4151 missing quotes Armin Kunaschik
2016-05-09 16:22 ` Eric Sunshine
2016-05-09 16:26 ` Eric Sunshine
2016-05-09 16:35 ` Armin Kunaschik
2016-05-09 17:57 ` Eric Sunshine
2016-05-09 18:11 ` Armin Kunaschik
2016-05-09 18:56 ` Junio C Hamano
2016-05-09 19:19 ` Stefan Beller
2016-05-09 20:16 ` Eric Sunshine
2016-05-09 20:45 ` Junio C Hamano [this message]
2016-05-09 21:35 ` Eric Sunshine
2016-05-10 13:44 ` Armin Kunaschik
2016-05-10 13:48 ` Jeff King
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=xmqqmvnzt0yx.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=megabreit@googlemail.com \
--cc=sunshine@sunshineco.com \
/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.