From: Phillip Wood <phillip.wood123@gmail•com>
To: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail•com>, git@vger•kernel.org
Cc: sunshine@sunshineco•com, gitster@pobox•com, karthik.188@gmail•com
Subject: Re: [RFC][PATCH 2/2] worktree: stop passing NULL as primary worktree
Date: Mon, 16 Feb 2026 16:18:58 +0000 [thread overview]
Message-ID: <66b0f03a-36ab-4305-814e-6d964f5d33c4@gmail.com> (raw)
In-Reply-To: <20260215090815.46544-1-shreyanshpaliwalcmsmn@gmail.com>
On 15/02/2026 08:56, Shreyansh Paliwal wrote:
>> I've cc'd Eric for a second opinion
>>
>> On 13/02/2026 22:29, Junio C Hamano wrote:
>>> Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail•com> writes:
>>>
>>>> diff --git a/path.c b/path.c
>>>> index d726537622..4ac86e1e58 100644
>>>> --- a/path.c
>>>> +++ b/path.c
>>>> @@ -408,9 +408,7 @@ static void strbuf_worktree_gitdir(struct strbuf *buf,
>>>> const struct repository *repo,
>>>> const struct worktree *wt)
>>>> {
>>>> - if (!wt)
>>>> - strbuf_addstr(buf, repo->gitdir);
>>>> - else if (!wt->id)
>>>> + if (is_main_worktree(wt))
>>>> strbuf_addstr(buf, repo->commondir);
>>>> else
>>>> repo_common_path_append(repo, buf, "worktrees/%s", wt->id);
>>>
>>> This is curious.
>>>
>>> We used to treat "wt==NULL" and "wt->id==NULL" differently. Now we
>>> use repo->commondir for both. For the primary worktree, it ought to
>>> be the same as repo->gitdir, so it should not matter, but makes me
>>> wonder what the reason behind this difference in the original.
>>>
>>> We have been assuming that wt==NULL and wt->id==NULL both meant the
>>> same thing: "we are talking about the primary worktree". But the
>>> code around here before this patch seems to behave differently. Is
>>> our assumption incorrect and are we making a mistake by conflating
>>> these two conditions into one?
>>
>> My understanding is that wt==NULL means "use the current worktree" and
>> wt->id==NULL means "this is the main worktree". That would explain why
>> we use repo->gitdir above when wt==NULL and repo->commondir when
>> wt->id==NULL, as repo->gitdir is the gitdir of the current worktree and
>> repo->commondir will be the gitdir of the main worktree. If we look at
>> the code in wt-status.c that's passing a NULL worktree it wants to know
>> about the status of the current worktree, not the main worktree.
>>
>> I think that we should add a new function
>>
>> struct worktree *get_current_worktree(struct repository*);
>>
>> to worktree.c that constructs a struct worktree using repo->gitdir etc.
>> The worktree id is the last path component of repo->gitdir when the
>> repo->gitdir and repo->commondir differ, otherwise it is NULL. Then we
>> can use that function to get the current worktree rather than passing
>> NULL when we call wt_status_check_{rebase,bisect} from
>> wt_status_get_state(). We should also think about whether we should
>> change wt_status_get_state() to take a "struct worktree*" rather than a
>> "struct repository*" instead (I've not looked at the callers to see if
>> that's sensible).
>>
>> With that, we can gradually clean up uses of wt==NULL in the rest of the
>> codebase overtime and eventually remove support for it from worktree.c
>> rather than having a big flag-day patch. I don't think we need to change
>> uses of wt-id==NULL.
>
> Thanks a lot for clarifying. This helps solve the doubt regarding the
> different usage of !wt and !wt->id in strbuf_worktree_gitdir(). I realize
> we have been under the wrong assumption about what wt == NULL represents.
>
> But I still have a few points where I’m a bit confused,
>
> If wt == NULL is meant to represent the current worktree, then what role
> wt->is_current plays in the present implementation, and if they both
> represent the same thing then wt->is_current wouldn't make sense if wt is
> already NULL in the case of a current worktree.
wt == NULL is a shorthand that callers can use if they don't have a
struct worktree to pass, it does not replace wt->is_current when listing
all worktrees with get_worktrees() which returns a NULL terminated list.
> Beyond representation, I’m not quite understanding on how call sites are
> logically differentiating on whether the intent is to 'operate on the
> worktree we are in' or 'operate on the primary one'.
We're nearly always interested in the current one. The primary worktree
is special in that it cannot be moved or deleted with "git worktree" but
git commands generally operate on the current worktree and occasionally
check the state of other worktrees (for example to avoid checking out
the same branch in two different worktrees).
> And I think if we included both in struct repository (r->main_wt, r->current_wt)
> so accessing either of them would be a whole lot easier and also would
> prevent confusion in the future.
It might be worth adding the current worktree (or probably the worktree
that the struct repository refers to) to struct repository in the future
but I think that is outside the scope of cleaning up wt-status.c
Thanks
Phillip
> Let me know what you think.
>
> Best,
> Shreyansh
>
next prev parent reply other threads:[~2026-02-16 16:19 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-02-13 11:59 [RFC][PATCH 0/2] worktree: change representation and usage of primary worktree Shreyansh Paliwal
2026-02-13 11:59 ` [RFC][PATCH 1/2] worktree: represent the primary worktree with '/' instead of NULL Shreyansh Paliwal
2026-02-13 21:35 ` Junio C Hamano
2026-02-14 9:54 ` Shreyansh Paliwal
2026-02-13 11:59 ` [RFC][PATCH 2/2] worktree: stop passing NULL as primary worktree Shreyansh Paliwal
2026-02-13 22:29 ` Junio C Hamano
2026-02-14 9:59 ` Shreyansh Paliwal
2026-02-14 14:30 ` Phillip Wood
2026-02-14 15:34 ` Junio C Hamano
2026-02-15 8:56 ` Shreyansh Paliwal
2026-02-16 16:18 ` Phillip Wood [this message]
2026-02-17 5:21 ` Junio C Hamano
2026-02-17 10:09 ` Shreyansh Paliwal
2026-02-16 16:18 ` [PATCH 0/2] worktree_git_path(): remove repository argument Phillip Wood
2026-02-16 16:18 ` [PATCH 1/2] wt-status: avoid passing NULL worktree Phillip Wood
2026-02-17 9:23 ` Phillip Wood
2026-02-17 10:18 ` Shreyansh Paliwal
2026-02-17 15:20 ` Phillip Wood
2026-02-17 16:38 ` Shreyansh Paliwal
2026-02-17 18:29 ` Junio C Hamano
2026-02-17 17:46 ` Karthik Nayak
2026-02-18 14:19 ` Phillip Wood
2026-02-17 18:47 ` Junio C Hamano
2026-02-18 14:18 ` Phillip Wood
2026-02-16 16:18 ` [PATCH 2/2] path: remove repository argument from worktree_git_path() Phillip Wood
2026-02-17 17:48 ` Karthik Nayak
2026-02-17 10:12 ` [PATCH 0/2] worktree_git_path(): remove repository argument Shreyansh Paliwal
2026-02-17 15:22 ` Phillip Wood
2026-02-17 16:45 ` Shreyansh Paliwal
2026-02-19 14:26 ` [PATCH v2 " Phillip Wood
2026-02-19 14:26 ` [PATCH v2 1/2] wt-status: avoid passing NULL worktree Phillip Wood
2026-02-19 19:30 ` Junio C Hamano
2026-02-19 20:37 ` Junio C Hamano
2026-02-25 16:39 ` Phillip Wood
2026-02-25 17:11 ` Junio C Hamano
2026-02-26 16:09 ` Phillip Wood
2026-02-26 16:15 ` Junio C Hamano
2026-02-19 14:26 ` [PATCH v2 2/2] path: remove repository argument from worktree_git_path() Phillip Wood
2026-02-19 19:34 ` Junio C Hamano
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=66b0f03a-36ab-4305-814e-6d964f5d33c4@gmail.com \
--to=phillip.wood123@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--cc=karthik.188@gmail$(echo .)com \
--cc=phillip.wood@dunelm$(echo .)org.uk \
--cc=shreyanshpaliwalcmsmn@gmail$(echo .)com \
--cc=sunshine@sunshineco$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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