From 81d340d40af506eda3182190b6132575547fa4c5 Mon Sep 17 00:00:00 2001 From: Felipe Contreras Date: Wed, 10 Apr 2013 17:15:52 -0400 Subject: transport-helper: report errors properly If a push fails because the remote-helper died (with fast-export), the user may not see any error message. We do correctly die with a failed exit code, as we notice that the helper has died while reading back the ref status from the helper. However, we don't print any message. This is OK if the helper itself printed a useful error message, but we cannot count on that; let's let the user know that the helper failed. In the long run, it may make more sense to propagate the error back up to push, so that it can present the usual status table and give a nicer message. But this is a much simpler fix that can help immediately. While we're adding tests, let's also confirm that the remote-helper dying is also detected when importing refs. We currently do so robustly when the helper uses the "done" feature (and that is what we test). We cannot do so reliably when the helper does not use the "done" feature, but it is not even worth testing; the right solution is for the helper to start using "done". Suggested-by: Jeff King Signed-off-by: Felipe Contreras Signed-off-by: Jeff King Acked-by: Sverre Rabbelier Signed-off-by: Junio C Hamano --- git-remote-testgit | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) (limited to 'git-remote-testgit') diff --git a/git-remote-testgit b/git-remote-testgit index b395c8de59..5fd09f965a 100755 --- a/git-remote-testgit +++ b/git-remote-testgit @@ -61,12 +61,31 @@ 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 + # consume input so fast-export doesn't get SIGPIPE; + # git would also notice that case, but we want + # to make sure we are exercising the later + # error checks + while read line; do + test "done" = "$line" && break + done + exit 1 + fi + before=$(git for-each-ref --format='%(refname) %(objectname)') git fast-import "${testgitmarks_args[@]}" --quiet -- cgit v1.2.3 From 126aac5cf3b5696481aafe840f8f596653087d8b Mon Sep 17 00:00:00 2001 From: Felipe Contreras Date: Fri, 10 May 2013 07:08:30 -0500 Subject: transport-helper: fix remote helper namespace regression Commit 664059f (transport-helper: update remote helper namespace) updates the namespace when the push succeeds or not; we should do it only when it succeeded. Signed-off-by: Felipe Contreras Signed-off-by: Junio C Hamano --- git-remote-testgit | 7 ++++++- t/t5801-remote-helpers.sh | 14 ++++++++++++++ transport-helper.c | 2 +- 3 files changed, 21 insertions(+), 2 deletions(-) (limited to 'git-remote-testgit') diff --git a/git-remote-testgit b/git-remote-testgit index 5fd09f965a..ff36d1d39d 100755 --- a/git-remote-testgit +++ b/git-remote-testgit @@ -97,7 +97,12 @@ do while read ref a b do test $a == $b && continue - echo "ok $ref" + if test -z "$GIT_REMOTE_TESTGIT_PUSH_ERROR" + then + echo "ok $ref" + else + echo "error $ref $GIT_REMOTE_TESTGIT_PUSH_ERROR" + fi done echo diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh index 0b13d10698..443e228ec5 100755 --- a/t/t5801-remote-helpers.sh +++ b/t/t5801-remote-helpers.sh @@ -167,6 +167,20 @@ test_expect_success 'push update refs' ' ) ' +test_expect_success 'push update refs failure' ' + (cd local && + git checkout update && + echo "update fail" >>file && + git commit -a -m "update fail" && + git rev-parse --verify testgit/origin/heads/update >expect && + GIT_REMOTE_TESTGIT_PUSH_ERROR="non-fast forward" && + export GIT_REMOTE_TESTGIT_PUSH_ERROR && + test_expect_code 1 git push origin update && + git rev-parse --verify testgit/origin/heads/update >actual && + test_cmp expect actual + ) +' + test_expect_success 'proper failure checks for fetching' ' (GIT_REMOTE_TESTGIT_FAILURE=1 && export GIT_REMOTE_TESTGIT_FAILURE && diff --git a/transport-helper.c b/transport-helper.c index 92174095ed..6cd0be90e0 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -701,7 +701,7 @@ static int push_update_ref_status(struct strbuf *buf, (*ref)->status = status; (*ref)->remote_status = msg; - return 0; + return !(status == REF_STATUS_OK); } static void push_update_refs_status(struct helper_data *data, -- cgit v1.2.3