From: René Scharfe Date: Sat, 18 Jul 2026 19:07:41 GMT Subject: Re: [PATCH] branch: report kind of checkout when rejecting delete Message-ID: In-Reply-To: On 7/18/26 7:34 PM, Junio C Hamano wrote: > René Scharfe 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 >> Signed-off-by: René Scharfe >> --- >> 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é