Re: [PATCH v2 2/2] merge-base: "--reflog" mode finds fork point from reflog entries

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 25, 2013, 16:53 UTC
Message-ID: <xmqqmwlxjnq6.fsf@gitster.dls.corp.google.com>
In-Reply-To: <526A19CA.9020609@viscovery.net>

Johannes Sixt <j.sixt@viscovery.net> writes:

Show 9 quoted lines

>> +	for count in 1 2 3 4 5
>> +	do
>> +		git merge-base --reflog base $(cat derived$count) >actual &&
>> +		test_cmp expect$count actual || break
>> +	done &&
>
> This does not work as intended because the exit code of 'break' is always
> zero. Unlike 'exit' and 'return', it does *not* pick up the exit code of
> the last command that was executed.

You are right. I obviously was not thinking straight.

> You could put the loops into a function from which you 'return',
> but that is obscure in this case. The first iteration was better,
> IMO.

I do not think using "return 1" is a good thing to do, either.

We saw breakages with different shells around the use of "return" and we know the original "exit 1 inside subshell" works reliably everywher.

I'll send out a revamped version later today, updating not just the test but the implementation.

Thank for a dose of sanity.