DWIM ref names for push/fetch

DWIM ref names for push/fetch

[Daniel Barkalow]

AFAICT, the rules are currently:

for push, we compare the source against the list of local refs, and do 
some complicated stuff to figure out what it matches. Then we compare the 
destination against the list of remote refs (with no destination being 
interpreted as the full name of the source), and do the same complicated 
stuff.

for fetch, we accept anything that starts with "refs/heads/", 
"refs/tags/", or "refs/remotes/"; prepend "refs/" to anything that starts 
with "heads/", "tags/", or "remotes/"; and prepend "refs/heads/" to 
anything else. (NB: I'm looking at Julian's C translation, rather than the 
original shell scripts, which I find inpenetrable).

Is this difference simply due to the different languages the matching 
portions of these were originally written in? Would it be okay to change 
fetch to work like push? It would be nice to have a single procedure for 
determining which (if any) of a list of refs is the match for some string, 
regardless of why we're looking.

	-Daniel
*This .sig left intentionally blank*

Re: DWIM ref names for push/fetch

[Junio C Hamano]

Daniel Barkalow <barkalow@iabervon.org> writes:

> Is this difference simply due to the different languages the matching 
> portions of these were originally written in?

If anything, the semantics on the fetch side is _very_ much
intentional and is done deliberately that way to be usable.  

On the other hand, push started as "matching only", and then
"match tail part of the name" as an afterthought.  It was so
afterthought that it had an idiotic behaviour of independently
match the source and destination side even when there is no
colon, which was fixed only recently.

So if you would want to match fetch and push, you should not
change the semantics on fetch to match what push does, as the
latter was done pretty much without design.

Having said that, I think fetch and push DWIMmery are
fundamentally different, especially when you do not have a
colon.  push without storing anything on the receiving end would
not make any sense whatsoever, but fetch without using tracking
branches does make perfect sense, so push does pretend dst side
has what matched with src side pattern, while fetch treats no
colon pattern as not storing.  IOW, even if we wanted to reuse
the code on both sides as much as possible, I suspect we would
need to have details different between them.

Re: DWIM ref names for push/fetch

[Daniel Barkalow]

On Sun, 24 Jun 2007, Junio C Hamano wrote:

> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> > Is this difference simply due to the different languages the matching 
> > portions of these were originally written in?
> 
> If anything, the semantics on the fetch side is _very_ much
> intentional and is done deliberately that way to be usable.  
> 
> On the other hand, push started as "matching only", and then
> "match tail part of the name" as an afterthought.  It was so
> afterthought that it had an idiotic behaviour of independently
> match the source and destination side even when there is no
> colon, which was fixed only recently.
> 
> So if you would want to match fetch and push, you should not
> change the semantics on fetch to match what push does, as the
> latter was done pretty much without design.
> 
> Having said that, I think fetch and push DWIMmery are
> fundamentally different, especially when you do not have a
> colon.  push without storing anything on the receiving end would
> not make any sense whatsoever, but fetch without using tracking
> branches does make perfect sense, so push does pretend dst side
> has what matched with src side pattern, while fetch treats no
> colon pattern as not storing.  IOW, even if we wanted to reuse
> the code on both sides as much as possible, I suspect we would
> need to have details different between them.

The no-colon handling is right as it is, as well as the semantics of 
fetching without tracking refs.

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. 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/.

	-Daniel
*This .sig left intentionally blank*

Re: DWIM ref names for push/fetch

[Junio C Hamano]

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.

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.

Re: DWIM ref names for push/fetch

[Daniel Barkalow]

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.

> 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?

In any case, the big question is whether the push code should use these 
rules, too, for the corresponding portions, in which case I can share the 
code (and, for that matter, the documentation, which would be even nicer, 
because we've currently got a lot of hints about refspecs in different 
places but nothing complete anywhere).

	-Daniel
*This .sig left intentionally blank*

Re: DWIM ref names for push/fetch

[Julian Phillips]

On Mon, 25 Jun 2007, Daniel Barkalow wrote:

> 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.

>
>> 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