From: Junio C Hamano Date: Wed, 24 Aug 2011 22:27:58 GMT Subject: Re: Buggy handling of non-canonical ref names Message-ID: <7vaaayo369.fsf@alter.siamese.dyndns.org> In-Reply-To: <7vaaayps9z.fsf@alter.siamese.dyndns.org> Junio C Hamano writes: > Michael Haggerty writes: > >> What is the policy about reference names and their canonicalization? > > The overall policy has been that we care about well-formed input, and > everything else is "undefined", even though as you found out some of them > try to work sensibly. > >> $ git check-ref-format /foo/bar ; echo $? >> 0 >> >> $ git check-ref-format --print /foo/bar >> /foo/bar > > I think these are bogus. Patches welcome. I actually think the former is correct and the latter should strip the leading slash. Essentially what "check-ref-format" (and the underlying check_ref_format() function) validates is if the given user string can be used under $GIT_DIR/refs/ to name a ref, and $GIT_DIR/refs//foo/bar is (because we "tolerate duplicated slashes" cf. comment in the function) the same as $GIT_DIR/refs/foo/bar and is allowed. I do not know what the original motivation behind --print was, but it seems to want to show it canonicalized, so it should strip the leading slash as well. I think what is missing is a unified way to canonicalize the refnames (which led to the inconsistencies you observed), and I strongly suspect that check_ref_format() should learn to return the canonicalized format (if asked by the caller) and the caller should use the canonicalized version after feeding end-user input to it. Then the plumbing "check-ref-format --print" can use it just like any other caller that should be (or already are) using check_ref_format() to validate the end-user input. Yes, such a change will update the overall policy I stated earlier and narrow the scope of "undefined" down a bit, by uniformly codifying that leading and duplicate slashes are removed to be nice to the user.