On Tue, Aug 06, 2013 at 09:28:28AM -0700, Junio C Hamano wrote:
Show 29 quoted lines
> Jeff King <peff@peff.net> writes:
>
> > @@ -170,6 +172,20 @@ static struct ref *get_ref_map(struct transport *transport,
> > rm->fetch_head_status = FETCH_HEAD_MERGE;
> > if (tags == TAGS_SET)
> > get_fetch_map(remote_refs, tag_refspec, &tail, 0);
> > +
> > + /*
> > + * For any refs that we happen to be fetching via command-line
> > + * arguments, take the opportunity to update their configured
> > + * counterparts. However, we do not want to mention these
> > + * entries in FETCH_HEAD at all, as they would simply be
> > + * duplicates of existing entries.
> > + */
> > + old_tail = tail;
> > + for (i = 0; i < transport->remote->fetch_refspec_nr; i++)
> > + get_fetch_map(ref_map, &transport->remote->fetch[i],
> > + &tail, 0);
> > + for (rm = *old_tail; rm; rm = rm->next)
> > + rm->fetch_head_status = FETCH_HEAD_IGNORE;
>
> Was there a reason why this change was done by appending new ref at
> the tail of the ref_map list? I would have expected that a naïve
> and obvious implementation would be to iterate existing refs over
> ref_map to find refs with an empty RHS whose LHS is configured to
> usually store the fetched result to somewhere and to update their
> RHS in place.
>
> Being curious.
Two reasons:
1. The implementation you suggest above behaves differently than the
current code. It does not look for refspecs with an empty RHS. It
looks for any LHS that matches our configured entries. So if you do
"git fetch origin master:foobar", we will update both
"refs/heads/foobar" as well as "refs/remotes/origin/master". That means it is purely an opportunistic "hey, during another
operation we happened to find out something new about
origin/master, so let's update our tracking branch". Whereas what
you stated above is more like "we are fetching into FETCH_HEAD, so
let's also update the tracking branch". 2. The list of refs after get_ref_map is actually more of an
instruction/command list for the rest of the code to follow. It
gets fed to store_updated_ref, but also impacts the status table we
show. You could implement it such that a single ref entry had
multiple storage destinations, but that would require changes to
all of the consumers of the instruction list. Since we already need
to handle extra refspecs (e.g., you can say "master:foobar
master:refs/remotes/origin/master" on the command line already), we
can treat these the same way. We append to the instruction list,
and the rest of the code treats it as if you specified it on the
command line.-Peff
Jeff King <peff@peff.net>