Re: [PATCH v14 17/27] bisect--helper: `bisect_autostart` shell function in C

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 26, 2016, 21:09 UTC
Message-ID: <xmqqy43jdyos.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <01020156b73fe6d7-8b80c663-7c77-469e-811f-40200ec6dbb1-000000@eu-west-1.amazonses.com>

Pranit Bauva <pranit.bauva@gmail.com> writes:

Show 8 quoted lines

> @@ -410,6 +413,7 @@ static int bisect_next(struct bisect_terms *terms, const char *prefix)
>  {
>  	int res, no_checkout;
>  
> +	bisect_autostart(terms);
>  	/* In case of mistaken revs or checkout error, or signals received,
>  	 * "bisect_auto_next" below may exit or misbehave.
>  	 * We have to trap this to be able to clean up using

This change is extremely hard to reason about.

Why wasn't auto-start done at the very beginning of this function before? Why does this start calling it all of a sudden? Before autostart was rewritten in C in this step, who made sure the shell version of autostart was called before bisect_next is called (which must have become unnecessary with this step, and I fully expected a removal of such a call in the shell version, but I didn't find such a change in this patch)?

Or is this series simply broken by lossage of autostart feature in "bisect next" between the step where bisect_next is rewritten in C and this step?