git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 1/2] worktree: fix "add -B"

From
Duy Nguyen <pclouds@gmail.com>
Date
Feb 16, 2016, 01:15 UTC
Message-ID
<CACsJy8BzN4Gskj3Vr_fZyJW-+cPtLE-590LD+kSfjDm1Y9gaLQ@mail.gmail.com>
In-Reply-To
<xmqq8u2lo7hq.fsf@gitster.mtv.corp.google.com>
On Tue, Feb 16, 2016 at 6:53 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 34 quoted lines
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> Current code does not update "symref" when -B is used. This string
>> contains the new HEAD. Because it's empty "git worktree add -B" fails at
>> symbolic-ref step.
>>
>> Because branch creation is already done before calling add_worktree(),
>> -B is equivalent to -b from add_worktree() point of view. We do not need
>> the special case for -B.
>>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>> ---
>>  Complete patch.
>>
>>  builtin/worktree.c      | 4 +---
>>  t/t2025-worktree-add.sh | 8 ++++++++
>>  2 files changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/builtin/worktree.c b/builtin/worktree.c
>> index 475b958..6b9c946 100644
>> --- a/builtin/worktree.c
>> +++ b/builtin/worktree.c
>> @@ -201,9 +201,7 @@ static int add_worktree(const char *path, const char *refname,
>>               die(_("'%s' already exists"), path);
>>
>>       /* is 'refname' a branch or commit? */
>> -     if (opts->force_new_branch) /* definitely a branch */
>> -             ;
>> -     else if (!opts->detach && !strbuf_check_branch_ref(&symref, refname) &&
>> +     if (!opts->detach && !strbuf_check_branch_ref(&symref, refname) &&
>>                ref_exists(symref.buf)) { /* it's a branch */
>
> Makes a reader wonder why the original thought it was OK to do
> nothing when -B is given here.

The bug is quite subtle. This code is added in f7c9dac. At that commit, I believe symref is simply a temporary var, to be used by ref_exists() and nothing else. The next commit 7f44e3d replaces git-checkout with git-symbolic-ref and symref is used again. But the symref initialization code is not in the diff context lines, so it's hard to see that there's one case where symref remains empty.

> What does symref.buf have at this point in the codeflow?
Empty.
> Will it always an existing branch?
It should be.
> In what case can it be the name of a branch that does not yet exist?
You can do "worktree add <path> <non-existing-ref>".
-- 
Duy
Previous: Junio C Hamano
Message 6 of 6 in “git worktree fails to recreate existing branch even with -B”
  1. Kirill LikhodedovFeb 9, 2016
  2. Duy NguyenFeb 15, 2016
  3. 1/2 worktree: fix "add -B"Nguyễn Thái Ngọc Duy, Feb 15, 2016
  4. 2/2 worktree add -B: do the checkout test before update branchNguyễn Thái Ngọc Duy, Feb 15, 2016
  5. Junio C HamanoFeb 15, 2016
  6. Duy NguyenFeb 16, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.