{"thread":{"id":"16778","subject":"Suggestion: better error reporting when setting HEAD","startedAt":"2008-12-18T05:58:25Z","lastAt":"2008-12-18T08:25:51Z","messageCount":2,"participants":["Francois Marier","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"98228","messageId":"20081218055825.GQ6708@isafjordur.dyndns.org","threadId":"16778","inReplyTo":null,"subject":"Suggestion: better error reporting when setting HEAD","fromName":"Francois Marier","fromEmail":"francois@debian.org","sentAt":"2008-12-18T05:58:25Z","receivedAt":"2008-12-18T05:58:25Z","isPatch":false,"sender":{"key":"francois@debian.org","avatar":null},"body":"(Please CC me on your replies, thanks)\n\nIf you take any valid git repo and then do the following:\n\n  git symbolic-ref HEAD master\n  (instead of \"git symbolic-ref HEAD refs/heads/master\")\n\nthen other git commands (e.g. \"git log\") will return this error message:\n\n  fatal: Not a git repository\n\nI'd like to suggest two \"usability\" improvements:\n\n1- changing the error message to explain why the directory is no longer\n   considered a git repo. Maybe something along the lines of:\n\n     fatal: Invalid reference in .git/HEAD\n\n2- To prevent this error from happening in the first place, git-symbolic-ref\n   could refuse to change HEAD to a non-existent ref.\n\nCheers,\nFrancois\n"},{"id":"98237","messageId":"20081218082551.GA29356@coredump.intra.peff.net","threadId":"16778","inReplyTo":"20081218055825.GQ6708@isafjordur.dyndns.org","subject":"Re: Suggestion: better error reporting when setting HEAD","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-12-18T08:25:51Z","receivedAt":"2008-12-18T08:25:51Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"[Junio cc'd, as I think he may have comments on the patch at the end]\n\nOn Thu, Dec 18, 2008 at 06:58:25PM +1300, Francois Marier wrote:\n\n> If you take any valid git repo and then do the following:\n> \n>   git symbolic-ref HEAD master\n>   (instead of \"git symbolic-ref HEAD refs/heads/master\")\n> \n> then other git commands (e.g. \"git log\") will return this error message:\n> \n>   fatal: Not a git repository\n\nI've done this exact same thing, and it is very annoying.\n\n> I'd like to suggest two \"usability\" improvements:\n> \n> 1- changing the error message to explain why the directory is no longer\n>    considered a git repo. Maybe something along the lines of:\n> \n>      fatal: Invalid reference in .git/HEAD\n\nThis can be done, but is more complex than you might hope; the test for\n\"is this is a git directory\" may get run multiple times while trying to\nfind out if we are in a repo. So giving a reason why we aren't in a git\nrepo is going to end up something like:\n\n  I tried X, but its HEAD ref had cruft in it;\n  then I tried Y, but it had no HEAD file;\n  then I gave up\n\nObviously with a working tree, the first directory with the '.git' repo\nis a likely candidate for being the right thing, but with an error. But\nif you are looking for a bare repository, then it could be any parent\ndirectory. Maybe it is worth just spewing \"there is a HEAD file here,\nbut it doesn't look right\" when it fails to validate. It would give a\nfalse positive if you had a file named HEAD (and you weren't actually in\na git repo), but that is unlikely to happen. And if you get multiple\nones, so be it.\n\n> 2- To prevent this error from happening in the first place, git-symbolic-ref\n>    could refuse to change HEAD to a non-existent ref.\n\nActually, we sometimes need to point to non-existent refs; these are\n\"branches yet to be born\". The reason that the new HEAD doesn't validate\nis that it is supposed to start with \"refs/\".\n\nWe could fix it like this:\n\ndiff --git a/builtin-symbolic-ref.c b/builtin-symbolic-ref.c\nindex bfc78bb..08fa3ce 100644\n--- a/builtin-symbolic-ref.c\n+++ b/builtin-symbolic-ref.c\n@@ -44,6 +44,8 @@ int cmd_symbolic_ref(int argc, const char **argv, const char *prefix)\n \t\tcheck_symref(argv[0], quiet);\n \t\tbreak;\n \tcase 2:\n+\t\tif (prefixcmp(argv[1], \"refs/\"))\n+\t\t\tdie(\"Refusing to point %s outside of refs/\", argv[0]);\n \t\tcreate_symref(argv[0], argv[1], msg);\n \t\tbreak;\n \tdefault:\n\nwhich tries to match the behavior used by validate_headref, but there\nare a few open questions:\n\n - Should it require this of _all_ symbolic refs, or just HEAD?\n\n - Should there be a \"-f\" force option?\n\n - Should it be moved to the create_symref library function. It gets\n   called in several places, and I didn't audit all of them.\n\n - I looked at trying to share validate_headref's logic (so they don't\n   get out of sync), but really the only thing that applies is the\n   \"refs\" check. The rest is about checking parts of the symref that are\n   outside of the actual content.\n\n - This doesn't allow a sha1 for detached HEAD. However, that currently\n   doesn't work _anyway_, as it creates a HEAD with \"ref: $SHA1\", which\n   is totally bogus.\n\n-Peff\n"}]}