git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Dmitry <wipedout@yandex.ru>
Cc: git@vger.kernel.org
Subject: [PATCH] remote-curl: do not complain on EOF from parent git
Date: Wed, 9 Jul 2014 17:20:43 -0400	[thread overview]
Message-ID: <20140709212043.GC25854@sigill.intra.peff.net> (raw)
In-Reply-To: <20140709205902.GB25854@sigill.intra.peff.net>

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).
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)
-- 
2.0.0.566.gfe3e6b2

  reply	other threads:[~2014-07-09 21:20 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-09  7:37 "Unexpected end of command stream" message looks irrelevant when I try to pull a non-existing branch Dmitry
2014-07-09 20:59 ` Jeff King
2014-07-09 21:20   ` Jeff King [this message]
2014-07-09 21:25     ` [PATCH] remote-curl: do not complain on EOF from parent git Keller, Jacob E
2014-07-09 21:42       ` Jeff King
2014-07-09 21:45     ` [PATCH 1/3] " Jeff King
2014-07-09 21:47     ` [PATCH 2/3] remote-curl: use error instead of fprintf(stderr) Jeff King
2014-07-09 21:47     ` [PATCH 3/3] remote-curl: mark helper-protocol errors more clearly Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140709212043.GC25854@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=wipedout@yandex.ru \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).