Re: Bad objects error since upgrading GitHub servers to 1.6.1

[Junio C Hamano <gitster@pobox.com>]

Junio C Hamano <gitster@pobox.com> writes:

> The extra "we also have these" advertisement happened as a result of this
> discussion:
>
>     http://thread.gmane.org/gmane.comp.version-control.git/95072/focus=95256
>
> I think I know what is going on.
>
> Consider this sequence of events.
>
>  (0) Alice creates a project and pushes to public.
>
>     alice$ cd $HOME/existing-tarball-extract
>     alice$ git init
>     alice$ git add .
>     alice$ git push /pub/alice.git master
>     
>
>  (1) Bob forks it.
>
>     bob$ git clone --bare --reference /pub/alice.git /pub/bob.git

I need another /pub/alice.git here, I think, but I hope I got the point
across to people who are capable of helping us to resolve this issue.

>
>  (2) Bob clones his.
>
>     bob$ cd $HOME && git clone /pub/bob.git bob
>
>  (3) Alice works more and pushes
>
>     alice$ edit foo
>     alice$ git add foo
>     alice$ git commit -a -m 'more'
>     alice$ git push /pub/alice.git master
>
>  (4) Bob works more and tries to push to his.
>
>     bob$ cd $HOME/bob
>     bob$ edit bar
>     bob$ git add bar
>     bob$ git commit -a -m 'yet more'
>     bob$ git push /pub/bob.git master
>
> Now, the new server advertises the objects reachable from alice's branch
> tips as usable cut-off points for pack-objects bob will run when sending.
>
> And new builtin-send-pack.c has new code that feeds "extra" refs as
>
> 	^SHA1\n
>
> to the pack-objects process.
>
> The latest commit Alice created and pushed into her repository is one such
> commit.
>
> But the problem is that Bob does *NOT* have it.  His "push" will run pack
> object telling it that objects reachable from Alice's top commit do not
> have to be sent, which was the whole point of doing this new "we also have
> these" advertisement, but instead of ignoring that unknown commit,
> pack-objects would say "Huh?  I do not even know that commit" and dies.
>
> This can and should be solved by client updates, as 1.6.1 server can work
> with older client just fine.

Here is a *wrong* fix that should work most of the time.  It will
certainly appear to fix the issue in the above reproduction recipe.
You may want to ask your users to try this to see if it makes their
symptom disappear.

When we receive ".have" advertisement, this wrong fix checks if that
object is available locally, and it ignores it otherwise.

This won't be acceptable as the official fix.  We should be doing the
full connectivity check; in other words, not just "do we have it", but "do
we have it *and* is it reachable from any of our own refs".

 connect.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git c/connect.c w/connect.c
index 2f23ab3..8026850 100644
--- c/connect.c
+++ w/connect.c
@@ -43,6 +43,9 @@ int check_ref_type(const struct ref *ref, int flags)
 
 static void add_extra_have(struct extra_have_objects *extra, unsigned char *sha1)
 {
+	if (!has_sha1_file(sha1))
+		return;
+
 	ALLOC_GROW(extra->array, extra->nr + 1, extra->alloc);
 	hashcpy(&(extra->array[extra->nr][0]), sha1);
 	extra->nr++;