Re: [PATCH] remote-curl: do not complain on EOF from parent git

["Keller, Jacob E" <jacob.e.keller@intel.com>]

On Wed, 2014-07-09 at 17:20 -0400, Jeff King wrote:
> The parent git process is supposed to send us an empty line
> to indicate that the conversation is over. However, the
> parent process may die() if there is a problem with the
> operaiton (e.g., we try to fetch a ref that does not exist). 

Nitpick, but you probably meant operation here.

Thanks,
Jake

> In this case, it produces a useful message, but then
> remote-curl _also_ produces an unhelpful message:
> 
>   $ git pull origin matser
>   fatal: couldn't find remote ref matser
>   Unexpected end of command stream
> 
> The "right" way to fix this is to teach the parent git to
> always cleanly close the connection to the helper, letting
> it know that we are done. Implementing that is rather
> clunky, though, as it would involve either replacing die()
> operations with returning errors up the stack (until we
> disconnect the transport), or adding an atexit handler to
> clean up any transport helpers left open.
> 
> It's much simpler to just suppress the EOF message in
> remote-curl. It was not added to address any real-world
> situation in the first place, but rather a "we should
> probably report unexpected things" suggestion[1].
> 
> It is the parent git which drives the operation, and whose
> exit value actually matters. If the parent dies, then the
> helper has no need to complain (except as a debugging aid).
> In the off chance that the pipe is closed without the parent
> dying, the parent can still notice the non-zero exit code.
> 
> [1] http://article.gmane.org/gmane.comp.version-control.git/176036
> 
> Reported-by: Dmitry <wipedout@yandex.ru>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> The original discussion that led to this code being implemented was due
> to us checking the helper's exit code in the first place. However, we
> seem to be inconsistent about doing so. I'm not inclined to pursue it
> further, though, as these subtle details of the transport helper code
> usually turn into a can of worms, and more importantly, I don't think it
> hurts anything in the real world. Either the parent git gets the
> expected protocol output from the helper or it doesn't, and we report
> errors on that. An error from a helper after the operation completes is
> not really important to the parent git either way.
> 
>  remote-curl.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/remote-curl.c b/remote-curl.c
> index 4493b38..0454ffc 100644
> --- a/remote-curl.c
> +++ b/remote-curl.c
> @@ -971,8 +971,6 @@ int main(int argc, const char **argv)
>  		if (strbuf_getline(&buf, stdin, '\n') == EOF) {
>  			if (ferror(stdin))
>  				fprintf(stderr, "Error reading command stream\n");
> -			else
> -				fprintf(stderr, "Unexpected end of command stream\n");
>  			return 1;
>  		}
>  		if (buf.len == 0)