Re: How to check new commit availability without full fetch?

From: Nicolas Pitre <nico@fluxnic.net>
Date: Jan 11, 2010, 20:52 UTC
Message-ID: <alpine.LFD.2.00.1001111501360.10143@xanadu.home>
In-Reply-To: <7vmy0kjvms.fsf@alter.siamese.dyndns.org>

On Mon, 11 Jan 2010, Junio C Hamano wrote:

Show 24 quoted lines

> Ahh, you are right.  It walks from objects the remote side told us are at
> the tip, and stops at what we know are complete (i.e. reachable from our
> tip of objects); immediately after --dry-run slurped objects, the next
> fetch will prove everything is locally available and complete before going
> over the network.
> 
> But either I am very confused or the use of fields from "struct ref" is
> unintuitive in this codepath.
> 
> Why does it feed ref->old_sha1?  We are feeding _their_ tip commits to:
> 
>     rev-list --objects --stdin --not --all
> 
> and expecting it to report failure when some of their tip commits lead to
> what we don't have yet.  The reason why we have old_sha1[] vs new_sha1[]
> is because we want to report what changed from what, and also to protect
> us from simultaneous updates by doing compare-and-swap using the value we
> read from our refs when we started in old_sha1[], so I would have expected
> that ref_map elements would have _their_ commits on the new_sha1[] side,
> but apparently that is not what is happening, and it has been this way for
> a long time.  The use of old_sha1[] came from 4191c35 (git-fetch: avoid
> local fetching from alternate (again), 2007-11-11), so it is a lot more
> likely that I am confused than the code is wrong and nobody noticed so
> far.

Very confusing indeed. I first discovered about quickfetch() myself when I fixed shallow clone leading to commit 86386829.

If old_sha1[] was our refs then quickfetch() would always succeed and we'd never fetch anything.

> What am I missing?

Digging a bit, it looks like get_remote_heads() is storing the remote's heads into old_sha1. And so is performed in get_refs_from_bundle(), and in insert_packed_refs() from get_refs_via_rsync(), etc.

Looking at the struct ref definition, we can see:

struct ref {
        struct ref *next;
        unsigned char old_sha1[20];
        unsigned char new_sha1[20];
        [...]
        struct ref *peer_ref; /* when renaming */
        [...]
};

And apparently store_updated_refs() ends up using that peer_ref like this:

        for (rm = ref_map; rm; rm = rm->next) {
                struct ref *ref = NULL;

                if (rm->peer_ref) {
                        ref = xcalloc(1, sizeof(*ref) + strlen(rm->peer_ref->name) + 1);
                        strcpy(ref->name, rm->peer_ref->name);
                        hashcpy(ref->old_sha1, rm->peer_ref->old_sha1);
                        hashcpy(ref->new_sha1, rm->old_sha1);
                        ref->force = rm->peer_ref->force;
                }

So.... Doesn't this all look like a total mess of needless (and even leaked in this case) allocations and duplications, besides being completely unintuitive? Both hashcpy() above certainly throw my sense of logic aside...

Nicolas