Re: DWIM ref names for push/fetch

From: Julian Phillips <julian@quantumfyre.co.uk>
Date: Jun 25, 2007, 18:45 UTC
Message-ID: <Pine.LNX.4.64.0706251937470.11026@beast.quantumfyre.co.uk>
In-Reply-To: <Pine.LNX.4.64.0706251216210.4740@iabervon.org>

On Mon, 25 Jun 2007, Daniel Barkalow wrote:

Show 27 quoted lines

> On Sun, 24 Jun 2007, Junio C Hamano wrote:
>
>> Daniel Barkalow <barkalow@iabervon.org> writes:
>>
>>> I was actually thinking exclusively of the matching of strings like "HEAD"
>>> or "origin/next" or "master" to refs from the list of available refs. It
>>> seems to me like the push code does a better job of handling the same
>>> sorts of things that get_sha1() handles.
>>>
>>> In particular, the handling of "refs/my/funny/thing" is really wrong: it
>>> gets treated as refs/heads/refs/my/funny/thing.
>>
>> git-parse-remote.sh::canon_refs_list_for_fetch() seems to say
>> otherwise, though.
>>
>>  - When unspecified, or explicitly spelled HEAD, take HEAD;
>>  - Anything that begins with refs/, use it as is;
>>  - Anything that begins with heads/, tags/, remotes/, assume
>>    it is a branch, a tag, or a tracking branch;
>>  - Otherwise assume a branch;
>>
>> So I suspect refs/my/funny/thing is covered by the second rule.
>
> Ah, okay. I think a few bits got lost somewhat in Julian's translation to
> C. I agree with the first three rules there, and with the last rule being
> the last rule, and sticking more things in between those sets is easy
> enough.

Just for the record as it were, the difference between my C code and git-parse-remote.sh is simply that commit 96f12b5 changed the shell script behaviour after I had already started, and translating the code to C was hard enough without also trying to track a moving target.

Effectively Daniel is working slightly in the past, and has spotted the same issue that Alex has already fixed.

Mea Culpa.  Sorry.

Show 19 quoted lines

>
>> But I do agree "otherwise assume a branch" part has huge room
>> for improvement.  Especially...
>>
>>> I think that "origin/next"
>>> should also be assumed to be refs/remotes/origin/next instead of
>>> refs/heads/origin/next, at least if you have refs/remotes/origin/ and not
>>> refs/heads/origin/.
>>
>> ... I think that makes perfect sense -- the code should
>> interpret your example as a request to start using a new
>> tracking branch refs/remotes/origin/next.
>
> Currently, it doesn't even notice if you've got the tracking branch
> already. Should it have some rule to prefer things that exist over things
> that don't?
>
> When refs/remotes/origin/next doesn't exist, should it require that
> refs/remotes/origin/ already exist?

It should at least require that a remote called origin exists perhaps?

-- 
Julian

  ---
<rcw> those apparently-bacteria-like multicolor worms coming out of
       microsoft's backorifice
<rcw> that's the backoffice logo