From: Junio C Hamano <gitster@pobox.com>
Date: Mon, 20 Jun 2011 19:51:47 GMT
Subject: Re: [PATCH v2 15/20] transport-helper: use the new done feature where possible
Message-ID: <7vzklcb7sc.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20110620114556.GA29818@elie>

Jonathan Nieder <jrnieder@gmail.com> writes:

>> @@ -412,11 +413,8 @@ static int fetch_with_import(struct transport *transport,
>>  		sendline(data, &buf);
>>  		strbuf_reset(&buf);
>>  	}
>> -	if (disconnect_helper(transport))
>> -		die("Error while disconnecting helper");
>>  	if (finish_command(&fastimport))
>>  		die("Error while running fast-import");
>> -
>
> What is this change about?  Is the plan to allow other commands after
> a fetch_with_import?  Sounds reasonable; I think it should be
> advertised in the log message, though.
>
> When does the disconnect_helper call happen (to avoid leaks)?  Ah, in
> release_helper; phew.
>
> The disconnect_helper call writes the blank line that terminates the
> list of "import %s" commands to start the import, so there would need
> to be a
>
> 	strbuf_reset(&buf);
> 	strbuf_addf(&buf, "\n");
> 	sendline(data, &buf);
>
> in its place.

All sensible suggestions for a potential reroll.