send-pack: limit on negative references

[Andy Whitcroft <apw@shadowen.org>]

[-- Attachment #1: Type: text/plain, Size: 1508 bytes --]

I've been having trouble with git push apparently resending the entire
commit trace for the branch each and every time I push; even with short
branch names.  This seems to be related to the changes made to handle
the case where the remote end has a large number of branches (>900).

The problem there seemed to be that we would fill list-revs' argument
list in reference order with negative and positive limits.  This could
lead to us to exceed the limit 900 argument limit and bomb.  The changes
made in commit 797656e58ddbd82ac461a5142ed726db3a4d0ac0 split the
parameter loading into two phases.  First we load the +ve and -ve
references for the branches we are definatly trying to send; bailing if
we cannot fit those references onto the command line.  Second we load up
the remaining remote references as -ve references to limit duplicate
object sends.

This second phase loads up to 16 additional references onto the command
line.  Now this effectively limits you to 16 branchs in your remote
repository if you wish to ensure you catch a parental head to copy from.
 This can lead us to send the entire commit stream for the branch even
though the branch may contain no new commits.

It seems to me that any and all negative references we have are helpful
and as such we should be loading as many as we can into the list.  If we
cannot fit them all we should _not_ error out, but we should not limit
ourselves to any less than as many as we can fit.

Can anyone see a downside to the patch below?

-apw

[-- Attachment #2: send-pack-supply-as-many-negative-references-as-we-can-fit --]
[-- Type: text/plain, Size: 1211 bytes --]

send-pack: supply as many negative references as we can fit

In commit 797656e58ddbd82ac461a5142ed726db3a4d0ac0 list-revs' command
line was reordered to ensure the branches we are pushing are first on
the list, and then a subset of the remaining remote references are
passed to limit our push.  Given that we now have the needed positive
references on the list first, it seems reasonable to load as many
negative references onto the list as will fit without error.

Signed-off-by: Andy Whitcroft <andyw@uk.ibm.com>
---
diff --git a/send-pack.c b/send-pack.c
index 10bc8bc..5d55241 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -40,7 +40,7 @@ static void exec_rev_list(struct ref *re
 {
 	struct ref *ref;
 	static const char *args[1000];
-	int i = 0, j;
+	int i = 0;
 
 	args[i++] = "rev-list";	/* 0 */
 	if (use_thin_pack)	/* 1 */
@@ -69,8 +69,8 @@ static void exec_rev_list(struct ref *re
 	/* Then a handful of the remainder
 	 * NEEDSWORK: we would be better off if used the newer ones first.
 	 */
-	for (ref = refs, j = i + 16;
-	     i < 900 && i < j && ref;
+	for (ref = refs;
+	     i < 900 && ref;
 	     ref = ref->next) {
 		if (is_zero_sha1(ref->new_sha1) &&
 		    !is_zero_sha1(ref->old_sha1) &&