git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Felipe Contreras <felipe.contreras@gmail.com>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>, Jeff King <peff@peff.net>,
	Sverre Rabbelier <srabbelier@gmail.com>,
	Felipe Contreras <felipe.contreras@gmail.com>
Subject: [PATCH v4] transport-helper: report errors properly
Date: Mon,  8 Apr 2013 09:40:04 -0500	[thread overview]
Message-ID: <1365432004-20132-1-git-send-email-felipe.contreras@gmail.com> (raw)

If a push fails because the remote-helper died (with fast-export), the
user won't see any error message. So let's add one.

At the same time lets add tests to ensure this error is reported, and
while we are at it, check the error from fast-import

Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 git-remote-testgit        | 13 +++++++++++++
 t/t5801-remote-helpers.sh | 21 +++++++++++++++++++++
 transport-helper.c        |  2 +-
 3 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/git-remote-testgit b/git-remote-testgit
index b395c8d..2eb7889 100755
--- a/git-remote-testgit
+++ b/git-remote-testgit
@@ -61,12 +61,25 @@ do
 			echo "feature import-marks=$gitmarks"
 			echo "feature export-marks=$gitmarks"
 		fi
+
+		if test -n "$GIT_REMOTE_TESTGIT_FAILURE"
+		then
+			echo "feature done"
+			exit 1
+		fi
+
 		echo "feature done"
 		git fast-export "${testgitmarks_args[@]}" $refs |
 		sed -e "s#refs/heads/#${prefix}/heads/#g"
 		echo "done"
 		;;
 	export)
+		if test -n "$GIT_REMOTE_TESTGIT_FAILURE"
+		then
+			sleep 1 # don't let fast-export get SIGPIPE
+			exit 1
+		fi
+
 		before=$(git for-each-ref --format='%(refname) %(objectname)')
 
 		git fast-import "${testgitmarks_args[@]}" --quiet
diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index f387027..2dfcf64 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -166,4 +166,25 @@ test_expect_success 'push ref with existing object' '
 	compare_refs local dup server dup
 '
 
+test_expect_success 'proper failure checks for fetching' '
+	(GIT_REMOTE_TESTGIT_FAILURE=1 &&
+	export GIT_REMOTE_TESTGIT_FAILURE &&
+	cd local &&
+	test_must_fail git fetch 2> error &&
+	cat error &&
+	grep -q "Error while running fast-import" error
+	)
+'
+
+# We sleep to give fast-export a chance to catch the SIGPIPE
+test_expect_success 'proper failure checks for pushing' '
+	(GIT_REMOTE_TESTGIT_FAILURE=1 &&
+	export GIT_REMOTE_TESTGIT_FAILURE &&
+	cd local &&
+	test_must_fail git push --all 2> error &&
+	cat error &&
+	grep -q "Reading from remote helper failed" error
+	)
+'
+
 test_done
diff --git a/transport-helper.c b/transport-helper.c
index cb3ef7d..96081cc 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -54,7 +54,7 @@ static int recvline_fh(FILE *helper, struct strbuf *buffer)
 	if (strbuf_getline(buffer, helper, '\n') == EOF) {
 		if (debug)
 			fprintf(stderr, "Debug: Remote helper quit.\n");
-		exit(128);
+		die("Reading from remote helper failed");
 	}
 
 	if (debug)
-- 
1.8.2

             reply	other threads:[~2013-04-08 16:32 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-08 14:40 Felipe Contreras [this message]
2013-04-08 18:20 ` [PATCH v4] transport-helper: report errors properly Sverre Rabbelier
2013-04-08 19:30   ` Jeff King
2013-04-08 19:28 ` Jeff King
2013-04-09 21:38 ` Thomas Rast
2013-04-09 21:50   ` Jeff King
2013-04-10 21:13 ` [PATCH v5 0/2] reporting transport helper errors Jeff King
2013-04-10 21:15   ` [PATCH 1/2] transport-helper: report errors properly Jeff King
2013-04-10 21:22     ` Sverre Rabbelier
2013-04-10 21:46     ` Eric Sunshine
2013-04-11 13:22     ` Felipe Contreras
2013-04-11 16:18       ` Jeff King
2013-04-11 16:49         ` Felipe Contreras
2013-04-11 16:59           ` Jeff King
2013-04-11 17:57             ` Felipe Contreras
2013-04-11 18:49               ` Junio C Hamano
2013-04-11 21:35                 ` Felipe Contreras
2013-04-11 18:44       ` Junio C Hamano
2013-04-11 21:21         ` Felipe Contreras
2013-04-11 23:05           ` Junio C Hamano
2013-04-13  5:42             ` Felipe Contreras
2013-04-13  6:00               ` Jeff King
2013-04-13  6:43                 ` Felipe Contreras
2013-04-14  5:23                   ` Junio C Hamano
2013-04-14 15:54                     ` Felipe Contreras
2013-04-10 21:16   ` [PATCH 2/2] transport-helper: mention helper name when it dies Jeff King
2013-04-10 21:23     ` Sverre Rabbelier
2013-04-10 21:28       ` Jeff King
2013-04-10 21:35         ` Sverre Rabbelier
2013-04-10 23:13 ` [PATCH v4] transport-helper: report errors properly rh
2013-04-11  3:39   ` 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=1365432004-20132-1-git-send-email-felipe.contreras@gmail.com \
    --to=felipe.contreras@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=srabbelier@gmail.com \
    /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).