From 37943e4c3867323593699d5458d2d7925fdf32fc Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 9 Jul 2014 17:45:43 -0400 Subject: remote-curl: do not complain on EOF from parent git 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 operation (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, it can still notice the non-zero exit code. [1] http://article.gmane.org/gmane.comp.version-control.git/176036 Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- remote-curl.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'remote-curl.c') diff --git a/remote-curl.c b/remote-curl.c index 10cb0114ea..711a7c64ed 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) -- cgit v1.2.1 From b725b270d1474819d46161909416ef73ba2170ff Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 9 Jul 2014 17:47:05 -0400 Subject: remote-curl: use error instead of fprintf(stderr) We usually prefix our error messages with "error: ", but many error messages from remote-curl are simply printed with fprintf. This can make the output a little harder to read (especially because such message may be intermingled with errors from the parent git process). There is no reason to avoid error(), as we are already calling it many places (in addition to libgit.a functions which use it). While we're adjusting the messages, we can also drop the capitalization which makes them unlike other git error messages. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- remote-curl.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'remote-curl.c') diff --git a/remote-curl.c b/remote-curl.c index 711a7c64ed..5f1d85135f 100644 --- a/remote-curl.c +++ b/remote-curl.c @@ -396,7 +396,7 @@ static curlioerr rpc_ioctl(CURL *handle, int cmd, void *clientp) rpc->pos = 0; return CURLIOE_OK; } - fprintf(stderr, "Unable to rewind rpc post data - try increasing http.postBuffer\n"); + error("unable to rewind rpc post data - try increasing http.postBuffer"); return CURLIOE_FAILRESTART; default: @@ -709,7 +709,7 @@ static int fetch_dumb(int nr_heads, struct ref **to_fetch) free(targets[i]); free(targets); - return ret ? error("Fetch failed.") : 0; + return ret ? error("fetch failed.") : 0; } static int fetch_git(struct discovery *heads, @@ -949,7 +949,7 @@ int main(int argc, const char **argv) git_extract_argv0_path(argv[0]); setup_git_directory_gently(&nongit); if (argc < 2) { - fprintf(stderr, "Remote needed\n"); + error("remote needed"); return 1; } @@ -970,7 +970,7 @@ int main(int argc, const char **argv) do { if (strbuf_getline(&buf, stdin, '\n') == EOF) { if (ferror(stdin)) - fprintf(stderr, "Error reading command stream\n"); + error("error reading command stream"); return 1; } if (buf.len == 0) @@ -1014,7 +1014,7 @@ int main(int argc, const char **argv) printf("\n"); fflush(stdout); } else { - fprintf(stderr, "Unknown command '%s'\n", buf.buf); + error("unknown command '%s'", buf.buf); return 1; } strbuf_reset(&buf); -- cgit v1.2.1 From cdaa4e98ca56586f78a455bd624457248347a56d Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 9 Jul 2014 17:47:20 -0400 Subject: remote-curl: mark helper-protocol errors more clearly When we encounter an error in remote-curl, we generally just report it to stderr. There is no need for the user to care that the "could not connect to server" error was generated by git-remote-https rather than a function in the parent git-fetch process. However, when the error is in the protocol between git and the helper, it makes sense to clearly identify which side is complaining. These cases shouldn't ever happen, but when they do, we can make them less confusing by being more verbose. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- remote-curl.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'remote-curl.c') diff --git a/remote-curl.c b/remote-curl.c index 5f1d85135f..449c24defa 100644 --- a/remote-curl.c +++ b/remote-curl.c @@ -949,7 +949,7 @@ int main(int argc, const char **argv) git_extract_argv0_path(argv[0]); setup_git_directory_gently(&nongit); if (argc < 2) { - error("remote needed"); + error("remote-curl: usage: git remote-curl []"); return 1; } @@ -970,14 +970,14 @@ int main(int argc, const char **argv) do { if (strbuf_getline(&buf, stdin, '\n') == EOF) { if (ferror(stdin)) - error("error reading command stream"); + error("remote-curl: error reading command stream from git"); return 1; } if (buf.len == 0) break; if (starts_with(buf.buf, "fetch ")) { if (nongit) - die("Fetch attempted without a local repo"); + die("remote-curl: fetch attempted without a local repo"); parse_fetch(&buf); } else if (!strcmp(buf.buf, "list") || starts_with(buf.buf, "list ")) { @@ -1014,7 +1014,7 @@ int main(int argc, const char **argv) printf("\n"); fflush(stdout); } else { - error("unknown command '%s'", buf.buf); + error("remote-curl: unknown command '%s' from git", buf.buf); return 1; } strbuf_reset(&buf); -- cgit v1.2.1