Re: [PATCH] branch: report kind of checkout when rejecting delete

["René Scharfe" <l.s.r@web.de>]

On 7/18/26 7:34 PM, Junio C Hamano wrote:
> René Scharfe <l.s.r@web.de> writes:
> 
>> git branch refuses to delete branches that are currently checked out
>> with a message like this: "error: cannot delete branch 'foo' used by
>> worktree at '/path/of/worktree'".  This can be confusing with internal
>> checkouts, e.g. if one tries to delete a branch associated with an
>> active bisect run.
>>
>> Mention the kind of internal checkout, if any, to spare the user from
>> remembering that they might have forgotten a bisect or rebase.  To do
>> that, register the checkout reason in a strintmap alongside the existing
>> strmap that stores the worktree path.
>>
>> Suggested-by: stsp <stsp2@yandex.ru>
>> Signed-off-by: René Scharfe <l.s.r@web.de>
>> ---
>> Original message:
>> https://lore.kernel.org/git/cae34516-5437-49d3-8d39-16f4059a81a8@yandex.ru/
> 
> This reminds me of another recent discussion on rewriting a branch
> that is checked out elsewhere, where the "git history" command
> forgot to apply the same safety check:
> 
> https://lore.kernel.org/git/e7dbcede-4486-459c-aa64-e44690e01fe0@gmail.com/
> 
> We definitely need an easy-to-use API to determine consistently
> which branches are in use, and to teach all commands that repoint
> branch tips to use it to offer the same safety to users.  The
> framework that this patch introduces might be a good starting point
> for that effort.

branch_checked_out() already allows to check whether a branch is in use,
but I guess git history needs to respond differently depending on the
kind of use, e.g. leave active bisects and rebases untouched and update
checked out branches.  branch_checkout_kind() would allow that, but I
now wonder if it suffices for cases where branches appear in multiple
worktrees.  So perhaps the query we need to enable are "is this branch
used by a rebase/bisect" and not the more limited "tell me one use of
this branch"?

>> diff --git a/branch.h b/branch.h
>> index 3dc6e2a0ff..d1073fe1cd 100644
>> --- a/branch.h
>> +++ b/branch.h
>> @@ -15,6 +15,14 @@ enum branch_track {
>>  	BRANCH_TRACK_SIMPLE,
>>  };
>>  
>> +enum branch_checkout_kind {
>> +	BRANCH_CHECKOUT_KIND_UNSPECIFIED = 0,
>> +	BRANCH_CHECKOUT_KIND_CHECKOUT,
>> +	BRANCH_CHECKOUT_KIND_REBASE,
>> +	BRANCH_CHECKOUT_KIND_BISECT,
>> +	BRANCH_CHECKOUT_KIND_UPDATE_REF,
>> +};
>> +
>> ...
>> +/*
>> + * If the branch at 'refname' is currently checked out in a worktree,
>> + * then return the kind of checkout, i.e. whether it was done by an
>> + * actual checkout or a rebase etc.
>> + */
>> +enum branch_checkout_kind branch_checkout_kind(const char *refname);
> 
> OK.
> 
>> diff --git a/builtin/branch.c b/builtin/branch.c
>> index dede60d27b..3223347129 100644
>> --- a/builtin/branch.c
>> +++ b/builtin/branch.c
>> @@ -266,9 +266,34 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
>>  		if (kinds == FILTER_REFS_BRANCHES) {
>>  			const char *path;
>>  			if ((path = branch_checked_out(name))) {
>> -				error(_("cannot delete branch '%s' "
>> -					"used by worktree at '%s'"),
>> -				      bname.buf, path);
>> +				int kind = branch_checkout_kind(name);
> 
> Not "enum branch_checkout_kind" but "int"?

Yes, it doesn't matter for the switch and is easier to print.

> 
>> +				switch (kind) {
>> +				case BRANCH_CHECKOUT_KIND_CHECKOUT:
>> +					error(_("cannot delete branch '%s' "
>> +						"used by worktree at '%s'"),
>> +					      bname.buf, path);
>> +					break;
> 
> We may want to be more explicit and say "cannot delete
> branch 'frotz' checked out in worktree at '/tmp/nitfol'"
> instead.  Unless this is a catch-all entry for states that
> are neither 'rebase', 'bisect', nor 'rebase-merges' but are
> somehow otherwise in use, that is.
> 
>> +				case BRANCH_CHECKOUT_KIND_UPDATE_REF:
>> +					error(_("cannot delete branch '%s' "
>> +						"used by worktree at '%s' "
>> +						"for update-ref"),
>> +					      bname.buf, path);
>> +					break;
> 
> I was quite lost when searching for cases where this 'update-ref'
> state might be encountered, and I still lack confidence.  Can
> we make the diagnostic message a bit friendlier to our users?
> 
> For instance, something like: 'You are rebasing a history with
> merges in that other worktree, and the tip of this branch will
> be updated when that process completes, so you cannot delete
> it from here.'  (Naturally, I may have misidentified the exact
> nature of the error, but this illustrates the level of detail and
> user-facing clarity I hope to see.)

That's quite long.  Would it make sense to throw that update-ref
case into the rebase bin, i.e. only distinguish between checkout,
bisect and rebase?

> 
>> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
>> index e2682a83a0..e5df493b66 100755
>> --- a/t/t3200-branch.sh
>> +++ b/t/t3200-branch.sh
>> @@ -930,7 +930,7 @@ test_expect_success 'deleting currently checked out branch fails' '
>>  	git worktree add -b my7 my7 &&
>>  	test_must_fail git -C my7 branch -d my7 &&
>>  	test_must_fail git branch -d my7 2>actual &&
>> -	grep "^error: cannot delete branch .my7. used by worktree at " actual &&
>> +	test_grep "^error: cannot delete branch '"'"'my7'"'"' used by worktree at '"'.*'\$"'" actual &&
>>  	rm -r my7 &&
>>  	git worktree prune
>>  '
>> @@ -941,7 +941,7 @@ test_expect_success 'deleting in-use branch fails' '
>>  	git -C my7 bisect start HEAD HEAD~2 &&
>>  	test_must_fail git -C my7 branch -d my7 &&
>>  	test_must_fail git branch -d my7 2>actual &&
>> -	grep "^error: cannot delete branch .my7. used by worktree at " actual &&
>> +	test_grep "^error: cannot delete branch '"'"'my7'"'"' used by worktree at '"'.*' for bisect\$"'" actual &&
>>  	rm -r my7 &&
>>  	git worktree prune
>>  '
> 
> We distinguish four kinds in the code but we test only two of them?
Laziness, and it was enough for the user requirement..

René