Git development
 help / color / mirror / Atom feed
From: Phillip Wood <phillip.wood123@gmail.com>
To: Jasper Mayone <me@jaspermayone.com>, git@vger.kernel.org
Cc: gitster@pobox.com, sunshine@sunshineco.com,
	kristofferhaugsbakk@fastmail.com, ben.knoble@gmail.com,
	haraldnordgren@gmail.com
Subject: Re: [PATCH 2/2] ref-filter: name the worktree of a branch being rebased or bisected
Date: Mon, 2 Nov 2026 14:06:05 +0000	[thread overview]
Message-ID: <37d53d40-66b6-46bb-a38d-0d3534ca112a@gmail.com> (raw)
In-Reply-To: <20261030142137.54750-3-me@jaspermayone.com>

Hi Jasper

On 30/10/2026 14:21, Jasper Mayone wrote:
> %(worktreepath) gives the worktree that has a branch checked out, and
> "git branch" uses it to mark such branches with "+". It only looks at
> the HEAD of each worktree, so while a worktree is rebasing or
> bisecting a branch, its HEAD is detached and the branch shows no
> worktree at all.
> 
> The rest of Git already treats that branch as checked out: "git
> worktree add" and "git branch -d" refuse it, because finishing the
> rebase or bisect will check it out again.

Those commands also reject branches that are being updated by a rebase 
via "update-ref" commands. If we're interested in checkouts we don't 
want to list those, but it means that "%(worktree)" will still give 
different results.

> "git worktree list" shows
> such a worktree as "(detached HEAD)", so %(worktreepath) is the only
> place a script could find out which worktree is working on the
> branch.

I don't really follow this logic. At the moment neither "git 
for-each-ref", nor "git worktree list" show this  - there is no way for 
a script to find out which worktree is working on a given branch. We can 
choose to add it to one or both commands.

> Also map the branch that a worktree is rebasing or bisecting to that
> worktree, as prepare_checked_out_branches() in branch.c does.

If we want it to behave like prepare_checked_out_branches() we should 
use that function, rather than copying some of it to a separate file. If 
we want to only list checked out branches we could do as I previously 
suggested and add a function to branch.c that lists the worktrees where 
a branch is checked out.

> Branches that "rebase --update-refs" will update are not mapped: that
> worktree has not checked them out, it only moves them when the rebase
> is done.

I think that's sensible

> As a result "git branch" marks such a branch with "+", also when it
> runs in the worktree that is rebasing or bisecting it, where it shows
> both "* (no branch, rebasing topic)" and "+ topic". Document that.

That sounds a bit confusing - "* (no branch, rebasing topic)" already 
tells us what we need to know, why do we need "+ topic" as well? The 
output of git branch is intended for human consumption, not scripts.

> diff --git a/Documentation/git-branch.adoc b/Documentation/git-branch.adoc
> index bfdf459329..d499d7e0d4 100644
> --- a/Documentation/git-branch.adoc
> +++ b/Documentation/git-branch.adoc
> @@ -33,7 +33,10 @@ DESCRIPTION
>   If `--list` is given, or if there are no non-option arguments, existing
>   branches are listed; the current branch will be highlighted in green and
>   marked with an asterisk.  Any branches checked out in linked worktrees will
> -be highlighted in cyan and marked with a plus sign. Option `-r` causes the
> +be highlighted in cyan and marked with a plus sign. A branch that a
> +worktree is rebasing or bisecting counts as checked out there, so it is
> +marked in the same way,

I follow this up to here

> also in the worktree that is rebasing or bisecting it.

but what is this trying to say?

> diff --git a/Documentation/git-for-each-ref.adoc b/Documentation/git-for-each-ref.adoc
> index a7b31e8aaf..90ff37259e 100644
> --- a/Documentation/git-for-each-ref.adoc
> +++ b/Documentation/git-for-each-ref.adoc
> @@ -182,7 +182,9 @@ but if used together the last option is selected.
>   
>   `worktreepath`::
>   	The absolute path to the worktree in which the ref is checked
> -	out, if it is checked out in any linked worktree. Empty string
> +	out, if it is checked out in any linked worktree. A branch that
> +	a worktree is rebasing or bisecting counts as checked out there,
> +	although that worktree's `HEAD` is detached. Empty string
>   	otherwise.

