On Tue, Oct 11, 2011 at 01:14:26PM -0700, Junio C Hamano wrote: > Junio C Hamano <gitster@xxxxxxxxx> writes: > > >> I think we've discussed tightening it a few years ago already. > >> > >> HEAD, MERGE_HEAD, FETCH_HEAD, etc. all are "^[_A-Z]*$" and it may even be > >> a good idea to insist "^[_A-Z]*HEAD$" or even "^([A-Z][A-Z]*_)?HEAD$". > > > > Perhaps like this? Only compile tested... > > Not quite. There are at least three bugs in the patch. > > - Some subsystems use random refnames like NOTES_MERGE_PARTIAL that would > not match "^([A-Z][A-Z]*_)?HEAD$". The rule needs to be relaxed; Yeah, I found one of these also. I thought at first we could rename things like NOTES_MERGE_PARTIAL, as it's more about internal communication within a specific version of git than it is about letting an external program peek at our state. But there really are several of them. And I think it makes sense to keep this safety valve conservative, and try to document existing practice rather than dictating it. I'm worried that some porcelain or other tool is using their own all-caps name, and that tightening it too much would be breaking that. Your relaxed rule does match things like COMMIT_EDITMSG and NOTES_MSG, which are obviously bogus. At the same time, I'm not sure it's a big deal. The point of this is to restrict the lookup to a class of names which are likely magical to git, and users should probably avoid the magical namespace (i.e., it's still not a good idea to call your branch "HEAD"). Something like "config" is an easy branch name to unknowingly use. Something like "COMMIT_EDITMSG" is not. Your rule does disallow RENAMED-REF, which is used in branch renaming. However, I'm having trouble figuring out what it's really for. It's not mentioned in the documentation at all, and c976d41, its origin, says only: This also indroduces $GIT_DIR/RENAME_REF which is a "metabranch" used when renaming branches. It will always hold the original sha1 for the latest renamed branch. Additionally, if $GIT_DIR/logs/RENAME_REF exists, all branch rename events are logged there. But in the code, it is spelled RENAMED-REF (with a dash). And as far as I can tell, does not actually create a reflog. And it's not documented anywhere, so I suspect nobody is using it. Maybe it is worth switching that name. > - dwim_ref() can be fed "refs/heads/master" and is expected to dwim it to > the master branch. It looks like your code will allow any subdirectory. I had thought to limit it to "refs/". Otherwise, my "config" example could be "objects/pack", or "lost-found/commits", "remotes/foo", or something. Obviously the longer the name, the smaller the possibility of an accidental collision. But I couldn't think of any other subdirectory into which refs should go. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html