From e860d96bf89fca63f664eb2d507f2f14537a9008 Mon Sep 17 00:00:00 2001 From: Matt McCutchen Date: Wed, 22 Feb 2017 11:01:22 -0500 Subject: fetch-pack: move code to report unmatched refs to a function Prepare to reuse this code in transport.c for "git fetch". While we're here, internationalize the existing error message. Signed-off-by: Matt McCutchen Signed-off-by: Junio C Hamano --- builtin/fetch-pack.c | 7 +------ fetch-pack.c | 13 +++++++++++++ fetch-pack.h | 6 ++++++ t/t5500-fetch-pack.sh | 6 +++--- 4 files changed, 23 insertions(+), 9 deletions(-) diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c index cfe9e447c2..2a1c1c213f 100644 --- a/builtin/fetch-pack.c +++ b/builtin/fetch-pack.c @@ -219,12 +219,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix) * remote no-such-ref' would silently succeed without issuing * an error. */ - for (i = 0; i < nr_sought; i++) { - if (!sought[i] || sought[i]->matched) - continue; - error("no such remote ref %s", sought[i]->name); - ret = 1; - } + ret |= report_unmatched_refs(sought, nr_sought); while (ref) { printf("%s %s\n", diff --git a/fetch-pack.c b/fetch-pack.c index 601f0779a1..7c8d44c38b 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -1094,3 +1094,16 @@ struct ref *fetch_pack(struct fetch_pack_args *args, clear_shallow_info(&si); return ref_cpy; } + +int report_unmatched_refs(struct ref **sought, int nr_sought) +{ + int i, ret = 0; + + for (i = 0; i < nr_sought; i++) { + if (!sought[i] || sought[i]->matched) + continue; + error(_("no such remote ref %s"), sought[i]->name); + ret = 1; + } + return ret; +} diff --git a/fetch-pack.h b/fetch-pack.h index c912e3d321..a2d46e6e75 100644 --- a/fetch-pack.h +++ b/fetch-pack.h @@ -45,4 +45,10 @@ struct ref *fetch_pack(struct fetch_pack_args *args, struct sha1_array *shallow, char **pack_lockfile); +/* + * Print an appropriate error message for each sought ref that wasn't + * matched. Return 0 if all sought refs were matched, otherwise 1. + */ +int report_unmatched_refs(struct ref **sought, int nr_sought); + #endif diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh index 505e1b4a7f..b5865b385d 100755 --- a/t/t5500-fetch-pack.sh +++ b/t/t5500-fetch-pack.sh @@ -484,7 +484,7 @@ test_expect_success 'test lonely missing ref' ' cd client && test_must_fail git fetch-pack --no-progress .. refs/heads/xyzzy ) >/dev/null 2>error-m && - test_cmp expect-error error-m + test_i18ncmp expect-error error-m ' test_expect_success 'test missing ref after existing' ' @@ -492,7 +492,7 @@ test_expect_success 'test missing ref after existing' ' cd client && test_must_fail git fetch-pack --no-progress .. refs/heads/A refs/heads/xyzzy ) >/dev/null 2>error-em && - test_cmp expect-error error-em + test_i18ncmp expect-error error-em ' test_expect_success 'test missing ref before existing' ' @@ -500,7 +500,7 @@ test_expect_success 'test missing ref before existing' ' cd client && test_must_fail git fetch-pack --no-progress .. refs/heads/xyzzy refs/heads/A ) >/dev/null 2>error-me && - test_cmp expect-error error-me + test_i18ncmp expect-error error-me ' test_expect_success 'test --all, --depth, and explicit head' ' -- cgit v1.2.3 From e70a65c5d89e4543491082d4b361398fd87433db Mon Sep 17 00:00:00 2001 From: Matt McCutchen Date: Wed, 22 Feb 2017 11:02:15 -0500 Subject: fetch_refs_via_pack: call report_unmatched_refs "git fetch" currently doesn't bother to check that it got all refs it sought, because the common case of requesting a nonexistent ref triggers a die() in get_fetch_map. However, there's at least one case that slipped through: "git fetch REMOTE SHA1" if the server doesn't allow requests for unadvertised objects. Make fetch_refs_via_pack (which is on the "git fetch" code path) call report_unmatched_refs so that we at least get an error message in that case. Signed-off-by: Matt McCutchen Signed-off-by: Junio C Hamano --- t/t5516-fetch-push.sh | 3 ++- transport.c | 14 +++++++++----- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index 26b2cafc47..0d13a4556c 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -1098,7 +1098,8 @@ test_expect_success 'fetch exact SHA1' ' test_must_fail git cat-file -t $the_commit && # fetching the hidden object should fail by default - test_must_fail git fetch -v ../testrepo $the_commit:refs/heads/copy && + test_must_fail git fetch -v ../testrepo $the_commit:refs/heads/copy 2>err && + test_i18ngrep "no such remote ref" err && test_must_fail git rev-parse --verify refs/heads/copy && # the server side can allow it to succeed diff --git a/transport.c b/transport.c index 04e5d6623e..c377907d4f 100644 --- a/transport.c +++ b/transport.c @@ -204,6 +204,7 @@ static struct ref *get_refs_via_connect(struct transport *transport, int for_pus static int fetch_refs_via_pack(struct transport *transport, int nr_heads, struct ref **to_fetch) { + int ret = 0; struct git_transport_data *data = transport->data; struct ref *refs; char *dest = xstrdup(transport->url); @@ -241,19 +242,22 @@ static int fetch_refs_via_pack(struct transport *transport, &transport->pack_lockfile); close(data->fd[0]); close(data->fd[1]); - if (finish_connect(data->conn)) { - free_refs(refs); - refs = NULL; - } + if (finish_connect(data->conn)) + ret = -1; data->conn = NULL; data->got_remote_heads = 0; data->options.self_contained_and_connected = args.self_contained_and_connected; + if (refs == NULL) + ret = -1; + if (report_unmatched_refs(to_fetch, nr_heads)) + ret = -1; + free_refs(refs_tmp); free_refs(refs); free(dest); - return (refs ? 0 : -1); + return ret; } static int push_had_errors(struct ref *ref) -- cgit v1.2.3 From d56583ded679f2eade3994d855c8d605e2964710 Mon Sep 17 00:00:00 2001 From: Matt McCutchen Date: Wed, 22 Feb 2017 11:05:57 -0500 Subject: fetch-pack: add specific error for fetching an unadvertised object Enhance filter_refs (which decides whether a request for an unadvertised object should be sent to the server) to record a new match status on the "struct ref" when a request is not allowed, and have report_unmatched_refs check for this status and print a special error message, "Server does not allow request for unadvertised object". Signed-off-by: Matt McCutchen Signed-off-by: Junio C Hamano --- fetch-pack.c | 42 +++++++++++++++++++++++++++--------------- remote.h | 9 +++++++-- t/t5516-fetch-push.sh | 2 +- 3 files changed, 35 insertions(+), 18 deletions(-) diff --git a/fetch-pack.c b/fetch-pack.c index 7c8d44c38b..f12bfcdbb1 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -578,7 +578,7 @@ static void filter_refs(struct fetch_pack_args *args, break; /* definitely do not have it */ else if (cmp == 0) { keep = 1; /* definitely have it */ - sought[i]->matched = 1; + sought[i]->match_status = REF_MATCHED; } i++; } @@ -598,22 +598,24 @@ static void filter_refs(struct fetch_pack_args *args, } /* Append unmatched requests to the list */ - if ((allow_unadvertised_object_request & - (ALLOW_TIP_SHA1 | ALLOW_REACHABLE_SHA1))) { - for (i = 0; i < nr_sought; i++) { - unsigned char sha1[20]; + for (i = 0; i < nr_sought; i++) { + unsigned char sha1[20]; - ref = sought[i]; - if (ref->matched) - continue; - if (get_sha1_hex(ref->name, sha1) || - ref->name[40] != '\0' || - hashcmp(sha1, ref->old_oid.hash)) - continue; + ref = sought[i]; + if (ref->match_status != REF_NOT_MATCHED) + continue; + if (get_sha1_hex(ref->name, sha1) || + ref->name[40] != '\0' || + hashcmp(sha1, ref->old_oid.hash)) + continue; - ref->matched = 1; + if ((allow_unadvertised_object_request & + (ALLOW_TIP_SHA1 | ALLOW_REACHABLE_SHA1))) { + ref->match_status = REF_MATCHED; *newtail = copy_ref(ref); newtail = &(*newtail)->next; + } else { + ref->match_status = REF_UNADVERTISED_NOT_ALLOWED; } } *refs = newlist; @@ -1100,9 +1102,19 @@ int report_unmatched_refs(struct ref **sought, int nr_sought) int i, ret = 0; for (i = 0; i < nr_sought; i++) { - if (!sought[i] || sought[i]->matched) + if (!sought[i]) continue; - error(_("no such remote ref %s"), sought[i]->name); + switch (sought[i]->match_status) { + case REF_MATCHED: + continue; + case REF_NOT_MATCHED: + error(_("no such remote ref %s"), sought[i]->name); + break; + case REF_UNADVERTISED_NOT_ALLOWED: + error(_("Server does not allow request for unadvertised object %s"), + sought[i]->name); + break; + } ret = 1; } return ret; diff --git a/remote.h b/remote.h index 924881169d..0b9d8c4589 100644 --- a/remote.h +++ b/remote.h @@ -89,8 +89,13 @@ struct ref { force:1, forced_update:1, expect_old_sha1:1, - deletion:1, - matched:1; + deletion:1; + + enum { + REF_NOT_MATCHED = 0, /* initial value */ + REF_MATCHED, + REF_UNADVERTISED_NOT_ALLOWED + } match_status; /* * Order is important here, as we write to FETCH_HEAD diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index 0d13a4556c..78f3b8ef22 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -1099,7 +1099,7 @@ test_expect_success 'fetch exact SHA1' ' # fetching the hidden object should fail by default test_must_fail git fetch -v ../testrepo $the_commit:refs/heads/copy 2>err && - test_i18ngrep "no such remote ref" err && + test_i18ngrep "Server does not allow request for unadvertised object" err && test_must_fail git rev-parse --verify refs/heads/copy && # the server side can allow it to succeed -- cgit v1.2.3