From: Junio C Hamano Date: Fri, 19 Oct 2018 04:34:07 +0000 (+0900) Subject: Merge branch 'jt/avoid-ls-refs' X-Git-Tag: v2.20.0-rc0~150 X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/0527fbab689929de47ab3fa7baf1e7618ffa704f?ds=inline;hp=-c Merge branch 'jt/avoid-ls-refs' Over some transports, fetching objects with an exact commit object name can be done without first seeing the ref advertisements. The code has been optimized to exploit this. * jt/avoid-ls-refs: fetch: do not list refs if fetching only hashes transport: list refs before fetch if necessary transport: do not list refs if possible transport: allow skipping of ref listing --- 0527fbab689929de47ab3fa7baf1e7618ffa704f diff --combined builtin/fetch.c index 6ce882ce82,4c4f8fa194..8f7249f2b1 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@@ -931,11 -931,10 +931,11 @@@ static int store_updated_refs(const cha * everything we are going to fetch already exists and is connected * locally. */ -static int quickfetch(struct ref *ref_map) +static int check_exist_and_connected(struct ref *ref_map) { struct ref *rm = ref_map; struct check_connected_options opt = CHECK_CONNECTED_INIT; + struct ref *r; /* * If we are deepening a shallow clone we already have these @@@ -946,23 -945,13 +946,23 @@@ */ if (deepen) return -1; + + /* + * check_connected() allows objects to merely be promised, but + * we need all direct targets to exist. + */ + for (r = rm; r; r = r->next) { + if (!has_object_file(&r->old_oid)) + return -1; + } + opt.quiet = 1; return check_connected(iterate_ref_map, &rm, &opt); } static int fetch_refs(struct transport *transport, struct ref *ref_map) { - int ret = quickfetch(ref_map); + int ret = check_exist_and_connected(ref_map); if (ret) ret = transport_fetch_refs(transport, ref_map); if (!ret) @@@ -1186,6 -1175,7 +1186,7 @@@ static int do_fetch(struct transport *t int retcode = 0; const struct ref *remote_refs; struct argv_array ref_prefixes = ARGV_ARRAY_INIT; + int must_list_refs = 1; if (tags == TAGS_DEFAULT) { if (transport->remote->fetch_tags == 2) @@@ -1201,17 -1191,36 +1202,36 @@@ goto cleanup; } - if (rs->nr) + if (rs->nr) { + int i; + refspec_ref_prefixes(rs, &ref_prefixes); - else if (transport->remote && transport->remote->fetch.nr) + + /* + * We can avoid listing refs if all of them are exact + * OIDs + */ + must_list_refs = 0; + for (i = 0; i < rs->nr; i++) { + if (!rs->items[i].exact_sha1) { + must_list_refs = 1; + break; + } + } + } else if (transport->remote && transport->remote->fetch.nr) refspec_ref_prefixes(&transport->remote->fetch, &ref_prefixes); - if (ref_prefixes.argc && - (tags == TAGS_SET || (tags == TAGS_DEFAULT))) { - argv_array_push(&ref_prefixes, "refs/tags/"); + if (tags == TAGS_SET || tags == TAGS_DEFAULT) { + must_list_refs = 1; + if (ref_prefixes.argc) + argv_array_push(&ref_prefixes, "refs/tags/"); } - remote_refs = transport_get_remote_refs(transport, &ref_prefixes); + if (must_list_refs) + remote_refs = transport_get_remote_refs(transport, &ref_prefixes); + else + remote_refs = NULL; + argv_array_clear(&ref_prefixes); ref_map = get_ref_map(transport->remote, remote_refs, rs, diff --combined fetch-pack.c index 49ab2666b9,15652b4776..bff05ee69e --- a/fetch-pack.c +++ b/fetch-pack.c @@@ -253,10 -253,8 +253,10 @@@ static int find_common(struct fetch_neg if (args->stateless_rpc && multi_ack == 1) die(_("--stateless-rpc requires multi_ack_detailed")); - mark_tips(negotiator, args->negotiation_tips); - for_each_cached_alternate(negotiator, insert_one_alternate_object); + if (!args->no_dependents) { + mark_tips(negotiator, args->negotiation_tips); + for_each_cached_alternate(negotiator, insert_one_alternate_object); + } fetching = 0; for ( ; refs ; refs = refs->next) { @@@ -273,12 -271,8 +273,12 @@@ * We use lookup_object here because we are only * interested in the case we *know* the object is * reachable and we have already scanned it. + * + * Do this only if args->no_dependents is false (if it is true, + * we cannot trust the object flags). */ - if (((o = lookup_object(the_repository, remote->hash)) != NULL) && + if (!args->no_dependents && + ((o = lookup_object(the_repository, remote->hash)) != NULL) && (o->flags & COMPLETE)) { continue; } @@@ -532,14 -526,21 +532,14 @@@ static void add_refs_to_oidset(struct o oidset_insert(oids, &refs->old_oid); } -static int tip_oids_contain(struct oidset *tip_oids, - struct ref *unmatched, struct ref *newlist, - const struct object_id *id) +static int is_unmatched_ref(const struct ref *ref) { - /* - * Note that this only looks at the ref lists the first time it's - * called. This works out in filter_refs() because even though it may - * add to "newlist" between calls, the additions will always be for - * oids that are already in the set. - */ - if (!tip_oids->map.map.tablesize) { - add_refs_to_oidset(tip_oids, unmatched); - add_refs_to_oidset(tip_oids, newlist); - } - return oidset_contains(tip_oids, id); + struct object_id oid; + const char *p; + return ref->match_status == REF_NOT_MATCHED && + !parse_oid_hex(ref->name, &oid, &p) && + *p == '\0' && + oideq(&oid, &ref->old_oid); } static void filter_refs(struct fetch_pack_args *args, @@@ -552,8 -553,6 +552,8 @@@ struct ref *ref, *next; struct oidset tip_oids = OIDSET_INIT; int i; + int strict = !(allow_unadvertised_object_request & + (ALLOW_TIP_SHA1 | ALLOW_REACHABLE_SHA1)); i = 0; for (ref = *refs; ref; ref = next) { @@@ -590,25 -589,23 +590,25 @@@ } } + if (strict) { + for (i = 0; i < nr_sought; i++) { + ref = sought[i]; + if (!is_unmatched_ref(ref)) + continue; + + add_refs_to_oidset(&tip_oids, unmatched); + add_refs_to_oidset(&tip_oids, newlist); + break; + } + } + /* Append unmatched requests to the list */ for (i = 0; i < nr_sought; i++) { - struct object_id oid; - const char *p; - ref = sought[i]; - if (ref->match_status != REF_NOT_MATCHED) - continue; - if (parse_oid_hex(ref->name, &oid, &p) || - *p != '\0' || - !oideq(&oid, &ref->old_oid)) + if (!is_unmatched_ref(ref)) continue; - if ((allow_unadvertised_object_request & - (ALLOW_TIP_SHA1 | ALLOW_REACHABLE_SHA1)) || - tip_oids_contain(&tip_oids, unmatched, newlist, - &ref->old_oid)) { + if (!strict || oidset_contains(&tip_oids, &ref->old_oid)) { ref->match_status = REF_MATCHED; *newtail = copy_ref(ref); newtail = &(*newtail)->next; @@@ -713,29 -710,31 +713,29 @@@ static void mark_complete_and_common_re oidset_clear(&loose_oid_set); - if (!args->no_dependents) { - if (!args->deepen) { - for_each_ref(mark_complete_oid, NULL); - for_each_cached_alternate(NULL, mark_alternate_complete); - commit_list_sort_by_date(&complete); - if (cutoff) - mark_recent_complete_commits(args, cutoff); - } + if (!args->deepen) { + for_each_ref(mark_complete_oid, NULL); + for_each_cached_alternate(NULL, mark_alternate_complete); + commit_list_sort_by_date(&complete); + if (cutoff) + mark_recent_complete_commits(args, cutoff); + } - /* - * Mark all complete remote refs as common refs. - * Don't mark them common yet; the server has to be told so first. - */ - for (ref = *refs; ref; ref = ref->next) { - struct object *o = deref_tag(the_repository, - lookup_object(the_repository, - ref->old_oid.hash), - NULL, 0); + /* + * Mark all complete remote refs as common refs. + * Don't mark them common yet; the server has to be told so first. + */ + for (ref = *refs; ref; ref = ref->next) { + struct object *o = deref_tag(the_repository, + lookup_object(the_repository, + ref->old_oid.hash), + NULL, 0); - if (!o || o->type != OBJ_COMMIT || !(o->flags & COMPLETE)) - continue; + if (!o || o->type != OBJ_COMMIT || !(o->flags & COMPLETE)) + continue; - negotiator->known_common(negotiator, - (struct commit *)o); - } + negotiator->known_common(negotiator, + (struct commit *)o); } save_commit_buffer = old_save_commit_buffer; @@@ -991,15 -990,11 +991,15 @@@ static struct ref *do_fetch_pack(struc if (!server_supports("deepen-relative") && args->deepen_relative) die(_("Server does not support --deepen")); - mark_complete_and_common_ref(&negotiator, args, &ref); - filter_refs(args, &ref, sought, nr_sought); - if (everything_local(args, &ref)) { - packet_flush(fd[1]); - goto all_done; + if (!args->no_dependents) { + mark_complete_and_common_ref(&negotiator, args, &ref); + filter_refs(args, &ref, sought, nr_sought); + if (everything_local(args, &ref)) { + packet_flush(fd[1]); + goto all_done; + } + } else { + filter_refs(args, &ref, sought, nr_sought); } if (find_common(&negotiator, args, fd, &oid, ref) < 0) if (!args->keep_pack) @@@ -1045,7 -1040,7 +1045,7 @@@ static void add_shallow_requests(struc } } -static void add_wants(const struct ref *wants, struct strbuf *req_buf) +static void add_wants(int no_dependents, const struct ref *wants, struct strbuf *req_buf) { int use_ref_in_want = server_supports_feature("fetch", "ref-in-want", 0); @@@ -1062,12 -1057,8 +1062,12 @@@ * We use lookup_object here because we are only * interested in the case we *know* the object is * reachable and we have already scanned it. + * + * Do this only if args->no_dependents is false (if it is true, + * we cannot trust the object flags). */ - if (((o = lookup_object(the_repository, remote->hash)) != NULL) && + if (!no_dependents && + ((o = lookup_object(the_repository, remote->hash)) != NULL) && (o->flags & COMPLETE)) { continue; } @@@ -1164,7 -1155,7 +1164,7 @@@ static int send_fetch_request(struct fe } /* add wants */ - add_wants(wants, &req_buf); + add_wants(args->no_dependents, wants, &req_buf); if (args->no_dependents) { packet_buf_write(&req_buf, "done"); @@@ -1355,21 -1346,16 +1355,21 @@@ static struct ref *do_fetch_pack_v2(str args->deepen = 1; /* Filter 'ref' by 'sought' and those that aren't local */ - mark_complete_and_common_ref(&negotiator, args, &ref); - filter_refs(args, &ref, sought, nr_sought); - if (everything_local(args, &ref)) - state = FETCH_DONE; - else + if (!args->no_dependents) { + mark_complete_and_common_ref(&negotiator, args, &ref); + filter_refs(args, &ref, sought, nr_sought); + if (everything_local(args, &ref)) + state = FETCH_DONE; + else + state = FETCH_SEND_REQUEST; + + mark_tips(&negotiator, args->negotiation_tips); + for_each_cached_alternate(&negotiator, + insert_one_alternate_object); + } else { + filter_refs(args, &ref, sought, nr_sought); state = FETCH_SEND_REQUEST; - - mark_tips(&negotiator, args->negotiation_tips); - for_each_cached_alternate(&negotiator, - insert_one_alternate_object); + } break; case FETCH_SEND_REQUEST: if (send_fetch_request(&negotiator, fd[1], args, ref, @@@ -1612,21 -1598,7 +1612,21 @@@ struct ref *fetch_pack(struct fetch_pac if (nr_sought) nr_sought = remove_duplicates_in_refs(sought, nr_sought); + if (args->no_dependents && !args->filter_options.choice) { + /* + * The protocol does not support requesting that only the + * wanted objects be sent, so approximate this by setting a + * "blob:none" filter if no filter is already set. This works + * for all object types: note that wanted blobs will still be + * sent because they are directly specified as a "want". + * + * NEEDSWORK: Add an option in the protocol to request that + * only the wanted objects be sent, and implement it. + */ + parse_list_objects_filter(&args->filter_options, "blob:none"); + } + - if (!ref) { + if (version != protocol_v2 && !ref) { packet_flush(fd[1]); die(_("no matching remote head")); } diff --combined t/t5551-http-fetch-smart.sh index 3dc8f8ecec,12b339d239..8630b0cc39 --- a/t/t5551-http-fetch-smart.sh +++ b/t/t5551-http-fetch-smart.sh @@@ -23,26 -23,26 +23,26 @@@ test_expect_success 'create http-access setup_askpass_helper -cat >exp < GET /smart/repo.git/info/refs?service=git-upload-pack HTTP/1.1 -> Accept: */* -> Accept-Encoding: ENCODINGS -> Pragma: no-cache -< HTTP/1.1 200 OK -< Pragma: no-cache -< Cache-Control: no-cache, max-age=0, must-revalidate -< Content-Type: application/x-git-upload-pack-advertisement -> POST /smart/repo.git/git-upload-pack HTTP/1.1 -> Accept-Encoding: ENCODINGS -> Content-Type: application/x-git-upload-pack-request -> Accept: application/x-git-upload-pack-result -> Content-Length: xxx -< HTTP/1.1 200 OK -< Pragma: no-cache -< Cache-Control: no-cache, max-age=0, must-revalidate -< Content-Type: application/x-git-upload-pack-result -EOF test_expect_success 'clone http repository' ' + cat >exp <<-\EOF && + > GET /smart/repo.git/info/refs?service=git-upload-pack HTTP/1.1 + > Accept: */* + > Accept-Encoding: ENCODINGS + > Pragma: no-cache + < HTTP/1.1 200 OK + < Pragma: no-cache + < Cache-Control: no-cache, max-age=0, must-revalidate + < Content-Type: application/x-git-upload-pack-advertisement + > POST /smart/repo.git/git-upload-pack HTTP/1.1 + > Accept-Encoding: ENCODINGS + > Content-Type: application/x-git-upload-pack-request + > Accept: application/x-git-upload-pack-result + > Content-Length: xxx + < HTTP/1.1 200 OK + < Pragma: no-cache + < Cache-Control: no-cache, max-age=0, must-revalidate + < Content-Type: application/x-git-upload-pack-result + EOF GIT_TRACE_CURL=true git clone --quiet $HTTPD_URL/smart/repo.git clone 2>err && test_cmp file clone/file && tr '\''\015'\'' Q exp <exp <<-\EOF && + GET /smart/repo.git/info/refs?service=git-upload-pack HTTP/1.1 200 + POST /smart/repo.git/git-upload-pack HTTP/1.1 200 + GET /smart/repo.git/info/refs?service=git-upload-pack HTTP/1.1 200 + POST /smart/repo.git/git-upload-pack HTTP/1.1 200 + EOF check_access_log exp ' @@@ -203,19 -203,19 +203,19 @@@ test_expect_success 'dumb clone via htt test_cmp expect actual ' -cat >cookies.txt <expect_cookies.txt <cookies.txt <<-\EOF && + 127.0.0.1 FALSE /smart_cookies/ FALSE 0 othername othervalue + EOF + sort >expect_cookies.txt <<-\EOF && + + 127.0.0.1 FALSE /smart_cookies/ FALSE 0 othername othervalue + 127.0.0.1 FALSE /smart_cookies/repo.git/info/ FALSE 0 name value + EOF git config http.cookiefile cookies.txt && git config http.savecookies true && git ls-remote $HTTPD_URL/smart_cookies/repo.git master && - tail -3 cookies.txt >cookies_tail.txt && + tail -3 cookies.txt | sort >cookies_tail.txt && test_cmp expect_cookies.txt cookies_tail.txt ' @@@ -381,6 -381,21 +381,21 @@@ test_expect_success 'using fetch comman test_cmp expect actual ' + test_expect_success 'fetch by SHA-1 without tag following' ' + SERVER="$HTTPD_DOCUMENT_ROOT_PATH/server" && + rm -rf "$SERVER" client && + + git init "$SERVER" && + test_commit -C "$SERVER" foo && + + git clone $HTTPD_URL/smart/server client && + + test_commit -C "$SERVER" bar && + git -C "$SERVER" rev-parse bar >bar_hash && + git -C client -c protocol.version=0 fetch \ + --no-tags origin $(cat bar_hash) + ' + test_expect_success 'GIT_REDACT_COOKIES redacts cookies' ' rm -rf clone && echo "Set-Cookie: Foo=1" >cookies && diff --combined t/t5702-protocol-v2.sh index 98fbf39da3,b82b5f8f7e..8360188c01 --- a/t/t5702-protocol-v2.sh +++ b/t/t5702-protocol-v2.sh @@@ -29,7 -29,7 +29,7 @@@ test_expect_success 'list refs with git grep "git< version 2" log && git ls-remote --symref "$GIT_DAEMON_URL/parent" >expect && - test_cmp actual expect + test_cmp expect actual ' test_expect_success 'ref advertisment is filtered with ls-remote using protocol v2' ' @@@ -42,7 -42,7 +42,7 @@@ $(git -C "$daemon_parent" rev-parse refs/heads/master)$(printf "\t")refs/heads/master EOF - test_cmp actual expect + test_cmp expect actual ' test_expect_success 'clone with git:// using protocol v2' ' @@@ -79,6 -79,19 +79,19 @@@ test_expect_success 'fetch with git:// grep "fetch< version 2" log ' + test_expect_success 'fetch by hash without tag following with protocol v2 does not list refs' ' + test_when_finished "rm -f log" && + + test_commit -C "$daemon_parent" two_a && + git -C "$daemon_parent" rev-parse two_a >two_a_hash && + + GIT_TRACE_PACKET="$(pwd)/log" git -C daemon_child -c protocol.version=2 \ + fetch --no-tags origin $(cat two_a_hash) && + + grep "fetch< version 2" log && + ! grep "fetch> command=ls-refs" log + ' + test_expect_success 'pull with git:// using protocol v2' ' test_when_finished "rm -f log" && @@@ -138,7 -151,7 +151,7 @@@ test_expect_success 'list refs with fil grep "git< version 2" log && git ls-remote --symref "file://$(pwd)/file_parent" >expect && - test_cmp actual expect + test_cmp expect actual ' test_expect_success 'ref advertisment is filtered with ls-remote using protocol v2' ' @@@ -151,7 -164,7 +164,7 @@@ $(git -C file_parent rev-parse refs/heads/master)$(printf "\t")refs/heads/master EOF - test_cmp actual expect + test_cmp expect actual ' test_expect_success 'server-options are sent when using ls-remote' ' @@@ -164,7 -177,7 +177,7 @@@ $(git -C file_parent rev-parse refs/heads/master)$(printf "\t")refs/heads/master EOF - test_cmp actual expect && + test_cmp expect actual && grep "server-option=hello" log && grep "server-option=world" log ' @@@ -271,7 -284,7 +284,7 @@@ test_expect_success 'partial clone' grep "version 2" trace && # Ensure that the old version of the file is missing - git -C client rev-list master --quiet --objects --missing=print \ + git -C client rev-list --quiet --objects --missing=print master \ >observed.oids && grep "$(git -C server rev-parse message1:a.txt)" observed.oids && @@@ -286,6 -299,10 +299,10 @@@ test_expect_success 'dynamically fetch grep "version 2" trace ' + test_expect_success 'when dynamically fetching missing object, do not list refs' ' + ! grep "git> command=ls-refs" trace + ' + test_expect_success 'partial fetch' ' rm -rf client "$(pwd)/trace" && git init client && @@@ -297,7 -314,7 +314,7 @@@ grep "version 2" trace && # Ensure that the old version of the file is missing - git -C client rev-list other --quiet --objects --missing=print \ + git -C client rev-list --quiet --objects --missing=print other \ >observed.oids && grep "$(git -C server rev-parse message1:a.txt)" observed.oids && @@@ -334,7 -351,7 +351,7 @@@ test_expect_success 'even with handcraf git -C server config uploadpack.allowfilter 0 && # Custom request that tries to filter even though it is not advertised. - test-pkt-line pack >in <<-EOF && + test-tool pkt-line pack >in <<-EOF && command=fetch 0001 want $(git -C server rev-parse master) diff --combined transport.c index dd7bd3436c,ea72fff6a6..046b2f6d8c --- a/transport.c +++ b/transport.c @@@ -252,8 -252,18 +252,18 @@@ static int connect_setup(struct transpo return 0; } - static struct ref *get_refs_via_connect(struct transport *transport, int for_push, - const struct argv_array *ref_prefixes) + /* + * Obtains the protocol version from the transport and writes it to + * transport->data->version, first connecting if not already connected. + * + * If the protocol version is one that allows skipping the listing of remote + * refs, and must_list_refs is 0, the listing of remote refs is skipped and + * this function returns NULL. Otherwise, this function returns the list of + * remote refs. + */ + static struct ref *handshake(struct transport *transport, int for_push, + const struct argv_array *ref_prefixes, + int must_list_refs) { struct git_transport_data *data = transport->data; struct ref *refs = NULL; @@@ -268,8 -278,10 +278,10 @@@ data->version = discover_version(&reader); switch (data->version) { case protocol_v2: - get_remote_refs(data->fd[1], &reader, &refs, for_push, - ref_prefixes, transport->server_options); + if (must_list_refs) + get_remote_refs(data->fd[1], &reader, &refs, for_push, + ref_prefixes, + transport->server_options); break; case protocol_v1: case protocol_v0: @@@ -283,9 -295,18 +295,18 @@@ } data->got_remote_heads = 1; + if (reader.line_peeked) + BUG("buffer must be empty at the end of handshake()"); + return refs; } + static struct ref *get_refs_via_connect(struct transport *transport, int for_push, + const struct argv_array *ref_prefixes) + { + return handshake(transport, for_push, ref_prefixes, 1); + } + static int fetch_refs_via_pack(struct transport *transport, int nr_heads, struct ref **to_fetch) { @@@ -320,8 -341,17 +341,17 @@@ args.server_options = transport->server_options; args.negotiation_tips = data->options.negotiation_tips; - if (!data->got_remote_heads) - refs_tmp = get_refs_via_connect(transport, 0, NULL); + if (!data->got_remote_heads) { + int i; + int must_list_refs = 0; + for (i = 0; i < nr_heads; i++) { + if (!to_fetch[i]->exact_oid) { + must_list_refs = 1; + break; + } + } + refs_tmp = handshake(transport, 0, NULL, must_list_refs); + } switch (data->version) { case protocol_v2: @@@ -703,6 -733,7 +733,7 @@@ static int disconnect_git(struct transp } static struct transport_vtable taken_over_vtable = { + 1, NULL, get_refs_via_connect, fetch_refs_via_pack, @@@ -852,6 -883,7 +883,7 @@@ void transport_check_allowed(const cha } static struct transport_vtable bundle_vtable = { + 0, NULL, get_refs_from_bundle, fetch_refs_from_bundle, @@@ -861,6 -893,7 +893,7 @@@ }; static struct transport_vtable builtin_smart_vtable = { + 1, NULL, get_refs_via_connect, fetch_refs_via_pack, @@@ -1139,8 -1172,7 +1172,8 @@@ int transport_push(struct transport *tr oid_array_append(&commits, &ref->new_oid); - if (!push_unpushed_submodules(&commits, + if (!push_unpushed_submodules(&the_index, + &commits, transport->remote, rs, transport->push_options, @@@ -1164,10 -1196,8 +1197,10 @@@ oid_array_append(&commits, &ref->new_oid); - if (find_unpushed_submodules(&commits, transport->remote->name, - &needs_pushing)) { + if (find_unpushed_submodules(&the_index, + &commits, + transport->remote->name, + &needs_pushing)) { oid_array_clear(&commits); die_with_unpushed_submodules(&needs_pushing); } @@@ -1227,6 -1257,15 +1260,15 @@@ int transport_fetch_refs(struct transpo struct ref **heads = NULL; struct ref *rm; + if (!transport->vtable->fetch_without_list) + /* + * Some transports (e.g. the built-in bundle transport and the + * transport helper interface) do not work when fetching is + * done immediately after transport creation. List the remote + * refs anyway (if not already listed) as a workaround. + */ + transport_get_remote_refs(transport, NULL); + for (rm = refs; rm; rm = rm->next) { nr_refs++; if (rm->peer_ref &&