Re: [PATCH] branch: report kind of checkout when rejecting delete
- From
René Scharfe <l.s.r@web.de>
- Date
- Jul 18, 2026, 19:07 UTC
- Message-ID
- <c7357faf-3d2b-46c6-99e7-88d3e2c72a77@web.de>
- In-Reply-To
- <xmqqjyqsqk1a.fsf@gitster.g>
On 7/18/26 7:34 PM, Junio C Hamano wrote:
Show 30 quoted lines
> 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"?
Show 40 quoted lines
>> 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.
Show 31 quoted lines
>
>> + 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?
Show 25 quoted lines
> >> 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é