Re: [PATCH] builtin/checkout: check the branch used in -B with worktrees
From: Rubén Justo <hidden>
Date: 2023-01-17 00:53:32
Hi Carlo. Thank you for working on this. On 16/1/23 18:28, Carlo Marcelo Arenas Belón wrote:
quoted hunk ↗ jump to hunk
@@ -1533,13 +1534,12 @@ static int checkout_branch(struct checkout_opts *opts, if (!opts->can_switch_when_in_progress) die_if_some_operation_in_progress(); - if (new_branch_info->path && !opts->force_detach && !opts->new_branch && - !opts->ignore_other_worktrees) { + if (check_branch_info->path && !opts->force_detach && !opts->ignore_other_worktrees) { int flag; char *head_ref = resolve_refdup("HEAD", 0, NULL, &flag); if (head_ref && - (!(flag & REF_ISSYMREF) || strcmp(head_ref, new_branch_info->path))) - die_if_checked_out(new_branch_info->path, 1); + (!(flag & REF_ISSYMREF) || strcmp(head_ref, check_branch_info->path))) + die_if_checked_out(check_branch_info->path, 1);
You adjusted this block to accommodate the problem with "checkout -B", which is sensible, but we may need to do something different here. What we are doing here, if I understand it correctly, is dying if the branch is _not checked out in the current worktree_ and _checked out in any other worktree_. Which is OK for normal checkout/switch. But IMHO with "checkout -B" we have to die if the branch is checked out in any other worktree, regardless of whether or not it is checked out in the current working tree. Perhaps the scenario where the user has the same branch checked out in multiple worktrees and tries to reset in one of them, is one of those where we could use the "if it hurts, don't do it". But we are providing the "--ignore-other-worktrees" modifier, so I think we should disallow the "-B" if the branch is checked out in any other worktree, and let the user use "--ignore-other-worktrees" if he wants to go wild. But.. die_if_checked_out() does not correctly honor the "ignore_current_worktree" in this situation. I have submitted a patch to fix this, in case you want to consider all of this. Un saludo.