From 5c3b801dab9d29c63c6c929405f808f064c11b77 Mon Sep 17 00:00:00 2001 From: Jonathan Tan Date: Mon, 17 Aug 2020 21:01:35 -0700 Subject: fetch-pack: do not lazy-fetch during ref iteration In order to determine negotiation tips, "fetch-pack" iterates over all refs and dereferences all annotated tags found. This causes the existence of targets of refs and annotated tags to be checked. Avoiding this is especially important when we use "git fetch" (which invokes "fetch-pack") to perform lazy fetches in a partial clone because a target of such a ref or annotated tag may need to be itself lazy-fetched (and otherwise causing an infinite loop). Therefore, teach "fetch-pack" not to lazy fetch whenever iterating over refs. This is done by using the raw form of ref iteration and by dereferencing tags ourselves. Signed-off-by: Jonathan Tan Signed-off-by: Junio C Hamano --- fetch-pack.c | 79 +++++++++++++++++++++++++++++++++--------------------------- 1 file changed, 44 insertions(+), 35 deletions(-) (limited to 'fetch-pack.c') diff --git a/fetch-pack.c b/fetch-pack.c index 7f20eca4f8..71dcc322c5 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -108,24 +108,48 @@ static void for_each_cached_alternate(struct fetch_negotiator *negotiator, cb(negotiator, cache.items[i]); } +static struct commit *deref_without_lazy_fetch(const struct object_id *oid, + int mark_tags_complete) +{ + enum object_type type; + struct object_info info = { .typep = &type }; + + while (1) { + if (oid_object_info_extended(the_repository, oid, &info, + OBJECT_INFO_SKIP_FETCH_OBJECT | OBJECT_INFO_QUICK)) + return NULL; + if (type == OBJ_TAG) { + struct tag *tag = (struct tag *) + parse_object(the_repository, oid); + + if (!tag->tagged) + return NULL; + if (mark_tags_complete) + tag->object.flags |= COMPLETE; + oid = &tag->tagged->oid; + } else { + break; + } + } + if (type == OBJ_COMMIT) + return (struct commit *) parse_object(the_repository, oid); + return NULL; +} + static int rev_list_insert_ref(struct fetch_negotiator *negotiator, - const char *refname, const struct object_id *oid) { - struct object *o = deref_tag(the_repository, - parse_object(the_repository, oid), - refname, 0); - - if (o && o->type == OBJ_COMMIT) - negotiator->add_tip(negotiator, (struct commit *)o); + struct commit *c = deref_without_lazy_fetch(oid, 0); + if (c) + negotiator->add_tip(negotiator, c); return 0; } static int rev_list_insert_ref_oid(const char *refname, const struct object_id *oid, int flag, void *cb_data) { - return rev_list_insert_ref(cb_data, refname, oid); + return rev_list_insert_ref(cb_data, oid); } enum ack_type { @@ -201,7 +225,7 @@ static void send_request(struct fetch_pack_args *args, static void insert_one_alternate_object(struct fetch_negotiator *negotiator, struct object *obj) { - rev_list_insert_ref(negotiator, NULL, &obj->oid); + rev_list_insert_ref(negotiator, &obj->oid); } #define INITIAL_FLUSH 16 @@ -230,13 +254,12 @@ static void mark_tips(struct fetch_negotiator *negotiator, int i; if (!negotiation_tips) { - for_each_ref(rev_list_insert_ref_oid, negotiator); + for_each_rawref(rev_list_insert_ref_oid, negotiator); return; } for (i = 0; i < negotiation_tips->nr; i++) - rev_list_insert_ref(negotiator, NULL, - &negotiation_tips->oid[i]); + rev_list_insert_ref(negotiator, &negotiation_tips->oid[i]); return; } @@ -503,21 +526,11 @@ static struct commit_list *complete; static int mark_complete(const struct object_id *oid) { - struct object *o = parse_object(the_repository, oid); - - while (o && o->type == OBJ_TAG) { - struct tag *t = (struct tag *) o; - if (!t->tagged) - break; /* broken repository */ - o->flags |= COMPLETE; - o = parse_object(the_repository, &t->tagged->oid); - } - if (o && o->type == OBJ_COMMIT) { - struct commit *commit = (struct commit *)o; - if (!(commit->object.flags & COMPLETE)) { - commit->object.flags |= COMPLETE; - commit_list_insert(commit, &complete); - } + struct commit *commit = deref_without_lazy_fetch(oid, 1); + + if (commit && !(commit->object.flags & COMPLETE)) { + commit->object.flags |= COMPLETE; + commit_list_insert(commit, &complete); } return 0; } @@ -702,7 +715,7 @@ static void mark_complete_and_common_ref(struct fetch_negotiator *negotiator, */ trace2_region_enter("fetch-pack", "mark_complete_local_refs", NULL); if (!args->deepen) { - for_each_ref(mark_complete_oid, NULL); + for_each_rawref(mark_complete_oid, NULL); for_each_cached_alternate(NULL, mark_alternate_complete); commit_list_sort_by_date(&complete); if (cutoff) @@ -716,16 +729,12 @@ static void mark_complete_and_common_ref(struct fetch_negotiator *negotiator, */ trace2_region_enter("fetch-pack", "mark_common_remote_refs", NULL); for (ref = *refs; ref; ref = ref->next) { - struct object *o = deref_tag(the_repository, - lookup_object(the_repository, - &ref->old_oid), - NULL, 0); + struct commit *c = deref_without_lazy_fetch(&ref->old_oid, 0); - if (!o || o->type != OBJ_COMMIT || !(o->flags & COMPLETE)) + if (!c || !(c->object.flags & COMPLETE)) continue; - negotiator->known_common(negotiator, - (struct commit *)o); + negotiator->known_common(negotiator, c); } trace2_region_leave("fetch-pack", "mark_common_remote_refs", NULL); -- cgit v1.3 From 9dfa8dbeee18a126aabcdd36a06e5d6b5eb6a58a Mon Sep 17 00:00:00 2001 From: Jonathan Tan Date: Mon, 17 Aug 2020 21:01:37 -0700 Subject: fetch-pack: remove no_dependents code Now that Git has switched to using a subprocess to lazy-fetch missing objects, remove the no_dependents code as it is no longer used. Signed-off-by: Jonathan Tan Signed-off-by: Junio C Hamano --- builtin/fetch-pack.c | 4 -- fetch-pack.c | 110 ++++++++++++++------------------------------------- fetch-pack.h | 14 ------- remote-curl.c | 6 --- transport.c | 4 -- transport.h | 7 ---- 6 files changed, 30 insertions(+), 115 deletions(-) (limited to 'fetch-pack.c') diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c index bbb5c96167..58b7c1fbdc 100644 --- a/builtin/fetch-pack.c +++ b/builtin/fetch-pack.c @@ -153,10 +153,6 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix) args.from_promisor = 1; continue; } - if (!strcmp("--no-dependents", arg)) { - args.no_dependents = 1; - continue; - } if (skip_prefix(arg, ("--" CL_ARG__FILTER "="), &arg)) { parse_list_objects_filter(&args.filter_options, arg); continue; diff --git a/fetch-pack.c b/fetch-pack.c index 71dcc322c5..6c576a98e9 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -285,10 +285,8 @@ static int find_common(struct fetch_negotiator *negotiator, PACKET_READ_CHOMP_NEWLINE | PACKET_READ_DIE_ON_ERR_PACKET); - if (!args->no_dependents) { - mark_tips(negotiator, args->negotiation_tips); - for_each_cached_alternate(negotiator, insert_one_alternate_object); - } + mark_tips(negotiator, args->negotiation_tips); + for_each_cached_alternate(negotiator, insert_one_alternate_object); fetching = 0; for ( ; refs ; refs = refs->next) { @@ -305,12 +303,8 @@ static int find_common(struct fetch_negotiator *negotiator, * 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 (!args->no_dependents && - ((o = lookup_object(the_repository, remote)) != NULL) && + if (((o = lookup_object(the_repository, remote)) != NULL) && (o->flags & COMPLETE)) { continue; } @@ -410,8 +404,6 @@ static int find_common(struct fetch_negotiator *negotiator, trace2_region_enter("fetch-pack", "negotiation_v0_v1", the_repository); flushes = 0; retval = -1; - if (args->no_dependents) - goto done; while ((oid = negotiator->next(negotiator))) { packet_buf_write(&req_buf, "have %s\n", oid_to_hex(oid)); print_verbose(args, "have %s", oid_to_hex(oid)); @@ -666,9 +658,7 @@ struct loose_object_iter { /* * Mark recent commits available locally and reachable from a local ref as - * COMPLETE. If args->no_dependents is false, also mark COMPLETE remote refs as - * COMMON_REF (otherwise, we are not planning to participate in negotiation, and - * thus do not need COMMON_REF marks). + * COMPLETE. * * The cutoff time for recency is determined by this heuristic: it is the * earliest commit time of the objects in refs that are commits and that we know @@ -969,12 +959,8 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args, struct fetch_negotiator negotiator_alloc; struct fetch_negotiator *negotiator; - if (args->no_dependents) { - negotiator = NULL; - } else { - negotiator = &negotiator_alloc; - fetch_negotiator_init(r, negotiator); - } + negotiator = &negotiator_alloc; + fetch_negotiator_init(r, negotiator); sort_ref_list(&ref, ref_compare_name); QSORT(sought, nr_sought, cmp_ref_by_name); @@ -1062,15 +1048,11 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args, if (!server_supports_hash(the_hash_algo->name, NULL)) die(_("Server does not support this repository's object format")); - 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); + 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 (find_common(negotiator, args, fd, &oid, ref) < 0) if (!args->keep_pack) @@ -1119,7 +1101,7 @@ static void add_shallow_requests(struct strbuf *req_buf, packet_buf_write(req_buf, "deepen-relative\n"); } -static void add_wants(int no_dependents, const struct ref *wants, struct strbuf *req_buf) +static void add_wants(const struct ref *wants, struct strbuf *req_buf) { int use_ref_in_want = server_supports_feature("fetch", "ref-in-want", 0); @@ -1136,12 +1118,8 @@ static void add_wants(int no_dependents, const struct ref *wants, struct strbuf * 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 (!no_dependents && - ((o = lookup_object(the_repository, remote)) != NULL) && + if (((o = lookup_object(the_repository, remote)) != NULL) && (o->flags & COMPLETE)) { continue; } @@ -1275,19 +1253,14 @@ static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out, } /* add wants */ - add_wants(args->no_dependents, wants, &req_buf); + add_wants(wants, &req_buf); - if (args->no_dependents) { - packet_buf_write(&req_buf, "done"); - ret = 1; - } else { - /* Add all of the common commits we've found in previous rounds */ - add_common(&req_buf, common); + /* Add all of the common commits we've found in previous rounds */ + add_common(&req_buf, common); - /* Add initial haves */ - ret = add_haves(negotiator, seen_ack, &req_buf, - haves_to_send, in_vain); - } + /* Add initial haves */ + ret = add_haves(negotiator, seen_ack, &req_buf, + haves_to_send, in_vain); /* Send request */ packet_buf_flush(&req_buf); @@ -1547,12 +1520,8 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args, struct string_list packfile_uris = STRING_LIST_INIT_DUP; int i; - if (args->no_dependents) { - negotiator = NULL; - } else { - negotiator = &negotiator_alloc; - fetch_negotiator_init(r, negotiator); - } + negotiator = &negotiator_alloc; + fetch_negotiator_init(r, negotiator); packet_reader_init(&reader, fd[0], NULL, 0, PACKET_READ_CHOMP_NEWLINE | @@ -1576,21 +1545,16 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args, args->deepen = 1; /* Filter 'ref' by 'sought' and those that aren't local */ - 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); + 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); break; case FETCH_SEND_REQUEST: if (!negotiation_started) { @@ -1911,20 +1875,6 @@ struct ref *fetch_pack(struct fetch_pack_args *args, 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 (version != protocol_v2 && !ref) { packet_flush(fd[1]); die(_("no matching remote head")); diff --git a/fetch-pack.h b/fetch-pack.h index 85d1e39fe7..bbe2938059 100644 --- a/fetch-pack.h +++ b/fetch-pack.h @@ -42,20 +42,6 @@ struct fetch_pack_args { unsigned deepen:1; unsigned from_promisor:1; - /* - * Attempt to fetch only the wanted objects, and not any objects - * referred to by them. Due to protocol limitations, extraneous - * objects may still be included. (When fetching non-blob - * objects, only blobs are excluded; when fetching a blob, the - * blob itself will still be sent. The client does not need to - * know whether a wanted object is a blob or not.) - * - * If 1, fetch_pack() will also not modify any object flags. - * This allows fetch_pack() to safely be called by any function, - * regardless of which object flags it uses (if any). - */ - unsigned no_dependents:1; - /* * Because fetch_pack() overwrites the shallow file upon a * successful deepening non-clone fetch, if this struct diff --git a/remote-curl.c b/remote-curl.c index 62b3a45cde..ce470f27b9 100644 --- a/remote-curl.c +++ b/remote-curl.c @@ -40,7 +40,6 @@ struct options { push_cert : 2, deepen_relative : 1, from_promisor : 1, - no_dependents : 1, atomic : 1, object_format : 1; const struct git_hash_algo *hash_algo; @@ -190,9 +189,6 @@ static int set_option(const char *name, const char *value) } else if (!strcmp(name, "from-promisor")) { options.from_promisor = 1; return 0; - } else if (!strcmp(name, "no-dependents")) { - options.no_dependents = 1; - return 0; } else if (!strcmp(name, "filter")) { options.filter = xstrdup(value); return 0; @@ -1175,8 +1171,6 @@ static int fetch_git(struct discovery *heads, strvec_push(&args, "--deepen-relative"); if (options.from_promisor) strvec_push(&args, "--from-promisor"); - if (options.no_dependents) - strvec_push(&args, "--no-dependents"); if (options.filter) strvec_pushf(&args, "--filter=%s", options.filter); strvec_push(&args, url.buf); diff --git a/transport.c b/transport.c index 2d4fd851dc..7efe32309a 100644 --- a/transport.c +++ b/transport.c @@ -232,9 +232,6 @@ static int set_git_option(struct git_transport_options *opts, } else if (!strcmp(name, TRANS_OPT_FROM_PROMISOR)) { opts->from_promisor = !!value; return 0; - } else if (!strcmp(name, TRANS_OPT_NO_DEPENDENTS)) { - opts->no_dependents = !!value; - return 0; } else if (!strcmp(name, TRANS_OPT_LIST_OBJECTS_FILTER)) { list_objects_filter_die_if_populated(&opts->filter_options); parse_list_objects_filter(&opts->filter_options, value); @@ -359,7 +356,6 @@ static int fetch_refs_via_pack(struct transport *transport, args.cloning = transport->cloning; args.update_shallow = data->options.update_shallow; args.from_promisor = data->options.from_promisor; - args.no_dependents = data->options.no_dependents; args.filter_options = data->options.filter_options; args.stateless_rpc = transport->stateless_rpc; args.server_options = transport->server_options; diff --git a/transport.h b/transport.h index 1be4013dec..fe5a0a6662 100644 --- a/transport.h +++ b/transport.h @@ -16,7 +16,6 @@ struct git_transport_options { unsigned update_shallow : 1; unsigned deepen_relative : 1; unsigned from_promisor : 1; - unsigned no_dependents : 1; /* * If this transport supports connect or stateless-connect, @@ -201,12 +200,6 @@ void transport_check_allowed(const char *type); /* Indicate that these objects are being fetched by a promisor */ #define TRANS_OPT_FROM_PROMISOR "from-promisor" -/* - * Indicate that only the objects wanted need to be fetched, not their - * dependents - */ -#define TRANS_OPT_NO_DEPENDENTS "no-dependents" - /* Filter objects for partial clone and fetch */ #define TRANS_OPT_LIST_OBJECTS_FILTER "filter" -- cgit v1.3