It is not a new problem, but I think it would be helpful to add 
something like

     As the path may contain a newline, it is recommended to either use
     one of the quoting options, or follow `%(worktreepath)` with `%00`
     and parse the output by looking for the `NUL` after the worktree
     path and then the LF that terminates each output record.

     Note that if a branch checked out in more than one worktree,
     `%(worktreepath)` will only show one of those worktrees.

> diff --git a/ref-filter.c b/ref-filter.c
> index bdf54f6f59..1aebea243e 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> [...]
> +/*
> + * Map each branch to the worktree that has it checked out. A worktree
> + * that is rebasing or bisecting a branch has it checked out, too,
> + * even though its HEAD may be detached, as for
> + * prepare_checked_out_branches() in branch.c.
> + */
>   static void populate_worktree_map(struct hashmap *map, struct worktree **worktrees)
>   {
> -	int i;
> +	struct strbuf ref = STRBUF_INIT;
>   
> -	for (i = 0; worktrees[i]; i++) {
> -		if (worktrees[i]->head_ref) {
> -			struct ref_to_worktree_entry *entry;
> -			entry = xmalloc(sizeof(*entry));
> -			entry->wt = worktrees[i];
> -			hashmap_entry_init(&entry->ent,
> -					strhash(worktrees[i]->head_ref));
> +	for (size_t i = 0; worktrees[i]; i++) {
> +		struct worktree *wt = worktrees[i];
> +		struct wt_status_state state = { 0 };
>   
> -			hashmap_add(map, &entry->ent);
> +		if (wt->is_bare)
> +			continue;

This is a change in behavior. We currently print the git_dir of bare 
worktrees. I think you can argue that this change is a bug fix as the 
user asked for the worktree path, but it should be explained in the 
commit message.

It is a shame that we end up duplicating code that is already in 
branch.c. As I said last time, I think that it would be better to have a 
single implementation that maps branches to worktrees in branch.c and 
call that function rather than duplicating it here.

Thanks

Phillip

> +
> +		if (wt->head_ref)
> +			add_worktree_entry(map, wt, wt->head_ref);
> +
> +		if (wt_status_check_rebase(wt, &state) &&
> +		    (state.rebase_in_progress ||
> +		     state.rebase_interactive_in_progress) &&
> +		    state.branch) {
> +			strbuf_reset(&ref);
> +			strbuf_addf(&ref, "refs/heads/%s", state.branch);
> +			add_worktree_entry(map, wt, ref.buf);
>   		}
> +		wt_status_state_free_buffers(&state);
> +
> +		if (wt_status_check_bisect(wt, &state) &&
> +		    state.bisecting_from) {
> +			strbuf_reset(&ref);
> +			strbuf_addf(&ref, "refs/heads/%s", state.bisecting_from);
> +			add_worktree_entry(map, wt, ref.buf);
> +		}
> +		wt_status_state_free_buffers(&state);
>   	}
> +	strbuf_release(&ref);
>   }
>   
>   static void lazy_init_worktree_map(void)
> diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
> index 7c44186341..e04599392d 100755
> --- a/t/t3203-branch-output.sh
> +++ b/t/t3203-branch-output.sh
> @@ -442,6 +442,41 @@ test_expect_success 'worktree colors correct' '
>   	test_cmp expect actual
>   '
>   
> +test_expect_success 'branch being rebased in a worktree is marked' '
> +	test_when_finished "git -C rebase_dir rebase --abort; rm -rf rebase_dir; git worktree prune; git branch -D rebasing" &&
> +	git worktree add -b rebasing rebase_dir branch-two &&
> +	git -C rebase_dir commit --allow-empty -m rebase-me &&
> +	test_must_fail git -C rebase_dir rebase -f -x false HEAD~1 &&
> +	test_must_fail git -C rebase_dir symbolic-ref -q HEAD &&
> +	git branch --list rebasing >actual &&
> +	echo "+ rebasing" >expect &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'branch being rebased is marked in its own worktree' '
> +	test_when_finished "git -C rebase_dir rebase --abort; rm -rf rebase_dir; git worktree prune; git branch -D rebasing" &&
> +	git worktree add -b rebasing rebase_dir branch-two &&
> +	git -C rebase_dir commit --allow-empty -m rebase-me &&
> +	test_must_fail git -C rebase_dir rebase -f -x false HEAD~1 &&
> +	git -C rebase_dir branch --list rebasing >actual &&
> +	echo "+ rebasing" >expect &&
> +	test_cmp expect actual &&
> +	git -C rebase_dir branch >actual &&
> +	test_grep "^\* (no branch, rebasing rebasing)" actual
> +'
> +
> +test_expect_success 'branch being bisected in a worktree is marked' '
> +	test_when_finished "git -C bisect_dir bisect reset; rm -rf bisect_dir; git worktree prune; git branch -D bisecting" &&
> +	git worktree add -b bisecting bisect_dir branch-two &&
> +	git -C bisect_dir commit --allow-empty -m bisect-one &&
> +	git -C bisect_dir commit --allow-empty -m bisect-two &&
> +	git -C bisect_dir bisect start HEAD HEAD~2 &&
> +	test_must_fail git -C bisect_dir symbolic-ref -q HEAD &&
> +	git branch --list bisecting >actual &&
> +	echo "+ bisecting" >expect &&
> +	test_cmp expect actual
> +'
> +
>   test_expect_success "set up color tests" '
>   	echo "<RED>main<RESET>" >expect.color &&
>   	echo "main" >expect.bare &&
> diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
> index 7f060d97bf..d656901f21 100755
> --- a/t/t6302-for-each-ref-filter.sh
> +++ b/t/t6302-for-each-ref-filter.sh
> @@ -3,6 +3,7 @@
>   test_description='test for-each-refs usage of ref-filter APIs'
>   
>   . ./test-lib.sh
> +. "$TEST_DIRECTORY"/lib-rebase.sh
>   . "$TEST_DIRECTORY"/lib-gpg.sh
>   
>   test_expect_success 'setup some history and refs' '
> @@ -541,6 +542,68 @@ test_expect_success 'validate worktree atom' '
>   	test_cmp expect actual
>   '
>   
> +test_expect_success 'worktree atom names a worktree rebasing the branch' '
> +	test_when_finished "git -C rebasing rebase --abort; rm -rf rebasing; git worktree prune; git branch -D rebasing" &&
> +	git worktree add -b rebasing rebasing main &&
> +	git -C rebasing commit --allow-empty -m rebase-me &&
> +	test_must_fail git -C rebasing rebase -f -x false main &&
> +	test_must_fail git -C rebasing symbolic-ref -q HEAD &&
> +	echo "$(pwd)/rebasing" >expect &&
> +	git for-each-ref --format="%(worktreepath)" refs/heads/rebasing >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'worktree atom names a worktree bisecting the branch' '
> +	test_when_finished "git -C bisecting bisect reset; rm -rf bisecting; git worktree prune; git branch -D bisecting" &&
> +	git worktree add -b bisecting bisecting main &&
> +	git -C bisecting commit --allow-empty -m bisect-one &&
> +	git -C bisecting commit --allow-empty -m bisect-two &&
> +	git -C bisecting bisect start HEAD main &&
> +	test_must_fail git -C bisecting symbolic-ref -q HEAD &&
> +	echo "$(pwd)/bisecting" >expect &&
> +	git for-each-ref --format="%(worktreepath)" refs/heads/bisecting >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'worktree atom with an interactive rebase stopped at edit' '
> +	test_when_finished "git -C editing rebase --abort; rm -rf editing; git worktree prune; git branch -D editing" &&
> +	git worktree add -b editing editing main &&
> +	git -C editing commit --allow-empty -m edit-me &&
> +	(
> +		set_fake_editor &&
> +		FAKE_LINES="edit 1" git -C editing rebase -i --keep-empty HEAD~1
> +	) &&
> +	test_must_fail git -C editing symbolic-ref -q HEAD &&
> +	echo "$(pwd)/editing" >expect &&
> +	git for-each-ref --format="%(worktreepath)" refs/heads/editing >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'worktree atom ignores branches rebase --update-refs will move' '
> +	test_when_finished "git -C updating rebase --abort; rm -rf updating; git worktree prune; git branch -D updating mid" &&
> +	git worktree add -b updating updating main &&
> +	git -C updating commit --allow-empty -m one &&
> +	git -C updating branch mid &&
> +	git -C updating commit --allow-empty -m two &&
> +	test_must_fail git -C updating rebase -f --update-refs -x false main &&
> +	git for-each-ref --format="%(refname:short) [%(worktreepath)]" \
> +		refs/heads/updating refs/heads/mid >actual &&
> +	cat >expect <<-EOF &&
> +	mid []
> +	updating [$(pwd)/updating]
> +	EOF
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'worktree atom ignores a rebase of a detached HEAD' '
> +	test_when_finished "git -C detached-rebase rebase --abort; rm -rf detached-rebase; git worktree prune" &&
> +	git worktree add --detach detached-rebase main &&
> +	git -C detached-rebase commit --allow-empty -m detached &&
> +	test_must_fail git -C detached-rebase rebase -f -x false main &&
> +	git for-each-ref --format="%(worktreepath)" refs/heads/ >actual &&
> +	test_grep ! detached-rebase actual
> +'
> +
>   test_expect_success 'start after with empty value' '
>   	cat >expect <<-\EOF &&
>   	refs/heads/main


  reply	other threads:[~2026-11-02 14:06 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-21 17:33 [RFC PATCH 0/1] worktree list: add --merged and --no-merged Jasper Mayone
2026-10-21 17:33 ` [RFC PATCH 1/1] " Jasper Mayone
2026-10-23 18:30   ` Junio C Hamano
2026-10-24  6:25     ` Eric Sunshine
2026-10-24 16:53       ` [PATCH v2] worktree list: add --branch to select worktrees by branch Jasper Mayone
2026-10-25 14:47         ` Phillip Wood
2026-10-26 11:02           ` Phillip Wood
2026-10-28  6:02           ` Eric Sunshine
2026-10-28 15:27             ` Junio C Hamano
2026-10-28 11:32         ` Kristoffer Haugsbakk
2026-10-29  4:46           ` Jasper Mayone
2026-10-29  5:01             ` Eric Sunshine
2026-10-29 11:14             ` Phillip Wood
2026-10-29 14:23               ` Ben Knoble
2026-10-29 16:10                 ` Phillip Wood
2026-10-30 14:21         ` [PATCH 0/2] worktree: treat a branch being rebased or bisected as checked out Jasper Mayone
2026-10-30 14:21           ` [PATCH 1/2] worktree remove: refuse a worktree in the middle of a rebase or bisect Jasper Mayone
2026-11-02 14:07             ` Phillip Wood
2026-11-02 17:57               ` Jasper Mayone
2026-10-30 14:21           ` [PATCH 2/2] ref-filter: name the worktree of a branch being rebased or bisected Jasper Mayone
2026-11-02 14:06             ` Phillip Wood [this message]
2026-11-02 17:57               ` Jasper Mayone
2026-10-30 22:55           ` [PATCH 0/2] worktree: treat a branch being rebased or bisected as checked out Junio C Hamano
2026-10-30 23:06             ` Jasper Mayone
2026-11-02 18:29           ` [PATCH v4 0/4] " Jasper Mayone
2026-11-02 18:29             ` [PATCH v4 1/4] worktree remove: refuse a worktree in the middle of an operation Jasper Mayone
2026-11-02 18:29             ` [PATCH v4 2/4] doc: for-each-ref: say how to read %(worktreepath) safely Jasper Mayone
2026-11-02 18:29             ` [PATCH v4 3/4] ref-filter: name the worktree of a branch being rebased or bisected Jasper Mayone
2026-11-02 18:29             ` [PATCH v4 4/4] branch: do not mark the branch the current worktree is rebasing Jasper Mayone
2026-10-21 21:36 ` [RFC PATCH 0/1] worktree list: add --merged and --no-merged Harald Nordgren
2026-10-21 22:12   ` Junio C Hamano
2026-10-22  7:21     ` Harald Nordgren
2026-10-22 17:58   ` Kristoffer Haugsbakk

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=37d53d40-66b6-46bb-a38d-0d3534ca112a@gmail.com \
    --to=phillip.wood123@gmail.com \
    --cc=ben.knoble@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=haraldnordgren@gmail.com \
    --cc=kristofferhaugsbakk@fastmail.com \
    --cc=me@jaspermayone.com \
    --cc=phillip.wood@dunelm.org.uk \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox