git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v3 5/7] fetch-pack: do not lazy-fetch during ref iteration

From
JTJonathan Tan <jonathantanmy@google.com>
Date
Aug 18, 2020, 04:01 UTC
Message-ID
<f56c24dd1b05081b9c66e87746a67e12394ef012.1597722942.git.jonathantanmy@google.com>
In-Reply-To
<cover.1597722941.git.jonathantanmy@google.com>

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 <jonathantanmy@google.com>
---
 fetch-pack.c             | 79 ++++++++++++++++++++++------------------
 t/t5616-partial-clone.sh | 20 ++++++++++
 2 files changed, 64 insertions(+), 35 deletions(-)
diff --git a/fetch-pack.c b/fetch-pack.c
index 80fb3bd899..707bbc31fd 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);
 
diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh
index 8a27452a51..e53492d595 100755
--- a/t/t5616-partial-clone.sh
+++ b/t/t5616-partial-clone.sh
@@ -384,6 +384,26 @@ test_expect_success 'fetch lazy-fetches only to resolve deltas, protocol v2' '
 	grep "want $(cat hash)" trace
 '
 
+test_expect_success 'fetch does not lazy-fetch missing targets of its refs' '
+	rm -rf server client trace &&
+
+	test_create_repo server &&
+	test_config -C server uploadpack.allowfilter 1 &&
+	test_config -C server uploadpack.allowanysha1inwant 1 &&
+	test_commit -C server foo &&
+
+	git clone --filter=blob:none "file://$(pwd)/server" client &&
+	# Make all refs point to nothing by deleting all objects.
+	rm client/.git/objects/pack/* &&
+
+	test_commit -C server bar &&
+	GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
+		--no-tags --recurse-submodules=no \
+		origin refs/tags/bar &&
+	FOO_HASH=$(git -C server rev-parse foo) &&
+	! grep "want $FOO_HASH" trace
+'
+
 # The following two tests must be in this order. It is important that
 # the srv.bare repository did not have tags during clone, but has tags
 # in the fetch.

-- 
2.28.0.220.ged08abb693-goog
Previous: Jonathan TanNext: Jonathan Tan
Message 50 of 57 in “Modify fetch-pack to no longer die on error?”
  1. Modify fetch-pack to no longer die on error?Jonathan Tan, Jul 24, 2020
  2. Junio C HamanoJul 24, 2020
  3. Junio C HamanoJul 24, 2020
  4. Jeff KingJul 25, 2020
  5. Junio C HamanoJul 25, 2020
  6. Jeff KingJul 27, 2020
  7. Jonathan TanJul 28, 2020
  8. Jeff KingJul 28, 2020
  9. Jonathan TanJul 29, 2020
  10. Jeff KingJul 29, 2020
  11. Junio C HamanoJul 29, 2020
  12. Jonathan TanJul 29, 2020
  13. 0/7 Lazy fetch with subprocessJonathan Tan, Aug 5, 2020
  14. 1/7 fetch-pack: allow NULL negotiator->add_tipJonathan Tan, Aug 5, 2020
  15. Junio C HamanoAug 5, 2020
  16. Jonathan TanAug 7, 2020
  17. 3/7 negotiator/null: add null fetch negotiatorJonathan Tan, Aug 5, 2020
  18. 2/7 fetch-pack: allow NULL negotiator->known_commonJonathan Tan, Aug 5, 2020
  19. Junio C HamanoAug 5, 2020
  20. Junio C HamanoAug 5, 2020
  21. Jonathan TanAug 7, 2020
  22. 4/7 fetch: --stdinJonathan Tan, Aug 5, 2020
  23. Junio C HamanoAug 5, 2020
  24. Jonathan TanAug 7, 2020
  25. Junio C HamanoAug 7, 2020
  26. Jonathan TanAug 7, 2020
  27. 5/7 fetch: submodule configJonathan Tan, Aug 5, 2020
  28. 7/7 promisor-remote: use subprocess to fetchJonathan Tan, Aug 5, 2020
  29. 6/7 fetch: only populate existing_refs if neededJonathan Tan, Aug 5, 2020
  30. 0/7 Lazy fetch with subprocessJonathan Tan, Aug 11, 2020
  31. 1/7 negotiator/null: add null fetch negotiatorJonathan Tan, Aug 11, 2020
  32. Derrick StoleeAug 12, 2020
  33. Junio C HamanoAug 12, 2020
  34. Jonathan TanAug 12, 2020
  35. 2/7 fetch: allow refspecs specified through stdinJonathan Tan, Aug 11, 2020
  36. 3/7 fetch: avoid reading submodule config until neededJonathan Tan, Aug 11, 2020
  37. Junio C HamanoAug 12, 2020
  38. 4/7 fetch: only populate existing_refs if neededJonathan Tan, Aug 11, 2020
  39. Junio C HamanoAug 12, 2020
  40. 5/7 fetch-pack: do not lazy-fetch during ref iterationJonathan Tan, Aug 11, 2020
  41. Junio C HamanoAug 12, 2020
  42. 6/7 promisor-remote: lazy-fetch objects in subprocessJonathan Tan, Aug 11, 2020
  43. Junio C HamanoAug 12, 2020
  44. 7/7 fetch-pack: remove no_dependents codeJonathan Tan, Aug 11, 2020
  45. Derrick StoleeAug 12, 2020
  46. 0/7 Lazy fetch with subprocessJonathan Tan, Aug 18, 2020
  47. 1/7 negotiator/noop: add noop fetch negotiatorJonathan Tan, Aug 18, 2020
  48. 2/7 fetch: allow refspecs specified through stdinJonathan Tan, Aug 18, 2020
  49. 4/7 fetch: only populate existing_refs if neededJonathan Tan, Aug 18, 2020
  50. 5/7 fetch-pack: do not lazy-fetch during ref iterationJonathan Tan, Aug 18, 2020
  51. 3/7 fetch: avoid reading submodule config until neededJonathan Tan, Aug 18, 2020
  52. 6/7 promisor-remote: lazy-fetch objects in subprocessJonathan Tan, Aug 18, 2020
  53. 7/7 fetch-pack: remove no_dependents codeJonathan Tan, Aug 18, 2020
  54. Junio C HamanoAug 18, 2020
  55. Junio C HamanoAug 18, 2020
  56. fixup! promisor-remote: lazy-fetch objects in subprocessJonathan Tan, Aug 18, 2020
  57. Junio C HamanoAug 18, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.