diff mbox series

[v3,3/3] index-pack --promisor: also check commits' trees

Message ID f9f9969a8f399fda701b0ccd44dd124e953bf36d.1733262662.git.jonathantanmy@google.com (mailing list archive)
State Accepted
Commit 1a14c857dbac02715725e8abcff35266c4c5ac93
Headers show
Series Performance improvements for repacking non-promisor objects | expand

Commit Message

Jonathan Tan Dec. 3, 2024, 9:52 p.m. UTC
Commit c08589efdc (index-pack: repack local links into promisor packs,
2024-11-01) seems to contain an oversight in that the tree of a commit
is not checked. Teach git to check these trees.

The fix slows down a fetch from a certain repo at $DAYJOB from 2m2.127s
to 2m45.052s, but in order to make the fetch correct, it seems worth it.

In order to test this, we could create server and client repos as
follows...

 C   S
  \ /
   O

(O and C are commits both on the client and server. S is a commit
only on the server. C and S have the same tree but different commit
messages. The diff between O and C is non-zero.)

...and then, from the client, fetch S from the server.

In theory, the client declares "have C" and the server can use this
information to exclude S's tree (since it knows that the client has C's
tree, which is the same as S's tree). However, it is also possible for
the server to compute that it needs to send S and not O, and proceed
from there; therefore the objects of C are not considered at all when
determining what to send in the packfile. In order to prevent a test of
client functionality from having such a dependence on server behavior, I
have not included such a test.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
 builtin/index-pack.c | 1 +
 1 file changed, 1 insertion(+)
diff mbox series

Patch

diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 2e90fe186e..1594f2b81d 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -865,6 +865,7 @@  static void do_record_outgoing_links(struct object *obj)
 		struct commit *commit = (struct commit *) obj;
 		struct commit_list *parents = commit->parents;
 
+		record_outgoing_link(get_commit_tree_oid(commit));
 		for (; parents; parents = parents->next)
 			record_outgoing_link(&parents->item->object.oid);
 	} else if (obj->type == OBJ_TAG) {