{"thread":{"id":"46661","subject":"Undocumented change in `git branch -M` behavior","startedAt":"2017-08-23T20:13:43Z","lastAt":"2017-08-27T05:39:46Z","messageCount":6,"participants":["Nish Aravamudan","Kevin Daudt","Duy Nguyen","Nguyễn Thái Ngọc Duy","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"327079","messageId":"20170823201334.bz42s6t5ti4jdaqm@pitfall","threadId":"46661","inReplyTo":null,"subject":"Undocumented change in `git branch -M` behavior","fromName":"Nish Aravamudan","fromEmail":"nish.aravamudan@canonical.com","sentAt":"2017-08-23T20:13:34Z","receivedAt":"2017-08-23T20:13:43Z","isPatch":false,"sender":{"key":"nish.aravamudan@canonical.com","avatar":null},"body":"Hello,\n\nHopefully, I've got this right -- I noticed a change in behavior in git\nwith Ubuntu 17.10, which recently got 2.14.1. Specifically, that when in\nan orphaned branch, -M ends up moving HEAD to the new branch name,\nclobbering the working tree. As far as I know, from the manpages,\norphaned branches are still supported and should work?\n\nI think an example will demonstrate more than words (the following are\ndone in LXD containers, hence the root user):\n\n# git --version\ngit version 2.14.1\n# mkdir test && cd test && git init .\nInitialized empty Git repository in /root/test/.git/\n# git checkout -b a\nSwitched to a new branch 'a'\n# touch testfile && git add testfile && git commit -m 'initial commit'\n[a (root-commit) 6061193] initial commit\n Committer: root \n 1 file changed, 0 insertions(+), 0 deletions(-)\n create mode 100644 testfile\n# git checkout --orphan master\nSwitched to a new branch 'master'\n# git status\nOn branch master\n\nNo commits yet\n\nChanges to be committed:\n (use \"git rm --cached ...\" to unstage)\n\n new file: testfile\n\n# git reset --hard && git status\nOn branch master\n\nNo commits yet\n\nnothing to commit (create/copy files and use \"git add\" to track)\n# git branch -M a b\n# git status\nOn branch b\nChanges to be committed:\n (use \"git reset HEAD ...\" to unstage)\n\n deleted: testfile\n\nThis is very unexpected. I force-renamed a branch I wasn't currently\nchecked out to and now I'm checked out to it *and* I have staged file\nremovals (I think what is effectively happening is my current working\ndirectory (empty) is being staged into the new branch, but I'm not\n100%).\n\nFor comparision, on 17.04:\n\n# git --version\ngit version 2.11.0\n# mkdir test && cd test && git init .\nInitialized empty Git repository in /root/test/.git/\n# git checkout -b a\nSwitched to a new branch 'a'\n# touch testfile && git add testfile && git commit -m 'initial commit'\n[a (root-commit) f8d0d53] initial commit\n Committer: root \n 1 file changed, 0 insertions(+), 0 deletions(-)\n create mode 100644 testfile\n# git checkout --orphan master\nSwitched to a new branch 'master'\n# git status\nOn branch master\n\nNo commits yet\n\nChanges to be committed:\n (use \"git rm --cached ...\" to unstage)\n\n new file: testfile\n\n# git reset --hard && git status\nOn branch master\n\nNo commits yet\n\nnothing to commit (create/copy files and use \"git add\" to track)\n# git branch -M a b\n# git status\nOn branch master\n\nInitial commit\n\nnothing to commit (create/copy files and use \"git add\" to track)\n\nThis is what I expect to see, the branch rename has no effect on HEAD.\n\nI haven't yet bisected this (but I can if necessary). My initial\nsuspicion is\nhttps://github.com/git/git/commit/70999e9ceca47e03b8900bfb310b2f804125811e#diff-d18f86ea14e2f1e5bff391b2e54438cb\nwhere a comparison between the oldname of the branch and HEAD was\nperformed before attempting to move HEAD (so that HEAD followed to the\nnew branch name, I believe). That change was dropped, though and perhaps\nthe new check in replace_each_worktree_head_symref of\n\n strcmp(oldref, worktrees[i]->head_ref)\n\ndoes not work for orphaned branches? I am unfamiliar with all the\ndetails of the git internals, so please correct me if I'm wrong!\n\nThanks,\nNish\n\n-- \nNishanth Aravamudan\nUbuntu Server\nCanonical Ltd\n"},{"id":"327105","messageId":"20170824053213.GF3839@alpha.vpn.ikke.info","threadId":"46661","inReplyTo":"20170823201334.bz42s6t5ti4jdaqm@pitfall","subject":"Re: Undocumented change in `git branch -M` behavior","fromName":"Kevin Daudt","fromEmail":"me@ikke.info","sentAt":"2017-08-24T05:32:13Z","receivedAt":"2017-08-24T05:35:25Z","isPatch":false,"sender":{"key":"me@ikke.info","avatar":"https://avatars.githubusercontent.com/u/135698?v=4"},"body":"On Wed, Aug 23, 2017 at 01:13:34PM -0700, Nish Aravamudan wrote:\n> Hello,\n> \n> Hopefully, I've got this right -- I noticed a change in behavior in git\n> with Ubuntu 17.10, which recently got 2.14.1. Specifically, that when in\n> an orphaned branch, -M ends up moving HEAD to the new branch name,\n> clobbering the working tree. As far as I know, from the manpages,\n> orphaned branches are still supported and should work?\n> \n> I think an example will demonstrate more than words (the following are\n> done in LXD containers, hence the root user):\n> \n> # git --version\n> git version 2.14.1\n> # mkdir test && cd test && git init .\n> Initialized empty Git repository in /root/test/.git/\n> # git checkout -b a\n> Switched to a new branch 'a'\n> # touch testfile && git add testfile && git commit -m 'initial commit'\n> [a (root-commit) 6061193] initial commit\n> Committer: root \n> 1 file changed, 0 insertions(+), 0 deletions(-)\n> create mode 100644 testfile\n> # git checkout --orphan master\n> Switched to a new branch 'master'\n> # git status\n> On branch master\n> \n> No commits yet\n> \n> Changes to be committed:\n> (use \"git rm --cached ...\" to unstage)\n> \n> new file: testfile\n> \n> # git reset --hard && git status\n> On branch master\n> \n> No commits yet\n> \n> nothing to commit (create/copy files and use \"git add\" to track)\n> # git branch -M a b\n> # git status\n> On branch b\n> Changes to be committed:\n> (use \"git reset HEAD ...\" to unstage)\n> \n> deleted: testfile\n> \n> This is very unexpected. I force-renamed a branch I wasn't currently\n> checked out to and now I'm checked out to it *and* I have staged file\n> removals (I think what is effectively happening is my current working\n> directory (empty) is being staged into the new branch, but I'm not\n> 100%).\n> \n> For comparision, on 17.04:\n> \n> # git --version\n> git version 2.11.0\n> # mkdir test && cd test && git init .\n> Initialized empty Git repository in /root/test/.git/\n> # git checkout -b a\n> Switched to a new branch 'a'\n> # touch testfile && git add testfile && git commit -m 'initial commit'\n> [a (root-commit) f8d0d53] initial commit\n> Committer: root \n> 1 file changed, 0 insertions(+), 0 deletions(-)\n> create mode 100644 testfile\n> # git checkout --orphan master\n> Switched to a new branch 'master'\n> # git status\n> On branch master\n> \n> No commits yet\n> \n> Changes to be committed:\n> (use \"git rm --cached ...\" to unstage)\n> \n> new file: testfile\n> \n> # git reset --hard && git status\n> On branch master\n> \n> No commits yet\n> \n> nothing to commit (create/copy files and use \"git add\" to track)\n> # git branch -M a b\n> # git status\n> On branch master\n> \n> Initial commit\n> \n> nothing to commit (create/copy files and use \"git add\" to track)\n> \n> This is what I expect to see, the branch rename has no effect on HEAD.\n> \n> I haven't yet bisected this (but I can if necessary). My initial\n> suspicion is\n> https://github.com/git/git/commit/70999e9ceca47e03b8900bfb310b2f804125811e#diff-d18f86ea14e2f1e5bff391b2e54438cb\n> where a comparison between the oldname of the branch and HEAD was\n> performed before attempting to move HEAD (so that HEAD followed to the\n> new branch name, I believe). That change was dropped, though and perhaps\n> the new check in replace_each_worktree_head_symref of\n> \n> strcmp(oldref, worktrees[i]->head_ref)\n> \n> does not work for orphaned branches? I am unfamiliar with all the\n> details of the git internals, so please correct me if I'm wrong!\n> \n> Thanks,\n> Nish\n> \n> -- \n> Nishanth Aravamudan\n> Ubuntu Server\n> Canonical Ltd\n\nThanks for this report. I've bisected it down to \nfa099d232 (worktree.c: kill parse_ref() in favor of refs_resolve_ref_unsafe(), 2017-04-24) \n\nI've CC'ed Duy, who made that commit.\n\nKevin\n"},{"id":"327117","messageId":"CACsJy8C=D_MHRJHdbqjZHBvr-Xs_VXxzRU_KenpWrvX1Nb_iyw@mail.gmail.com","threadId":"46661","inReplyTo":"20170823201334.bz42s6t5ti4jdaqm@pitfall","subject":"Re: Undocumented change in `git branch -M` behavior","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-08-24T08:59:57Z","receivedAt":"2017-08-24T09:00:33Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Aug 24, 2017 at 3:13 AM, Nish Aravamudan\n wrote:\n> Hello,\n>\n> Hopefully, I've got this right -- I noticed a change in behavior in git\n> with Ubuntu 17.10, which recently got 2.14.1. Specifically, that when in\n> an orphaned branch, -M ends up moving HEAD to the new branch name,\n> clobbering the working tree.\n\nThanks for the report. I think I see what's going on. I need some more\ntime to come up with a patch but yes this is definitely a bug (and\ncaused by my commit)\n-- \nDuy\n"},{"id":"327123","messageId":"20170824104124.6263-1-pclouds@gmail.com","threadId":"46661","inReplyTo":"20170823201334.bz42s6t5ti4jdaqm@pitfall","subject":"[PATCH] Fix branch renaming not updating HEADs correctly","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-08-24T10:41:24Z","receivedAt":"2017-08-24T10:41:53Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"There are two bugs that sort of work together and cause problems. Let's\nstart with one in replace_each_worktree_head_symref.\n\nBefore fa099d2322 (worktree.c: kill parse_ref() in favor of\nrefs_resolve_ref_unsafe() - 2017-04-24), this code looks like this:\n\n if (strcmp(oldref, worktrees[i]->head_ref))\n continue;\n set_worktree_head_symref(...);\n\nAfter fa099d2322, it is possible that head_ref can be NULL. However, the\nupdated code takes the wrong exit. In the error case (NULL head_ref), we\nshould \"continue;\" to the next worktree. The updated code makes us\n_skip_ \"continue;\" and update HEAD anyway.\n\nThe NULL head_ref is triggered by the second bug in add_head_info (in\nthe same commit). With the flag RESOLVE_REF_READING, resolve_ref_unsafe()\nwill abort if it cannot resolve the target ref. For orphan checkouts,\nHEAD always points to an unborned branch, resolving target ref will\nalways fail. Now we have NULL head_ref. Now we always update HEAD.\n\nCorrect the logic in replace_ function so that we don't accidentally\nupdate HEAD on error. As it turns out, correcting the logic bug above\nbreaks branch renaming completely, thanks to the second bug.\n\n\"git branch -[Mm]\" does two steps (on a normal checkout, no orphan!):\n\n- rename the branch on disk (e.g. refs/heads/abc to refs/heads/def)\n- update HEAD if it points to the branch being renamed.\n\nAt the second step, since the branch pointed to by HEAD (e.g. \"abc\") no\nlonger exists on disk, we run into a temporary orphan checkout situation\nthat has been just corrected to _not_ update HEAD. But we need to update\nHEAD since it's not actually an orphan checkout. We need to update HEAD\nto move out of that orphan state.\n\nCorrect add_head_info(), remove RESOLVE_REF_READING flag. With the flag\ngone, we should always return good \"head_ref\" in orphan checkouts (either\ntemporary or permanent). With good head_ref, things start to work again.\n\nNoticed-by: Nish Aravamudan \nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n branch.c | 5 +++--\n t/t3200-branch.sh | 13 +++++++++++++\n worktree.c | 2 +-\n 3 files changed, 17 insertions(+), 3 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex 36541d05cd..86360d36b3 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -357,8 +357,9 @@ int replace_each_worktree_head_symref(const char *oldref, const char *newref,\n \n \t\tif (worktrees[i]->is_detached)\n \t\t\tcontinue;\n-\t\tif (worktrees[i]->head_ref &&\n-\t\t strcmp(oldref, worktrees[i]->head_ref))\n+\t\tif (!worktrees[i]->head_ref)\n+\t\t\tcontinue;\n+\t\tif (strcmp(oldref, worktrees[i]->head_ref))\n \t\t\tcontinue;\n \n \t\trefs = get_worktree_ref_store(worktrees[i]);\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex dd37ac47c5..902cb87ea8 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -162,6 +162,19 @@ test_expect_success 'git branch -M baz bam should add entries to .git/logs/HEAD'\n \tgrep \"^0\\{40\\}.*$msg$\" .git/logs/HEAD\n '\n \n+test_expect_success 'git branch -M should leave orphaned HEAD alone' '\n+\tgit init orphan &&\n+\t(\n+\t\tcd orphan &&\n+\t\ttest_commit initial &&\n+\t\tgit checkout --orphan lonely &&\n+\t\tgrep lonely .git/HEAD &&\n+\t\ttest_path_is_missing .git/refs/head/lonely &&\n+\t\tgit branch -M master mistress &&\n+\t\tgrep lonely .git/HEAD\n+\t)\n+'\n+\n test_expect_success 'resulting reflog can be shown by log -g' '\n \toid=$(git rev-parse HEAD) &&\n \tcat >expect <<-EOF &&\ndiff --git a/worktree.c b/worktree.c\nindex e28ffbeb09..c0c5a2b373 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -30,7 +30,7 @@ static void add_head_info(struct worktree *wt)\n \n \ttarget = refs_resolve_ref_unsafe(get_worktree_ref_store(wt),\n \t\t\t\t\t \"HEAD\",\n-\t\t\t\t\t RESOLVE_REF_READING,\n+\t\t\t\t\t 0,\n \t\t\t\t\t wt->head_sha1, &flags);\n \tif (!target)\n \t\treturn;\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"327145","messageId":"20170824190418.tdhg45fugibtfgwb@pitfall","threadId":"46661","inReplyTo":"20170824104124.6263-1-pclouds@gmail.com","subject":"Re: [PATCH] Fix branch renaming not updating HEADs correctly","fromName":"Nish Aravamudan","fromEmail":"nish.aravamudan@canonical.com","sentAt":"2017-08-24T19:04:18Z","receivedAt":"2017-08-24T19:04:28Z","isPatch":true,"sender":{"key":"nish.aravamudan@canonical.com","avatar":null},"body":"On 24.08.2017 [17:41:24 +0700], Nguyễn Thái Ngọc Duy wrote:\n> There are two bugs that sort of work together and cause problems. Let's\n> start with one in replace_each_worktree_head_symref.\n\n\n\n> Correct add_head_info(), remove RESOLVE_REF_READING flag. With the flag\n> gone, we should always return good \"head_ref\" in orphan checkouts (either\n> temporary or permanent). With good head_ref, things start to work again.\n> \n> Noticed-by: Nish Aravamudan \n> Signed-off-by: Nguyễn Thái Ngọc Duy \nTested-by: Nish Aravamudan \n\nThank you for the quick turnaround on the patch!\n\n-Nish\n-- \nNishanth Aravamudan\nUbuntu Server\nCanonical Ltd\n"},{"id":"327253","messageId":"xmqqd17hr2lj.fsf@gitster.mtv.corp.google.com","threadId":"46661","inReplyTo":"20170824104124.6263-1-pclouds@gmail.com","subject":"Re: [PATCH] Fix branch renaming not updating HEADs correctly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-27T05:39:36Z","receivedAt":"2017-08-27T05:39:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks. I'll merge this down to at least 'next' before I go\noffline for a week.\n\n"}]}