Re: [PATCH] bisect: fix "reset" when branch is checked out elsewhere

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 26, 2023, 17:06 UTC
Message-ID: <xmqqzga5b4yz.fsf@gitster.g>
In-Reply-To: <0d04f8ed-6933-9354-1f64-24d827424c71@gmail.com>

Rubén Justo <rjusto@gmail.com> writes:

Show 14 quoted lines

> But, and this is what makes me think that "checking out will fail" is the wrong
> choice for "bisect", while bisecting, with the worktree in a detached HEAD
> state, the branch to which "bisect reset" will check out back (BISECT_START),
> is still considered checked out in the working tree:
>
> 	$ git checkout -b work
> 	$ git bisect start HEAD HEAD~3
> 	... bisect detaches the current worktree ...
> 	$ git worktree add work
> 	Preparing worktree (checkout out 'work')
> 	fatal: 'work' is already checked out at ...
>
> So, checking out back to the implicitly checked out branch sounds like it
> should not fail.

If that is what you are aiming at, I suspect that the posted patch is doing it in a wrong way. Instead, we should just declare that the branch being bisected does not mean the branch cannot be checked out elsewhere, so that

	$ git worktree add --detach ../another HEAD^0
	$ git checkout -b work
	$ git bisect start work work~3
        ... detaches ...
	$ git -C ../another checkout work

should just work, no?

I admit I haven't thought things through, but I tend to be sympathetic to such a declaration. After all, "bisect" is a read-only operation as far as the branch you happened to be on when you started a bisect session is concerned. Jumping around and materializing tree states recorded in various commits leading to the tip of the branch and inspecting them would not change anything on the branch itself. And more importantly, the branch being checked out in another worktree and modified there should not break the bisection, EXCEPT that the final "git bisect reset" (without arguments) would fail if the other worktree removed the branch.

So, how about removing the is_worktree_being_bisected() check from find_shared_symref(), so that not just "worktree add" and "bisect reset", but "checkout" and "switch" are allowed to make the branch current even it is being bisected elsewhere?

That would affect the other topic, I suspect, as well. It may be a positive change. Or are there cases I missed, where the branch being bisected should not be touched from elsewhere, and we cannot remove the check from find_shared_symref()?

Thanks.