{"thread":{"id":"8706","subject":"DWIM ref names for push/fetch","startedAt":"2007-06-24T22:25:12Z","lastAt":"2007-06-25T18:45:54Z","messageCount":6,"participants":["Daniel Barkalow","Junio C Hamano","Julian Phillips"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"45708","messageId":"Pine.LNX.4.64.0706241808550.4740@iabervon.org","threadId":"8706","inReplyTo":null,"subject":"DWIM ref names for push/fetch","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2007-06-24T22:25:12Z","receivedAt":"2007-06-24T22:25:12Z","isPatch":false,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"AFAICT, the rules are currently:\n\nfor push, we compare the source against the list of local refs, and do \nsome complicated stuff to figure out what it matches. Then we compare the \ndestination against the list of remote refs (with no destination being \ninterpreted as the full name of the source), and do the same complicated \nstuff.\n\nfor fetch, we accept anything that starts with \"refs/heads/\", \n\"refs/tags/\", or \"refs/remotes/\"; prepend \"refs/\" to anything that starts \nwith \"heads/\", \"tags/\", or \"remotes/\"; and prepend \"refs/heads/\" to \nanything else. (NB: I'm looking at Julian's C translation, rather than the \noriginal shell scripts, which I find inpenetrable).\n\nIs this difference simply due to the different languages the matching \nportions of these were originally written in? Would it be okay to change \nfetch to work like push? It would be nice to have a single procedure for \ndetermining which (if any) of a list of refs is the match for some string, \nregardless of why we're looking.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"45722","messageId":"7v3b0gq4id.fsf@assigned-by-dhcp.cox.net","threadId":"8706","inReplyTo":"Pine.LNX.4.64.0706241808550.4740@iabervon.org","subject":"Re: DWIM ref names for push/fetch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-06-25T00:09:14Z","receivedAt":"2007-06-25T00:09:14Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Barkalow writes:\n\n> Is this difference simply due to the different languages the matching \n> portions of these were originally written in?\n\nIf anything, the semantics on the fetch side is _very_ much\nintentional and is done deliberately that way to be usable. \n\nOn the other hand, push started as \"matching only\", and then\n\"match tail part of the name\" as an afterthought. It was so\nafterthought that it had an idiotic behaviour of independently\nmatch the source and destination side even when there is no\ncolon, which was fixed only recently.\n\nSo if you would want to match fetch and push, you should not\nchange the semantics on fetch to match what push does, as the\nlatter was done pretty much without design.\n\nHaving said that, I think fetch and push DWIMmery are\nfundamentally different, especially when you do not have a\ncolon. push without storing anything on the receiving end would\nnot make any sense whatsoever, but fetch without using tracking\nbranches does make perfect sense, so push does pretend dst side\nhas what matched with src side pattern, while fetch treats no\ncolon pattern as not storing. IOW, even if we wanted to reuse\nthe code on both sides as much as possible, I suspect we would\nneed to have details different between them.\n"},{"id":"45727","messageId":"Pine.LNX.4.64.0706242234250.4740@iabervon.org","threadId":"8706","inReplyTo":"7v3b0gq4id.fsf@assigned-by-dhcp.cox.net","subject":"Re: DWIM ref names for push/fetch","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2007-06-25T03:12:38Z","receivedAt":"2007-06-25T03:12:38Z","isPatch":false,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Sun, 24 Jun 2007, Junio C Hamano wrote:\n\n> Daniel Barkalow writes:\n> \n> > Is this difference simply due to the different languages the matching \n> > portions of these were originally written in?\n> \n> If anything, the semantics on the fetch side is _very_ much\n> intentional and is done deliberately that way to be usable. \n> \n> On the other hand, push started as \"matching only\", and then\n> \"match tail part of the name\" as an afterthought. It was so\n> afterthought that it had an idiotic behaviour of independently\n> match the source and destination side even when there is no\n> colon, which was fixed only recently.\n> \n> So if you would want to match fetch and push, you should not\n> change the semantics on fetch to match what push does, as the\n> latter was done pretty much without design.\n> \n> Having said that, I think fetch and push DWIMmery are\n> fundamentally different, especially when you do not have a\n> colon. push without storing anything on the receiving end would\n> not make any sense whatsoever, but fetch without using tracking\n> branches does make perfect sense, so push does pretend dst side\n> has what matched with src side pattern, while fetch treats no\n> colon pattern as not storing. IOW, even if we wanted to reuse\n> the code on both sides as much as possible, I suspect we would\n> need to have details different between them.\n\nThe no-colon handling is right as it is, as well as the semantics of \nfetching without tracking refs.\n\nI was actually thinking exclusively of the matching of strings like \"HEAD\" \nor \"origin/next\" or \"master\" to refs from the list of available refs. It \nseems to me like the push code does a better job of handling the same \nsorts of things that get_sha1() handles.\n\nIn particular, the handling of \"refs/my/funny/thing\" is really wrong: it \ngets treated as refs/heads/refs/my/funny/thing. I think that \"origin/next\" \nshould also be assumed to be refs/remotes/origin/next instead of \nrefs/heads/origin/next, at least if you have refs/remotes/origin/ and not \nrefs/heads/origin/.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"45733","messageId":"7vodj4o973.fsf@assigned-by-dhcp.cox.net","threadId":"8706","inReplyTo":"Pine.LNX.4.64.0706242234250.4740@iabervon.org","subject":"Re: DWIM ref names for push/fetch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-06-25T06:10:56Z","receivedAt":"2007-06-25T06:10:56Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Barkalow writes:\n\n> I was actually thinking exclusively of the matching of strings like \"HEAD\" \n> or \"origin/next\" or \"master\" to refs from the list of available refs. It \n> seems to me like the push code does a better job of handling the same \n> sorts of things that get_sha1() handles.\n>\n> In particular, the handling of \"refs/my/funny/thing\" is really wrong: it \n> gets treated as refs/heads/refs/my/funny/thing.\n\ngit-parse-remote.sh::canon_refs_list_for_fetch() seems to say\notherwise, though.\n\n - When unspecified, or explicitly spelled HEAD, take HEAD;\n - Anything that begins with refs/, use it as is;\n - Anything that begins with heads/, tags/, remotes/, assume\n it is a branch, a tag, or a tracking branch;\n - Otherwise assume a branch;\n\nSo I suspect refs/my/funny/thing is covered by the second rule.\n\nBut I do agree \"otherwise assume a branch\" part has huge room\nfor improvement. Especially...\n\n> I think that \"origin/next\" \n> should also be assumed to be refs/remotes/origin/next instead of \n> refs/heads/origin/next, at least if you have refs/remotes/origin/ and not \n> refs/heads/origin/.\n\n... I think that makes perfect sense -- the code should\ninterpret your example as a request to start using a new\ntracking branch refs/remotes/origin/next.\n"},{"id":"45779","messageId":"Pine.LNX.4.64.0706251216210.4740@iabervon.org","threadId":"8706","inReplyTo":"7vodj4o973.fsf@assigned-by-dhcp.cox.net","subject":"Re: DWIM ref names for push/fetch","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2007-06-25T16:27:01Z","receivedAt":"2007-06-25T16:27:01Z","isPatch":false,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Sun, 24 Jun 2007, Junio C Hamano wrote:\n\n> Daniel Barkalow writes:\n> \n> > I was actually thinking exclusively of the matching of strings like \"HEAD\" \n> > or \"origin/next\" or \"master\" to refs from the list of available refs. It \n> > seems to me like the push code does a better job of handling the same \n> > sorts of things that get_sha1() handles.\n> >\n> > In particular, the handling of \"refs/my/funny/thing\" is really wrong: it \n> > gets treated as refs/heads/refs/my/funny/thing.\n> \n> git-parse-remote.sh::canon_refs_list_for_fetch() seems to say\n> otherwise, though.\n> \n> - When unspecified, or explicitly spelled HEAD, take HEAD;\n> - Anything that begins with refs/, use it as is;\n> - Anything that begins with heads/, tags/, remotes/, assume\n> it is a branch, a tag, or a tracking branch;\n> - Otherwise assume a branch;\n> \n> So I suspect refs/my/funny/thing is covered by the second rule.\n\nAh, okay. I think a few bits got lost somewhat in Julian's translation to \nC. I agree with the first three rules there, and with the last rule being \nthe last rule, and sticking more things in between those sets is easy \nenough.\n\n> But I do agree \"otherwise assume a branch\" part has huge room\n> for improvement. Especially...\n> \n> > I think that \"origin/next\" \n> > should also be assumed to be refs/remotes/origin/next instead of \n> > refs/heads/origin/next, at least if you have refs/remotes/origin/ and not \n> > refs/heads/origin/.\n> \n> ... I think that makes perfect sense -- the code should\n> interpret your example as a request to start using a new\n> tracking branch refs/remotes/origin/next.\n\nCurrently, it doesn't even notice if you've got the tracking branch \nalready. Should it have some rule to prefer things that exist over things \nthat don't?\n\nWhen refs/remotes/origin/next doesn't exist, should it require that \nrefs/remotes/origin/ already exist?\n\nIn any case, the big question is whether the push code should use these \nrules, too, for the corresponding portions, in which case I can share the \ncode (and, for that matter, the documentation, which would be even nicer, \nbecause we've currently got a lot of hints about refspecs in different \nplaces but nothing complete anywhere).\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"45784","messageId":"Pine.LNX.4.64.0706251937470.11026@beast.quantumfyre.co.uk","threadId":"8706","inReplyTo":"Pine.LNX.4.64.0706251216210.4740@iabervon.org","subject":"Re: DWIM ref names for push/fetch","fromName":"Julian Phillips","fromEmail":"julian@quantumfyre.co.uk","sentAt":"2007-06-25T18:45:54Z","receivedAt":"2007-06-25T18:45:54Z","isPatch":false,"sender":{"key":"julian@quantumfyre.co.uk","avatar":"https://avatars.githubusercontent.com/u/948888?v=4"},"body":"On Mon, 25 Jun 2007, Daniel Barkalow wrote:\n\n> On Sun, 24 Jun 2007, Junio C Hamano wrote:\n>\n>> Daniel Barkalow writes:\n>>\n>>> I was actually thinking exclusively of the matching of strings like \"HEAD\"\n>>> or \"origin/next\" or \"master\" to refs from the list of available refs. It\n>>> seems to me like the push code does a better job of handling the same\n>>> sorts of things that get_sha1() handles.\n>>>\n>>> In particular, the handling of \"refs/my/funny/thing\" is really wrong: it\n>>> gets treated as refs/heads/refs/my/funny/thing.\n>>\n>> git-parse-remote.sh::canon_refs_list_for_fetch() seems to say\n>> otherwise, though.\n>>\n>> - When unspecified, or explicitly spelled HEAD, take HEAD;\n>> - Anything that begins with refs/, use it as is;\n>> - Anything that begins with heads/, tags/, remotes/, assume\n>> it is a branch, a tag, or a tracking branch;\n>> - Otherwise assume a branch;\n>>\n>> So I suspect refs/my/funny/thing is covered by the second rule.\n>\n> Ah, okay. I think a few bits got lost somewhat in Julian's translation to\n> C. I agree with the first three rules there, and with the last rule being\n> the last rule, and sticking more things in between those sets is easy\n> enough.\n\nJust for the record as it were, the difference between my C code and \ngit-parse-remote.sh is simply that commit 96f12b5 changed the shell script \nbehaviour after I had already started, and translating the code to C was \nhard enough without also trying to track a moving target.\n\nEffectively Daniel is working slightly in the past, and has spotted the \nsame issue that Alex has already fixed.\n\nMea Culpa. Sorry.\n\n>\n>> But I do agree \"otherwise assume a branch\" part has huge room\n>> for improvement. Especially...\n>>\n>>> I think that \"origin/next\"\n>>> should also be assumed to be refs/remotes/origin/next instead of\n>>> refs/heads/origin/next, at least if you have refs/remotes/origin/ and not\n>>> refs/heads/origin/.\n>>\n>> ... I think that makes perfect sense -- the code should\n>> interpret your example as a request to start using a new\n>> tracking branch refs/remotes/origin/next.\n>\n> Currently, it doesn't even notice if you've got the tracking branch\n> already. Should it have some rule to prefer things that exist over things\n> that don't?\n>\n> When refs/remotes/origin/next doesn't exist, should it require that\n> refs/remotes/origin/ already exist?\n\nIt should at least require that a remote called origin exists perhaps?\n\n-- \nJulian\n\n ---\n those apparently-bacteria-like multicolor worms coming out of\n microsoft's backorifice\n that's the backoffice logo\n"}]}