Re: [PATCH] disallow refs containing successive slashes

From: Jens Lehmann <jens.lehmann@web.de>
Date: Oct 11, 2009, 10:42 UTC
Message-ID: <4AD1B6A4.8060405@web.de>
In-Reply-To: <7vws327wbp.fsf@alter.siamese.dyndns.org>

Junio C Hamano schrieb:

Show 16 quoted lines

> Jens Lehmann <Jens.Lehmann@web.de> writes:
> 
>> When creating branches using names starting with '/' or containing a '//',
>> a leading slash would just vanish and successive slashes were 'compressed'
>> into just one slash.
> 
> Hmm.  We already do that without your patch.
> 
>     $ git branch /foo//bar
>     $ git for-each-ref --format='%(refname)'
>     refs/heads/foo/bar
>     refs/heads/master
>     $ git branch -d /foo//bar
>     Deleted branch /foo//bar (was deadbeef)
>     $ git for-each-ref --format='%(refname)'
>     refs/heads/master

Yes, but git changes the name silently from '/foo//bar' to 'foo/bar'!

The automagical removal of some slashes leads to strange behaviour when using such names:

    $ git checkout -b foo/bar
    Switched to a new branch 'foo/bar'
    $ git checkout -b /foo//bar
    fatal: A branch named '/foo//bar' already exists.
    $ git for-each-ref --format='%(refname)'
    refs/heads/foo/bar
    refs/heads/master

git claims wrongly that "A branch named '/foo//bar' already exists.".

And that same happens to tag and repo names, leading to other strange effects:

    $ git tag /foo//bar
    $ git tag
    foo/bar

That is not the tagname the user provided.

    $ git remote add /foo//bar git://git.kernel.org/pub/scm/git/git.git
    $ git remote show
    warning: Config remote shorthand cannot begin with '/': /foo//bar.url
    warning: Config remote shorthand cannot begin with '/': /foo//bar.fetch
    $ git fetch /foo//bar
    warning: Config remote shorthand cannot begin with '/': /foo//bar.url
    warning: Config remote shorthand cannot begin with '/': /foo//bar.fetch
    fatal: 'foo/bar' does not appear to be a git repository
    fatal: The remote end hung up unexpectedly

Note: git fetch uses both versions of the remote name in its output.

Show 13 quoted lines

>> I became aware of this issue while looking into problems occuring
>> when a user created a branch starting with a '/' in git gui (e.g.
>> "/foo"). Strange things happen, while git gui shows the current
>> branch as "/foo" under the hood a branch "foo" (without the slash)
>> had been created. But then you can't delete "/foo" from git gui,
>> because a branch of that name doesn't exist.
> 
> Perhaps an interface to give a cleaned-up version, e.g.
> 
>     $ git check-ref-format --print refs/heads//foo/bar
>     refs/heads/foo/bar
> 
> is what you want in order to fix git-gui?  I dunno.

Yes, one solution could be to fix every application handling branch, tag or repo names to mimic the namechange done in the bowels of git. But i think it is not worth the hassle. Every application i tested (git gui, gitk and a handful of git commands) just assumes - and i think rightfully so - that refnames will not change while they are being created (And they do call "git check-ref-format" or strbuf_check_branch_ref() and friends to make sure they have a valid refname, but it gets changed nonetheless).

With this patch you get an error every time you try to create such a refname in the first place and the strange effects and the ambiguity of refnames ('/foo//bar' is a synonym for 'foo/bar' right now) just go away.

The motivation for this patch was a confused user at my dayjob. IMHO most people don't use '/foo//bar' on purpose but the extra slashes get there through typos, copy & paste errors and such. Then generating an error might just be the Right Thing to do to avoid the problems with the changed refname afterwards.