Re: [PATCH 7/7] Factor ref updating out of fetch_with_import
- From
Daniel Barkalow <barkalow@iabervon.org>
- Date
- Oct 29, 2009, 17:22 UTC
- Message-ID
- <alpine.LNX.2.00.0910291311570.14365@iabervon.org>
- In-Reply-To
- <fabb9a1e0910290932u45c9c416m4d0ba0a8b2f5b01d@mail.gmail.com>
On Thu, 29 Oct 2009, Sverre Rabbelier wrote:
Show 36 quoted lines
> Heya,
>
> On Thu, Oct 29, 2009 at 08:56, Shawn O. Pearce <spearce@spearce.org> 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*