{"thread":{"id":"41376","subject":"git worktree fails to recreate existing branch even with -B","startedAt":"2016-02-09T14:54:01Z","lastAt":"2016-02-16T01:15:16Z","messageCount":6,"participants":["Kirill Likhodedov","Duy Nguyen","Nguyễn Thái Ngọc Duy","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"277801","messageId":"344AE670-BF9A-4779-AFB0-50B870E1D488@jetbrains.com","threadId":"41376","inReplyTo":null,"subject":"git worktree fails to recreate existing branch even with -B","fromName":"Kirill Likhodedov","fromEmail":"kirill.likhodedov@jetbrains.com","sentAt":"2016-02-09T14:54:01Z","receivedAt":"2016-02-09T14:54:01Z","isPatch":false,"sender":{"key":"kirill.likhodedov@jetbrains.com","avatar":"https://gravatar.com/avatar/9fd55d2a110e8e96deb3ed7b503d7f3b9b5b1ed5ca9ef6683c15525b11b94e63?d=mp&s=160"},"body":"Git doesn’t allow me to execute \n    git worktree add -B <branch> <path> <start-point>\nwhere <branch> already exists in the repository.\n\nThe command prints the following:\n    Preparing <path> (identifier <branch>)\n    fatal: Refusing to point HEAD outside of refs/\n\nI’m trying to create a worktree on an existing branch <branch>, which should point to <start-point>. This obviously should fail with “-b”, but I use “-B” and expect it to be reset to <start-point> as mentioned in the docs:\n\n    By default, -b refuses to create a new branch if it already exists.  \n    -B overrides this safeguard, resetting <new-branch> to <branch>.\n\nDo I miss something or there is a bug?\n\nSteps to reproduce:\n\ngit init wt\ncd wt\necho 'asd' > a.txt ; git add a.txt ; git commit -m initial\ngit branch br1\ngit worktree add -B br1 ~/temp/br1 master\n\nError message:\nPreparing /Users/loki/temp/br1 (identifier br1)\nfatal: Refusing to point HEAD outside of refs/\n\nThanks a lot!\n-- Kirill"},{"id":"278146","messageId":"20160215094154.GA11698@lanh","threadId":"41376","inReplyTo":"344AE670-BF9A-4779-AFB0-50B870E1D488@jetbrains.com","subject":"Re: git worktree fails to recreate existing branch even with -B","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-02-15T09:41:54Z","receivedAt":"2016-02-15T09:41:54Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Feb 09, 2016 at 05:54:01PM +0300, Kirill Likhodedov wrote:\n> Git doesn’t allow me to execute \n>     git worktree add -B <branch> <path> <start-point>\n> where <branch> already exists in the repository.\n> \n> The command prints the following:\n>     Preparing <path> (identifier <branch>)\n>     fatal: Refusing to point HEAD outside of refs/\n> \n> I’m trying to create a worktree on an existing branch <branch>,\n> which should point to <start-point>. This obviously should fail with\n> “-b”, but I use “-B” and expect it to be reset to <start-point> as\n> mentioned in the docs:\n> \n>     By default, -b refuses to create a new branch if it already exists.  \n>     -B overrides this safeguard, resetting <new-branch> to <branch>.\n> \n> Do I miss something or there is a bug?\n\nAccording to the man page, this looks like a bug.\n\n> Steps to reproduce:\n> \n> git init wt\n> cd wt\n> echo 'asd' > a.txt ; git add a.txt ; git commit -m initial\n> git branch br1\n> git worktree add -B br1 ~/temp/br1 master\n> \n> Error message:\n> Preparing /Users/loki/temp/br1 (identifier br1)\n> fatal: Refusing to point HEAD outside of refs/\n\nGIT_TRACE=2 gives me\n\ntrace: built-in: git 'symbolic-ref' 'HEAD' ''\nfatal: Refusing to point HEAD outside of refs/\n\nSo we pass wrong argument to symbolic-ref. The '' should be\n'refs/heads/br1'. This patch seems to fix it.\n\n-- 8< --\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 475b958..d5b319f 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -202,7 +202,7 @@ static int add_worktree(const char *path, const char *refname,\n \n \t/* is 'refname' a branch or commit? */\n \tif (opts->force_new_branch) /* definitely a branch */\n-\t\t;\n+\t\tstrbuf_addf(&symref, \"refs/heads/%s\", refname);\n \telse if (!opts->detach && !strbuf_check_branch_ref(&symref, refname) &&\n \t\t ref_exists(symref.buf)) { /* it's a branch */\n \t\tif (!opts->force)\n-- 8< --\n--\nDuy\n"},{"id":"278159","messageId":"1455543333-25814-1-git-send-email-pclouds@gmail.com","threadId":"41376","inReplyTo":"20160215094154.GA11698@lanh","subject":"[PATCH 1/2] worktree: fix \"add -B\"","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-02-15T13:35:32Z","receivedAt":"2016-02-15T13:35:32Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Current code does not update \"symref\" when -B is used. This string\ncontains the new HEAD. Because it's empty \"git worktree add -B\" fails at\nsymbolic-ref step.\n\nBecause branch creation is already done before calling add_worktree(),\n-B is equivalent to -b from add_worktree() point of view. We do not need\nthe special case for -B.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n Complete patch.\n\n builtin/worktree.c      | 4 +---\n t/t2025-worktree-add.sh | 8 ++++++++\n 2 files changed, 9 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 475b958..6b9c946 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -201,9 +201,7 @@ static int add_worktree(const char *path, const char *refname,\n \t\tdie(_(\"'%s' already exists\"), path);\n \n \t/* is 'refname' a branch or commit? */\n-\tif (opts->force_new_branch) /* definitely a branch */\n-\t\t;\n-\telse if (!opts->detach && !strbuf_check_branch_ref(&symref, refname) &&\n+\tif (!opts->detach && !strbuf_check_branch_ref(&symref, refname) &&\n \t\t ref_exists(symref.buf)) { /* it's a branch */\n \t\tif (!opts->force)\n \t\t\tdie_if_checked_out(symref.buf);\ndiff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh\nindex 0a804da..a4d36c0 100755\n--- a/t/t2025-worktree-add.sh\n+++ b/t/t2025-worktree-add.sh\n@@ -193,6 +193,14 @@ test_expect_success '\"add\" -B/--detach mutually exclusive' '\n \ttest_must_fail git worktree add -B poodle --detach bamboo master\n '\n \n+test_expect_success 'add -B' '\n+\tgit worktree add -B poodle bamboo2 master^ &&\n+\tgit -C bamboo2 symbolic-ref HEAD >actual &&\n+\techo refs/heads/poodle >expected &&\n+\ttest_cmp expected actual &&\n+\ttest_cmp_rev master^ poodle\n+'\n+\n test_expect_success 'local clone from linked checkout' '\n \tgit clone --local here here-clone &&\n \t( cd here-clone && git fsck )\n-- \n2.7.0.377.g4cd97dd\n"},{"id":"278160","messageId":"1455543333-25814-2-git-send-email-pclouds@gmail.com","threadId":"41376","inReplyTo":"1455543333-25814-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 2/2] worktree add -B: do the checkout test before update branch","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-02-15T13:35:33Z","receivedAt":"2016-02-15T13:35:33Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"If --force is not given but -B is, we should not proceed if the given\nbranch is already checked out elsewhere. add_worktree() has this test,\nbut it kicks in too late when \"git branch --force\" is already\nexecuted. As a result, even though we correctly refuse to create a new\nworktree, we have already updated the branch and mess up the other\ncheckout.\n\nRepeat the die_if_checked_out() test again for this specific case before\n\"git branch\" runs.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n Something extra while I was studying this code. I'm not so sure if\n this is the right way.\n \n Another option is do it in \"git branch\" which rejects with \"Cannot\n force update the current branch\", but only for current worktree.\n\n builtin/worktree.c      | 11 ++++++++++-\n t/t2025-worktree-add.sh |  7 +++++++\n 2 files changed, 17 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 6b9c946..20cf67a 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -334,9 +334,18 @@ static int add(int ac, const char **av, const char *prefix)\n \tbranch = ac < 2 ? \"HEAD\" : av[1];\n \n \topts.force_new_branch = !!new_branch_force;\n-\tif (opts.force_new_branch)\n+\tif (opts.force_new_branch) {\n+\t\tstruct strbuf symref = STRBUF_INIT;\n+\n \t\topts.new_branch = new_branch_force;\n \n+\t\tif (!opts.force &&\n+\t\t    !strbuf_check_branch_ref(&symref, opts.new_branch) &&\n+\t\t    ref_exists(symref.buf))\n+\t\t\tdie_if_checked_out(symref.buf);\n+\t\tstrbuf_release(&symref);\n+\t}\n+\n \tif (ac < 2 && !opts.new_branch && !opts.detach) {\n \t\tint n;\n \t\tconst char *s = worktree_basename(path, &n);\ndiff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh\nindex a4d36c0..cbfa41e 100755\n--- a/t/t2025-worktree-add.sh\n+++ b/t/t2025-worktree-add.sh\n@@ -193,6 +193,13 @@ test_expect_success '\"add\" -B/--detach mutually exclusive' '\n \ttest_must_fail git worktree add -B poodle --detach bamboo master\n '\n \n+test_expect_success '\"add -B\" fails if the branch is checked out' '\n+\tgit rev-parse newmaster >before &&\n+\ttest_must_fail git worktree add -B newmaster bamboo master &&\n+\tgit rev-parse newmaster >after &&\n+\ttest_cmp before after\n+'\n+\n test_expect_success 'add -B' '\n \tgit worktree add -B poodle bamboo2 master^ &&\n \tgit -C bamboo2 symbolic-ref HEAD >actual &&\n-- \n2.7.0.377.g4cd97dd\n"},{"id":"278248","messageId":"xmqq8u2lo7hq.fsf@gitster.mtv.corp.google.com","threadId":"41376","inReplyTo":"1455543333-25814-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 1/2] worktree: fix \"add -B\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-02-15T23:53:53Z","receivedAt":"2016-02-15T23:53:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n\n> Current code does not update \"symref\" when -B is used. This string\n> contains the new HEAD. Because it's empty \"git worktree add -B\" fails at\n> symbolic-ref step.\n>\n> Because branch creation is already done before calling add_worktree(),\n> -B is equivalent to -b from add_worktree() point of view. We do not need\n> the special case for -B.\n>\n> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n> ---\n>  Complete patch.\n>\n>  builtin/worktree.c      | 4 +---\n>  t/t2025-worktree-add.sh | 8 ++++++++\n>  2 files changed, 9 insertions(+), 3 deletions(-)\n>\n> diff --git a/builtin/worktree.c b/builtin/worktree.c\n> index 475b958..6b9c946 100644\n> --- a/builtin/worktree.c\n> +++ b/builtin/worktree.c\n> @@ -201,9 +201,7 @@ static int add_worktree(const char *path, const char *refname,\n>  \t\tdie(_(\"'%s' already exists\"), path);\n>  \n>  \t/* is 'refname' a branch or commit? */\n> -\tif (opts->force_new_branch) /* definitely a branch */\n> -\t\t;\n> -\telse if (!opts->detach && !strbuf_check_branch_ref(&symref, refname) &&\n> +\tif (!opts->detach && !strbuf_check_branch_ref(&symref, refname) &&\n>  \t\t ref_exists(symref.buf)) { /* it's a branch */\n\nMakes a reader wonder why the original thought it was OK to do\nnothing when -B is given here.\n\nWhat does symref.buf have at this point in the codeflow?  Will it\nalways an existing branch?  In what case can it be the name of a\nbranch that does not yet exist?\n\nThanks.\n\n>  \t\tif (!opts->force)\n>  \t\t\tdie_if_checked_out(symref.buf);\n> diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh\n> index 0a804da..a4d36c0 100755\n> --- a/t/t2025-worktree-add.sh\n> +++ b/t/t2025-worktree-add.sh\n> @@ -193,6 +193,14 @@ test_expect_success '\"add\" -B/--detach mutually exclusive' '\n>  \ttest_must_fail git worktree add -B poodle --detach bamboo master\n>  '\n>  \n> +test_expect_success 'add -B' '\n> +\tgit worktree add -B poodle bamboo2 master^ &&\n> +\tgit -C bamboo2 symbolic-ref HEAD >actual &&\n> +\techo refs/heads/poodle >expected &&\n> +\ttest_cmp expected actual &&\n> +\ttest_cmp_rev master^ poodle\n> +'\n> +\n>  test_expect_success 'local clone from linked checkout' '\n>  \tgit clone --local here here-clone &&\n>  \t( cd here-clone && git fsck )\n"},{"id":"278255","messageId":"CACsJy8BzN4Gskj3Vr_fZyJW-+cPtLE-590LD+kSfjDm1Y9gaLQ@mail.gmail.com","threadId":"41376","inReplyTo":"xmqq8u2lo7hq.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 1/2] worktree: fix \"add -B\"","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-02-16T01:15:16Z","receivedAt":"2016-02-16T01:15:16Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Feb 16, 2016 at 6:53 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n>\n>> Current code does not update \"symref\" when -B is used. This string\n>> contains the new HEAD. Because it's empty \"git worktree add -B\" fails at\n>> symbolic-ref step.\n>>\n>> Because branch creation is already done before calling add_worktree(),\n>> -B is equivalent to -b from add_worktree() point of view. We do not need\n>> the special case for -B.\n>>\n>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n>> ---\n>>  Complete patch.\n>>\n>>  builtin/worktree.c      | 4 +---\n>>  t/t2025-worktree-add.sh | 8 ++++++++\n>>  2 files changed, 9 insertions(+), 3 deletions(-)\n>>\n>> diff --git a/builtin/worktree.c b/builtin/worktree.c\n>> index 475b958..6b9c946 100644\n>> --- a/builtin/worktree.c\n>> +++ b/builtin/worktree.c\n>> @@ -201,9 +201,7 @@ static int add_worktree(const char *path, const char *refname,\n>>               die(_(\"'%s' already exists\"), path);\n>>\n>>       /* is 'refname' a branch or commit? */\n>> -     if (opts->force_new_branch) /* definitely a branch */\n>> -             ;\n>> -     else if (!opts->detach && !strbuf_check_branch_ref(&symref, refname) &&\n>> +     if (!opts->detach && !strbuf_check_branch_ref(&symref, refname) &&\n>>                ref_exists(symref.buf)) { /* it's a branch */\n>\n> Makes a reader wonder why the original thought it was OK to do\n> nothing when -B is given here.\n\nThe bug is quite subtle. This code is added in f7c9dac. At that\ncommit, I believe symref is simply a temporary var, to be used by\nref_exists() and nothing else. The next commit 7f44e3d replaces\ngit-checkout with git-symbolic-ref and symref is used again. But the\nsymref initialization code is not in the diff context lines, so it's\nhard to see that there's one case where symref remains empty.\n\n> What does symref.buf have at this point in the codeflow?\n\nEmpty.\n\n> Will it always an existing branch?\n\nIt should be.\n\n> In what case can it be the name of a branch that does not yet exist?\n\nYou can do \"worktree add <path> <non-existing-ref>\".\n-- \nDuy\n"}]}