From 6462d5eb9a5b23ab9cff4e3c92ff930600562d8f Mon Sep 17 00:00:00 2001 From: Jonathan Tan Date: Tue, 5 Nov 2019 10:56:19 -0800 Subject: fetch: remove fetch_if_missing=0 In fetch_pack() (and all functions it calls), pass OBJECT_INFO_SKIP_FETCH_OBJECT whenever we query an object that could be a tree or blob that we do not want to be lazy-fetched even if it is absent. Thus, the only lazy-fetches occurring for trees and blobs are when resolving deltas. Thus, we can remove fetch_if_missing=0 from builtin/fetch.c. Remove this, and also add a test ensuring that such objects are not lazy-fetched. (We might be able to remove fetch_if_missing=0 from other places too, but I have limited myself to builtin/fetch.c in this commit because I have not written tests for the other commands yet.) Note that commits and tags may still be lazy-fetched. I limited myself to objects that could be trees or blobs here because Git does not support creating such commit- and tag-excluding clones yet, and even if such a clone were manually created, Git does not have good support for fetching a single commit (when fetching a commit, it and all its ancestors would be sent). Signed-off-by: Jonathan Tan Signed-off-by: Junio C Hamano --- fetch-pack.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'fetch-pack.c') diff --git a/fetch-pack.c b/fetch-pack.c index 0130b44112..37178e2d34 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -673,7 +673,8 @@ static void mark_complete_and_common_ref(struct fetch_negotiator *negotiator, struct object *o; if (!has_object_file_with_flags(&ref->old_oid, - OBJECT_INFO_QUICK)) + OBJECT_INFO_QUICK | + OBJECT_INFO_SKIP_FETCH_OBJECT)) continue; o = parse_object(the_repository, &ref->old_oid); if (!o) -- cgit v1.2.3 From 603960b50edeb1f0afa694f2f0283e553c031129 Mon Sep 17 00:00:00 2001 From: Jonathan Tan Date: Tue, 12 Nov 2019 16:34:20 -0800 Subject: promisor-remote: remove fetch_if_missing=0 Commit 6462d5eb9a ("fetch: remove fetch_if_missing=0", 2019-11-08) strove to remove the need for fetch_if_missing=0 from the fetching mechanism, so it is plausible to attempt removing fetch_if_missing=0 from the lazy-fetching mechanism in promisor-remote as well. But doing so reveals a bug - when the server does not send an object pointed to by a tag object, an infinite loop occurs: Git attempts to fetch the missing object, which causes a deferencing of all refs (for negotiation), which causes a lazy fetch of that missing object, and so on. This bug is because of unnecessary use of the fetch negotiator during lazy fetching - it is not used after initialization, but it is still initialized (which causes the dereferencing of all refs). Thus, when the negotiator is not used during fetching, refrain from initializing it. Then, remove fetch_if_missing from promisor-remote. Signed-off-by: Jonathan Tan Signed-off-by: Junio C Hamano --- fetch-pack.c | 46 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 32 insertions(+), 14 deletions(-) (limited to 'fetch-pack.c') diff --git a/fetch-pack.c b/fetch-pack.c index 37178e2d34..490b111822 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -896,8 +896,15 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args, struct object_id oid; const char *agent_feature; int agent_len; - struct fetch_negotiator negotiator; - fetch_negotiator_init(r, &negotiator); + struct fetch_negotiator negotiator_alloc; + struct fetch_negotiator *negotiator; + + if (args->no_dependents) { + negotiator = NULL; + } else { + negotiator = &negotiator_alloc; + fetch_negotiator_init(r, negotiator); + } sort_ref_list(&ref, ref_compare_name); QSORT(sought, nr_sought, cmp_ref_by_name); @@ -984,7 +991,7 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args, die(_("Server does not support --deepen")); if (!args->no_dependents) { - mark_complete_and_common_ref(&negotiator, args, &ref); + mark_complete_and_common_ref(negotiator, args, &ref); filter_refs(args, &ref, sought, nr_sought); if (everything_local(args, &ref)) { packet_flush(fd[1]); @@ -993,7 +1000,7 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args, } else { filter_refs(args, &ref, sought, nr_sought); } - if (find_common(&negotiator, args, fd, &oid, ref) < 0) + if (find_common(negotiator, args, fd, &oid, ref) < 0) if (!args->keep_pack) /* When cloning, it is not unusual to have * no common commit. @@ -1013,7 +1020,8 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args, die(_("git fetch-pack: fetch failed.")); all_done: - negotiator.release(&negotiator); + if (negotiator) + negotiator->release(negotiator); return ref; } @@ -1231,7 +1239,8 @@ static int process_acks(struct fetch_negotiator *negotiator, struct commit *commit; oidset_insert(common, &oid); commit = lookup_commit(the_repository, &oid); - negotiator->ack(negotiator, commit); + if (negotiator) + negotiator->ack(negotiator, commit); } continue; } @@ -1383,8 +1392,16 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args, struct packet_reader reader; int in_vain = 0, negotiation_started = 0; int haves_to_send = INITIAL_FLUSH; - struct fetch_negotiator negotiator; - fetch_negotiator_init(r, &negotiator); + struct fetch_negotiator negotiator_alloc; + struct fetch_negotiator *negotiator; + + if (args->no_dependents) { + negotiator = NULL; + } else { + negotiator = &negotiator_alloc; + fetch_negotiator_init(r, negotiator); + } + packet_reader_init(&reader, fd[0], NULL, 0, PACKET_READ_CHOMP_NEWLINE | PACKET_READ_DIE_ON_ERR_PACKET); @@ -1408,15 +1425,15 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args, /* Filter 'ref' by 'sought' and those that aren't local */ if (!args->no_dependents) { - mark_complete_and_common_ref(&negotiator, args, &ref); + 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, + mark_tips(negotiator, args->negotiation_tips); + for_each_cached_alternate(negotiator, insert_one_alternate_object); } else { filter_refs(args, &ref, sought, nr_sought); @@ -1430,7 +1447,7 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args, "negotiation_v2", the_repository); } - if (send_fetch_request(&negotiator, fd[1], args, ref, + if (send_fetch_request(negotiator, fd[1], args, ref, &common, &haves_to_send, &in_vain, reader.use_sideband)) @@ -1440,7 +1457,7 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args, break; case FETCH_PROCESS_ACKS: /* Process ACKs/NAKs */ - switch (process_acks(&negotiator, &reader, &common)) { + switch (process_acks(negotiator, &reader, &common)) { case 2: state = FETCH_GET_PACK; break; @@ -1475,7 +1492,8 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args, } } - negotiator.release(&negotiator); + if (negotiator) + negotiator->release(negotiator); oidset_clear(&common); return ref; } -- cgit v1.2.3