Re: [RFC][PATCH 1/2] worktree: represent the primary worktree with '/' instead of NULL
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 13, 2026, 21:35 UTC
- Message-ID
- <xmqq7bsgl42j.fsf@gitster.g>
- In-Reply-To
- <20260213120529.15475-2-shreyanshpaliwalcmsmn@gmail.com>
Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com> writes:
Show 12 quoted lines
> diff --git a/worktree.c b/worktree.c
> index 9308389cb6..b29934407f 100644
> --- a/worktree.c
> +++ b/worktree.c
> @@ -101,6 +101,7 @@ static struct worktree *get_main_worktree(int skip_reading_head)
>
> CALLOC_ARRAY(worktree, 1);
> worktree->repo = the_repository;
> + worktree->id = xstrdup("/");
> worktree->path = strbuf_detach(&worktree_path, NULL);
> worktree->is_current = is_current_worktree(worktree);
> worktree->is_bare = (is_bare_repository_cfg == 1) ||Presumably we left .id = NULL from CALLOC_ARRAY(), so this looks sensible. When releasing resources from an instance of worktree, we'd blindly free(worktree->id) and in the old world, free(NULL) turned into no-op, and this xstrdup()'d copy will be freed in the new world, so there is nothing funny here, I hope? This one, and the change to is_main_worktree() go together.
Show 6 quoted lines
> @@ -127,6 +128,8 @@ struct worktree *get_linked_worktree(const char *id,
>
> if (!id)
> die("Missing linked worktree name");
> + if (!strcmp(id, "/"))
> + die("'/' is reserved for primary worktree");Makes me wonder if this is a BUG not die; where does id come from?
... goes and looks ...
The only caller is worktree.c:get_worktrees_internal() and it is feeding d->d_name that came from readdir_skip_dot_and_dotdot(), so it cannot be "/".
By the way, I suspect that get_linked_worktree() should become file-scope static, as there is no other caller.
Show 11 quoted lines
> @@ -206,9 +209,7 @@ struct worktree **get_worktrees_without_reading_head(void)
>
> char *get_worktree_git_dir(const struct worktree *wt)
> {
> - if (!wt)
> - return xstrdup(repo_get_git_dir(the_repository));
> - else if (!wt->id)
> + if (is_main_worktree(wt))
> return xstrdup(repo_get_common_dir(the_repository));
> else
> return repo_common_path(the_repository, "worktrees/%s", wt->id);Good spotting. This series needs to spot any and all places that use these other conventions (i.e. wt->id == NULL) to identify the primary worktree and rewrite them to call is_main_worktree(), which may be a chore, but once it is done, it would become a lot easier to follow the resulting code.
Show 7 quoted lines
> @@ -277,7 +278,7 @@ struct worktree *find_worktree_by_path(struct worktree **list, const char *p)
>
> int is_main_worktree(const struct worktree *wt)
> {
> - return !wt->id;
> + return !strcmp(wt->id, "/");
> }OK.
Show 6 quoted lines
> @@ -566,7 +567,7 @@ void strbuf_worktree_ref(const struct worktree *wt,
> {
> if (parse_worktree_ref(refname, NULL, NULL, NULL) ==
> REF_WORKTREE_CURRENT &&
> - wt && !wt->is_current) {
> + !wt->is_current) {OK.
Show 6 quoted lines
> @@ -629,6 +630,9 @@ static void repair_gitfile(struct worktree *wt, > char *path = NULL; > int err; > > + if (is_main_worktree(wt)) > + goto done;
This is a bit new.
The original did not say
if (!wt || !wt->id || !strcmp(wt->id, "/")) goto done;
The only caller is iterating over the resulting list of worktrees returned from get_worktrees_internal(1) *BUT* it already skips the primary worktree (the function MUST return the primary one as the first one, and the callers MUST be aware of the convention).
So I am not sure if the new check is even needed. Or rather, this ...
if (!wt || !wt->id || !strcmp(wt->id, "/"))
BUG("why are you feeding me the primary worktree???");
... might be more appropriate, perhaps? I dunno.> /* missing worktree can't be repaired */ > if (!file_exists(wt->path)) > goto done;