threads / discuss / 8706

DWIM ref names for push/fetch

Subject: DWIM ref names for push/fetch

## tl;dr

6 messages between Jun 24, 2007 and Jun 25, 2007.

replies: 5people: 3as markdown or json

Daniel Barkalow· Jun 24, 2007, 22:25 UTC · lore
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*
Junio C Hamano· Jun 25, 2007, 00:09 UTC · re: Daniel Barkalow · lore

Re: DWIM ref names for push/fetch

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.

Daniel Barkalow· Jun 25, 2007, 03:12 UTC · re: Junio C Hamano · lore

Re: DWIM ref names for push/fetch

On Sun, 24 Jun 2007, Junio C Hamano wrote:
Show 27 quoted lines
> 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*
Junio C Hamano· Jun 25, 2007, 06:10 UTC · re: Daniel Barkalow · lore

Re: DWIM ref names for push/fetch

Daniel Barkalow <barkalow@iabervon.org> writes:
Show 7 quoted lines
> 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.

Daniel Barkalow· Jun 25, 2007, 16:27 UTC · re: Junio C Hamano · lore

Re: DWIM ref names for push/fetch

On Sun, 24 Jun 2007, Junio C Hamano wrote:
Show 20 quoted lines
> 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.

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

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*
Julian Phillips· Jun 25, 2007, 18:45 UTC · re: Daniel Barkalow · lore

Re: DWIM ref names for push/fetch

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

← back to recent threads