Re: [PATCHv2 04/10] remote: Reject remote names containing '/'

On Mon, May 13, 2013 at 6:54 AM, Junio C Hamano [off-list ref] wrote:

Johan Herland [off-list ref] writes:

quoted

Although we definitely support and encourage use of multi-level branch
names, we have never conciously tried to give support for multi-level
remote names. Currently, they are allowed, but there is no evidence that
they are commonly used.

Now, they do provide a source of problems when trying to expand the
"$nick/$name" shorthand notation (where $nick matches a remote name)
into a full refname. Consider the shorthand "foo/bar/baz": Does this
parse as $nick = foo, $name = bar/baz, or $nick = foo/bar, $name = baz?

Since we need to be unambiguous about these things, we hereby declare
that a remote name shall never contain a '/' character, and that the
only correct way to parse "foo/bar/baz" is $nick = foo, $name = bar/baz.

I know I am guilty of hinting to go in this direction in the earlier
discussion, but I have to wonder how much more refactoring is needed
to see if there is only one unique possibility among many.

For a string with N slashes, you have only N possible ways to split
it into $nick and $name, and if you see a ref "bar/baz" copied from
remote "foo" but no ref "baz" copied from remote "foo/bar" (or you
may not even have a remote "foo/bar" in the first place), the user
is already very unambiguous. The declaration is merely being lazy.

I am not saying we must implement such a back-track to disambiguate
the user input better.  I am wondering how much more effort on top
of this series is needed if we want to get there (provided that the
disambiguation is a good thing to do in the first place).

I feel the problem with multi-level remote names runs a little deeper
than merely disambiguation when resolving remote-tracking refs: If you
have two remotes "foo" and "foo/bar", and they have branches "bar/baz"
and "baz", respectively, then they will (in the default current
configuration) end up clobbering eachother due to the overlapping
remote-tracking branch (refs/remotes/foo/bar/baz). Although the remote
ref namespace coincidentally resolves this by mapping the two to
"refs/peers/foo/heads/bar/baz" and "refs/peers/foo/bar/heads/baz"
respectively, you can easily create a different (although probably
even more unlikely) case where the remote ref namespace causes the
same kind of overlap: One remote "foo" with branch "heads/bar", and
another remote "foo/heads" with branch "bar" will both end up
clobbering eachother at "refs/peers/foo/heads/heads/bar"...

The disambiguation can probably be resolved, although the resulting
code will obviously be somewhat more cumbersome and ugly (and IMHO the
current code is plenty of that already...). Combine this with the
problems of clobbering of the same remote-tracking ref (describe
above), and the fact that AFAIK a multi-level remote name has never
been observed "in the wild" (none of the people I asked at the Git
Merge conference had ever observed multi-level remote names, nor did
they directly oppose banning them), I'm not at all sure it's worth
bothering about this at all. Simply disallowing multi-level remote
names seems like the simpler and naturally ambiguity-resistant
approach.


...Johan

-- 
Johan Herland, [off-list ref]
www.herland.net