On Thu, 29 Oct 2009, Sverre Rabbelier wrote:
> Heya,
>
> On Thu, Oct 29, 2009 at 08:56, Shawn O. Pearce <spearce@xxxxxxxxxxx> wrote:
> >> > I certainly have to wonder... if this is done in both fetch and
> >> > clone, why isn't it just part of fetch_refs?
> >>
> >> Because clone does not use fetch_refs, or am I missing something?
> >
> > Hmmph. Weird. Its been a while since I last looked at this code,
> > maybe I misunderstood it.
>
> Unless you mean transport_fetch_refs? It can't be done in
> transport_fetch_refs because the foreign helper transport needs to
> update all refs, including those that wanted_peer_refs decided not to
> fetch. Otherwise the 'HEAD' ref will not be updated, and it is left at
> all zeros. It is not as obvious that that is a problem in this patch,
> but when Junio merges with Nico's [5bdc32d3e "make 'git clone' ask the
> remote only for objects it cares about"] the code looks like this:
>
> if (refs) {
> struct ref *ref_cpy;
> mapped_refs = wanted_peer_refs(refs, refspec);
> ref_cpy = copy_ref_list(mapped_refs);
> transport_fetch_refs(transport, ref_cpy);
> if (transport->update_refs)
> {
> ref_cpy = copy_ref_list(refs);
> transport->update_refs(transport, ref_cpy);
> refs = ref_cpy;
> mapped_refs = wanted_peer_refs(refs, refspec);
> }
> }
>
> Does it make more sense now? transport_fetch_refs gets only a limited
> view of the refs, so it cannot pass all the refs to
> transport_update_refs as needed.
I think there are a bunch of bugs that you're working around:
(a) if HEAD is determined to be a symref, and we care about HEAD, we care
about whatever HEAD is a symref to; wanted_peer_refs() shouldn't be
filtering such things out.
(b) we don't want to make a whole bunch of copies of the ref list to avoid
updating the mutable ref list that we will look for updated values in; the
merge of my series with Nico's should replace my copy_ref_list with his
wanted_peer_refs, not include the copy afterwards. Correcting this should
lead to the value that matters in the list that will be used having been
updated directly by transport_fetch_refs().
-Daniel
*This .sig left intentionally blank*