Re: [PATCH] clone: allow detached checkout when --branch takes a tag

>> @@ -721,6 +722,14 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
>>                       strbuf_release(&head);
>>
>>                       if (!our_head_points_at) {
>> +                             strbuf_addstr(&head, "refs/tags/");
>> +                             strbuf_addstr(&head, option_branch);
>> +                             our_head_points_at =
>> +                                     find_ref_by_name(mapped_refs, head.buf);
>> +                             strbuf_release(&head);
>> +                     }
>> +
>> +                     if (!our_head_points_at) {
>
> Hmm. The context just above your patch that got snipped does this:
>
>    strbuf_addstr(&head, src_ref_prefix);
>    strbuf_addstr(&head, option_branch);
>    our_head_points_at =
>        find_ref_by_name(mapped_refs, head.buf);
>
> where src_ref_prefix typically is "refs/heads/", and clearly you are
> meaning to do the same thing for tags. But the use of "src_ref_prefix"
> is interesting.
>
> It is always "refs/heads/" unless we are cloning into a bare mirror, in
> which case it is "refs/". So with your patch in the non-mirror case,
> doing "--branch=foo" would try "refs/heads/foo" followed by
> "refs/tags/foo". Which makes sense. But in the mirror case, it will try
> "refs/foo" followed by "refs/tags/foo", which is kind of odd.
>
> I wonder, though, if the original code makes any sense. By using
> "refs/", I would have to say "--branch=heads/foo", which is kind of
> weird and undocumented. I think it should probably always be
> "refs/heads/", no matter if we are mirroring or not.