Re: [PATCH v3 12/13] worktree.c: check whether branch is bisected in another worktree

From: Eric Sunshine <sunshine@sunshineco.com>
Date: May 11, 2016, 05:45 UTC
Message-ID: <CAPig+cTor7CaYDomyocA+GMGc9Jpye1MRfN_Mhtv1JALsn+w0Q@mail.gmail.com>
In-Reply-To: <1461330096-21783-13-git-send-email-pclouds@gmail.com>

On Fri, Apr 22, 2016 at 9:01 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:

Show 8 quoted lines

> Similar to the rebase case, we want to detect if "HEAD" in some worktree
> is being bisected because
>
> 1) we do not want to checkout this branch in another worktree, after
>    bisect is done it will want to go back to this branch
>
> 2) we do not want to delete the branch is either or git bisect will
>    fail to return to the (long gone) branch

I'm very far behind with my reviews and I see that this series is already in 'next', so perhaps these comments are too late...

Show 13 quoted lines

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
> @@ -263,4 +263,17 @@ test_expect_success 'check out from current worktree branch ok' '
> +test_expect_success 'checkout a branch under bisect' '
> +       git worktree add under-bisect &&
> +       (
> +               cd under-bisect &&
> +               git bisect start &&
> +               git bisect bad &&
> +               git bisect good HEAD~2 &&
> +               git worktree list | grep "under-bisect.*detached HEAD" &&
> +               test_must_fail git worktree add new-bisect under-bisect &&

Nit: I realize that the checking 'worktree add' is sufficient, but
it's a bit confusing to read in the commit message about how deleting
the branch would be bad, but then see it testing only 'add'.

Show 10 quoted lines

> +               ! test -d new-bisect
> +       )
> +'
> diff --git a/worktree.c b/worktree.c
> @@ -234,6 +234,21 @@ static int is_worktree_being_rebased(const struct worktree *wt,
> +static int is_worktree_being_bisected(const struct worktree *wt,
> +                                     const char *target)
> +{
> +       struct wt_status_state state;
> +       int found_rebase;

s/rebase/bisect/ perhaps?

Show 8 quoted lines

> +       memset(&state, 0, sizeof(state));
> +       found_rebase = wt_status_check_bisect(wt, &state) &&
> +               state.branch &&
> +               starts_with(target, "refs/heads/") &&
> +               !strcmp(state.branch, target + strlen("refs/heads/"));
> +       free(state.branch);
> +       return found_rebase;
> +}