public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Michael J Gruber <git@drmicha•warpmail.net>
To: Jeff King <peff@peff•net>
Cc: git@vger•kernel.org, Junio C Hamano <gitster@pobox•com>
Subject: Re: [PATCHv2 4/6] t7510: exit for loop with test result
Date: Fri, 13 Jun 2014 14:22:36 +0200	[thread overview]
Message-ID: <539AED0C.8050107@drmicha.warpmail.net> (raw)
In-Reply-To: <539AE8CA.50009@drmicha.warpmail.net>

Michael J Gruber venit, vidit, dixit 13.06.2014 14:04:
> Jeff King venit, vidit, dixit 13.06.2014 13:46:
>> On Fri, Jun 13, 2014 at 12:42:46PM +0200, Michael J Gruber wrote:
>>
>>> When t7510 was introduced, the author made sure that a for loop in
>>> a subshell would return with the appropriate error code.
>>>
>>> Make sure this is true also the for the first line in each loop, which
>>> was missed.
>>>
>>> Signed-off-by: Michael J Gruber <git@drmicha•warpmail.net>
>>> ---
>>>  t/t7510-signed-commit.sh | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh
>>> index 5ddac1a..a5ba48e 100755
>>> --- a/t/t7510-signed-commit.sh
>>> +++ b/t/t7510-signed-commit.sh
>>> @@ -49,7 +49,7 @@ test_expect_success GPG 'show signatures' '
>>>  	(
>>>  		for commit in initial second merge fourth-signed fifth-signed sixth-signed master
>>>  		do
>>> -			git show --pretty=short --show-signature $commit >actual &&
>>> +			git show --pretty=short --show-signature $commit >actual || exit 1
>>>  			grep "Good signature from" actual || exit 1
>>
>> Hrm. The original is:
>>
>>   X &&
>>   Y || exit 1
>>
>> Won't that still exit (i.e., it is already correct)? Doing:
>>
>>   for X in true false; do
>>     for Y in true false; do
>>       ($X && $Y || exit 1)
>>       echo "$X/$Y: $?"
>>     done
>>   done
>>
>> yields:
>>
>>   true/true: 0
>>   true/false: 1
>>   false/true: 1
>>   false/false: 1
>>
>> (and should still short-circuit Y, because we go from left-to-right).
>>
>> I do not mind changing it to keep the style of each line consistent,
>> though. I would have written it as a series of "&&"-chains, with a
>> single exit at the end, but I think that is just a matter of preference.
> 
> If I remember correctly, I put something failing on the first line of
> the original version, and the test succeeded. I think the point is that
> we have a for loop in a subshell, and we need to make sure that the
> false of one iteration is not overwritten by the true of the next one -
> "exit 1" makes sure to "break" the for loop and exit the subshell.
> (The chain should do that as well, I'll recheck.)

... the chain does not, which is the point :)

With X && Y || exit 1 inside the loop, the loop statement will return
false, but the loop will continue (if X returns false), which is exactly
the problem that the exit avoids.

Make your example iterate over false true instead in the inner loop and
you'll see ;)

Michael

  reply	other threads:[~2014-06-13 12:22 UTC|newest]

Thread overview: 75+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-06 14:15 [PATCH 0/3] verify-commit: verify commit signatures Michael J Gruber
2014-06-06 14:15 ` [PATCH 1/3] pretty: free the gpg status buf Michael J Gruber
2014-06-06 14:15 ` [PATCH 2/3] gpg-interface: provide access to the payload Michael J Gruber
2014-06-13  7:55   ` Jeff King
2014-06-13  9:44     ` Michael J Gruber
2014-06-13 10:34       ` Jeff King
2014-06-06 14:15 ` [PATCH 3/3] verify-commit: scriptable commit signature verification Michael J Gruber
2014-06-11 19:48   ` Michael J Gruber
2014-06-13  8:02   ` Jeff King
2014-06-13  9:55     ` Michael J Gruber
2014-06-13 11:09       ` Jeff King
2014-06-13 17:06         ` Junio C Hamano
2014-06-16  9:21           ` Michael J Gruber
2014-06-16 19:54           ` Jeff King
2014-06-16 20:34             ` Junio C Hamano
2014-06-16 20:39               ` Jeff King
2014-06-27 12:31                 ` Michael J Gruber
2014-06-27 12:49                   ` Michael J Gruber
2014-06-27 13:06                     ` Michael J Gruber
2014-06-27 13:18                       ` [PATCH] log: correctly identify mergetag signature verification status Michael J Gruber
2014-06-28  0:44                         ` Jeff King
2014-07-10 22:27                           ` Junio C Hamano
2014-06-27 13:50                     ` [PATCH 3/3] verify-commit: scriptable commit signature verification Michael J Gruber
2014-06-27 18:55                       ` Junio C Hamano
2014-06-27 18:36                     ` Junio C Hamano
2014-06-28  0:32                       ` Jeff King
2014-06-30  6:14                         ` Junio C Hamano
2014-06-13 10:42 ` [PATCHv2 0/6] verify-commit: verify commit signatures Michael J Gruber
2014-06-13 10:42   ` [PATCHv2 1/6] pretty: free the gpg status buf Michael J Gruber
2014-06-13 11:39     ` Jeff King
2014-06-13 10:42   ` [PATCHv2 2/6] gpg-interface: provide access to the payload Michael J Gruber
2014-06-13 10:42   ` [PATCHv2 3/6] verify-commit: scriptable commit signature verification Michael J Gruber
2014-06-13 11:19     ` Jeff King
2014-06-13 11:45       ` Michael J Gruber
2014-06-13 11:50         ` Jeff King
2014-06-13 12:12           ` Michael J Gruber
2014-06-13 10:42   ` [PATCHv2 4/6] t7510: exit for loop with test result Michael J Gruber
2014-06-13 11:46     ` Jeff King
2014-06-13 12:04       ` Michael J Gruber
2014-06-13 12:22         ` Michael J Gruber [this message]
2014-06-13 12:33           ` Michael J Gruber
2014-06-13 12:45             ` Jeff King
2014-06-13 12:54             ` Johannes Sixt
2014-06-13 13:06               ` Michael J Gruber
2014-06-13 13:21                 ` Johannes Sixt
2014-06-13 13:30                   ` Jeff King
2014-06-13 13:31                   ` Michael J Gruber
2014-06-13 13:42                     ` Johannes Sixt
2014-06-13 18:23       ` Junio C Hamano
2014-06-13 10:42   ` [PATCHv2 5/6] t7510: test verify-commit Michael J Gruber
2014-06-13 11:51     ` Jeff King
2014-06-13 12:14       ` Michael J Gruber
2014-06-13 18:16         ` Junio C Hamano
2014-06-13 10:42   ` [PATCHv2 6/6] gpg-interface: provide clear helper for struct signature_check Michael J Gruber
2014-06-23  7:05   ` [PATCHv3 0/5] verify-commit: verify commit signatures Michael J Gruber
2014-06-23  7:05     ` [PATCHv3 1/5] gpg-interface: provide clear helper for struct signature_check Michael J Gruber
2014-06-23  7:05     ` [PATCHv3 2/5] gpg-interface: provide access to the payload Michael J Gruber
2014-06-23  7:05     ` [PATCHv3 3/5] verify-commit: scriptable commit signature verification Michael J Gruber
2014-06-23  7:05     ` [PATCHv3 4/5] t7510: exit for loop with test result Michael J Gruber
2014-06-23  7:05     ` [PATCHv3 5/5] t7510: test verify-commit Michael J Gruber
2014-06-23 23:02       ` Junio C Hamano
2014-06-23 17:28     ` [PATCHv3 0/5] verify-commit: verify commit signatures Jeff King
2014-06-23 17:52       ` Junio C Hamano
2014-06-23 21:09         ` Jeff King
2014-06-23 21:23           ` Junio C Hamano
2014-06-27 14:13             ` [PATCHv4 0/4] " Michael J Gruber
2014-06-27 14:13               ` [PATCHv4 1/4] gpg-interface: provide clear helper for struct signature_check Michael J Gruber
2014-06-27 14:13               ` [PATCHv4 2/4] gpg-interface: provide access to the payload Michael J Gruber
2014-06-27 14:13               ` [PATCHv4 3/4] verify-commit: scriptable commit signature verification Michael J Gruber
2014-06-27 14:13               ` [PATCHv4 4/4] t7510: test verify-commit Michael J Gruber
2014-06-27 19:32                 ` Junio C Hamano
2014-06-27 20:26                   ` Michael J Gruber
2014-06-27 19:07               ` [PATCHv4 0/4] verify-commit: verify commit signatures Junio C Hamano
2014-06-28  0:48                 ` Jeff King
2014-06-28  0:49               ` 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=539AED0C.8050107@drmicha.warpmail.net \
    --to=git@drmicha$(echo .)warpmail.net \
    --cc=git@vger$(echo .)kernel.org \
    --cc=gitster@pobox$(echo .)com \
    --cc=peff@peff$(echo .)net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox