From: Junio C Hamano <junkio@cox.net>
Date: Thu, 20 Oct 2005 01:16:11 GMT
Subject: Re: [PATCH] Do not send "want" lines for complete objects
Message-ID: <7vwtk9vvhg.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <7vzmp5xfwu.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> writes:

> Let's draw a single strand of pearl case to illustrate.  You
> have a commit chain A->B->C->D, and the other end says she has
> C.  At this point, telling her that you have C is enough, and by
> not telling her about A and B, you would save her from depreting
> MAX_HAVE slots.  Of course, if the other end has D as another
> head, then the above logic would give "^D^" to rev-list as well,
> telling it not to tell her about C, but that is what we want --
> because she already knows you have C too when you tell her that
> you have D.  I think I like this optimization.

This was subtly wrong.  ^D^ would barf if D is a tag that points
at a non commit (refs/tags/v2.6.11-tree).  Also it would do a
suboptimal thing for a merge commit, since it will not cull the
second and later parents.

Maybe something like this on top of your patch?  This is turning
out to be quite ugly.

---

diff --git a/fetch-pack.c b/fetch-pack.c
index 9dfd072..5cc3766 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -28,20 +28,29 @@ static int find_common(int fd[2], unsign
 	fetching = 0;
 	for ( ; refs ; refs = refs->next) {
 		unsigned char *remote = refs->old_sha1;
-
+		struct object *o;
+		struct commit *commit;
 		/*
-		   If that object is complete (i.e. it is a descendant of a
-		   local ref), we don't want it, nor its descendants.
-		*/
-		if (has_sha1_file(remote)
-				&& parse_object(remote)->flags & COMPLETE) {
-			if (rev_command_len + 44 < sizeof(rev_command)) {
+		 * If that object is complete (i.e. it is an ancestor of a
+		 * local ref), we tell them we have it but do not have to
+		 * tell them about its ancestors, which they already know
+		 * about.
+		 */
+		if (has_sha1_file(remote) &&
+		    ((o = parse_object(remote)) != NULL) &&
+		    (o->flags & COMPLETE) &&
+		    ((commit = (struct commit *) deref_tag(o)) != NULL) &&
+		    (commit->object.type = commit_type)) {
+			struct commit_list *p = commit->parents;
+			while (p && rev_command_len + 44 < sizeof(rev_command)) {
 				snprintf(rev_command + rev_command_len, 44,
-					" ^%s^", sha1_to_hex(remote));
+					 " ^%s",
+					 sha1_to_hex(p->item->object.sha1));
 				rev_command_len += 43;
+				p = p->next;
 			}
-
-			continue;
+			if (!p)
+				continue;
 		}
 
 		packet_write(fd[1], "want %s\n", sha1_to_hex(remote));