Re: [PATCH] branch: report kind of checkout when rejecting delete
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 18, 2026, 17:34 UTC
- Message-ID
- <xmqqjyqsqk1a.fsf@gitster.g>
- In-Reply-To
- <9865fc6b-e3fe-4614-9ffe-71af776e1796@web.de>
René Scharfe <l.s.r@web.de> writes:
Show 16 quoted lines
> 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.
Show 23 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.
Show 12 quoted lines
> 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"?
Show 6 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.
Show 6 quoted lines
> + 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.)
Show 22 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?
Thanks. I very much like the direction this is taking us.