Re: [PATCH] remote-curl: do not complain on EOF from parent git
- From
Keller, Jacob E <jacob.e.keller@intel.com>
- Date
- Jul 9, 2014, 21:25 UTC
- Message-ID
- <1404941118.23510.34.camel@jekeller-desk1.amr.corp.intel.com>
- In-Reply-To
- <20140709212043.GC25854@sigill.intra.peff.net>
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
Show 57 quoted lines
> 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)