Re: Bad objects error since upgrading GitHub servers to 1.6.1

From

Junio C Hamano <gitster@pobox.com>

Date

Jan 28, 2009, 04:38 UTC

Message-ID

<7v1vuoxcxk.fsf@gitster.siamese.dyndns.org>

In-Reply-To

<20090128033020.GF1321@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> writes:

> Junio C Hamano <gitster@pobox.com> wrote:
>> 
>> Hmm, I am puzzled.
>> 
>> I do not know how 41fa7d2 (Teach git-fetch to exploit server side
>> automatic tag following, 2008-03-03), which is about the conversation
>> between fetch-pack and upload-pack, is relevant to the issue at hand,
>> which is about the conversation between send-pack and receive-pack.
>
> Oh, right, its not.  I was pointing out that the last time the
> protocol changed in a way the server can infer something about the
> client, which IIRC was 41fa7d2, we still don't have a way to tell
> what the client is.

> PJ - the short story here is, to forever work around these buggy
> 1.6.1 clients, you'd have to either run an old server forever,
> or forever run a patched server that disables the newer ".have"
> extension in the advertised data written by git-upload-pack.
> There just isn't a way to hide this from the client.
>
> Really though, I'd recommend getting your users to upgrade to a
> non-buggy client.  Pasky has the same problem on repo.or.cz; if
> he doesn't have it already he will soon when he upgrades...

-- >8 --
Subject: [PATCH] send-pack: do not send unknown object name from ".have" to pack-objects

This implements a tentative fix.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-send-pack.c |   49 +++++++++++++++++++++++++++----------------------
 1 files changed, 27 insertions(+), 22 deletions(-)

diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index a9fdbf9..fae597b 100644
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -15,6 +15,26 @@ static struct send_pack_args args = {
 	/* .receivepack = */ "git-receive-pack",
 };
 
+static int feed_object(const unsigned char *theirs, int fd, int negative)
+{
+	char buf[42];
+
+	if (negative) {
+		if (!has_sha1_file(theirs))
+			return 1;
+		/*
+		 * NEEDSWORK: we should not be satisfied by simply having
+		 * theirs, but should be making sure it is reachable from
+		 * some of our refs.
+		 */
+	}
+
+	memcpy(buf + negative, sha1_to_hex(theirs), 40);
+	if (negative)
+		buf[0] = '^';
+	buf[40 + negative] = '\n';
+	return write_or_whine(fd, buf, 41 + negative, "send-pack: send refs");
+}
 /*
  * Make a pack stream and spit it out into file descriptor fd
  */
@@ -35,7 +55,6 @@ static int pack_objects(int fd, struct ref *refs, struct extra_have_objects *ext
 	};
 	struct child_process po;
 	int i;
-	char buf[42];
 
 	if (args.use_thin_pack)
 		argv[4] = "--thin";
@@ -51,31 +70,17 @@ static int pack_objects(int fd, struct ref *refs, struct extra_have_objects *ext
 	 * We feed the pack-objects we just spawned with revision
 	 * parameters by writing to the pipe.
 	 */
-	for (i = 0; i < extra->nr; i++) {
-		memcpy(buf + 1, sha1_to_hex(&extra->array[i][0]), 40);
-		buf[0] = '^';
-		buf[41] = '\n';
-		if (!write_or_whine(po.in, buf, 42, "send-pack: send refs"))
+	for (i = 0; i < extra->nr; i++)
+		if (!feed_object(extra->array[i], po.in, 1))
 			break;
-	}
 
 	while (refs) {
 		if (!is_null_sha1(refs->old_sha1) &&
-		    has_sha1_file(refs->old_sha1)) {
-			memcpy(buf + 1, sha1_to_hex(refs->old_sha1), 40);
-			buf[0] = '^';
-			buf[41] = '\n';
-			if (!write_or_whine(po.in, buf, 42,
-						"send-pack: send refs"))
-				break;
-		}
-		if (!is_null_sha1(refs->new_sha1)) {
-			memcpy(buf, sha1_to_hex(refs->new_sha1), 40);
-			buf[40] = '\n';
-			if (!write_or_whine(po.in, buf, 41,
-						"send-pack: send refs"))
-				break;
-		}
+		    !feed_object(refs->old_sha1, po.in, 1))
+			break;
+		if (!is_null_sha1(refs->new_sha1) &&
+		    !feed_object(refs->new_sha1, po.in, 0))
+			break;
 		refs = refs->next;
 	}
 

-- 
1.6.1.1.273.g0e555