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 +++++++++++++++++++ t/t5801-remote-helpers.sh | 20 ++++++++++++++++++++ transport-helper.c | 2 +- 3 files changed, 40 insertions(+), 1 deletion(-) 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 diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh index f387027c05..aafc46ac94 100755 --- a/t/t5801-remote-helpers.sh +++ b/t/t5801-remote-helpers.sh @@ -166,4 +166,24 @@ 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 + ) +' + +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 cb3ef7d38e..96081cc392 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) -- cgit v1.2.1 From c096955c5bb8e20186cc7c07d4d12b77ddcd01c6 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 10 Apr 2013 17:16:03 -0400 Subject: transport-helper: mention helper name when it dies When we try to read from a remote-helper and get EOF or an error, we print a message indicating that the helper died. However, users may not know that a remote helper was in use (e.g., when using git-over-http), or even what a remote helper is. Let's print the name of the helper (e.g., "git-remote-https"); this makes it more obvious what the program is for, and provides a useful token for reporting bugs or searching for more information (e.g., in manpages). Signed-off-by: Jeff King Acked-by: Sverre Rabbelier Signed-off-by: Junio C Hamano --- t/t5801-remote-helpers.sh | 2 +- transport-helper.c | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh index aafc46ac94..8b2cb68ec5 100755 --- a/t/t5801-remote-helpers.sh +++ b/t/t5801-remote-helpers.sh @@ -182,7 +182,7 @@ test_expect_success 'proper failure checks for pushing' ' cd local && test_must_fail git push --all 2> error && cat error && - grep -q "Reading from remote helper failed" error + grep -q "Reading from helper .git-remote-testgit. failed" error ) ' diff --git a/transport-helper.c b/transport-helper.c index 96081cc392..3fc43b97f5 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -46,7 +46,7 @@ static void sendline(struct helper_data *helper, struct strbuf *buffer) die_errno("Full write to remote helper failed"); } -static int recvline_fh(FILE *helper, struct strbuf *buffer) +static int recvline_fh(FILE *helper, struct strbuf *buffer, const char *name) { strbuf_reset(buffer); if (debug) @@ -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"); - die("Reading from remote helper failed"); + die("Reading from helper 'git-remote-%s' failed", name); } if (debug) @@ -64,7 +64,7 @@ static int recvline_fh(FILE *helper, struct strbuf *buffer) static int recvline(struct helper_data *helper, struct strbuf *buffer) { - return recvline_fh(helper->out, buffer); + return recvline_fh(helper->out, buffer, helper->name); } static void xchgline(struct helper_data *helper, struct strbuf *buffer) @@ -536,7 +536,7 @@ static int process_connect_service(struct transport *transport, goto exit; sendline(data, &cmdbuf); - recvline_fh(input, &cmdbuf); + recvline_fh(input, &cmdbuf, name); if (!strcmp(cmdbuf.buf, "")) { data->no_disconnect_req = 1; if (debug) -- cgit v1.2.1 From 5034fdea3017304fa37e50b3bd40c392503ebfd2 Mon Sep 17 00:00:00 2001 From: Felipe Contreras Date: Wed, 10 Apr 2013 19:07:11 -0500 Subject: transport-helper: improve push messages If there's already a remote-helper tracking ref, we can fetch the SHA-1 to report proper push messages (as opposed to always reporting [new branch]). The remote-helper currently can specify the old SHA-1 to avoid this problem, but there's no point in forcing all remote-helpers to be aware of git commit ids; they should be able to be agnostic of them. Signed-off-by: Felipe Contreras Reviewed-by: Jeff King Signed-off-by: Junio C Hamano --- t/t5801-remote-helpers.sh | 14 ++++++++++++++ transport-helper.c | 1 + 2 files changed, 15 insertions(+) diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh index 8b2cb68ec5..ff5fb852f4 100755 --- a/t/t5801-remote-helpers.sh +++ b/t/t5801-remote-helpers.sh @@ -186,4 +186,18 @@ test_expect_success 'proper failure checks for pushing' ' ) ' +test_expect_success 'push messages' ' + (cd local && + git checkout -b new_branch master && + echo new >>file && + git commit -a -m new && + git push origin new_branch && + git fetch origin && + echo new >>file && + git commit -a -m new && + git push origin new_branch 2> msg && + ! grep "\[new branch\]" msg + ) +' + test_done diff --git a/transport-helper.c b/transport-helper.c index 3fc43b97f5..56207eae2f 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -801,6 +801,7 @@ static int push_refs_with_export(struct transport *transport, if (private && !get_sha1(private, sha1)) { strbuf_addf(&buf, "^%s", private); string_list_append(&revlist_args, strbuf_detach(&buf, NULL)); + hashcpy(ref->old_sha1, sha1); } free(private); -- cgit v1.2.1 From 7a43c55415a224039a41684cbadd5b94be6f1a8e Mon Sep 17 00:00:00 2001 From: Felipe Contreras Date: Wed, 17 Apr 2013 23:14:28 -0500 Subject: transport-helper: clarify *:* refspec The *:* refspec doesn't work, and never has, clarify the code and documentation to reflect that. This in effect reverts commit 9e7673e (gitremote-helpers(1): clarify refspec behaviour). Signed-off-by: Felipe Contreras Signed-off-by: Junio C Hamano --- Documentation/gitremote-helpers.txt | 4 ++-- t/t5801-remote-helpers.sh | 15 --------------- transport-helper.c | 2 +- 3 files changed, 3 insertions(+), 18 deletions(-) diff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt index f506031ae4..0c91aba861 100644 --- a/Documentation/gitremote-helpers.txt +++ b/Documentation/gitremote-helpers.txt @@ -174,8 +174,8 @@ ref. This capability can be advertised multiple times. The first applicable refspec takes precedence. The left-hand of refspecs advertised with this capability must cover all refs reported by -the list command. If a helper does not need a specific 'refspec' -capability then it should advertise `refspec *:*`. +the list command. If no 'refspec' capability is advertised, +there is an implied `refspec *:*`. 'bidi-import':: This modifies the 'import' capability. diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh index ff5fb852f4..0b376e0229 100755 --- a/t/t5801-remote-helpers.sh +++ b/t/t5801-remote-helpers.sh @@ -120,21 +120,6 @@ test_expect_failure 'pushing without refspecs' ' compare_refs local2 HEAD server HEAD ' -test_expect_success 'pulling with straight refspec' ' - (cd local2 && - GIT_REMOTE_TESTGIT_REFSPEC="*:*" git pull) && - compare_refs local2 HEAD server HEAD -' - -test_expect_failure 'pushing with straight refspec' ' - test_when_finished "(cd local2 && git reset --hard origin)" && - (cd local2 && - echo content >>file && - git commit -a -m eleven && - GIT_REMOTE_TESTGIT_REFSPEC="*:*" git push) && - compare_refs local2 HEAD server HEAD -' - test_expect_success 'pulling without marks' ' (cd local2 && GIT_REMOTE_TESTGIT_NO_MARKS=1 git pull) && diff --git a/transport-helper.c b/transport-helper.c index 56207eae2f..018513baa0 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -469,7 +469,7 @@ static int fetch_with_import(struct transport *transport, * were fetching. * * (If no "refspec" capability was specified, for historical - * reasons we default to *:*.) + * reasons we default to the equivalent of *:*.) * * Store the result in to_fetch[i].old_sha1. Callers such * as "git fetch" can use the value to write feedback to the -- cgit v1.2.1 From bb0a5cc9dc574e1dda4ee6bcb1e0f166d29dcd65 Mon Sep 17 00:00:00 2001 From: Felipe Contreras Date: Wed, 17 Apr 2013 23:14:29 -0500 Subject: transport-helper: update refspec documentation The refspec capability is not only used by 'import', also by 'export', and it's recommended in both. Signed-off-by: Felipe Contreras Signed-off-by: Junio C Hamano --- Documentation/gitremote-helpers.txt | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt index 0c91aba861..ba7240c363 100644 --- a/Documentation/gitremote-helpers.txt +++ b/Documentation/gitremote-helpers.txt @@ -159,11 +159,11 @@ Miscellaneous capabilities carried out. 'refspec' :: - This modifies the 'import' capability, allowing the produced - fast-import stream to modify refs in a private namespace - instead of writing to refs/heads or refs/remotes directly. - It is recommended that all importers providing the 'import' - capability use this. + For remote helpers that implement 'import' or 'export', this capability + allows the refs to be constrained to a private namespace, instead of + writing to refs/heads or refs/remotes directly. + It is recommended that all importers providing the 'import' or 'export' + capabilities use this. + A helper advertising the capability `refspec refs/heads/*:refs/svn/origin/branches/*` -- cgit v1.2.1 From 21610d820b97583a8f4e3e7f4a48716c8e32fd92 Mon Sep 17 00:00:00 2001 From: Felipe Contreras Date: Wed, 17 Apr 2013 23:14:30 -0500 Subject: transport-helper: clarify pushing without refspecs This has never worked, since it's inception the code simply skips all the refs, essentially telling fast-export to do nothing. Let's at least tell the user what's going on. Signed-off-by: Felipe Contreras Signed-off-by: Junio C Hamano --- Documentation/gitremote-helpers.txt | 4 ++-- t/t5801-remote-helpers.sh | 6 +++--- transport-helper.c | 5 +++-- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt index ba7240c363..4d26e37d36 100644 --- a/Documentation/gitremote-helpers.txt +++ b/Documentation/gitremote-helpers.txt @@ -162,8 +162,8 @@ Miscellaneous capabilities For remote helpers that implement 'import' or 'export', this capability allows the refs to be constrained to a private namespace, instead of writing to refs/heads or refs/remotes directly. - It is recommended that all importers providing the 'import' or 'export' - capabilities use this. + It is recommended that all importers providing the 'import' + capability use this. It's mandatory for 'export'. + A helper advertising the capability `refspec refs/heads/*:refs/svn/origin/branches/*` diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh index 0b376e0229..f95003a97e 100755 --- a/t/t5801-remote-helpers.sh +++ b/t/t5801-remote-helpers.sh @@ -111,13 +111,13 @@ test_expect_success 'pulling without refspecs' ' compare_refs local2 HEAD server HEAD ' -test_expect_failure 'pushing without refspecs' ' +test_expect_success 'pushing without refspecs' ' test_when_finished "(cd local2 && git reset --hard origin)" && (cd local2 && echo content >>file && git commit -a -m ten && - GIT_REMOTE_TESTGIT_REFSPEC="" git push) && - compare_refs local2 HEAD server HEAD + GIT_REMOTE_TESTGIT_REFSPEC="" test_must_fail git push 2>../error) && + grep "remote-helper doesn.t support push; refspec needed" error ' test_expect_success 'pulling without marks' ' diff --git a/transport-helper.c b/transport-helper.c index 018513baa0..98ef8f6410 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -785,6 +785,9 @@ static int push_refs_with_export(struct transport *transport, struct string_list revlist_args = STRING_LIST_INIT_NODUP; struct strbuf buf = STRBUF_INIT; + if (!data->refspecs) + die("remote-helper doesn't support push; refspec needed"); + helper = get_helper(transport); write_constant(helper->in, "export\n"); @@ -795,8 +798,6 @@ static int push_refs_with_export(struct transport *transport, char *private; unsigned char sha1[20]; - if (!data->refspecs) - continue; private = apply_refspecs(data->refspecs, data->refspec_nr, ref->name); if (private && !get_sha1(private, sha1)) { strbuf_addf(&buf, "^%s", private); -- cgit v1.2.1 From a93b4a09acfed0e2a006770d0196c74968e65c25 Mon Sep 17 00:00:00 2001 From: Felipe Contreras Date: Wed, 17 Apr 2013 23:14:31 -0500 Subject: transport-helper: warn when refspec is not used For the modes that need it. In the future we should probably error out, instead of providing half-assed support. The reason we want to do this is because if it's not present, the remote helper might be updating refs/heads/*, or refs/remotes/origin/*, directly, and in the process fetch will get confused trying to update refs that are already updated, or older than what they should be. We shouldn't be messing with the rest of git. Signed-off-by: Felipe Contreras Signed-off-by: Junio C Hamano --- t/t5801-remote-helpers.sh | 6 ++++-- transport-helper.c | 2 ++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh index f95003a97e..f572271941 100755 --- a/t/t5801-remote-helpers.sh +++ b/t/t5801-remote-helpers.sh @@ -100,14 +100,16 @@ test_expect_failure 'push new branch with old:new refspec' ' test_expect_success 'cloning without refspec' ' GIT_REMOTE_TESTGIT_REFSPEC="" \ - git clone "testgit::${PWD}/server" local2 && + git clone "testgit::${PWD}/server" local2 2>error && + grep "This remote helper should implement refspec capability" error && compare_refs local2 HEAD server HEAD ' test_expect_success 'pulling without refspecs' ' (cd local2 && git reset --hard && - GIT_REMOTE_TESTGIT_REFSPEC="" git pull) && + GIT_REMOTE_TESTGIT_REFSPEC="" git pull 2>../error) && + grep "This remote helper should implement refspec capability" error && compare_refs local2 HEAD server HEAD ' diff --git a/transport-helper.c b/transport-helper.c index 98ef8f6410..2452b3bb6d 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -215,6 +215,8 @@ static struct child_process *get_helper(struct transport *transport) free((char *)refspecs[i]); } free(refspecs); + } else if (data->import || data->bidi_import || data->export) { + warning("This remote helper should implement refspec capability."); } strbuf_release(&buf); if (debug) -- cgit v1.2.1 From 9c51558cfb6ffe104da45d324194d9f1ebf9bc65 Mon Sep 17 00:00:00 2001 From: Felipe Contreras Date: Wed, 17 Apr 2013 23:14:32 -0500 Subject: transport-helper: trivial code shuffle Just shuffle the die() part to make it more explicit, and cleanup the code-style. Signed-off-by: Felipe Contreras Signed-off-by: Junio C Hamano --- transport-helper.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/transport-helper.c b/transport-helper.c index 2452b3bb6d..d48e00d03c 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -800,6 +800,9 @@ static int push_refs_with_export(struct transport *transport, char *private; unsigned char sha1[20]; + if (ref->deletion) + die("remote-helpers do not support ref deletion"); + private = apply_refspecs(data->refspecs, data->refspec_nr, ref->name); if (private && !get_sha1(private, sha1)) { strbuf_addf(&buf, "^%s", private); @@ -808,13 +811,8 @@ static int push_refs_with_export(struct transport *transport, } free(private); - if (ref->deletion) { - die("remote-helpers do not support ref deletion"); - } - if (ref->peer_ref) string_list_append(&revlist_args, ref->peer_ref->name); - } if (get_exporter(transport, &exporter, &revlist_args)) -- cgit v1.2.1 From 664059fb624467975da5c9a4bf6053c83126eaa7 Mon Sep 17 00:00:00 2001 From: Felipe Contreras Date: Wed, 17 Apr 2013 23:14:33 -0500 Subject: transport-helper: update remote helper namespace When pushing, the remote namespace is updated correctly (e.g. refs/origin/master), but not the remote helper's (e.g. refs/testgit/origin/master), which currently is only updated while fetching. Since the remote namespace is used to tell fast-export which commits to avoid (because they were already imported/exported), it makes sense to have them in sync so they don't get generated twice. If the remote helper was implemented properly, they would be ignored, if not, they probably would end up repeated. Signed-off-by: Felipe Contreras Signed-off-by: Junio C Hamano --- t/t5801-remote-helpers.sh | 12 ++++++++++++ transport-helper.c | 23 +++++++++++++++++++---- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh index f572271941..4dcf744f97 100755 --- a/t/t5801-remote-helpers.sh +++ b/t/t5801-remote-helpers.sh @@ -153,6 +153,18 @@ test_expect_success 'push ref with existing object' ' compare_refs local dup server dup ' +test_expect_success 'push update refs' ' + (cd local && + git checkout -b update master && + echo update >>file && + git commit -a -m update && + git push origin update + git rev-parse --verify remotes/origin/update >expect && + 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 d48e00d03c..92174095ed 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -11,6 +11,7 @@ #include "thread-utils.h" #include "sigchain.h" #include "argv-array.h" +#include "refs.h" static int debug; @@ -620,7 +621,7 @@ static int fetch(struct transport *transport, return -1; } -static void push_update_ref_status(struct strbuf *buf, +static int push_update_ref_status(struct strbuf *buf, struct ref **ref, struct ref *remote_refs) { @@ -686,7 +687,7 @@ static void push_update_ref_status(struct strbuf *buf, *ref = find_ref_by_name(remote_refs, refname); if (!*ref) { warning("helper reported unexpected status of %s", refname); - return; + return 1; } if ((*ref)->status != REF_STATUS_NONE) { @@ -695,11 +696,12 @@ static void push_update_ref_status(struct strbuf *buf, * status reported by the remote helper if the latter is 'no match'. */ if (status == REF_STATUS_NONE) - return; + return 1; } (*ref)->status = status; (*ref)->remote_status = msg; + return 0; } static void push_update_refs_status(struct helper_data *data, @@ -708,11 +710,24 @@ static void push_update_refs_status(struct helper_data *data, struct strbuf buf = STRBUF_INIT; struct ref *ref = remote_refs; for (;;) { + char *private; + recvline(data, &buf); if (!buf.len) break; - push_update_ref_status(&buf, &ref, remote_refs); + if (push_update_ref_status(&buf, &ref, remote_refs)) + continue; + + if (!data->refspecs) + continue; + + /* propagate back the update to the remote namespace */ + private = apply_refspecs(data->refspecs, data->refspec_nr, ref->name); + if (!private) + continue; + update_ref("update by helper", private, ref->new_sha1, NULL, 0, 0); + free(private); } strbuf_release(&buf); } -- cgit v1.2.1 From 1afe6e404453586a1095ce0f3f7e899e62ab237f Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 25 Apr 2013 10:18:37 -0700 Subject: t5801: "VAR=VAL shell_func args" is forbidden It is not a portable expectation that a single-shot environment variable assignment works when calling a shell function, not a command. Set and export the variable before calling "test_must_fail git push" instead. This change would not hurt because this is the last command in the subprocess and the environment will not seep through to later tests without using a single-shot assignment. Signed-off-by: Junio C Hamano --- t/t5801-remote-helpers.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh index 4dcf744f97..c956abd865 100755 --- a/t/t5801-remote-helpers.sh +++ b/t/t5801-remote-helpers.sh @@ -118,7 +118,9 @@ test_expect_success 'pushing without refspecs' ' (cd local2 && echo content >>file && git commit -a -m ten && - GIT_REMOTE_TESTGIT_REFSPEC="" test_must_fail git push 2>../error) && + GIT_REMOTE_TESTGIT_REFSPEC="" && + export GIT_REMOTE_TESTGIT_REFSPEC && + test_must_fail git push 2>../error) && grep "remote-helper doesn.t support push; refspec needed" error ' -- cgit v1.2.1 From d6ae7b2d366da8d90aa90cabfd54d285c24b5a86 Mon Sep 17 00:00:00 2001 From: Felipe Contreras Date: Fri, 10 May 2013 07:08:29 -0500 Subject: test: remote-helper: add missing and Signed-off-by: Felipe Contreras Signed-off-by: Junio C Hamano --- t/t5801-remote-helpers.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh index c956abd865..0b13d10698 100755 --- a/t/t5801-remote-helpers.sh +++ b/t/t5801-remote-helpers.sh @@ -160,7 +160,7 @@ test_expect_success 'push update refs' ' git checkout -b update master && echo update >>file && git commit -a -m update && - git push origin update + git push origin update && git rev-parse --verify remotes/origin/update >expect && git rev-parse --verify testgit/origin/heads/update >actual && test_cmp expect actual -- cgit v1.2.1 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(-) 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.1