[PATCH v2 2/3] index-pack --promisor: don't check blobs

From

JTJonathan Tan <jonathantanmy@google.com>

Date

Dec 3, 2024, 21:43 UTC

Message-ID

<5a63c9a5cac8088730cc536f33b0af052c90aca1.1733259949.git.jonathantanmy@google.com>

In-Reply-To

<cover.1733259949.git.jonathantanmy@google.com>

The tradeoff of not checking blobs is documented in a code comment.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
 builtin/index-pack.c | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index d1c777a6af..57b7888c42 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -817,6 +817,33 @@ static void record_outgoing_link(const struct object_id *oid)
 	oidset_insert(&outgoing_links, oid);
 }
 
+static void maybe_record_name_entry(const struct name_entry *entry)
+{
+	/*
+	 * The benefit of doing this is as above (fetch speedup), but the drawback
+is that if the packfile to be indexed references a local blob directly
+(that is, not through a local tree), that local blob is in danger of
+being garbage collected. Such a situation may arise if we push local
+commits, including one with a change to a blob in the root tree,
+and then the server incorporates them into its main branch through a
+"rebase" or "squash" merge strategy, and then we fetch the new main
+branch from the server.
+
+This situation has not been observed yet - we have only noticed missing
+commits, not missing trees or blobs. (In fact, if it were believed that
+only missing commits are problematic, one could argue that we should
+also exclude trees during the outgoing link check; but it is safer to
+include them.)
+
+Due to the rarity of the situation (it has not been observed to happen
+in real life), and because the "penalty" in such a situation is merely
+to refetch the missing blob when it's needed, the tradeoff seems
+worth it.
+	*/
+	if (S_ISDIR(entry->mode))
+		record_outgoing_link(&entry->oid);
+}
+
 static void do_record_outgoing_links(struct object *obj)
 {
 	if (obj->type == OBJ_TREE) {
@@ -831,7 +858,7 @@ static void do_record_outgoing_links(struct object *obj)
 			 */
 			return;
 		while (tree_entry_gently(&desc, &entry))
-			record_outgoing_link(&entry.oid);
+			maybe_record_name_entry(&entry);
 	} else if (obj->type == OBJ_COMMIT) {
 		struct commit *commit = (struct commit *) obj;
 		struct commit_list *parents = commit->parents;

-- 
2.47.0.338.g60cca15819-goog