{"thread":{"id":"47057","subject":"[BUG] Incosistent repository state when trying to rename HEAD in the middle of a rebase","startedAt":"2017-10-28T16:59:04Z","lastAt":"2017-11-03T09:23:49Z","messageCount":4,"participants":["Kaartic Sivaraam","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"331229","messageId":"1509209933.2256.4.camel@gmail.com","threadId":"47057","inReplyTo":null,"subject":"[BUG] Incosistent repository state when trying to rename HEAD in the middle of a rebase","fromName":"Kaartic Sivaraam","fromEmail":"kaarticsivaraam91196@gmail.com","sentAt":"2017-10-28T16:58:53Z","receivedAt":"2017-10-28T16:59:04Z","isPatch":false,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"I just noticed this recently while trying to see if a recent change [1]\nthat disallowed the possibility of creating HEAD also allowed renaming\nbranches named \"HEAD\" that were created using previous versions that\nallowed it. Unfortunately (or fortunately (?)), I was in the middle of\nan interactive rebase while trying this out and as a consequence\nobserved weird behaviour as shown in the following output,\n\n    $ git branch -m HEAD head-1\n    warning: Renamed a misnamed branch '|�?' away\n\nThe most interesting thing with the above output was that I really\ndidn't have any branch named \"HEAD\" while trying this. Further, the\nmost crucial thing about the above issue is it left the repository in\nan inconsistent state by removing \".git/HEAD\" and thus leaving the\nrepository in an inconsistent state. This results in git not\nrecognizing it as a git repository itself! I had to do a \"git init\" to\nfix this (I guess I lose at least something as a consequence as it\nchecks out the 'master' by default; though I'm not sure).\n\ngit 2.11.0 shows a \"branch doesn't exist\" error (not exactly) in the\nabove case,\n\n    $ hgit b -m HEAD head-1\n    error: refname refs/heads/HEAD not found\n    fatal: Branch rename failed\n\nMy reproduction recipe for the above issue is as follows,\n\n    git init test &&\n    cd test &&\n    echo \"Test file\" >test &&\n    git add test &&\n    git commit -m \"Initial commit\" &&echo \"Test\" >test &&git add test &&\n    git c -m \"Second commit\" &&\n    git rebase-i HEAD~\n\nAt this point use the 'edit' command for \"Second commit\" and close the\neditor and try the following,\n\n    $ git branch -m HEAD head-1\n\nThis should show the issue. Any git operation after this should report\nthe following error,\n\n    fatal: Not a git repository (or any parent up to mount point /)\n    Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set)\n\nRe-initializing the repository should bring the repository back to the\nposition it was before the issue (to some extent though the staging\narea might become cluttered as the 'master' branch is checked out by\ndefault.)\n\nI'm still surprised as to why \".git/HEAD\" was treated as a branch. I\nthought of digging into this but didn't get the time now so thought of\ninforming to the people in the mailing list. One last thing, I suspect\nthis to be a consequence of that change I specified in the beginning,\nthough I might be just guessing around.\n\n[1]: https://public-inbox.org/git/20171013051132.3973-1-gitster@pobox.com/\n\n-- \nKaartic\n"},{"id":"331645","messageId":"1509600814.9254.6.camel@gmail.com","threadId":"47057","inReplyTo":"1509209933.2256.4.camel@gmail.com","subject":"Re: [BUG] Incosistent repository state when trying to rename HEAD in the middle of a rebase","fromName":"Kaartic Sivaraam","fromEmail":"kaarticsivaraam91196@gmail.com","sentAt":"2017-11-02T05:33:34Z","receivedAt":"2017-11-02T05:36:27Z","isPatch":false,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"I was able to spare some time to dig into this and found a few things.\n\nFirst, it seems that the issue is more generic and the BUG kicks in\nwhenever HEAD is not a symbolic ref. I noticed that because when HEAD\nis a symbolic ref there was a change in the error message shown by git.\nIn (2.11.0) I get this error message,\n\n    error: refname refs/heads/HEAD not found\n    fatal: Branch rename failed\n\nwhile in the version build from 'next', I get the following error\nmessage,\n\n    error: refname HEAD is a symbolic ref, renaming it is not supported\n    fatal: Branch rename failed\n\n    That made me suspicious and I wanted to find where the error message\n    got changed and bisected this which pointed to,\n\n    9416812db (branch: forbid refs/heads/HEAD, 2017-10-13)\n\n    This is the same commit which also causes the bug of allowing HEAD to\n    be renamed when it is not a symbolic ref. I found a way to fix this but\n    am not so sure if it's the right way to do this. (the diff patch is\n    found at the end of this mail).\n\n    One more observation I made was that without the patch at the end it is\n    NOT possible to rename a branch named \"HEAD\" created using the older\n    version!\n\n    On Sat, 2017-10-28 at 22:28 +0530, Kaartic Sivaraam wrote:\n    >     git rebase-i HEAD~\n> \n\nSmall correction here. Now you could replace that with the simpler,\n\n    git checkout HEAD^\n\n\n\ndiff --git a/sha1_name.c b/sha1_name.c\nindex c7c5ab376..4345e14c9 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -1334,7 +1334,13 @@ int strbuf_check_branch_ref(struct strbuf *sb, const char *name)\n        strbuf_branchname(sb, name, INTERPRET_BRANCH_LOCAL);\n        if (name[0] == '-')\n                return -1;\n+\n        strbuf_splice(sb, 0, 0, \"refs/heads/\", 11);\n+\n+       /* HEAD is not to be used as a branch name */\n+       if(!strcmp(sb->buf, \"refs/heads/HEAD\"))\n+               return -1;\n+\n        return check_refname_format(sb->buf, 0);\n }\n \n\nHTH,\nKaartic\n"},{"id":"331661","messageId":"xmqqd151qft5.fsf@gitster.mtv.corp.google.com","threadId":"47057","inReplyTo":"1509600814.9254.6.camel@gmail.com","subject":"Re: [BUG] Incosistent repository state when trying to rename HEAD in the middle of a rebase","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-02T07:51:18Z","receivedAt":"2017-11-02T07:51:25Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> writes:\n\n> I was able to spare some time to dig into this and found a few things.\n>\n> First, it seems that the issue is more generic and the BUG kicks in\n> whenever HEAD is not a symbolic ref.\n\nInteresting.  \n\nShortly we'll be rewinding the tip of 'next', and it is a good\nopportunity to kick any not-so-well-cooked topic back to 'pu',\nso let's figure out what is going on after that happens (at which\npoint let's eject the \"branch name sanity\" topic out of 'next').\n\n"},{"id":"331765","messageId":"49563f7c-354e-334e-03a6-c3a40884b6d0@gmail.com","threadId":"47057","inReplyTo":"xmqqd151qft5.fsf@gitster.mtv.corp.google.com","subject":"Re: [BUG] Incosistent repository state when trying to rename HEAD in the middle of a rebase","fromName":"Kaartic Sivaraam","fromEmail":"kaarticsivaraam91196@gmail.com","sentAt":"2017-11-03T09:23:37Z","receivedAt":"2017-11-03T09:23:49Z","isPatch":false,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"On Thursday 02 November 2017 01:21 PM, Junio C Hamano wrote:\n> Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> writes:\n> \n>> I was able to spare some time to dig into this and found a few things.\n>>\n>> First, it seems that the issue is more generic and the BUG kicks in\n>> whenever HEAD is not a symbolic ref.\n> \n> Interesting.\n>\n\nLet me detail a little more about my observations just for the sake of \ncompleteness. The change that forbid refs/heads/HEAD caused issues only \nwhen HEAD wasn't a symbolic link because of the following reasons,\n\n1) The change resulted in 'strbuf_check_branch_ref' returning with \nfailure when the name it received (sb) was HEAD *without* interpreting \nit as \"refs/heads/HEAD\" into 'ref'. This resulted in the violation of \nthe expectation of it's callers that it would have interpret 'ref' which \nwas the major cause of the issue.\n\nIt wouldn't have been an issue if we had checked for the existence of a \n\"branch\" (refs/heads/) rather than checking for the existence of a \"ref\" \n(which allowed HEAD to pass the test).\n\n\n2) This did not cause issues when HEAD was a symbolic ref because there \nwas a check for attempting to rename in a symbolic ref in \n'files_copy_or_rename_ref'. The check throws an error when trying to \nrename a symbolic ref which resulted in suspicious error message I got.\n\nSo, IIUC the issue doesn't occur when 'ref' is intrepreted before the \ncheck for 'HEAD'. That's possibly why the diff patch I sent in the \nprevious mail fixed the issue.\n\nAlso, it would be nice if we check for the existence of a \"branch\" when \nwe want to know whether a branch exists rather than simply doing a \n'ref_exists' on the interpreted ref.\n\n\n> Shortly we'll be rewinding the tip of 'next', and it is a good\n> opportunity to kick any not-so-well-cooked topic back to 'pu',\n> so let's figure out what is going on after that happens (at which\n> point let's eject the \"branch name sanity\" topic out of 'next').\n\nDoes the above explanation give an idea about what's happening?\n\n\n---\nKaartic\n"}]}