Re: [PATCH] Add option -b/--branch to clone for select a new HEAD

From: Jeff King <peff@peff.net>
Date: Aug 26, 2009, 17:48 UTC
Message-ID: <20090826174823.GA1202@coredump.intra.peff.net>
In-Reply-To: <20090826165618.GA7477@atjola.homenet>

On Wed, Aug 26, 2009 at 06:56:18PM +0200, Björn Steinbrink wrote:

Show 15 quoted lines

> On 2009.08.26 12:10:59 -0400, Jeff King wrote:
> > So I think the code should probably look like this (totally untested):
> > 
> >   remote_head = find_ref_by_name(refs, "HEAD");
> >   if (option_branch) {
> >           strbuf_addf(&branch_head, "%s%s", src_ref_prefix, option_branch);
> >           head_points_at = find_ref_by_name(refs, branch_head.buf);
> >           if (!head_points_at)
> >                   warning("remote branch not found, etc");
> >   }
> >   if (!head_points_at)
> >     head_points_at = guess_remote_head(remote_head, mapped_refs, 0);
> 
> Hm, why "refs" for find_ref_by_name(), but "mapped_ref" for
> guess_remote_head()?

Blind copying of the current code? :)

Good question, though. AFAICT, the difference between mapped_refs and refs is that the former contains only the refs we are actually fetching, and its peer_ref member is filled in as appropriate.

Later in the code, we look at head_points_at->peer_ref, which means it _must_ come from mapped_refs. And which means the code I posted is bogus, as the ref we look up in "refs" will not have that member filled in. So I think we do need:

  head_points_at = find_ref_by_name(mapped_refs, branch_head.buf);

Good catch.

-Peff