{"thread":{"id":"64989","subject":"[RFC][PATCH 0/2] worktree: change representation and usage of primary worktree","startedAt":"2026-02-13T12:06:23Z","lastAt":"2026-02-26T16:15:06Z","messageCount":39,"participants":["Shreyansh Paliwal","Junio C Hamano","Phillip Wood","Karthik Nayak"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"535922","messageId":"20260213120529.15475-1-shreyanshpaliwalcmsmn@gmail.com","threadId":"64989","inReplyTo":null,"subject":"[RFC][PATCH 0/2] worktree: change representation and usage of primary worktree","fromName":"Shreyansh Paliwal","fromEmail":"shreyanshpaliwalcmsmn@gmail.com","sentAt":"2026-02-13T11:59:52Z","receivedAt":"2026-02-13T12:06:23Z","isPatch":true,"sender":{"key":"shreyanshpaliwalcmsmn@gmail.com","avatar":"https://avatars.githubusercontent.com/u/152720574?v=4"},"body":"While working on reducing global state in wt-status [1], it became clear\nthat some cleanup in the worktree API would be helpful before moving ahead.\n\nAs of now Primary worktree is represented in three ways,\n\n* by passing NULL as a worktree\n* by a struct worktree whose wt->id is NULL, and\n* in the ref-store map, by using \"/\" as the key.\n\nThis creates ambiguity, callers and helpers often need extra checks for a\nNULL worktree, and the implicit NULL form makes it harder to rely on\nfields like wt->repo without falling back to global state.\n\nSo this patch series involves making the worktree API more robust by\nremoving the usage of NULL as the representation and using wt->id = '/'\nas the marker for a primary worktree.\n\nIn patch 1/2, change the internal representation in worktree.c so that the\nmain worktree is identified by '/' instead of NULL.\n\nIn patch 2/2, update the API usage by modifying callers to obtain the\nprimary worktree via helper and pass an actual struct worktree rather than\nNULL.\n\nI would like to get feedback whether this implementation is in the right direction,\nand if there is anything else that I should be doing in this cleanup series.\n\n[1]- https://lore.kernel.org/git/20260209134439.14492-1-shreyanshpaliwalcmsmn@gmail.com/T/#u\n\nShreyansh Paliwal (2):\n  worktree: represent the primary worktree with \"/\" instead of NULL\n  worktree: stop passing NULL as primary worktree\n\n builtin/fsck.c     |  2 +-\n builtin/worktree.c | 10 +++++++---\n path.c             | 27 +++++++++++++--------------\n path.h             |  9 +++------\n refs.c             |  4 ++--\n revision.c         |  6 ++++--\n worktree.c         | 43 +++++++++++++++++++++++++------------------\n worktree.h         |  7 +++++++\n wt-status.c        | 22 ++++++++++++----------\n 9 files changed, 74 insertions(+), 56 deletions(-)\n\n--\n2.53.0\n"},{"id":"535923","messageId":"20260213120529.15475-2-shreyanshpaliwalcmsmn@gmail.com","threadId":"64989","inReplyTo":"20260213120529.15475-1-shreyanshpaliwalcmsmn@gmail.com","subject":"[RFC][PATCH 1/2] worktree: represent the primary worktree with '/' instead of NULL","fromName":"Shreyansh Paliwal","fromEmail":"shreyanshpaliwalcmsmn@gmail.com","sentAt":"2026-02-13T11:59:53Z","receivedAt":"2026-02-13T12:06:32Z","isPatch":true,"sender":{"key":"shreyanshpaliwalcmsmn@gmail.com","avatar":"https://avatars.githubusercontent.com/u/152720574?v=4"},"body":"The worktree API uses NULL to represent the primary worktree. As a result,\nmany callers pass NULL to implicitly refer to it, which in turn requires\nadditional checks to ensure that `wt` is defined before use.\n\nRepresent the main worktree explicitly by setting `wt->id` to \"/\" in\n`get_main_worktree()` and update `is_main_worktree()` accordingly. Replace\nchecks for wt to be defined in worktree.c functions with calls to\n`is_main_worktree()` (or strcmp(wt->id, \"/\")).\n\nSigned-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>\n---\n worktree.c | 14 +++++++++-----\n 1 file changed, 9 insertions(+), 5 deletions(-)\n\ndiff --git a/worktree.c b/worktree.c\nindex 9308389cb6..b29934407f 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -101,6 +101,7 @@ static struct worktree *get_main_worktree(int skip_reading_head)\n \n \tCALLOC_ARRAY(worktree, 1);\n \tworktree->repo = the_repository;\n+\tworktree->id = xstrdup(\"/\");\n \tworktree->path = strbuf_detach(&worktree_path, NULL);\n \tworktree->is_current = is_current_worktree(worktree);\n \tworktree->is_bare = (is_bare_repository_cfg == 1) ||\n@@ -127,6 +128,8 @@ struct worktree *get_linked_worktree(const char *id,\n \n \tif (!id)\n \t\tdie(\"Missing linked worktree name\");\n+\tif (!strcmp(id, \"/\"))\n+\t\tdie(\"'/' is reserved for primary worktree\");\n \n \trepo_common_path_append(the_repository, &path, \"worktrees/%s/gitdir\", id);\n \tif (strbuf_read_file(&worktree_path, path.buf, 0) <= 0)\n@@ -206,9 +209,7 @@ struct worktree **get_worktrees_without_reading_head(void)\n \n char *get_worktree_git_dir(const struct worktree *wt)\n {\n-\tif (!wt)\n-\t\treturn xstrdup(repo_get_git_dir(the_repository));\n-\telse if (!wt->id)\n+\tif (is_main_worktree(wt))\n \t\treturn xstrdup(repo_get_common_dir(the_repository));\n \telse\n \t\treturn repo_common_path(the_repository, \"worktrees/%s\", wt->id);\n@@ -277,7 +278,7 @@ struct worktree *find_worktree_by_path(struct worktree **list, const char *p)\n \n int is_main_worktree(const struct worktree *wt)\n {\n-\treturn !wt->id;\n+\treturn !strcmp(wt->id, \"/\");\n }\n \n const char *worktree_lock_reason(struct worktree *wt)\n@@ -566,7 +567,7 @@ void strbuf_worktree_ref(const struct worktree *wt,\n {\n \tif (parse_worktree_ref(refname, NULL, NULL, NULL) ==\n \t\t    REF_WORKTREE_CURRENT &&\n-\t    wt && !wt->is_current) {\n+\t    !wt->is_current) {\n \t\tif (is_main_worktree(wt))\n \t\t\tstrbuf_addstr(sb, \"main-worktree/\");\n \t\telse\n@@ -629,6 +630,9 @@ static void repair_gitfile(struct worktree *wt,\n \tchar *path = NULL;\n \tint err;\n \n+\tif (is_main_worktree(wt))\n+\t\tgoto done;\n+\n \t/* missing worktree can't be repaired */\n \tif (!file_exists(wt->path))\n \t\tgoto done;\n-- \n2.53.0\n\n"},{"id":"535924","messageId":"20260213120529.15475-3-shreyanshpaliwalcmsmn@gmail.com","threadId":"64989","inReplyTo":"20260213120529.15475-1-shreyanshpaliwalcmsmn@gmail.com","subject":"[RFC][PATCH 2/2] worktree: stop passing NULL as primary worktree","fromName":"Shreyansh Paliwal","fromEmail":"shreyanshpaliwalcmsmn@gmail.com","sentAt":"2026-02-13T11:59:54Z","receivedAt":"2026-02-13T12:06:35Z","isPatch":true,"sender":{"key":"shreyanshpaliwalcmsmn@gmail.com","avatar":"https://avatars.githubusercontent.com/u/152720574?v=4"},"body":"Some functions like repo_git_pathv() in path.c, wt_status_check_rebase()\nand wt_status_check_bisect() in wt-status.c, get_worktree_git_dir() in\nbuiltin/worktree.c are called with a NULL worktree. This makes it difficult\nto access fields like `wt->repo` and add extra handling for checking wt is\ndefined.\n\nAdd a helper function of get_worktrees_internal() as get_worktrees_repo()\nand pass struct repository down the callstack to get_main_worktree()\nfunction so that the primary worktree of a specific repository can be\naccessed instead of just the_repository.\n\nAccess the primary worktree by the help of get_worktrees_repo() and pass it\nto the functions as struct worktree instead of passing NULL.\n\nFurther `worktree_git_path()` no longer needs a separate\n`struct repository *` parameter. Use `wt->repo` instead and update its\ncallers accordingly.\n\nwhile at, replace any unecessary checks for wt to be defined with\nis_main_worktree(wt).\n\nSigned-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>\n---\n builtin/fsck.c     |  2 +-\n builtin/worktree.c | 10 +++++++---\n path.c             | 27 +++++++++++++--------------\n path.h             |  9 +++------\n refs.c             |  4 ++--\n revision.c         |  6 ++++--\n worktree.c         | 29 ++++++++++++++++-------------\n worktree.h         |  7 +++++++\n wt-status.c        | 22 ++++++++++++----------\n 9 files changed, 65 insertions(+), 51 deletions(-)\n\ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex 0512f78a87..42ba0afb91 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -1137,7 +1137,7 @@ int cmd_fsck(int argc,\n \t\t\t * and may get overwritten by other calls\n \t\t\t * while we're examining the index.\n \t\t\t */\n-\t\t\tpath = xstrdup(worktree_git_path(the_repository, wt, \"index\"));\n+\t\t\tpath = xstrdup(worktree_git_path(wt, \"index\"));\n \t\t\twt_gitdir = get_worktree_git_dir(wt);\n \n \t\t\tread_index_from(&istate, path, wt_gitdir);\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex fbdaf2eb2e..27c5889c89 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -328,6 +328,8 @@ static void check_candidate_path(const char *path,\n \twt = find_worktree_by_path(worktrees, path);\n \tif (!wt)\n \t\treturn;\n+\tif(is_main_worktree(wt))\n+\t\tdie(_(\"'%s' is the main worktree\"), path);\n \n \tlocked = !!worktree_lock_reason(wt);\n \tif ((!locked && force) || (locked && force > 1)) {\n@@ -660,7 +662,8 @@ static int can_use_local_refs(const struct add_opts *opts)\n \t\tif (!opts->quiet) {\n \t\t\tstruct strbuf path = STRBUF_INIT;\n \t\t\tstruct strbuf contents = STRBUF_INIT;\n-\t\t\tchar *wt_gitdir = get_worktree_git_dir(NULL);\n+\t\t\tstruct worktree **worktrees = get_worktrees_repo(the_repository);\n+\t\t\tchar *wt_gitdir = get_worktree_git_dir(worktrees[0]);\n \n \t\t\tstrbuf_add_real_path(&path, wt_gitdir);\n \t\t\tstrbuf_addstr(&path, \"/HEAD\");\n@@ -675,6 +678,7 @@ static int can_use_local_refs(const struct add_opts *opts)\n \t\t\tstrbuf_release(&path);\n \t\t\tstrbuf_release(&contents);\n \t\t\tfree(wt_gitdir);\n+\t\t\tfree_worktrees(worktrees);\n \t\t}\n \t\treturn 1;\n \t}\n@@ -1191,14 +1195,14 @@ static void validate_no_submodules(const struct worktree *wt)\n \n \twt_gitdir = get_worktree_git_dir(wt);\n \n-\tif (is_directory(worktree_git_path(the_repository, wt, \"modules\"))) {\n+\tif (is_directory(worktree_git_path(wt, \"modules\"))) {\n \t\t/*\n \t\t * There could be false positives, e.g. the \"modules\"\n \t\t * directory exists but is empty. But it's a rare case and\n \t\t * this simpler check is probably good enough for now.\n \t\t */\n \t\tfound_submodules = 1;\n-\t} else if (read_index_from(&istate, worktree_git_path(the_repository, wt, \"index\"),\n+\t} else if (read_index_from(&istate, worktree_git_path(wt, \"index\"),\n \t\t\t\t   wt_gitdir) > 0) {\n \t\tfor (i = 0; i < istate.cache_nr; i++) {\n \t\t\tstruct cache_entry *ce = istate.cache[i];\ndiff --git a/path.c b/path.c\nindex d726537622..4ac86e1e58 100644\n--- a/path.c\n+++ b/path.c\n@@ -408,9 +408,7 @@ static void strbuf_worktree_gitdir(struct strbuf *buf,\n \t\t\t\t   const struct repository *repo,\n \t\t\t\t   const struct worktree *wt)\n {\n-\tif (!wt)\n-\t\tstrbuf_addstr(buf, repo->gitdir);\n-\telse if (!wt->id)\n+\tif (is_main_worktree(wt))\n \t\tstrbuf_addstr(buf, repo->commondir);\n \telse\n \t\trepo_common_path_append(repo, buf, \"worktrees/%s\", wt->id);\n@@ -426,7 +424,7 @@ static void repo_git_pathv(struct repository *repo,\n \t\tstrbuf_addch(buf, '/');\n \tgitdir_len = buf->len;\n \tstrbuf_vaddf(buf, fmt, args);\n-\tif (!wt)\n+\tif (is_main_worktree(wt))\n \t\tadjust_git_path(repo, buf, gitdir_len);\n \tstrbuf_cleanup_path(buf);\n }\n@@ -437,8 +435,10 @@ char *repo_git_path(struct repository *repo,\n \tstruct strbuf path = STRBUF_INIT;\n \tva_list args;\n \tva_start(args, fmt);\n-\trepo_git_pathv(repo, NULL, &path, fmt, args);\n+\tstruct worktree **worktrees = get_worktrees_repo(repo);\n+\trepo_git_pathv(repo, worktrees[0], &path, fmt, args);\n \tva_end(args);\n+\tfree_worktrees(worktrees);\n \treturn strbuf_detach(&path, NULL);\n }\n \n@@ -448,8 +448,10 @@ const char *repo_git_path_append(struct repository *repo,\n {\n \tva_list args;\n \tva_start(args, fmt);\n-\trepo_git_pathv(repo, NULL, sb, fmt, args);\n+\tstruct worktree **worktrees = get_worktrees_repo(repo);\n+\trepo_git_pathv(repo, worktrees[0], sb, fmt, args);\n \tva_end(args);\n+\tfree_worktrees(worktrees);\n \treturn sb->buf;\n }\n \n@@ -460,8 +462,10 @@ const char *repo_git_path_replace(struct repository *repo,\n \tva_list args;\n \tstrbuf_reset(sb);\n \tva_start(args, fmt);\n-\trepo_git_pathv(repo, NULL, sb, fmt, args);\n+\tstruct worktree **worktrees = get_worktrees_repo(repo);\n+\trepo_git_pathv(repo, worktrees[0], sb, fmt, args);\n \tva_end(args);\n+\tfree_worktrees(worktrees);\n \treturn sb->buf;\n }\n \n@@ -486,17 +490,12 @@ const char *mkpath(const char *fmt, ...)\n \treturn cleanup_path(pathname->buf);\n }\n \n-const char *worktree_git_path(struct repository *r,\n-\t\t\t      const struct worktree *wt, const char *fmt, ...)\n+const char *worktree_git_path(const struct worktree *wt, const char *fmt, ...)\n {\n \tstruct strbuf *pathname = get_pathname();\n \tva_list args;\n-\n-\tif (wt && wt->repo != r)\n-\t\tBUG(\"worktree not connected to expected repository\");\n-\n \tva_start(args, fmt);\n-\trepo_git_pathv(r, wt, pathname, fmt, args);\n+\trepo_git_pathv(wt->repo, wt, pathname, fmt, args);\n \tva_end(args);\n \treturn pathname->buf;\n }\ndiff --git a/path.h b/path.h\nindex 0ec95a0b07..54e09b2883 100644\n--- a/path.h\n+++ b/path.h\n@@ -66,13 +66,10 @@ const char *repo_git_path_replace(struct repository *repo,\n \n /*\n  * Similar to repo_git_path() but can produce paths for a specified\n- * worktree instead of current one. When no worktree is given, then the path is\n- * computed relative to main worktree of the given repository.\n+ * worktree instead of current one.\n  */\n-const char *worktree_git_path(struct repository *r,\n-\t\t\t      const struct worktree *wt,\n-\t\t\t      const char *fmt, ...)\n-\t__attribute__((format (printf, 3, 4)));\n+const char *worktree_git_path(const struct worktree *wt, const char *fmt, ...)\n+\t__attribute__((format (printf, 2, 3)));\n \n /*\n  * The `repo_worktree_path` family of functions will construct a path into a\ndiff --git a/refs.c b/refs.c\nindex 627b7f8698..98df2235e7 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2324,12 +2324,12 @@ struct ref_store *get_worktree_ref_store(const struct worktree *wt)\n \tif (wt->is_current)\n \t\treturn get_main_ref_store(wt->repo);\n \n-\tid = wt->id ? wt->id : \"/\";\n+\tid = wt->id;\n \trefs = lookup_ref_store_map(&wt->repo->worktree_ref_stores, id);\n \tif (refs)\n \t\treturn refs;\n \n-\tif (wt->id) {\n+\tif (!is_main_worktree(wt)) {\n \t\tstruct strbuf common_path = STRBUF_INIT;\n \t\trepo_common_path_append(wt->repo, &common_path,\n \t\t\t\t\t\"worktrees/%s\", wt->id);\ndiff --git a/revision.c b/revision.c\nindex 9b131670f7..a9d4f796ed 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1730,12 +1730,14 @@ void add_reflogs_to_pending(struct rev_info *revs, unsigned flags)\n \n \tcb.all_revs = revs;\n \tcb.all_flags = flags;\n-\tcb.wt = NULL;\n+\tstruct worktree **worktrees = get_worktrees_repo(the_repository);\n+\tcb.wt = worktrees[0];\n \trefs_for_each_reflog(get_main_ref_store(the_repository),\n \t\t\t     handle_one_reflog, &cb);\n \n \tif (!revs->single_worktree)\n \t\tadd_other_reflogs_to_pending(&cb);\n+\tfree_worktrees(worktrees);\n }\n \n static void add_cache_tree(struct cache_tree *it, struct rev_info *revs,\n@@ -1847,7 +1849,7 @@ void add_index_objects_to_pending(struct rev_info *revs, unsigned int flags)\n \t\twt_gitdir = get_worktree_git_dir(wt);\n \n \t\tif (read_index_from(&istate,\n-\t\t\t\t    worktree_git_path(the_repository, wt, \"index\"),\n+\t\t\t\t    worktree_git_path(wt, \"index\"),\n \t\t\t\t    wt_gitdir) > 0)\n \t\t\tdo_add_index_objects_to_pending(revs, &istate, flags);\n \ndiff --git a/worktree.c b/worktree.c\nindex b29934407f..1059c18115 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -91,16 +91,16 @@ static int is_main_worktree_bare(struct repository *repo)\n /**\n  * get the main worktree\n  */\n-static struct worktree *get_main_worktree(int skip_reading_head)\n+static struct worktree *get_main_worktree(struct repository *repo, int skip_reading_head)\n {\n \tstruct worktree *worktree = NULL;\n \tstruct strbuf worktree_path = STRBUF_INIT;\n \n-\tstrbuf_add_real_path(&worktree_path, repo_get_common_dir(the_repository));\n+\tstrbuf_add_real_path(&worktree_path, repo_get_common_dir(repo));\n \tstrbuf_strip_suffix(&worktree_path, \"/.git\");\n \n \tCALLOC_ARRAY(worktree, 1);\n-\tworktree->repo = the_repository;\n+\tworktree->repo = repo;\n \tworktree->id = xstrdup(\"/\");\n \tworktree->path = strbuf_detach(&worktree_path, NULL);\n \tworktree->is_current = is_current_worktree(worktree);\n@@ -112,7 +112,7 @@ static struct worktree *get_main_worktree(int skip_reading_head)\n \t\t * This check is unnecessary if we're currently in the main worktree,\n \t\t * as prior checks already consulted all configs of the current worktree.\n \t\t */\n-\t\t(!worktree->is_current && is_main_worktree_bare(the_repository));\n+\t\t(!worktree->is_current && is_main_worktree_bare(repo));\n \n \tif (!skip_reading_head)\n \t\tadd_head_info(worktree);\n@@ -165,7 +165,7 @@ struct worktree *get_linked_worktree(const char *id,\n  * retrieving worktree metadata that could be used when the worktree is known\n  * to not be in a healthy state, e.g. when creating or repairing it.\n  */\n-static struct worktree **get_worktrees_internal(int skip_reading_head)\n+static struct worktree **get_worktrees_internal(struct repository *repo, int skip_reading_head)\n {\n \tstruct worktree **list = NULL;\n \tstruct strbuf path = STRBUF_INIT;\n@@ -175,9 +175,9 @@ static struct worktree **get_worktrees_internal(int skip_reading_head)\n \n \tALLOC_ARRAY(list, alloc);\n \n-\tlist[counter++] = get_main_worktree(skip_reading_head);\n+\tlist[counter++] = get_main_worktree(repo, skip_reading_head);\n \n-\tstrbuf_addf(&path, \"%s/worktrees\", repo_get_common_dir(the_repository));\n+\tstrbuf_addf(&path, \"%s/worktrees\", repo_get_common_dir(repo));\n \tdir = opendir(path.buf);\n \tstrbuf_release(&path);\n \tif (dir) {\n@@ -199,12 +199,15 @@ static struct worktree **get_worktrees_internal(int skip_reading_head)\n \n struct worktree **get_worktrees(void)\n {\n-\treturn get_worktrees_internal(0);\n+\treturn get_worktrees_repo(the_repository);\n+}\n+struct worktree **get_worktrees_repo(struct repository *repo)\n+{\n+\treturn get_worktrees_internal(repo, 0);\n }\n-\n struct worktree **get_worktrees_without_reading_head(void)\n {\n-\treturn get_worktrees_internal(1);\n+\treturn get_worktrees_internal(the_repository, 1);\n }\n \n char *get_worktree_git_dir(const struct worktree *wt)\n@@ -289,7 +292,7 @@ const char *worktree_lock_reason(struct worktree *wt)\n \tif (!wt->lock_reason_valid) {\n \t\tstruct strbuf path = STRBUF_INIT;\n \n-\t\tstrbuf_addstr(&path, worktree_git_path(the_repository, wt, \"locked\"));\n+\t\tstrbuf_addstr(&path, worktree_git_path(wt, \"locked\"));\n \t\tif (file_exists(path.buf)) {\n \t\t\tstruct strbuf lock_reason = STRBUF_INIT;\n \t\t\tif (strbuf_read_file(&lock_reason, path.buf, 0) < 0)\n@@ -690,7 +693,7 @@ static void repair_noop(int iserr UNUSED,\n \n void repair_worktrees(worktree_repair_fn fn, void *cb_data, int use_relative_paths)\n {\n-\tstruct worktree **worktrees = get_worktrees_internal(1);\n+\tstruct worktree **worktrees = get_worktrees_internal(the_repository, 1);\n \tstruct worktree **wt = worktrees + 1; /* +1 skips main worktree */\n \n \tif (!fn)\n@@ -735,7 +738,7 @@ void repair_worktree_after_gitdir_move(struct worktree *wt, const char *old_path\n \n void repair_worktrees_after_gitdir_move(const char *old_path)\n {\n-\tstruct worktree **worktrees = get_worktrees_internal(1);\n+\tstruct worktree **worktrees = get_worktrees_internal(the_repository, 1);\n \tstruct worktree **wt = worktrees + 1; /* +1 skips main worktree */\n \n \tfor (; *wt; wt++)\ndiff --git a/worktree.h b/worktree.h\nindex e4bcccdc0a..fc5f87b5b1 100644\n--- a/worktree.h\n+++ b/worktree.h\n@@ -30,6 +30,13 @@ struct worktree {\n  */\n struct worktree **get_worktrees(void);\n \n+/*\n+ * Like `get_worktrees`, but it returns the worktrees for the given repository.\n+ * This is useful for cases where local repository context is used rather than\n+ * the_repository global.\n+ */\n+struct worktree **get_worktrees_repo(struct repository *repo);\n+\n /*\n  * Like `get_worktrees`, but does not read HEAD. Skip reading HEAD allows to\n  * get the worktree without worrying about failures pertaining to parsing\ndiff --git a/wt-status.c b/wt-status.c\nindex e12adb26b9..11aea61c81 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1624,7 +1624,7 @@ static char *get_branch(const struct worktree *wt, const char *path)\n \tstruct object_id oid;\n \tconst char *branch_name;\n \n-\tif (strbuf_read_file(&sb, worktree_git_path(the_repository, wt, \"%s\", path), 0) <= 0)\n+\tif (strbuf_read_file(&sb, worktree_git_path(wt, \"%s\", path), 0) <= 0)\n \t\tgoto got_nothing;\n \n \twhile (sb.len && sb.buf[sb.len - 1] == '\\n')\n@@ -1723,18 +1723,18 @@ int wt_status_check_rebase(const struct worktree *wt,\n {\n \tstruct stat st;\n \n-\tif (!stat(worktree_git_path(the_repository, wt, \"rebase-apply\"), &st)) {\n-\t\tif (!stat(worktree_git_path(the_repository, wt, \"rebase-apply/applying\"), &st)) {\n+\tif (!stat(worktree_git_path(wt, \"rebase-apply\"), &st)) {\n+\t\tif (!stat(worktree_git_path(wt, \"rebase-apply/applying\"), &st)) {\n \t\t\tstate->am_in_progress = 1;\n-\t\t\tif (!stat(worktree_git_path(the_repository, wt, \"rebase-apply/patch\"), &st) && !st.st_size)\n+\t\t\tif (!stat(worktree_git_path(wt, \"rebase-apply/patch\"), &st) && !st.st_size)\n \t\t\t\tstate->am_empty_patch = 1;\n \t\t} else {\n \t\t\tstate->rebase_in_progress = 1;\n \t\t\tstate->branch = get_branch(wt, \"rebase-apply/head-name\");\n \t\t\tstate->onto = get_branch(wt, \"rebase-apply/onto\");\n \t\t}\n-\t} else if (!stat(worktree_git_path(the_repository, wt, \"rebase-merge\"), &st)) {\n-\t\tif (!stat(worktree_git_path(the_repository, wt, \"rebase-merge/interactive\"), &st))\n+\t} else if (!stat(worktree_git_path(wt, \"rebase-merge\"), &st)) {\n+\t\tif (!stat(worktree_git_path(wt, \"rebase-merge/interactive\"), &st))\n \t\t\tstate->rebase_interactive_in_progress = 1;\n \t\telse\n \t\t\tstate->rebase_in_progress = 1;\n@@ -1750,7 +1750,7 @@ int wt_status_check_bisect(const struct worktree *wt,\n {\n \tstruct stat st;\n \n-\tif (!stat(worktree_git_path(the_repository, wt, \"BISECT_LOG\"), &st)) {\n+\tif (!stat(worktree_git_path(wt, \"BISECT_LOG\"), &st)) {\n \t\tstate->bisect_in_progress = 1;\n \t\tstate->bisecting_from = get_branch(wt, \"BISECT_START\");\n \t\treturn 1;\n@@ -1795,18 +1795,19 @@ void wt_status_get_state(struct repository *r,\n \tstruct stat st;\n \tstruct object_id oid;\n \tenum replay_action action;\n+\tstruct worktree **worktrees = get_worktrees_repo(r);\n \n \tif (!stat(git_path_merge_head(r), &st)) {\n-\t\twt_status_check_rebase(NULL, state);\n+\t\twt_status_check_rebase(worktrees[0], state);\n \t\tstate->merge_in_progress = 1;\n-\t} else if (wt_status_check_rebase(NULL, state)) {\n+\t} else if (wt_status_check_rebase(worktrees[0], state)) {\n \t\t;\t\t/* all set */\n \t} else if (refs_ref_exists(get_main_ref_store(r), \"CHERRY_PICK_HEAD\") &&\n \t\t   !repo_get_oid(r, \"CHERRY_PICK_HEAD\", &oid)) {\n \t\tstate->cherry_pick_in_progress = 1;\n \t\toidcpy(&state->cherry_pick_head_oid, &oid);\n \t}\n-\twt_status_check_bisect(NULL, state);\n+\twt_status_check_bisect(worktrees[0], state);\n \tif (refs_ref_exists(get_main_ref_store(r), \"REVERT_HEAD\") &&\n \t    !repo_get_oid(r, \"REVERT_HEAD\", &oid)) {\n \t\tstate->revert_in_progress = 1;\n@@ -1824,6 +1825,7 @@ void wt_status_get_state(struct repository *r,\n \tif (get_detached_from)\n \t\twt_status_get_detached_from(r, state);\n \twt_status_check_sparse_checkout(r, state);\n+\tfree_worktrees(worktrees);\n }\n \n static void wt_longstatus_print_state(struct wt_status *s)\n-- \n2.53.0\n\n"},{"id":"535966","messageId":"xmqq7bsgl42j.fsf@gitster.g","threadId":"64989","inReplyTo":"20260213120529.15475-2-shreyanshpaliwalcmsmn@gmail.com","subject":"Re: [RFC][PATCH 1/2] worktree: represent the primary worktree with '/' instead of NULL","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-13T21:35:32Z","receivedAt":"2026-02-13T21:35:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com> writes:\n\n> diff --git a/worktree.c b/worktree.c\n> index 9308389cb6..b29934407f 100644\n> --- a/worktree.c\n> +++ b/worktree.c\n> @@ -101,6 +101,7 @@ static struct worktree *get_main_worktree(int skip_reading_head)\n>  \n>  \tCALLOC_ARRAY(worktree, 1);\n>  \tworktree->repo = the_repository;\n> +\tworktree->id = xstrdup(\"/\");\n>  \tworktree->path = strbuf_detach(&worktree_path, NULL);\n>  \tworktree->is_current = is_current_worktree(worktree);\n>  \tworktree->is_bare = (is_bare_repository_cfg == 1) ||\n\nPresumably we left .id = NULL from CALLOC_ARRAY(), so this looks\nsensible.  When releasing resources from an instance of worktree,\nwe'd blindly free(worktree->id) and in the old world, free(NULL)\nturned into no-op, and this xstrdup()'d copy will be freed in the\nnew world, so there is nothing funny here, I hope?  This one, and\nthe change to is_main_worktree() go together.\n\n> @@ -127,6 +128,8 @@ struct worktree *get_linked_worktree(const char *id,\n>  \n>  \tif (!id)\n>  \t\tdie(\"Missing linked worktree name\");\n> +\tif (!strcmp(id, \"/\"))\n> +\t\tdie(\"'/' is reserved for primary worktree\");\n\nMakes me wonder if this is a BUG not die; where does id come from?\n\n\t... goes and looks ...\n\nThe only caller is worktree.c:get_worktrees_internal() and it is\nfeeding d->d_name that came from readdir_skip_dot_and_dotdot(), so\nit cannot be \"/\".\n\nBy the way, I suspect that get_linked_worktree() should become\nfile-scope static, as there is no other caller.\n\n> @@ -206,9 +209,7 @@ struct worktree **get_worktrees_without_reading_head(void)\n>  \n>  char *get_worktree_git_dir(const struct worktree *wt)\n>  {\n> -\tif (!wt)\n> -\t\treturn xstrdup(repo_get_git_dir(the_repository));\n> -\telse if (!wt->id)\n> +\tif (is_main_worktree(wt))\n>  \t\treturn xstrdup(repo_get_common_dir(the_repository));\n>  \telse\n>  \t\treturn repo_common_path(the_repository, \"worktrees/%s\", wt->id);\n\nGood spotting.  This series needs to spot any and all places that\nuse these other conventions (i.e. wt->id == NULL) to identify the\nprimary worktree and rewrite them to call is_main_worktree(), which\nmay be a chore, but once it is done, it would become a lot easier to\nfollow the resulting code.\n\n> @@ -277,7 +278,7 @@ struct worktree *find_worktree_by_path(struct worktree **list, const char *p)\n>  \n>  int is_main_worktree(const struct worktree *wt)\n>  {\n> -\treturn !wt->id;\n> +\treturn !strcmp(wt->id, \"/\");\n>  }\n\nOK.\n\n> @@ -566,7 +567,7 @@ void strbuf_worktree_ref(const struct worktree *wt,\n>  {\n>  \tif (parse_worktree_ref(refname, NULL, NULL, NULL) ==\n>  \t\t    REF_WORKTREE_CURRENT &&\n> -\t    wt && !wt->is_current) {\n> +\t    !wt->is_current) {\n\nOK.\n\n> @@ -629,6 +630,9 @@ static void repair_gitfile(struct worktree *wt,\n>  \tchar *path = NULL;\n>  \tint err;\n>  \n> +\tif (is_main_worktree(wt))\n> +\t\tgoto done;\n\nThis is a bit new.\n\nThe original did not say \n\n\tif (!wt || !wt->id || !strcmp(wt->id, \"/\"))\n\t\tgoto done;\n\nThe only caller is iterating over the resulting list of worktrees\nreturned from get_worktrees_internal(1) *BUT* it already skips the\nprimary worktree (the function MUST return the primary one as the\nfirst one, and the callers MUST be aware of the convention).\n\nSo I am not sure if the new check is even needed.  Or rather, this ...\n\n\tif (!wt || !wt->id || !strcmp(wt->id, \"/\"))\n\t\tBUG(\"why are you feeding me the primary worktree???\");\n\t\n... might be more appropriate, perhaps?  I dunno.\n\n\n>  \t/* missing worktree can't be repaired */\n>  \tif (!file_exists(wt->path))\n>  \t\tgoto done;\n"},{"id":"535972","messageId":"xmqqcy28jmzs.fsf@gitster.g","threadId":"64989","inReplyTo":"20260213120529.15475-3-shreyanshpaliwalcmsmn@gmail.com","subject":"Re: [RFC][PATCH 2/2] worktree: stop passing NULL as primary worktree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-13T22:29:43Z","receivedAt":"2026-02-13T22:29:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com> writes:\n\n> -\t\t\tpath = xstrdup(worktree_git_path(the_repository, wt, \"index\"));\n> +\t\t\tpath = xstrdup(worktree_git_path(wt, \"index\"));\n\nWe'll understand this change when we read the changes to\nworktree_git_path() later in this patch, I guess.  I will not\ncomment on the same changes around worktree_git_path() callers.\n\n> diff --git a/builtin/worktree.c b/builtin/worktree.c\n> index fbdaf2eb2e..27c5889c89 100644\n> --- a/builtin/worktree.c\n> +++ b/builtin/worktree.c\n> @@ -328,6 +328,8 @@ static void check_candidate_path(const char *path,\n>  \twt = find_worktree_by_path(worktrees, path);\n>  \tif (!wt)\n>  \t\treturn;\n> +\tif(is_main_worktree(wt))\n> +\t\tdie(_(\"'%s' is the main worktree\"), path);\n\nStyle (missing SP between \"if\" and \"(condition)\").\n\nEarlier, a failure from find_worktree_by_path(), presumably meaning\n\"you gave me this path, but that is not where any of our worktrees\nlive\" gave wt==NULL and it silently returned.  Could the function\nreturned wt==NULL to signal that the path is where the primary\nworktree is?  I guess not (it seems to give us a worktree instance\nwith wt->id==NULL).\n\nSo, we never cared about wt being the primary worktree, but now we\ncare.  Why do we need to?\n\n> @@ -660,7 +662,8 @@ static int can_use_local_refs(const struct add_opts *opts)\n>  \t\tif (!opts->quiet) {\n>  \t\t\tstruct strbuf path = STRBUF_INIT;\n>  \t\t\tstruct strbuf contents = STRBUF_INIT;\n> -\t\t\tchar *wt_gitdir = get_worktree_git_dir(NULL);\n> +\t\t\tstruct worktree **worktrees = get_worktrees_repo(the_repository);\n> +\t\t\tchar *wt_gitdir = get_worktree_git_dir(worktrees[0]);\n\nWe used to pass NULL to get_worktree_git_dir() to ask about the\nprimary working tree, but the convention is no longer available.  So\nwe use get_worktrees_repo(), presumably is a new function, that\ngives all the worktrees honoring the \"first one in the resulting\nlist is the primary one\" convention, only to use the first element\nin the list.\n\nI wonder if making worktree.c:get_main_worktree(), which is a file\nscope static in worktree.c, available would allow us express this\nlogic more directly?\n\n>  \t\t\tstrbuf_add_real_path(&path, wt_gitdir);\n>  \t\t\tstrbuf_addstr(&path, \"/HEAD\");\n> @@ -675,6 +678,7 @@ static int can_use_local_refs(const struct add_opts *opts)\n>  \t\t\tstrbuf_release(&path);\n>  \t\t\tstrbuf_release(&contents);\n>  \t\t\tfree(wt_gitdir);\n> +\t\t\tfree_worktrees(worktrees);\n\nAnyway, we need to release the list of worktrees here.\n\n> @@ -1191,14 +1195,14 @@ static void validate_no_submodules(const struct worktree *wt)\n>  \n>  \twt_gitdir = get_worktree_git_dir(wt);\n>  \n> -\tif (is_directory(worktree_git_path(the_repository, wt, \"modules\"))) {\n> +\tif (is_directory(worktree_git_path(wt, \"modules\"))) {\n> -\t} else if (read_index_from(&istate, worktree_git_path(the_repository, wt, \"index\"),\n> +\t} else if (read_index_from(&istate, worktree_git_path(wt, \"index\"),\n\nDitto on worktree_git_path().\n\n> diff --git a/path.c b/path.c\n> index d726537622..4ac86e1e58 100644\n> --- a/path.c\n> +++ b/path.c\n> @@ -408,9 +408,7 @@ static void strbuf_worktree_gitdir(struct strbuf *buf,\n>  \t\t\t\t   const struct repository *repo,\n>  \t\t\t\t   const struct worktree *wt)\n>  {\n> -\tif (!wt)\n> -\t\tstrbuf_addstr(buf, repo->gitdir);\n> -\telse if (!wt->id)\n> +\tif (is_main_worktree(wt))\n>  \t\tstrbuf_addstr(buf, repo->commondir);\n>  \telse\n>  \t\trepo_common_path_append(repo, buf, \"worktrees/%s\", wt->id);\n\nThis is curious.\n\nWe used to treat \"wt==NULL\" and \"wt->id==NULL\" differently.  Now we\nuse repo->commondir for both.  For the primary worktree, it ought to\nbe the same as repo->gitdir, so it should not matter, but makes me\nwonder what the reason behind this difference in the original.\n\nWe have been assuming that wt==NULL and wt->id==NULL both meant the\nsame thing: \"we are talking about the primary worktree\".  But the\ncode around here before this patch seems to behave differently.  Is\nour assumption incorrect and are we making a mistake by conflating\nthese two conditions into one?\n\n> @@ -437,8 +435,10 @@ char *repo_git_path(struct repository *repo,\n>  \tstruct strbuf path = STRBUF_INIT;\n>  \tva_list args;\n>  \tva_start(args, fmt);\n> -\trepo_git_pathv(repo, NULL, &path, fmt, args);\n> +\tstruct worktree **worktrees = get_worktrees_repo(repo);\n> +\trepo_git_pathv(repo, worktrees[0], &path, fmt, args);\n>  \tva_end(args);\n> +\tfree_worktrees(worktrees);\n\nThe same \"to pass the primary worktree, we need to grab everybody\nand pass the first one\" pattern is repeated here.\n\n> @@ -448,8 +448,10 @@ const char *repo_git_path_append(struct repository *repo,\n>  {\n>  \tva_list args;\n>  \tva_start(args, fmt);\n> -\trepo_git_pathv(repo, NULL, sb, fmt, args);\n> +\tstruct worktree **worktrees = get_worktrees_repo(repo);\n> +\trepo_git_pathv(repo, worktrees[0], sb, fmt, args);\n>  \tva_end(args);\n> +\tfree_worktrees(worktrees);\n\nAnd again here.\n\n> @@ -460,8 +462,10 @@ const char *repo_git_path_replace(struct repository *repo,\n>  \tva_list args;\n>  \tstrbuf_reset(sb);\n>  \tva_start(args, fmt);\n> -\trepo_git_pathv(repo, NULL, sb, fmt, args);\n> +\tstruct worktree **worktrees = get_worktrees_repo(repo);\n> +\trepo_git_pathv(repo, worktrees[0], sb, fmt, args);\n>  \tva_end(args);\n> +\tfree_worktrees(worktrees);\n\nAnd again here.\n\n> @@ -486,17 +490,12 @@ const char *mkpath(const char *fmt, ...)\n>  \treturn cleanup_path(pathname->buf);\n>  }\n>  \n> -const char *worktree_git_path(struct repository *r,\n> -\t\t\t      const struct worktree *wt, const char *fmt, ...)\n> +const char *worktree_git_path(const struct worktree *wt, const char *fmt, ...)\n>  {\n\nSince we no longer use wt==NULL as the sign to work on the primary\nworking tree, we can rely on wt->repo being a valid repository the\ncaller wants to work with.\n\n>  \tstruct strbuf *pathname = get_pathname();\n>  \tva_list args;\n> -\n> -\tif (wt && wt->repo != r)\n> -\t\tBUG(\"worktree not connected to expected repository\");\n> -\n>  \tva_start(args, fmt);\n> -\trepo_git_pathv(r, wt, pathname, fmt, args);\n> +\trepo_git_pathv(wt->repo, wt, pathname, fmt, args);\n>  \tva_end(args);\n>  \treturn pathname->buf;\n>  }\n> diff --git a/path.h b/path.h\n> index 0ec95a0b07..54e09b2883 100644\n> --- a/path.h\n> +++ b/path.h\n> @@ -66,13 +66,10 @@ const char *repo_git_path_replace(struct repository *repo,\n>  \n>  /*\n>   * Similar to repo_git_path() but can produce paths for a specified\n> - * worktree instead of current one. When no worktree is given, then the path is\n> - * computed relative to main worktree of the given repository.\n> + * worktree instead of current one.\n>   */\n> -const char *worktree_git_path(struct repository *r,\n> -\t\t\t      const struct worktree *wt,\n> -\t\t\t      const char *fmt, ...)\n> -\t__attribute__((format (printf, 3, 4)));\n> +const char *worktree_git_path(const struct worktree *wt, const char *fmt, ...)\n> +\t__attribute__((format (printf, 2, 3)));\n\nGood.\n\n> diff --git a/refs.c b/refs.c\n> index 627b7f8698..98df2235e7 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -2324,12 +2324,12 @@ struct ref_store *get_worktree_ref_store(const struct worktree *wt)\n>  \tif (wt->is_current)\n>  \t\treturn get_main_ref_store(wt->repo);\n>  \n> -\tid = wt->id ? wt->id : \"/\";\n> +\tid = wt->id;\n>  \trefs = lookup_ref_store_map(&wt->repo->worktree_ref_stores, id);\n>  \tif (refs)\n>  \t\treturn refs;\n>  \n> -\tif (wt->id) {\n> +\tif (!is_main_worktree(wt)) {\n>  \t\tstruct strbuf common_path = STRBUF_INIT;\n>  \t\trepo_common_path_append(wt->repo, &common_path,\n>  \t\t\t\t\t\"worktrees/%s\", wt->id);\n\nGood.  The original uses local \"id\" as a pathname component (which\ncould be \"/\"), and wt->id==NULL as the sign that it is the primary.\nThe updated code is a faithful conversion of it to the new world\norder.\n\n> diff --git a/revision.c b/revision.c\n> index 9b131670f7..a9d4f796ed 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -1730,12 +1730,14 @@ void add_reflogs_to_pending(struct rev_info *revs, unsigned flags)\n>  \n>  \tcb.all_revs = revs;\n>  \tcb.all_flags = flags;\n> -\tcb.wt = NULL;\n> +\tstruct worktree **worktrees = get_worktrees_repo(the_repository);\n> +\tcb.wt = worktrees[0];\n>  \trefs_for_each_reflog(get_main_ref_store(the_repository),\n>  \t\t\t     handle_one_reflog, &cb);\n>  \n>  \tif (!revs->single_worktree)\n>  \t\tadd_other_reflogs_to_pending(&cb);\n> +\tfree_worktrees(worktrees);\n\nThe same \"to pass the primary worktree, we need to grab everybody\nand pass the first one\" pattern strikes again.\n\n> diff --git a/worktree.c b/worktree.c\n> index b29934407f..1059c18115 100644\n> --- a/worktree.c\n> +++ b/worktree.c\n> @@ -91,16 +91,16 @@ static int is_main_worktree_bare(struct repository *repo)\n>  /**\n>   * get the main worktree\n>   */\n> -static struct worktree *get_main_worktree(int skip_reading_head)\n> +static struct worktree *get_main_worktree(struct repository *repo, int skip_reading_head)\n>  {\n>  \tstruct worktree *worktree = NULL;\n>  \tstruct strbuf worktree_path = STRBUF_INIT;\n>  \n> -\tstrbuf_add_real_path(&worktree_path, repo_get_common_dir(the_repository));\n> +\tstrbuf_add_real_path(&worktree_path, repo_get_common_dir(repo));\n>  \tstrbuf_strip_suffix(&worktree_path, \"/.git\");\n>  \n>  \tCALLOC_ARRAY(worktree, 1);\n> -\tworktree->repo = the_repository;\n> +\tworktree->repo = repo;\n>  \tworktree->id = xstrdup(\"/\");\n>  \tworktree->path = strbuf_detach(&worktree_path, NULL);\n>  \tworktree->is_current = is_current_worktree(worktree);\n> @@ -112,7 +112,7 @@ static struct worktree *get_main_worktree(int skip_reading_head)\n>  \t\t * This check is unnecessary if we're currently in the main worktree,\n>  \t\t * as prior checks already consulted all configs of the current worktree.\n>  \t\t */\n> -\t\t(!worktree->is_current && is_main_worktree_bare(the_repository));\n> +\t\t(!worktree->is_current && is_main_worktree_bare(repo));\n>  \n>  \tif (!skip_reading_head)\n>  \t\tadd_head_info(worktree);\n\nWeaning the code from depending on the_repository is mixed into the\nrefactoring, which makes me wonder if it is better done in a\nseparate patch.  We seriously should consider making this function\nexternally visible, as so many callers want to get hold of it.\n\nI also wonder if \"struct repository\" wants to have a member that\npoints at the primary worktree instance, but I think I am getting\nway ahead of myself.\n\n> @@ -199,12 +199,15 @@ static struct worktree **get_worktrees_internal(int skip_reading_head)\n>  \n>  struct worktree **get_worktrees(void)\n>  {\n> -\treturn get_worktrees_internal(0);\n> +\treturn get_worktrees_repo(the_repository);\n> +}\n> +struct worktree **get_worktrees_repo(struct repository *repo)\n> +{\n> +\treturn get_worktrees_internal(repo, 0);\n>  }\n\nOK.\n\n> @@ -1795,18 +1795,19 @@ void wt_status_get_state(struct repository *r,\n>  \tstruct stat st;\n>  \tstruct object_id oid;\n>  \tenum replay_action action;\n> +\tstruct worktree **worktrees = get_worktrees_repo(r);\n>  \n>  \tif (!stat(git_path_merge_head(r), &st)) {\n> -\t\twt_status_check_rebase(NULL, state);\n> +\t\twt_status_check_rebase(worktrees[0], state);\n>  \t\tstate->merge_in_progress = 1;\n> -\t} else if (wt_status_check_rebase(NULL, state)) {\n> +\t} else if (wt_status_check_rebase(worktrees[0], state)) {\n>  \t\t;\t\t/* all set */\n>  \t} else if (refs_ref_exists(get_main_ref_store(r), \"CHERRY_PICK_HEAD\") &&\n>  \t\t   !repo_get_oid(r, \"CHERRY_PICK_HEAD\", &oid)) {\n>  \t\tstate->cherry_pick_in_progress = 1;\n>  \t\toidcpy(&state->cherry_pick_head_oid, &oid);\n>  \t}\n> -\twt_status_check_bisect(NULL, state);\n> +\twt_status_check_bisect(worktrees[0], state);\n>  \tif (refs_ref_exists(get_main_ref_store(r), \"REVERT_HEAD\") &&\n>  \t    !repo_get_oid(r, \"REVERT_HEAD\", &oid)) {\n>  \t\tstate->revert_in_progress = 1;\n> @@ -1824,6 +1825,7 @@ void wt_status_get_state(struct repository *r,\n>  \tif (get_detached_from)\n>  \t\twt_status_get_detached_from(r, state);\n>  \twt_status_check_sparse_checkout(r, state);\n> +\tfree_worktrees(worktrees);\n>  }\n\nThe same \"to pass the primary worktree, we need to grab everybody\nand pass the first one\" pattern strikes yet another time.\n\nThanks.\n"},{"id":"536007","messageId":"20260214095817.514765-1-shreyanshpaliwalcmsmn@gmail.com","threadId":"64989","inReplyTo":"xmqq7bsgl42j.fsf@gitster.g","subject":"Re: [RFC][PATCH 1/2] worktree: represent the primary worktree with '/' instead of NULL","fromName":"Shreyansh Paliwal","fromEmail":"shreyanshpaliwalcmsmn@gmail.com","sentAt":"2026-02-14T09:54:21Z","receivedAt":"2026-02-14T09:58:38Z","isPatch":true,"sender":{"key":"shreyanshpaliwalcmsmn@gmail.com","avatar":"https://avatars.githubusercontent.com/u/152720574?v=4"},"body":"> > diff --git a/worktree.c b/worktree.c\n> > index 9308389cb6..b29934407f 100644\n> > --- a/worktree.c\n> > +++ b/worktree.c\n> > @@ -101,6 +101,7 @@ static struct worktree *get_main_worktree(int skip_reading_head)\n> >\n> >  \tCALLOC_ARRAY(worktree, 1);\n> >  \tworktree->repo = the_repository;\n> > +\tworktree->id = xstrdup(\"/\");\n> >  \tworktree->path = strbuf_detach(&worktree_path, NULL);\n> >  \tworktree->is_current = is_current_worktree(worktree);\n> >  \tworktree->is_bare = (is_bare_repository_cfg == 1) ||\n>\n> Presumably we left .id = NULL from CALLOC_ARRAY(), so this looks\n> sensible.  When releasing resources from an instance of worktree,\n> we'd blindly free(worktree->id) and in the old world, free(NULL)\n> turned into no-op, and this xstrdup()'d copy will be freed in the\n> new world, so there is nothing funny here, I hope?  This one, and\n> the change to is_main_worktree() go together.\n\nHmm, I don't think free(worktree->id) should cause any issue in this case.\n\n> > @@ -127,6 +128,8 @@ struct worktree *get_linked_worktree(const char *id\n> >\n> >  \tif (!id)\n> >  \t\tdie(\"Missing linked worktree name\");\n> > +\tif (!strcmp(id, \"/\"))\n> > +\t\tdie(\"'/' is reserved for primary worktree\");\n>\n> Makes me wonder if this is a BUG not die; where does id come from?\n>\n> \t... goes and looks ...\n>\n> The only caller is worktree.c:get_worktrees_internal() and it is\n> feeding d->d_name that came from readdir_skip_dot_and_dotdot(), so\n> it cannot be \"/\".\n>\n> By the way, I suspect that get_linked_worktree() should become\n> file-scope static, as there is no other caller.\n\nActually get_linked_worktee(), along with worktree.c: get_worktrees_internal()\nis also called from builtin/worktree.c: add_worktree().\nSo at this point we should prefer die(), and if were to make\nget_linked_worktree() static, then we can add a helper for external uses\nmaybe using a struct repository* instead of the_repository in the future.\n\n> > @@ -629,6 +630,9 @@ static void repair_gitfile(struct worktree *wt,\n> >  \tchar *path = NULL;\n> >  \tint err;\n> >\n> > +\tif (is_main_worktree(wt))\n> > +\t\tgoto done;\n>\n> This is a bit new.\n>\n> The original did not say\n>\n> \tif (!wt || !wt->id || !strcmp(wt->id, \"/\"))\n> \t\tgoto done;\n>\n> The only caller is iterating over the resulting list of worktrees\n> returned from get_worktrees_internal(1) *BUT* it already skips the\n> primary worktree (the function MUST return the primary one as the\n> first one, and the callers MUST be aware of the convention).\n>\n> So I am not sure if the new check is even needed.  Or rather, this ...\n>\n> \tif (!wt || !wt->id || !strcmp(wt->id, \"/\"))\n> \t\tBUG(\"why are you feeding me the primary worktree???\");\n>\n> ... might be more appropriate, perhaps?  I dunno.\n\nActually this new check is to prevent accidently feeding '/' as wt->id in,\n\n\tpath = repo_common_path(the_repository, \"worktrees/%s\", wt->id);\n\nbut you are right as of now its only caller skips the primary worktree\nso we can just put a precautionary check, for the case if this function\nis used somewhere in future like this,\n\n    if(is_main_worktree(wt))\n        BUG(\"repair_gitfile() called for the main worktree\");\n"},{"id":"536008","messageId":"20260214101045.515941-1-shreyanshpaliwalcmsmn@gmail.com","threadId":"64989","inReplyTo":"xmqqcy28jmzs.fsf@gitster.g","subject":"Re: [RFC][PATCH 2/2] worktree: stop passing NULL as primary worktree","fromName":"Shreyansh Paliwal","fromEmail":"shreyanshpaliwalcmsmn@gmail.com","sentAt":"2026-02-14T09:59:33Z","receivedAt":"2026-02-14T10:10:55Z","isPatch":true,"sender":{"key":"shreyanshpaliwalcmsmn@gmail.com","avatar":"https://avatars.githubusercontent.com/u/152720574?v=4"},"body":"> > diff --git a/builtin/worktree.c b/builtin/worktree.c\n> > index fbdaf2eb2e..27c5889c89 100644\n> > --- a/builtin/worktree.c\n> > +++ b/builtin/worktree.c\n> > @@ -328,6 +328,8 @@ static void check_candidate_path(const char *path,\n> >  \twt = find_worktree_by_path(worktrees, path);\n> >  \tif (!wt)\n> >  \t\treturn;\n> > +\tif(is_main_worktree(wt))\n> > +\t\tdie(_(\"'%s' is the main worktree\"), path);\n>\n> Style (missing SP between \"if\" and \"(condition)\").\n\nmy bad, will fix that.\n\n> Earlier, a failure from find_worktree_by_path(), presumably meaning\n> \"you gave me this path, but that is not where any of our worktrees\n> live\" gave wt==NULL and it silently returned.  Could the function\n> returned wt==NULL to signal that the path is where the primary\n> worktree is?  I guess not (it seems to give us a worktree instance\n> with wt->id==NULL).\n>\n> So, we never cared about wt being the primary worktree, but now we\n> care.  Why do we need to?\n\nThe function check_candidate_path() is basically to check the path we can\nadd a worktree, so if we are receiving primary worktree it shouldn't proceed\nand further in the function we are calling delete_git_dir(wt->id)\nbelow which then further calls repo_common_path_append() with id.\nSo to prevent feeding '/' to this we need to check if the worktree is\nmain or not.\n\n> > @@ -660,7 +662,8 @@ static int can_use_local_refs(const struct add_opts *opts)\n> >  \t\tif (!opts->quiet) {\n> >  \t\t\tstruct strbuf path = STRBUF_INIT;\n> >  \t\t\tstruct strbuf contents = STRBUF_INIT;\n> > -\t\t\tchar *wt_gitdir = get_worktree_git_dir(NULL);\n> > +\t\t\tstruct worktree **worktrees = get_worktrees_repo(the_repository);\n> > +\t\t\tchar *wt_gitdir = get_worktree_git_dir(worktrees[0]);\n>\n> We used to pass NULL to get_worktree_git_dir() to ask about the\n> primary working tree, but the convention is no longer available.  So\n> we use get_worktrees_repo(), presumably is a new function, that\n> gives all the worktrees honoring the \"first one in the resulting\n> list is the primary one\" convention, only to use the first element\n> in the list.\n>\n> I wonder if making worktree.c:get_main_worktree(), which is a file\n> scope static in worktree.c, available would allow us express this\n> logic more directly?\n\nActually I thought about making get_main_worktree() available but then\nI saw helpers like get_worktrees() so I thought that get_worktrees_repo()\nwould be better for every use case.\nBut I agree since there are so many sites to access the main worktree, we can\nmake get_main_worktree() available and use it instead of introducing\nget_worktrees_repo().\n\n> > diff --git a/path.c b/path.c\n> > index d726537622..4ac86e1e58 100644\n> > --- a/path.c\n> > +++ b/path.c\n> > @@ -408,9 +408,7 @@ static void strbuf_worktree_gitdir(struct strbuf *buf,\n> >  \t\t\t\t   const struct repository *repo,\n> >  \t\t\t\t   const struct worktree *wt)\n> >  {\n> > -\tif (!wt)\n> > -\t\tstrbuf_addstr(buf, repo->gitdir);\n> > -\telse if (!wt->id)\n> > +\tif (is_main_worktree(wt))\n> >  \t\tstrbuf_addstr(buf, repo->commondir);\n> >  \telse\n> >  \t\trepo_common_path_append(repo, buf, \"worktrees/%s\", wt->id);\n>\n> This is curious.\n>\n> We used to treat \"wt==NULL\" and \"wt->id==NULL\" differently.  Now we\n> use repo->commondir for both.  For the primary worktree, it ought to\n> be the same as repo->gitdir, so it should not matter, but makes me\n> wonder what the reason behind this difference in the original.\n>\n> We have been assuming that wt==NULL and wt->id==NULL both meant the\n> same thing: \"we are talking about the primary worktree\".  But the\n> code around here before this patch seems to behave differently.  Is\n> our assumption incorrect and are we making a mistake by conflating\n> these two conditions into one?\n\nYes it came into my mind as well. So if we check were strbuf_worktree_gitdir()\nis called from, there is only one function repo_git_pathv() which is mostly\ncalled with a NULL wt (primary worktree) and called once with an actual\nworktreee once inside worktree_git_path() which had some NULL indirect callers\ninside wt-status.c which again meant wt being primary.\nI think different usecases for wt and wt->id being NULL was just an oversight\nat this particular instance and both of them should refer to the main worktree.\n\n> > diff --git a/worktree.c b/worktree.c\n> > index b29934407f..1059c18115 100644\n> > --- a/worktree.c\n> > +++ b/worktree.c\n> > @@ -91,16 +91,16 @@ static int is_main_worktree_bare(struct repository *repo)\n> >  /**\n> >   * get the main worktree\n> >   */\n> > -static struct worktree *get_main_worktree(int skip_reading_head)\n> > +static struct worktree *get_main_worktree(struct repository *repo, int skip_reading_head)\n> >  {\n> >  \tstruct worktree *worktree = NULL;\n> >  \tstruct strbuf worktree_path = STRBUF_INIT;\n> >\n> > -\tstrbuf_add_real_path(&worktree_path, repo_get_common_dir(the_repository));\n> > +\tstrbuf_add_real_path(&worktree_path, repo_get_common_dir(repo));\n> >  \tstrbuf_strip_suffix(&worktree_path, \"/.git\");\n> >\n> >  \tCALLOC_ARRAY(worktree, 1);\n> > -\tworktree->repo = the_repository;\n> > +\tworktree->repo = repo;\n> >  \tworktree->id = xstrdup(\"/\");\n> >  \tworktree->path = strbuf_detach(&worktree_path, NULL);\n> >  \tworktree->is_current = is_current_worktree(worktree);\n> > @@ -112,7 +112,7 @@ static struct worktree *get_main_worktree(int skip_reading_head)\n> >  \t\t * This check is unnecessary if we're currently in the main worktree,\n> >  \t\t * as prior checks already consulted all configs of the current worktree.\n> >  \t\t */\n> > -\t\t(!worktree->is_current && is_main_worktree_bare(the_repository));\n> > +\t\t(!worktree->is_current && is_main_worktree_bare(repo));\n> >\n> >  \tif (!skip_reading_head)\n> >  \t\tadd_head_info(worktree);\n>\n> Weaning the code from depending on the_repository is mixed into the\n> refactoring, which makes me wonder if it is better done in a\n> separate patch.  We seriously should consider making this function\n> externally visible, as so many callers want to get hold of it.\n>\n> I also wonder if \"struct repository\" wants to have a member that\n> points at the primary worktree instance, but I think I am getting\n> way ahead of myself.\n\nYes agreed. I will in a seperate patch make get_main_worktree() available\nwith a struct repository * argument instead of the_repository and then\nwe can use it in all the places where we need to access the main worktree.\n\nAnd I think that if we are making get_main_worktree() with a\nstruct repository * argument available, then we can skip adding a new member\nto struct repository and just call get_main_worktree() whenever we need to\naccess the main worktree with respect to whatever instance of repository we\nare working with.\n\nThanks for reviewing.\n\nBest,\nShreyansh\n"},{"id":"536020","messageId":"ebc16a74-0555-4951-8ec6-ff7fce6b6fcc@gmail.com","threadId":"64989","inReplyTo":"xmqqcy28jmzs.fsf@gitster.g","subject":"Re: [RFC][PATCH 2/2] worktree: stop passing NULL as primary worktree","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-02-14T14:30:22Z","receivedAt":"2026-02-14T14:30:26Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"I've cc'd Eric for a second opinion\n\nOn 13/02/2026 22:29, Junio C Hamano wrote:\n> Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com> writes:\n> \n>> diff --git a/path.c b/path.c\n>> index d726537622..4ac86e1e58 100644\n>> --- a/path.c\n>> +++ b/path.c\n>> @@ -408,9 +408,7 @@ static void strbuf_worktree_gitdir(struct strbuf *buf,\n>>   \t\t\t\t   const struct repository *repo,\n>>   \t\t\t\t   const struct worktree *wt)\n>>   {\n>> -\tif (!wt)\n>> -\t\tstrbuf_addstr(buf, repo->gitdir);\n>> -\telse if (!wt->id)\n>> +\tif (is_main_worktree(wt))\n>>   \t\tstrbuf_addstr(buf, repo->commondir);\n>>   \telse\n>>   \t\trepo_common_path_append(repo, buf, \"worktrees/%s\", wt->id);\n> \n> This is curious.\n> \n> We used to treat \"wt==NULL\" and \"wt->id==NULL\" differently.  Now we\n> use repo->commondir for both.  For the primary worktree, it ought to\n> be the same as repo->gitdir, so it should not matter, but makes me\n> wonder what the reason behind this difference in the original.\n> \n> We have been assuming that wt==NULL and wt->id==NULL both meant the\n> same thing: \"we are talking about the primary worktree\".  But the\n> code around here before this patch seems to behave differently.  Is\n> our assumption incorrect and are we making a mistake by conflating\n> these two conditions into one?\n\nMy understanding is that wt==NULL means \"use the current worktree\" and \nwt->id==NULL means \"this is the main worktree\". That would explain why \nwe use repo->gitdir above when wt==NULL and repo->commondir when \nwt->id==NULL, as repo->gitdir is the gitdir of the current worktree and \nrepo->commondir will be the gitdir of the main worktree. If we look at \nthe code in wt-status.c that's passing a NULL worktree it wants to know \nabout the status of the current worktree, not the main worktree.\n\nI think that we should add a new function\n\nstruct worktree *get_current_worktree(struct repository*);\n\nto worktree.c that constructs a struct worktree using repo->gitdir etc. \nThe worktree id is the last path component of repo->gitdir when the \nrepo->gitdir and repo->commondir differ, otherwise it is NULL. Then we \ncan use that function to get the current worktree rather than passing \nNULL when we call wt_status_check_{rebase,bisect} from \nwt_status_get_state(). We should also think about whether we should \nchange wt_status_get_state() to take a \"struct worktree*\" rather than a \n\"struct repository*\" instead (I've not looked at the callers to see if \nthat's sensible).\n\nWith that, we can gradually clean up uses of wt==NULL in the rest of the \ncodebase overtime and eventually remove support for it from worktree.c \nrather than having a big flag-day patch. I don't think we need to change \nuses of wt-id==NULL.\n\nThanks\n\nPhillip\n\n"},{"id":"536023","messageId":"xmqqqzqngwz9.fsf@gitster.g","threadId":"64989","inReplyTo":"ebc16a74-0555-4951-8ec6-ff7fce6b6fcc@gmail.com","subject":"Re: [RFC][PATCH 2/2] worktree: stop passing NULL as primary worktree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-14T15:34:34Z","receivedAt":"2026-02-14T15:34:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood <phillip.wood123@gmail.com> writes:\n\n> I've cc'd Eric for a second opinion\n>\n> On 13/02/2026 22:29, Junio C Hamano wrote:\n>> Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com> writes:\n>> \n>>> diff --git a/path.c b/path.c\n>>> index d726537622..4ac86e1e58 100644\n>>> --- a/path.c\n>>> +++ b/path.c\n>>> @@ -408,9 +408,7 @@ static void strbuf_worktree_gitdir(struct strbuf *buf,\n>>>   \t\t\t\t   const struct repository *repo,\n>>>   \t\t\t\t   const struct worktree *wt)\n>>>   {\n>>> -\tif (!wt)\n>>> -\t\tstrbuf_addstr(buf, repo->gitdir);\n>>> -\telse if (!wt->id)\n>>> +\tif (is_main_worktree(wt))\n>>>   \t\tstrbuf_addstr(buf, repo->commondir);\n>>>   \telse\n>>>   \t\trepo_common_path_append(repo, buf, \"worktrees/%s\", wt->id);\n>> \n>> This is curious.\n>> \n>> We used to treat \"wt==NULL\" and \"wt->id==NULL\" differently.  Now we\n>> use repo->commondir for both.  For the primary worktree, it ought to\n>> be the same as repo->gitdir, so it should not matter, but makes me\n>> wonder what the reason behind this difference in the original.\n>> \n>> We have been assuming that wt==NULL and wt->id==NULL both meant the\n>> same thing: \"we are talking about the primary worktree\".  But the\n>> code around here before this patch seems to behave differently.  Is\n>> our assumption incorrect and are we making a mistake by conflating\n>> these two conditions into one?\n>\n> My understanding is that wt==NULL means \"use the current worktree\" and \n> wt->id==NULL means \"this is the main worktree\". That would explain why \n> we use repo->gitdir above when wt==NULL and repo->commondir when \n> wt->id==NULL, as repo->gitdir is the gitdir of the current worktree and \n> repo->commondir will be the gitdir of the main worktree. If we look at \n> the code in wt-status.c that's passing a NULL worktree it wants to know \n> about the status of the current worktree, not the main worktree.\n\nOh, boy.  If that is what wt==NULL means, the above confusion about\nthe original is perfectly cleared.  We have been operating under a\ntotally wrong assumption.\n\n> I think that we should add a new function\n>\n> struct worktree *get_current_worktree(struct repository*);\n>\n> to worktree.c that constructs a struct worktree using repo->gitdir etc. \n\nCertainly.\n\n> We should also think about whether we should \n> change wt_status_get_state() to take a \"struct worktree *\" rather than a \n> \"struct repository *\" instead (I've not looked at the callers to see if \n> that's sensible).\n>\n> With that, we can gradually clean up uses of wt==NULL in the rest of the \n> codebase overtime and eventually remove support for it from worktree.c \n> rather than having a big flag-day patch. I don't think we need to change \n> uses of wt-id==NULL.\n\nOK.  I think wt->id==NULL vs wt->id==\"/\" is about correcting\ninconsistencies between the worktree.c and refs.c and certainly can\nbe done in a separate patch.\n\nWe probably should think about how often we use the current one\n(presumably almost all the time, given that even wt-status.c API\nfunctions seem to take one), and if the current implementation is\nthe best way to signal, among a list of worktrees, which one is the\ncurrent and which one is the primary.\n\nI do not mind too much about \"the primary sits at the beginning of\nthe resulting list all the time\" convention, but wt->is_current bit\nlooks like a disaster waiting to happen.  To find the current one,\nyou need to construct a full list and then iterate over the list to\nfind one with that bit on?  What if there is nobody with the bit or\nmore than one?  Are callers prepared to notice and report such bugs?\n\nAs you said, comparison between gitdir and commondir is sufficient,\nthen we can lose that bit.  One fewer thing that can go out of sync\ntakes us one step closer to a cleaner world.\n\nThanks.\n"},{"id":"536048","messageId":"20260215090815.46544-1-shreyanshpaliwalcmsmn@gmail.com","threadId":"64989","inReplyTo":"ebc16a74-0555-4951-8ec6-ff7fce6b6fcc@gmail.com","subject":"Re: [RFC][PATCH 2/2] worktree: stop passing NULL as primary worktree","fromName":"Shreyansh Paliwal","fromEmail":"shreyanshpaliwalcmsmn@gmail.com","sentAt":"2026-02-15T08:56:36Z","receivedAt":"2026-02-15T09:08:28Z","isPatch":true,"sender":{"key":"shreyanshpaliwalcmsmn@gmail.com","avatar":"https://avatars.githubusercontent.com/u/152720574?v=4"},"body":"> I've cc'd Eric for a second opinion\n>\n> On 13/02/2026 22:29, Junio C Hamano wrote:\n> > Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com> writes:\n> >\n> >> diff --git a/path.c b/path.c\n> >> index d726537622..4ac86e1e58 100644\n> >> --- a/path.c\n> >> +++ b/path.c\n> >> @@ -408,9 +408,7 @@ static void strbuf_worktree_gitdir(struct strbuf *buf,\n> >>   \t\t\t\t   const struct repository *repo,\n> >>   \t\t\t\t   const struct worktree *wt)\n> >>   {\n> >> -\tif (!wt)\n> >> -\t\tstrbuf_addstr(buf, repo->gitdir);\n> >> -\telse if (!wt->id)\n> >> +\tif (is_main_worktree(wt))\n> >>   \t\tstrbuf_addstr(buf, repo->commondir);\n> >>   \telse\n> >>   \t\trepo_common_path_append(repo, buf, \"worktrees/%s\", wt->id);\n> >\n> > This is curious.\n> >\n> > We used to treat \"wt==NULL\" and \"wt->id==NULL\" differently.  Now we\n> > use repo->commondir for both.  For the primary worktree, it ought to\n> > be the same as repo->gitdir, so it should not matter, but makes me\n> > wonder what the reason behind this difference in the original.\n> >\n> > We have been assuming that wt==NULL and wt->id==NULL both meant the\n> > same thing: \"we are talking about the primary worktree\".  But the\n> > code around here before this patch seems to behave differently.  Is\n> > our assumption incorrect and are we making a mistake by conflating\n> > these two conditions into one?\n>\n> My understanding is that wt==NULL means \"use the current worktree\" and\n> wt->id==NULL means \"this is the main worktree\". That would explain why\n> we use repo->gitdir above when wt==NULL and repo->commondir when\n> wt->id==NULL, as repo->gitdir is the gitdir of the current worktree and\n> repo->commondir will be the gitdir of the main worktree. If we look at\n> the code in wt-status.c that's passing a NULL worktree it wants to know\n> about the status of the current worktree, not the main worktree.\n>\n> I think that we should add a new function\n>\n> struct worktree *get_current_worktree(struct repository*);\n>\n> to worktree.c that constructs a struct worktree using repo->gitdir etc.\n> The worktree id is the last path component of repo->gitdir when the\n> repo->gitdir and repo->commondir differ, otherwise it is NULL. Then we\n> can use that function to get the current worktree rather than passing\n> NULL when we call wt_status_check_{rebase,bisect} from\n> wt_status_get_state(). We should also think about whether we should\n> change wt_status_get_state() to take a \"struct worktree*\" rather than a\n> \"struct repository*\" instead (I've not looked at the callers to see if\n> that's sensible).\n>\n> With that, we can gradually clean up uses of wt==NULL in the rest of the\n> codebase overtime and eventually remove support for it from worktree.c\n> rather than having a big flag-day patch. I don't think we need to change\n> uses of wt-id==NULL.\n\nThanks a lot for clarifying. This helps solve the doubt regarding the\ndifferent usage of !wt and !wt->id in strbuf_worktree_gitdir(). I realize\nwe have been under the wrong assumption about what wt == NULL represents.\n\nBut I still have a few points where I’m a bit confused,\n\nIf wt == NULL is meant to represent the current worktree, then what role\nwt->is_current plays in the present implementation, and if they both\nrepresent the same thing then wt->is_current wouldn't make sense if wt is\nalready NULL in the case of a current worktree.\n\nBeyond representation, I’m not quite understanding on how call sites are\nlogically differentiating on whether the intent is to 'operate on the\nworktree we are in' or 'operate on the primary one'.\n\nAnd I think if we included both in struct repository (r->main_wt, r->current_wt)\nso accessing either of them would be a whole lot easier and also would\nprevent confusion in the future.\n\nLet me know what you think.\n\nBest,\nShreyansh\n"},{"id":"536124","messageId":"cover.1771258688.git.phillip.wood@dunelm.org.uk","threadId":"64989","inReplyTo":"ebc16a74-0555-4951-8ec6-ff7fce6b6fcc@gmail.com","subject":"[PATCH 0/2] worktree_git_path(): remove repository argument","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-02-16T16:18:08Z","receivedAt":"2026-02-16T16:18:40Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nOn 14/02/2026 14:30, Phillip Wood wrote:\n>\n> I think that we should add a new function\n>\n> struct worktree *get_current_worktree(struct repository*);\n>\n> to worktree.c that constructs a struct worktree using repo->gitdir etc.\n> The worktree id is the last path component of repo->gitdir when the\n> repo->gitdir and repo->commondir differ, otherwise it is NULL. Then we\n> can use that function to get the current worktree rather than passing\n> NULL when we call wt_status_check_{rebase,bisect} from\n> wt_status_get_state().\n\nHere's what that looks like, the first patch adds\nget_worktree_from_repository() and uses it to avoid passing a NULL\nworktree to worktree_git_path(). The second patch then removes the\nrepository argument from that function and always uses wt->repo instead.\n\nShreyansh - I think your patches to clean up wt-status.c can probably proceed\nseparately to these if you remove the changes to\nwt_status_check_{bisect,rebase}().\n\nBase-Commit: 852829b3dd2fe4e7c7fc4d8badde644cf1b66c74\nPublished-As: https://github.com/phillipwood/git/releases/tag/pw%2Fget-current-worktree%2Fv1\nView-Changes-At: https://github.com/phillipwood/git/compare/852829b3d...23b8a355b\nFetch-It-Via: git fetch https://github.com/phillipwood/git pw/get-current-worktree/v1\n\n\nPhillip Wood (2):\n  wt-status: avoid passing NULL worktree\n  path: remove repository argument from worktree_git_path()\n\n builtin/fsck.c     |  2 +-\n builtin/worktree.c |  4 ++--\n path.c             |  9 ++++-----\n path.h             |  8 +++-----\n revision.c         |  2 +-\n worktree.c         | 22 +++++++++++++++++++++-\n worktree.h         |  5 ++++-\n wt-status.c        | 29 +++++++++++++++++++----------\n 8 files changed, 55 insertions(+), 26 deletions(-)\n\n-- \n2.52.0.362.g884e03848a9\n\n"},{"id":"536125","messageId":"409871a7d521b76c9eb811d3c49747e04de8defc.1771258688.git.phillip.wood@dunelm.org.uk","threadId":"64989","inReplyTo":"cover.1771258688.git.phillip.wood@dunelm.org.uk","subject":"[PATCH 1/2] wt-status: avoid passing NULL worktree","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-02-16T16:18:09Z","receivedAt":"2026-02-16T16:18:41Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nIn preparation for removing the repository argument from\nworktree_git_path() add a function to construct a \"struct worktree\"\nfrom a \"struct repository\" and use that to avoid passing a NULL\nworktree to wt_status_check_bisect() and wt_status_check_rebase().\n\nwt_status_check_bisect() and wt_status_check_rebase() have the following\ncallers:\n\n - branch.c:prepare_checked_out_branches() which loops over all\n   worktrees.\n\n - worktree.c:is_worktree_being_rebased() which is called from\n   builtin/branch.c:reject_rebase_or_bisect_branch() that loops over all\n   worktrees and worktree.c:is_shared_symref() which dereferences wt\n   earlier in the function.\n\n - wt-status:wt_status_get_state() which is updated to avoid passing a\n   NULL worktree by this patch.\n\nThis updates the only callers that pass a NULL worktree to\nworktree_git_path().\n\nSigned-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n---\n worktree.c  | 20 ++++++++++++++++++++\n worktree.h  |  5 ++++-\n wt-status.c | 15 ++++++++++++---\n 3 files changed, 36 insertions(+), 4 deletions(-)\n\ndiff --git a/worktree.c b/worktree.c\nindex 9308389cb6f..fd182c319b7 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -66,6 +66,26 @@ static int is_current_worktree(struct worktree *wt)\n \treturn is_current;\n }\n \n+struct worktree *get_worktree_from_repository(struct repository *repo)\n+{\n+\tstruct worktree *wt = xcalloc(1, sizeof(*wt));\n+\tchar *gitdir = absolute_pathdup(repo->gitdir);\n+\tchar *commondir = absolute_pathdup(repo->commondir);\n+\n+\twt->repo = repo;\n+\tif (repo->worktree)\n+\t\twt->path = absolute_pathdup(repo->worktree);\n+\twt->is_bare = !!repo->worktree;\n+\tif (fspathcmp(gitdir, commondir))\n+\t\twt->id = xstrdup(find_last_dir_sep(commondir) + 1);\n+\twt->is_current = is_current_worktree(wt);\n+\tadd_head_info(wt);\n+\n+\tfree(gitdir);\n+\tfree(commondir);\n+\treturn wt;\n+}\n+\n /*\n * When in a secondary worktree, and when extensions.worktreeConfig\n * is true, only $commondir/config and $commondir/worktrees/<id>/\ndiff --git a/worktree.h b/worktree.h\nindex e4bcccdc0ae..b162bbabd50 100644\n--- a/worktree.h\n+++ b/worktree.h\n@@ -38,7 +38,10 @@ struct worktree **get_worktrees(void);\n  */\n struct worktree **get_worktrees_without_reading_head(void);\n \n-/*\n+/* Construct a struct worktree from a struct repository */\n+struct worktree *get_worktree_from_repository(struct repository *repo);\n+\n+ /*\n  * Returns 1 if linked worktrees exist, 0 otherwise.\n  */\n int submodule_uses_worktrees(const char *path);\ndiff --git a/wt-status.c b/wt-status.c\nindex 95942399f8c..2debda534c1 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1747,6 +1747,9 @@ int wt_status_check_rebase(const struct worktree *wt,\n {\n \tstruct stat st;\n \n+\tif (!wt)\n+\t\tBUG(\"wt_status_check_rebase() called with NULL worktree\");\n+\n \tif (!stat(worktree_git_path(the_repository, wt, \"rebase-apply\"), &st)) {\n \t\tif (!stat(worktree_git_path(the_repository, wt, \"rebase-apply/applying\"), &st)) {\n \t\t\tstate->am_in_progress = 1;\n@@ -1774,6 +1777,9 @@ int wt_status_check_bisect(const struct worktree *wt,\n {\n \tstruct stat st;\n \n+\tif (!wt)\n+\t\tBUG(\"wt_status_check_bisect() called with NULL worktree\");\n+\n \tif (!stat(worktree_git_path(the_repository, wt, \"BISECT_LOG\"), &st)) {\n \t\tstate->bisect_in_progress = 1;\n \t\tstate->bisecting_from = get_branch(wt, \"BISECT_START\");\n@@ -1819,18 +1825,19 @@ void wt_status_get_state(struct repository *r,\n \tstruct stat st;\n \tstruct object_id oid;\n \tenum replay_action action;\n+\tstruct worktree *wt = get_worktree_from_repository(r);\n \n \tif (!stat(git_path_merge_head(r), &st)) {\n-\t\twt_status_check_rebase(NULL, state);\n+\t\twt_status_check_rebase(wt, state);\n \t\tstate->merge_in_progress = 1;\n-\t} else if (wt_status_check_rebase(NULL, state)) {\n+\t} else if (wt_status_check_rebase(wt, state)) {\n \t\t;\t\t/* all set */\n \t} else if (refs_ref_exists(get_main_ref_store(r), \"CHERRY_PICK_HEAD\") &&\n \t\t   !repo_get_oid(r, \"CHERRY_PICK_HEAD\", &oid)) {\n \t\tstate->cherry_pick_in_progress = 1;\n \t\toidcpy(&state->cherry_pick_head_oid, &oid);\n \t}\n-\twt_status_check_bisect(NULL, state);\n+\twt_status_check_bisect(wt, state);\n \tif (refs_ref_exists(get_main_ref_store(r), \"REVERT_HEAD\") &&\n \t    !repo_get_oid(r, \"REVERT_HEAD\", &oid)) {\n \t\tstate->revert_in_progress = 1;\n@@ -1848,6 +1855,8 @@ void wt_status_get_state(struct repository *r,\n \tif (get_detached_from)\n \t\twt_status_get_detached_from(r, state);\n \twt_status_check_sparse_checkout(r, state);\n+\n+\tfree_worktree(wt);\n }\n \n static void wt_longstatus_print_state(struct wt_status *s)\n-- \n2.52.0.362.g884e03848a9\n\n"},{"id":"536126","messageId":"23b8a355b414da2b6216a50006bf2276dd3ea6ae.1771258688.git.phillip.wood@dunelm.org.uk","threadId":"64989","inReplyTo":"cover.1771258688.git.phillip.wood@dunelm.org.uk","subject":"[PATCH 2/2] path: remove repository argument from worktree_git_path()","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-02-16T16:18:10Z","receivedAt":"2026-02-16T16:18:42Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nworktree_git_path() takes a struct repository and a struct worktree\nwhich also contains a struct repository. The repository argument\nwas added by a973f60dc7c (path: stop relying on `the_repository` in\n`worktree_git_path()`, 2024-08-13) and exists because the worktree\nargument is optional. Having two ways of passing a repository is\na potential foot-gun as if the the worktree argument is present the\nrepository argument must match the worktree's repository member. Since\nthe last commit there are no callers that pass a NULL worktree so lets\nremove the repository argument. This removes the potential confusion\nand lets us delete a number of uses of \"the_repository\".\n\nworktree_git_path() has the following callers:\n\n - builtin/worktree.c:validate_no_submodules() which is called from\n   check_clean_worktree() and move_worktree(), both of which supply\n   a non-NULL worktree.\n\n - builtin/fsck.c:cmd_fsck() which loops over all worktrees.\n\n - revision.c:add_index_objects_to_pending() which loops over all\n   worktrees.\n\n - worktree.c:worktree_lock_reason() which dereferences wt before\n   calling worktree_git_path().\n\n - wt-status.c:wt_status_check_bisect() and wt_status_check_rebase()\n   which are always called with a non-NULL worktree after the last\n   commit.\n\n - wt-status.c:git_branch() which is only called by\n   wt_status_check_bisect() and wt_status_check_rebase().\n\nSigned-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n---\n builtin/fsck.c     |  2 +-\n builtin/worktree.c |  4 ++--\n path.c             |  9 ++++-----\n path.h             |  8 +++-----\n revision.c         |  2 +-\n worktree.c         |  2 +-\n wt-status.c        | 14 +++++++-------\n 7 files changed, 19 insertions(+), 22 deletions(-)\n\ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex 0512f78a87f..42ba0afb91a 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -1137,7 +1137,7 @@ int cmd_fsck(int argc,\n \t\t\t * and may get overwritten by other calls\n \t\t\t * while we're examining the index.\n \t\t\t */\n-\t\t\tpath = xstrdup(worktree_git_path(the_repository, wt, \"index\"));\n+\t\t\tpath = xstrdup(worktree_git_path(wt, \"index\"));\n \t\t\twt_gitdir = get_worktree_git_dir(wt);\n \n \t\t\tread_index_from(&istate, path, wt_gitdir);\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 3d6547c23b4..62fd4642e5d 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -1191,14 +1191,14 @@ static void validate_no_submodules(const struct worktree *wt)\n \n \twt_gitdir = get_worktree_git_dir(wt);\n \n-\tif (is_directory(worktree_git_path(the_repository, wt, \"modules\"))) {\n+\tif (is_directory(worktree_git_path(wt, \"modules\"))) {\n \t\t/*\n \t\t * There could be false positives, e.g. the \"modules\"\n \t\t * directory exists but is empty. But it's a rare case and\n \t\t * this simpler check is probably good enough for now.\n \t\t */\n \t\tfound_submodules = 1;\n-\t} else if (read_index_from(&istate, worktree_git_path(the_repository, wt, \"index\"),\n+\t} else if (read_index_from(&istate, worktree_git_path(wt, \"index\"),\n \t\t\t\t   wt_gitdir) > 0) {\n \t\tfor (i = 0; i < istate.cache_nr; i++) {\n \t\t\tstruct cache_entry *ce = istate.cache[i];\ndiff --git a/path.c b/path.c\nindex d726537622c..073f631b914 100644\n--- a/path.c\n+++ b/path.c\n@@ -486,17 +486,16 @@ const char *mkpath(const char *fmt, ...)\n \treturn cleanup_path(pathname->buf);\n }\n \n-const char *worktree_git_path(struct repository *r,\n-\t\t\t      const struct worktree *wt, const char *fmt, ...)\n+const char *worktree_git_path(const struct worktree *wt, const char *fmt, ...)\n {\n \tstruct strbuf *pathname = get_pathname();\n \tva_list args;\n \n-\tif (wt && wt->repo != r)\n-\t\tBUG(\"worktree not connected to expected repository\");\n+\tif (!wt)\n+\t\tBUG(\"%s() called with NULL worktree\", __func__);\n \n \tva_start(args, fmt);\n-\trepo_git_pathv(r, wt, pathname, fmt, args);\n+\trepo_git_pathv(wt->repo, wt, pathname, fmt, args);\n \tva_end(args);\n \treturn pathname->buf;\n }\ndiff --git a/path.h b/path.h\nindex 0ec95a0b079..cbcad254a0a 100644\n--- a/path.h\n+++ b/path.h\n@@ -66,13 +66,11 @@ const char *repo_git_path_replace(struct repository *repo,\n \n /*\n  * Similar to repo_git_path() but can produce paths for a specified\n- * worktree instead of current one. When no worktree is given, then the path is\n- * computed relative to main worktree of the given repository.\n+ * worktree instead of current one.\n  */\n-const char *worktree_git_path(struct repository *r,\n-\t\t\t      const struct worktree *wt,\n+const char *worktree_git_path(const struct worktree *wt,\n \t\t\t      const char *fmt, ...)\n-\t__attribute__((format (printf, 3, 4)));\n+\t__attribute__((format (printf, 2, 3)));\n \n /*\n  * The `repo_worktree_path` family of functions will construct a path into a\ndiff --git a/revision.c b/revision.c\nindex 29972c3a198..ca3481c1902 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1847,7 +1847,7 @@ void add_index_objects_to_pending(struct rev_info *revs, unsigned int flags)\n \t\twt_gitdir = get_worktree_git_dir(wt);\n \n \t\tif (read_index_from(&istate,\n-\t\t\t\t    worktree_git_path(the_repository, wt, \"index\"),\n+\t\t\t\t    worktree_git_path(wt, \"index\"),\n \t\t\t\t    wt_gitdir) > 0)\n \t\t\tdo_add_index_objects_to_pending(revs, &istate, flags);\n \ndiff --git a/worktree.c b/worktree.c\nindex fd182c319b7..efd2b75608d 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -308,7 +308,7 @@ const char *worktree_lock_reason(struct worktree *wt)\n \tif (!wt->lock_reason_valid) {\n \t\tstruct strbuf path = STRBUF_INIT;\n \n-\t\tstrbuf_addstr(&path, worktree_git_path(the_repository, wt, \"locked\"));\n+\t\tstrbuf_addstr(&path, worktree_git_path(wt, \"locked\"));\n \t\tif (file_exists(path.buf)) {\n \t\t\tstruct strbuf lock_reason = STRBUF_INIT;\n \t\t\tif (strbuf_read_file(&lock_reason, path.buf, 0) < 0)\ndiff --git a/wt-status.c b/wt-status.c\nindex 2debda534c1..68257d6dfd2 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1648,7 +1648,7 @@ static char *get_branch(const struct worktree *wt, const char *path)\n \tstruct object_id oid;\n \tconst char *branch_name;\n \n-\tif (strbuf_read_file(&sb, worktree_git_path(the_repository, wt, \"%s\", path), 0) <= 0)\n+\tif (strbuf_read_file(&sb, worktree_git_path(wt, \"%s\", path), 0) <= 0)\n \t\tgoto got_nothing;\n \n \twhile (sb.len && sb.buf[sb.len - 1] == '\\n')\n@@ -1750,18 +1750,18 @@ int wt_status_check_rebase(const struct worktree *wt,\n \tif (!wt)\n \t\tBUG(\"wt_status_check_rebase() called with NULL worktree\");\n \n-\tif (!stat(worktree_git_path(the_repository, wt, \"rebase-apply\"), &st)) {\n-\t\tif (!stat(worktree_git_path(the_repository, wt, \"rebase-apply/applying\"), &st)) {\n+\tif (!stat(worktree_git_path(wt, \"rebase-apply\"), &st)) {\n+\t\tif (!stat(worktree_git_path(wt, \"rebase-apply/applying\"), &st)) {\n \t\t\tstate->am_in_progress = 1;\n-\t\t\tif (!stat(worktree_git_path(the_repository, wt, \"rebase-apply/patch\"), &st) && !st.st_size)\n+\t\t\tif (!stat(worktree_git_path(wt, \"rebase-apply/patch\"), &st) && !st.st_size)\n \t\t\t\tstate->am_empty_patch = 1;\n \t\t} else {\n \t\t\tstate->rebase_in_progress = 1;\n \t\t\tstate->branch = get_branch(wt, \"rebase-apply/head-name\");\n \t\t\tstate->onto = get_branch(wt, \"rebase-apply/onto\");\n \t\t}\n-\t} else if (!stat(worktree_git_path(the_repository, wt, \"rebase-merge\"), &st)) {\n-\t\tif (!stat(worktree_git_path(the_repository, wt, \"rebase-merge/interactive\"), &st))\n+\t} else if (!stat(worktree_git_path(wt, \"rebase-merge\"), &st)) {\n+\t\tif (!stat(worktree_git_path(wt, \"rebase-merge/interactive\"), &st))\n \t\t\tstate->rebase_interactive_in_progress = 1;\n \t\telse\n \t\t\tstate->rebase_in_progress = 1;\n@@ -1780,7 +1780,7 @@ int wt_status_check_bisect(const struct worktree *wt,\n \tif (!wt)\n \t\tBUG(\"wt_status_check_bisect() called with NULL worktree\");\n \n-\tif (!stat(worktree_git_path(the_repository, wt, \"BISECT_LOG\"), &st)) {\n+\tif (!stat(worktree_git_path(wt, \"BISECT_LOG\"), &st)) {\n \t\tstate->bisect_in_progress = 1;\n \t\tstate->bisecting_from = get_branch(wt, \"BISECT_START\");\n \t\treturn 1;\n-- \n2.52.0.362.g884e03848a9\n\n"},{"id":"536127","messageId":"66b0f03a-36ab-4305-814e-6d964f5d33c4@gmail.com","threadId":"64989","inReplyTo":"20260215090815.46544-1-shreyanshpaliwalcmsmn@gmail.com","subject":"Re: [RFC][PATCH 2/2] worktree: stop passing NULL as primary worktree","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-02-16T16:18:58Z","receivedAt":"2026-02-16T16:19:01Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 15/02/2026 08:56, Shreyansh Paliwal wrote:\n>> I've cc'd Eric for a second opinion\n>>\n>> On 13/02/2026 22:29, Junio C Hamano wrote:\n>>> Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com> writes:\n>>>\n>>>> diff --git a/path.c b/path.c\n>>>> index d726537622..4ac86e1e58 100644\n>>>> --- a/path.c\n>>>> +++ b/path.c\n>>>> @@ -408,9 +408,7 @@ static void strbuf_worktree_gitdir(struct strbuf *buf,\n>>>>    \t\t\t\t   const struct repository *repo,\n>>>>    \t\t\t\t   const struct worktree *wt)\n>>>>    {\n>>>> -\tif (!wt)\n>>>> -\t\tstrbuf_addstr(buf, repo->gitdir);\n>>>> -\telse if (!wt->id)\n>>>> +\tif (is_main_worktree(wt))\n>>>>    \t\tstrbuf_addstr(buf, repo->commondir);\n>>>>    \telse\n>>>>    \t\trepo_common_path_append(repo, buf, \"worktrees/%s\", wt->id);\n>>>\n>>> This is curious.\n>>>\n>>> We used to treat \"wt==NULL\" and \"wt->id==NULL\" differently.  Now we\n>>> use repo->commondir for both.  For the primary worktree, it ought to\n>>> be the same as repo->gitdir, so it should not matter, but makes me\n>>> wonder what the reason behind this difference in the original.\n>>>\n>>> We have been assuming that wt==NULL and wt->id==NULL both meant the\n>>> same thing: \"we are talking about the primary worktree\".  But the\n>>> code around here before this patch seems to behave differently.  Is\n>>> our assumption incorrect and are we making a mistake by conflating\n>>> these two conditions into one?\n>>\n>> My understanding is that wt==NULL means \"use the current worktree\" and\n>> wt->id==NULL means \"this is the main worktree\". That would explain why\n>> we use repo->gitdir above when wt==NULL and repo->commondir when\n>> wt->id==NULL, as repo->gitdir is the gitdir of the current worktree and\n>> repo->commondir will be the gitdir of the main worktree. If we look at\n>> the code in wt-status.c that's passing a NULL worktree it wants to know\n>> about the status of the current worktree, not the main worktree.\n>>\n>> I think that we should add a new function\n>>\n>> struct worktree *get_current_worktree(struct repository*);\n>>\n>> to worktree.c that constructs a struct worktree using repo->gitdir etc.\n>> The worktree id is the last path component of repo->gitdir when the\n>> repo->gitdir and repo->commondir differ, otherwise it is NULL. Then we\n>> can use that function to get the current worktree rather than passing\n>> NULL when we call wt_status_check_{rebase,bisect} from\n>> wt_status_get_state(). We should also think about whether we should\n>> change wt_status_get_state() to take a \"struct worktree*\" rather than a\n>> \"struct repository*\" instead (I've not looked at the callers to see if\n>> that's sensible).\n>>\n>> With that, we can gradually clean up uses of wt==NULL in the rest of the\n>> codebase overtime and eventually remove support for it from worktree.c\n>> rather than having a big flag-day patch. I don't think we need to change\n>> uses of wt-id==NULL.\n> \n> Thanks a lot for clarifying. This helps solve the doubt regarding the\n> different usage of !wt and !wt->id in strbuf_worktree_gitdir(). I realize\n> we have been under the wrong assumption about what wt == NULL represents.\n> \n> But I still have a few points where I’m a bit confused,\n> \n> If wt == NULL is meant to represent the current worktree, then what role\n> wt->is_current plays in the present implementation, and if they both\n> represent the same thing then wt->is_current wouldn't make sense if wt is\n> already NULL in the case of a current worktree.\n\nwt == NULL is a shorthand that callers can use if they don't have a \nstruct worktree to pass, it does not replace wt->is_current when listing \nall worktrees with get_worktrees() which returns a NULL terminated list.\n\n> Beyond representation, I’m not quite understanding on how call sites are\n> logically differentiating on whether the intent is to 'operate on the\n> worktree we are in' or 'operate on the primary one'.\n\nWe're nearly always interested in the current one. The primary worktree \nis special in that it cannot be moved or deleted with \"git worktree\" but \ngit commands generally operate on the current worktree and occasionally \ncheck the state of other worktrees (for example to avoid checking out \nthe same branch in two different worktrees).\n\n> And I think if we included both in struct repository (r->main_wt, r->current_wt)\n> so accessing either of them would be a whole lot easier and also would\n> prevent confusion in the future.\n\nIt might be worth adding the current worktree (or probably the worktree \nthat the struct repository refers to) to struct repository in the future \nbut I think that is outside the scope of cleaning up wt-status.c\n\nThanks\n\nPhillip\n\n> Let me know what you think.\n> \n> Best,\n> Shreyansh\n> \n\n"},{"id":"536140","messageId":"xmqqldgsdjy2.fsf@gitster.g","threadId":"64989","inReplyTo":"66b0f03a-36ab-4305-814e-6d964f5d33c4@gmail.com","subject":"Re: [RFC][PATCH 2/2] worktree: stop passing NULL as primary worktree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-17T05:21:09Z","receivedAt":"2026-02-17T05:21:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood <phillip.wood123@gmail.com> writes:\n\n> It might be worth adding the current worktree (or probably the worktree \n> that the struct repository refers to) to struct repository in the future \n> but I think that is outside the scope of cleaning up wt-status.c\n\nThanks for being conservative.  I would agree that we would need\nfurther thought before making such a change, and if we do not need\nit if we have \"give me the current worktree struct\" call to clean up\nthe wt-status.c where passing NULL to indicate the \"current\" is\nproblematic.\n"},{"id":"536162","messageId":"89c78ce2-1783-416d-9ae5-ef51f6bde58d@gmail.com","threadId":"64989","inReplyTo":"409871a7d521b76c9eb811d3c49747e04de8defc.1771258688.git.phillip.wood@dunelm.org.uk","subject":"Re: [PATCH 1/2] wt-status: avoid passing NULL worktree","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-02-17T09:23:04Z","receivedAt":"2026-02-17T09:23:10Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 16/02/2026 16:18, Phillip Wood wrote:\n> \n> +struct worktree *get_worktree_from_repository(struct repository *repo)\n> +{\n> +\tstruct worktree *wt = xcalloc(1, sizeof(*wt));\n> +\tchar *gitdir = absolute_pathdup(repo->gitdir);\n> +\tchar *commondir = absolute_pathdup(repo->commondir);\n> +\n> +\twt->repo = repo;\n> +\tif (repo->worktree)\n> +\t\twt->path = absolute_pathdup(repo->worktree);\n> +\twt->is_bare = !!repo->worktree;\n> +\tif (fspathcmp(gitdir, commondir))\n> +\t\twt->id = xstrdup(find_last_dir_sep(commondir) + 1);\n\nOops s/commondir/gitdir/ - I'll wait to see if there are any other \ncomments before re-rolling (perhaps with a test that runs git status on \na rebase in a linked worktree)\n\nThanks\n\nPhillip\n\n> +\twt->is_current = is_current_worktree(wt);\n> +\tadd_head_info(wt);\n> +\n> +\tfree(gitdir);\n> +\tfree(commondir);\n> +\treturn wt;\n> +}\n> +\n>   /*\n>   * When in a secondary worktree, and when extensions.worktreeConfig\n>   * is true, only $commondir/config and $commondir/worktrees/<id>/\n> diff --git a/worktree.h b/worktree.h\n> index e4bcccdc0ae..b162bbabd50 100644\n> --- a/worktree.h\n> +++ b/worktree.h\n> @@ -38,7 +38,10 @@ struct worktree **get_worktrees(void);\n>    */\n>   struct worktree **get_worktrees_without_reading_head(void);\n>   \n> -/*\n> +/* Construct a struct worktree from a struct repository */\n> +struct worktree *get_worktree_from_repository(struct repository *repo);\n> +\n> + /*\n>    * Returns 1 if linked worktrees exist, 0 otherwise.\n>    */\n>   int submodule_uses_worktrees(const char *path);\n> diff --git a/wt-status.c b/wt-status.c\n> index 95942399f8c..2debda534c1 100644\n> --- a/wt-status.c\n> +++ b/wt-status.c\n> @@ -1747,6 +1747,9 @@ int wt_status_check_rebase(const struct worktree *wt,\n>   {\n>   \tstruct stat st;\n>   \n> +\tif (!wt)\n> +\t\tBUG(\"wt_status_check_rebase() called with NULL worktree\");\n> +\n>   \tif (!stat(worktree_git_path(the_repository, wt, \"rebase-apply\"), &st)) {\n>   \t\tif (!stat(worktree_git_path(the_repository, wt, \"rebase-apply/applying\"), &st)) {\n>   \t\t\tstate->am_in_progress = 1;\n> @@ -1774,6 +1777,9 @@ int wt_status_check_bisect(const struct worktree *wt,\n>   {\n>   \tstruct stat st;\n>   \n> +\tif (!wt)\n> +\t\tBUG(\"wt_status_check_bisect() called with NULL worktree\");\n> +\n>   \tif (!stat(worktree_git_path(the_repository, wt, \"BISECT_LOG\"), &st)) {\n>   \t\tstate->bisect_in_progress = 1;\n>   \t\tstate->bisecting_from = get_branch(wt, \"BISECT_START\");\n> @@ -1819,18 +1825,19 @@ void wt_status_get_state(struct repository *r,\n>   \tstruct stat st;\n>   \tstruct object_id oid;\n>   \tenum replay_action action;\n> +\tstruct worktree *wt = get_worktree_from_repository(r);\n>   \n>   \tif (!stat(git_path_merge_head(r), &st)) {\n> -\t\twt_status_check_rebase(NULL, state);\n> +\t\twt_status_check_rebase(wt, state);\n>   \t\tstate->merge_in_progress = 1;\n> -\t} else if (wt_status_check_rebase(NULL, state)) {\n> +\t} else if (wt_status_check_rebase(wt, state)) {\n>   \t\t;\t\t/* all set */\n>   \t} else if (refs_ref_exists(get_main_ref_store(r), \"CHERRY_PICK_HEAD\") &&\n>   \t\t   !repo_get_oid(r, \"CHERRY_PICK_HEAD\", &oid)) {\n>   \t\tstate->cherry_pick_in_progress = 1;\n>   \t\toidcpy(&state->cherry_pick_head_oid, &oid);\n>   \t}\n> -\twt_status_check_bisect(NULL, state);\n> +\twt_status_check_bisect(wt, state);\n>   \tif (refs_ref_exists(get_main_ref_store(r), \"REVERT_HEAD\") &&\n>   \t    !repo_get_oid(r, \"REVERT_HEAD\", &oid)) {\n>   \t\tstate->revert_in_progress = 1;\n> @@ -1848,6 +1855,8 @@ void wt_status_get_state(struct repository *r,\n>   \tif (get_detached_from)\n>   \t\twt_status_get_detached_from(r, state);\n>   \twt_status_check_sparse_checkout(r, state);\n> +\n> +\tfree_worktree(wt);\n>   }\n>   \n>   static void wt_longstatus_print_state(struct wt_status *s)\n\n"},{"id":"536165","messageId":"20260217101016.13641-1-shreyanshpaliwalcmsmn@gmail.com","threadId":"64989","inReplyTo":"66b0f03a-36ab-4305-814e-6d964f5d33c4@gmail.com","subject":"Re: [RFC][PATCH 2/2] worktree: stop passing NULL as primary worktree","fromName":"Shreyansh Paliwal","fromEmail":"shreyanshpaliwalcmsmn@gmail.com","sentAt":"2026-02-17T10:09:34Z","receivedAt":"2026-02-17T10:10:52Z","isPatch":true,"sender":{"key":"shreyanshpaliwalcmsmn@gmail.com","avatar":"https://avatars.githubusercontent.com/u/152720574?v=4"},"body":"> On 15/02/2026 08:56, Shreyansh Paliwal wrote:\n> >> I've cc'd Eric for a second opinion\n> >>\n> >> On 13/02/2026 22:29, Junio C Hamano wrote:\n> >>> Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com> writes:\n> >>>\n> >>>> diff --git a/path.c b/path.c\n> >>>> index d726537622..4ac86e1e58 100644\n> >>>> --- a/path.c\n> >>>> +++ b/path.c\n> >>>> @@ -408,9 +408,7 @@ static void strbuf_worktree_gitdir(struct strbuf *buf,\n> >>>>    \t\t\t\t   const struct repository *repo,\n> >>>>    \t\t\t\t   const struct worktree *wt)\n> >>>>    {\n> >>>> -\tif (!wt)\n> >>>> -\t\tstrbuf_addstr(buf, repo->gitdir);\n> >>>> -\telse if (!wt->id)\n> >>>> +\tif (is_main_worktree(wt))\n> >>>>    \t\tstrbuf_addstr(buf, repo->commondir);\n> >>>>    \telse\n> >>>>    \t\trepo_common_path_append(repo, buf, \"worktrees/%s\", wt->id);\n> >>>\n> >>> This is curious.\n> >>>\n> >>> We used to treat \"wt==NULL\" and \"wt->id==NULL\" differently.  Now we\n> >>> use repo->commondir for both.  For the primary worktree, it ought to\n> >>> be the same as repo->gitdir, so it should not matter, but makes me\n> >>> wonder what the reason behind this difference in the original.\n> >>>\n> >>> We have been assuming that wt==NULL and wt->id==NULL both meant the\n> >>> same thing: \"we are talking about the primary worktree\".  But the\n> >>> code around here before this patch seems to behave differently.  Is\n> >>> our assumption incorrect and are we making a mistake by conflating\n> >>> these two conditions into one?\n> >>\n> >> My understanding is that wt==NULL means \"use the current worktree\" and\n> >> wt->id==NULL means \"this is the main worktree\". That would explain why\n> >> we use repo->gitdir above when wt==NULL and repo->commondir when\n> >> wt->id==NULL, as repo->gitdir is the gitdir of the current worktree and\n> >> repo->commondir will be the gitdir of the main worktree. If we look at\n> >> the code in wt-status.c that's passing a NULL worktree it wants to know\n> >> about the status of the current worktree, not the main worktree.\n> >>\n> >> I think that we should add a new function\n> >>\n> >> struct worktree *get_current_worktree(struct repository*);\n> >>\n> >> to worktree.c that constructs a struct worktree using repo->gitdir etc.\n> >> The worktree id is the last path component of repo->gitdir when the\n> >> repo->gitdir and repo->commondir differ, otherwise it is NULL. Then we\n> >> can use that function to get the current worktree rather than passing\n> >> NULL when we call wt_status_check_{rebase,bisect} from\n> >> wt_status_get_state(). We should also think about whether we should\n> >> change wt_status_get_state() to take a \"struct worktree*\" rather than a\n> >> \"struct repository*\" instead (I've not looked at the callers to see if\n> >> that's sensible).\n> >>\n> >> With that, we can gradually clean up uses of wt==NULL in the rest of the\n> >> codebase overtime and eventually remove support for it from worktree.c\n> >> rather than having a big flag-day patch. I don't think we need to change\n> >> uses of wt-id==NULL.\n> >\n> > Thanks a lot for clarifying. This helps solve the doubt regarding the\n> > different usage of !wt and !wt->id in strbuf_worktree_gitdir(). I realize\n> > we have been under the wrong assumption about what wt == NULL represents.\n> >\n> > But I still have a few points where I’m a bit confused,\n> >\n> > If wt == NULL is meant to represent the current worktree, then what role\n> > wt->is_current plays in the present implementation, and if they both\n> > represent the same thing then wt->is_current wouldn't make sense if wt is\n> > already NULL in the case of a current worktree.\n>\n> wt == NULL is a shorthand that callers can use if they don't have a\n> struct worktree to pass, it does not replace wt->is_current when listing\n> all worktrees with get_worktrees() which returns a NULL terminated list.\n\nAh, yes got it.\n\n> > Beyond representation, I’m not quite understanding on how call sites are\n> > logically differentiating on whether the intent is to 'operate on the\n> > worktree we are in' or 'operate on the primary one'.\n>\n> We're nearly always interested in the current one. The primary worktree\n> is special in that it cannot be moved or deleted with \"git worktree\" but\n> git commands generally operate on the current worktree and occasionally\n> check the state of other worktrees (for example to avoid checking out\n> the same branch in two different worktrees).\n\nHmm. Understood.\n\n> > And I think if we included both in struct repository (r->main_wt, r->current_wt)\n> > so accessing either of them would be a whole lot easier and also would\n> > prevent confusion in the future.\n>\n> It might be worth adding the current worktree (or probably the worktree\n> that the struct repository refers to) to struct repository in the future\n> but I think that is outside the scope of cleaning up wt-status.c\n\nRight.\n\nThanks for clearing it out :)\n\nBest,\nShreyansh\n"},{"id":"536166","messageId":"20260217101242.14688-1-shreyanshpaliwalcmsmn@gmail.com","threadId":"64989","inReplyTo":"cover.1771258688.git.phillip.wood@dunelm.org.uk","subject":"Re: [PATCH 0/2] worktree_git_path(): remove repository argument","fromName":"Shreyansh Paliwal","fromEmail":"shreyanshpaliwalcmsmn@gmail.com","sentAt":"2026-02-17T10:12:21Z","receivedAt":"2026-02-17T10:12:52Z","isPatch":true,"sender":{"key":"shreyanshpaliwalcmsmn@gmail.com","avatar":"https://avatars.githubusercontent.com/u/152720574?v=4"},"body":"> On 14/02/2026 14:30, Phillip Wood wrote:\n> >\n> > I think that we should add a new function\n> >\n> > struct worktree *get_current_worktree(struct repository*);\n> >\n> > to worktree.c that constructs a struct worktree using repo->gitdir etc.\n> > The worktree id is the last path component of repo->gitdir when the\n> > repo->gitdir and repo->commondir differ, otherwise it is NULL. Then we\n> > can use that function to get the current worktree rather than passing\n> > NULL when we call wt_status_check_{rebase,bisect} from\n> > wt_status_get_state().\n>\n> Here's what that looks like, the first patch adds\n> get_worktree_from_repository() and uses it to avoid passing a NULL\n> worktree to worktree_git_path(). The second patch then removes the\n> repository argument from that function and always uses wt->repo instead.\n>\n> Shreyansh - I think your patches to clean up wt-status.c can probably proceed\n> separately to these if you remove the changes to\n> wt_status_check_{bisect,rebase}().\n\nCool. I'll send a revised version on the original thread.\n\nBest,\nShreyansh\n"},{"id":"536168","messageId":"20260217101950.15731-1-shreyanshpaliwalcmsmn@gmail.com","threadId":"64989","inReplyTo":"89c78ce2-1783-416d-9ae5-ef51f6bde58d@gmail.com","subject":"Re: [PATCH 1/2] wt-status: avoid passing NULL worktree","fromName":"Shreyansh Paliwal","fromEmail":"shreyanshpaliwalcmsmn@gmail.com","sentAt":"2026-02-17T10:18:38Z","receivedAt":"2026-02-17T10:20:00Z","isPatch":true,"sender":{"key":"shreyanshpaliwalcmsmn@gmail.com","avatar":"https://avatars.githubusercontent.com/u/152720574?v=4"},"body":"> On 16/02/2026 16:18, Phillip Wood wrote:\n> >\n> > +struct worktree *get_worktree_from_repository(struct repository *repo)\n> > +{\n> > +\tstruct worktree *wt = xcalloc(1, sizeof(*wt));\n> > +\tchar *gitdir = absolute_pathdup(repo->gitdir);\n> > +\tchar *commondir = absolute_pathdup(repo->commondir);\n> > +\n> > +\twt->repo = repo;\n> > +\tif (repo->worktree)\n> > +\t\twt->path = absolute_pathdup(repo->worktree);\n> > +\twt->is_bare = !!repo->worktree;\n> > +\tif (fspathcmp(gitdir, commondir))\n> > +\t\twt->id = xstrdup(find_last_dir_sep(commondir) + 1);\n>\n> Oops s/commondir/gitdir/ - I'll wait to see if there are any other\n> comments before re-rolling (perhaps with a test that runs git status on\n> a rebase in a linked worktree)\n>\n> Thanks\n>\n> Phillip\n\nI wanted to just check for my understanding: the NULL usage of worktree in\nget_worktree_git_dir() caller, repo_git_pathv() callers and inside function\nadd_reflogs_to_pending() is intentionally left unchanged for now,\nand is meant for a follow-up once this gets gets finalized.\nor it is out of scope wrt this cleanup?\n\nBest,\nShreyansh\n"},{"id":"536192","messageId":"0619603c-278c-42e3-a186-a674a124a451@gmail.com","threadId":"64989","inReplyTo":"20260217101950.15731-1-shreyanshpaliwalcmsmn@gmail.com","subject":"Re: [PATCH 1/2] wt-status: avoid passing NULL worktree","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-02-17T15:20:27Z","receivedAt":"2026-02-17T15:20:31Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 17/02/2026 10:18, Shreyansh Paliwal wrote:\n> \n> I wanted to just check for my understanding: the NULL usage of worktree in\n> get_worktree_git_dir() caller, repo_git_pathv() callers and inside function\n> add_reflogs_to_pending() is intentionally left unchanged for now,\n> and is meant for a follow-up once this gets gets finalized.\n> or it is out of scope wrt this cleanup?\n\nI left those out as they're not needed for cleaning up wt-status.c. They \ncan be cleaned up separately if you're still interesting in working on \nthat. It would certainly be worth removing \"the_repository\" from \nget_worktree_git_dir(). The others are not quite so bad as they don't \nuse \"the_repository\".\n\nThanks\n\nPhillip\n\n"},{"id":"536194","messageId":"d7fe45b3-4a75-4a28-aa0e-74619fbe6a2f@gmail.com","threadId":"64989","inReplyTo":"20260217101242.14688-1-shreyanshpaliwalcmsmn@gmail.com","subject":"Re: [PATCH 0/2] worktree_git_path(): remove repository argument","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-02-17T15:22:59Z","receivedAt":"2026-02-17T15:23:02Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 17/02/2026 10:12, Shreyansh Paliwal wrote:\n>> On 14/02/2026 14:30, Phillip Wood wrote:\n>>>\n>>> I think that we should add a new function\n>>>\n>>> struct worktree *get_current_worktree(struct repository*);\n>>>\n>>> to worktree.c that constructs a struct worktree using repo->gitdir etc.\n>>> The worktree id is the last path component of repo->gitdir when the\n>>> repo->gitdir and repo->commondir differ, otherwise it is NULL. Then we\n>>> can use that function to get the current worktree rather than passing\n>>> NULL when we call wt_status_check_{rebase,bisect} from\n>>> wt_status_get_state().\n>>\n>> Here's what that looks like, the first patch adds\n>> get_worktree_from_repository() and uses it to avoid passing a NULL\n>> worktree to worktree_git_path(). The second patch then removes the\n>> repository argument from that function and always uses wt->repo instead.\n>>\n>> Shreyansh - I think your patches to clean up wt-status.c can probably proceed\n>> separately to these if you remove the changes to\n>> wt_status_check_{bisect,rebase}().\n> \n> Cool. I'll send a revised version on the original thread.\n\nGreat, I hope I'm not stepping on your toes posting these patches. By \nthe time I'd worked out what was needed and checked all the callers were \npassing a non-NULL worktree argument I had the code changes and commit \nmessages so I thought I'd post them.\n\nThanks\n\nPhillip\n"},{"id":"536201","messageId":"20260217163909.55094-1-shreyanshpaliwalcmsmn@gmail.com","threadId":"64989","inReplyTo":"0619603c-278c-42e3-a186-a674a124a451@gmail.com","subject":"Re: [PATCH 1/2] wt-status: avoid passing NULL worktree","fromName":"Shreyansh Paliwal","fromEmail":"shreyanshpaliwalcmsmn@gmail.com","sentAt":"2026-02-17T16:38:59Z","receivedAt":"2026-02-17T16:39:19Z","isPatch":true,"sender":{"key":"shreyanshpaliwalcmsmn@gmail.com","avatar":"https://avatars.githubusercontent.com/u/152720574?v=4"},"body":"> On 17/02/2026 10:18, Shreyansh Paliwal wrote:\n> >\n> > I wanted to just check for my understanding: the NULL usage of worktree in\n> > get_worktree_git_dir() caller, repo_git_pathv() callers and inside function\n> > add_reflogs_to_pending() is intentionally left unchanged for now,\n> > and is meant for a follow-up once this gets gets finalized.\n> > or it is out of scope wrt this cleanup?\n>\n> I left those out as they're not needed for cleaning up wt-status.c. They\n> can be cleaned up separately if you're still interesting in working on\n> that. It would certainly be worth removing \"the_repository\" from\n> get_worktree_git_dir(). The others are not quite so bad as they don't\n> use \"the_repository\".\n\nThanks, that makes sense.\nI will send a patch for get_worktree_git_dir() sometime later.\n\nBest,\nShreyansh\n"},{"id":"536202","messageId":"20260217164615.55916-1-shreyanshpaliwalcmsmn@gmail.com","threadId":"64989","inReplyTo":"d7fe45b3-4a75-4a28-aa0e-74619fbe6a2f@gmail.com","subject":"Re: [PATCH 0/2] worktree_git_path(): remove repository argument","fromName":"Shreyansh Paliwal","fromEmail":"shreyanshpaliwalcmsmn@gmail.com","sentAt":"2026-02-17T16:45:49Z","receivedAt":"2026-02-17T16:46:27Z","isPatch":true,"sender":{"key":"shreyanshpaliwalcmsmn@gmail.com","avatar":"https://avatars.githubusercontent.com/u/152720574?v=4"},"body":"> On 17/02/2026 10:12, Shreyansh Paliwal wrote:\n> >> On 14/02/2026 14:30, Phillip Wood wrote:\n> >>>\n> >>> I think that we should add a new function\n> >>>\n> >>> struct worktree *get_current_worktree(struct repository*);\n> >>>\n> >>> to worktree.c that constructs a struct worktree using repo->gitdir etc.\n> >>> The worktree id is the last path component of repo->gitdir when the\n> >>> repo->gitdir and repo->commondir differ, otherwise it is NULL. Then we\n> >>> can use that function to get the current worktree rather than passing\n> >>> NULL when we call wt_status_check_{rebase,bisect} from\n> >>> wt_status_get_state().\n> >>\n> >> Here's what that looks like, the first patch adds\n> >> get_worktree_from_repository() and uses it to avoid passing a NULL\n> >> worktree to worktree_git_path(). The second patch then removes the\n> >> repository argument from that function and always uses wt->repo instead.\n> >>\n> >> Shreyansh - I think your patches to clean up wt-status.c can probably proceed\n> >> separately to these if you remove the changes to\n> >> wt_status_check_{bisect,rebase}().\n> >\n> > Cool. I'll send a revised version on the original thread.\n>\n> Great, I hope I'm not stepping on your toes posting these patches. By\n> the time I'd worked out what was needed and checked all the callers were\n> passing a non-NULL worktree argument I had the code changes and commit\n> messages so I thought I'd post them.\n\nNot at all, I’m glad you posted them. They clarified the right direction\nand are logically more fit.\nI got to learn a lot about the worktree API while working on this,\nso that was very helpful. Thanks :)\n\nBest,\nShreyansh\n"},{"id":"536208","messageId":"CAOLa=ZQKLqFn4w3s7PD87FZ_120gohoqKX5c3uLKo2vASsbxfA@mail.gmail.com","threadId":"64989","inReplyTo":"409871a7d521b76c9eb811d3c49747e04de8defc.1771258688.git.phillip.wood@dunelm.org.uk","subject":"Re: [PATCH 1/2] wt-status: avoid passing NULL worktree","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-02-17T17:46:21Z","receivedAt":"2026-02-17T17:46:24Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Phillip Wood <phillip.wood123@gmail.com> writes:\n\n> From: Phillip Wood <phillip.wood@dunelm.org.uk>\n>\n> In preparation for removing the repository argument from\n> worktree_git_path() add a function to construct a \"struct worktree\"\n> from a \"struct repository\" and use that to avoid passing a NULL\n> worktree to wt_status_check_bisect() and wt_status_check_rebase().\n>\n\nOkay this makes sense, I'm curious how 'wt->id = NULL' is going to be\nhandled. Let's see\n\n> wt_status_check_bisect() and wt_status_check_rebase() have the following\n> callers:\n>\n>  - branch.c:prepare_checked_out_branches() which loops over all\n>    worktrees.\n>\n>  - worktree.c:is_worktree_being_rebased() which is called from\n>    builtin/branch.c:reject_rebase_or_bisect_branch() that loops over all\n>    worktrees and worktree.c:is_shared_symref() which dereferences wt\n>    earlier in the function.\n>\n>  - wt-status:wt_status_get_state() which is updated to avoid passing a\n>    NULL worktree by this patch.\n>\n> This updates the only callers that pass a NULL worktree to\n> worktree_git_path().\n>\n\nI was thinking surely there must be other places where we also pass NULL\nfor worktree, but doesn't seem like there are any such instances.\n\n> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n> ---\n>  worktree.c  | 20 ++++++++++++++++++++\n>  worktree.h  |  5 ++++-\n>  wt-status.c | 15 ++++++++++++---\n>  3 files changed, 36 insertions(+), 4 deletions(-)\n>\n> diff --git a/worktree.c b/worktree.c\n> index 9308389cb6f..fd182c319b7 100644\n> --- a/worktree.c\n> +++ b/worktree.c\n> @@ -66,6 +66,26 @@ static int is_current_worktree(struct worktree *wt)\n>  \treturn is_current;\n>  }\n>\n> +struct worktree *get_worktree_from_repository(struct repository *repo)\n> +{\n> +\tstruct worktree *wt = xcalloc(1, sizeof(*wt));\n> +\tchar *gitdir = absolute_pathdup(repo->gitdir);\n> +\tchar *commondir = absolute_pathdup(repo->commondir);\n> +\n> +\twt->repo = repo;\n> +\tif (repo->worktree)\n> +\t\twt->path = absolute_pathdup(repo->worktree);\n\nShouldn't this always be set? I guess my question is, will\n`repo->worktree` ever be NULL?\n\n> +\twt->is_bare = !!repo->worktree;\n> +\tif (fspathcmp(gitdir, commondir))\n> +\t\twt->id = xstrdup(find_last_dir_sep(commondir) + 1);\n\nSo here we continue to treat NULL as the main worktree. Okay.\n\n> +\twt->is_current = is_current_worktree(wt);\n\nSince we're getting the worktree from the repo, shouldn't this be\n'true'?\n\n> +\tadd_head_info(wt);\n> +\n> +\tfree(gitdir);\n> +\tfree(commondir);\n> +\treturn wt;\n> +}\n> +\n>  /*\n>  * When in a secondary worktree, and when extensions.worktreeConfig\n>  * is true, only $commondir/config and $commondir/worktrees/<id>/\n> diff --git a/worktree.h b/worktree.h\n> index e4bcccdc0ae..b162bbabd50 100644\n> --- a/worktree.h\n> +++ b/worktree.h\n> @@ -38,7 +38,10 @@ struct worktree **get_worktrees(void);\n>   */\n>  struct worktree **get_worktrees_without_reading_head(void);\n>\n> -/*\n> +/* Construct a struct worktree from a struct repository */\n> +struct worktree *get_worktree_from_repository(struct repository *repo);\n> +\n> + /*\n\nNit: extra space?\n\n>   * Returns 1 if linked worktrees exist, 0 otherwise.\n>   */\n>  int submodule_uses_worktrees(const char *path);\n\n[snip]\n"},{"id":"536209","messageId":"CAOLa=ZQ5YUz7c8w7PY=cfAn57wV0NNwOj2czqbojQ65=jVuAWw@mail.gmail.com","threadId":"64989","inReplyTo":"23b8a355b414da2b6216a50006bf2276dd3ea6ae.1771258688.git.phillip.wood@dunelm.org.uk","subject":"Re: [PATCH 2/2] path: remove repository argument from worktree_git_path()","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-02-17T17:48:59Z","receivedAt":"2026-02-17T17:49:01Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Phillip Wood <phillip.wood123@gmail.com> writes:\n\n> From: Phillip Wood <phillip.wood@dunelm.org.uk>\n>\n> worktree_git_path() takes a struct repository and a struct worktree\n> which also contains a struct repository. The repository argument\n> was added by a973f60dc7c (path: stop relying on `the_repository` in\n> `worktree_git_path()`, 2024-08-13) and exists because the worktree\n> argument is optional. Having two ways of passing a repository is\n> a potential foot-gun as if the the worktree argument is present the\n> repository argument must match the worktree's repository member. Since\n> the last commit there are no callers that pass a NULL worktree so lets\n> remove the repository argument. This removes the potential confusion\n> and lets us delete a number of uses of \"the_repository\".\n>\n> worktree_git_path() has the following callers:\n>\n>  - builtin/worktree.c:validate_no_submodules() which is called from\n>    check_clean_worktree() and move_worktree(), both of which supply\n>    a non-NULL worktree.\n>\n>  - builtin/fsck.c:cmd_fsck() which loops over all worktrees.\n>\n>  - revision.c:add_index_objects_to_pending() which loops over all\n>    worktrees.\n>\n>  - worktree.c:worktree_lock_reason() which dereferences wt before\n>    calling worktree_git_path().\n>\n>  - wt-status.c:wt_status_check_bisect() and wt_status_check_rebase()\n>    which are always called with a non-NULL worktree after the last\n>    commit.\n>\n>  - wt-status.c:git_branch() which is only called by\n>    wt_status_check_bisect() and wt_status_check_rebase().\n>\n\nNice. Well explained, the patch looks good to me :)\n\n[snip]\n"},{"id":"536214","messageId":"xmqqikbvcjgi.fsf@gitster.g","threadId":"64989","inReplyTo":"89c78ce2-1783-416d-9ae5-ef51f6bde58d@gmail.com","subject":"Re: [PATCH 1/2] wt-status: avoid passing NULL worktree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-17T18:29:17Z","receivedAt":"2026-02-17T18:29:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood <phillip.wood123@gmail.com> writes:\n\n> Oops s/commondir/gitdir/ - I'll wait to see if there are any other \n> comments before re-rolling (perhaps with a test that runs git status on \n> a rebase in a linked worktree)\n\nSuch a test that exposes behaviour difference would be very much\nappreciated.\n\nThanks.\n"},{"id":"536215","messageId":"xmqqa4x7cile.fsf@gitster.g","threadId":"64989","inReplyTo":"409871a7d521b76c9eb811d3c49747e04de8defc.1771258688.git.phillip.wood@dunelm.org.uk","subject":"Re: [PATCH 1/2] wt-status: avoid passing NULL worktree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-17T18:47:57Z","receivedAt":"2026-02-17T18:48:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood <phillip.wood123@gmail.com> writes:\n\n> From: Phillip Wood <phillip.wood@dunelm.org.uk>\n>\n> In preparation for removing the repository argument from\n> worktree_git_path() add a function to construct a \"struct worktree\"\n> from a \"struct repository\" and use that to avoid passing a NULL\n> worktree to wt_status_check_bisect() and wt_status_check_rebase().\n\nHmph, I am afraid that\n\n    \"Construct a struct worktree from a struct repository\"\n\nis not quite sufficient.  A repository can have more than one\nworktrees, so if you give a repository as a parameter, there needs a\nway for the implementation of this helper function to identify which\none of them to construct a struct worktree for, and more importantly\nfor you as the caller to be able to expect which one the implementation\nwould pick, and what that particular worktree among many _means_ to you.\n\nI know that the implementation uses repo->worktree but what does\nthat path mean in the world-view of the worktree API set?\n\nI am guessing that it is what the worktree API calls \"current\", but\nif so, perhaps the function should be explained with that word in\nit, and the function name should also contain that word, no?\n\n> +struct worktree *get_worktree_from_repository(struct repository *repo)\n> +{\n> +\tstruct worktree *wt = xcalloc(1, sizeof(*wt));\n> +\tchar *gitdir = absolute_pathdup(repo->gitdir);\n> +\tchar *commondir = absolute_pathdup(repo->commondir);\n> +\n> +\twt->repo = repo;\n> +\tif (repo->worktree)\n> +\t\twt->path = absolute_pathdup(repo->worktree);\n\nSo, if the repository instance knows where the worktree is, we use\nthat to wt->path.  Otherwise wt->path is left NULL.\n\n> +\twt->is_bare = !!repo->worktree;\n\nI may be confused but don't we have one ! too many?  If we have a\nworktree directory, \"git checkout\" would check the files there, and\nthat is not quite a \"bare\" repository, no?\n\n> +\tif (fspathcmp(gitdir, commondir))\n> +\t\twt->id = xstrdup(find_last_dir_sep(commondir) + 1);\n\nOK.  So gitdir and commondir would be the same for the primary and\nfor everybody else we'd have \"id\" as the last directory component of\nthe commondir.\n\n> +\twt->is_current = is_current_worktree(wt);\n\nOh, so I guessed wrong and this is not about \"current\" worktree?\nWhat does the directory pointed at by repo->worktree mean to the\ncallers of this function?  I somehow thought that is_current would\nbe always 1 here,.\n"},{"id":"536290","messageId":"f03fa5a8-b408-4b4e-a254-b0a39b87e636@gmail.com","threadId":"64989","inReplyTo":"xmqqa4x7cile.fsf@gitster.g","subject":"Re: [PATCH 1/2] wt-status: avoid passing NULL worktree","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-02-18T14:18:50Z","receivedAt":"2026-02-18T14:18:53Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 17/02/2026 18:47, Junio C Hamano wrote:\n> Phillip Wood <phillip.wood123@gmail.com> writes:\n> \n>> From: Phillip Wood <phillip.wood@dunelm.org.uk>\n>>\n>> In preparation for removing the repository argument from\n>> worktree_git_path() add a function to construct a \"struct worktree\"\n>> from a \"struct repository\" and use that to avoid passing a NULL\n>> worktree to wt_status_check_bisect() and wt_status_check_rebase().\n> \n> Hmph, I am afraid that\n> \n>      \"Construct a struct worktree from a struct repository\"\n> \n> is not quite sufficient.  A repository can have more than one\n> worktrees, so if you give a repository as a parameter, there needs a\n> way for the implementation of this helper function to identify which\n> one of them to construct a struct worktree for, and more importantly\n> for you as the caller to be able to expect which one the implementation\n> would pick, and what that particular worktree among many _means_ to you.\n> \n> I know that the implementation uses repo->worktree but what does\n> that path mean in the world-view of the worktree API set?\n\nWhile a repository can have multiple worktrees, a \"struct repository\" \npoints to a particular worktree within that repository via the gitdir \nand worktree members. I'll try and make it clearer that the function \nreturns a struct worktree corresponding to those members.\n\n> I am guessing that it is what the worktree API calls \"current\", but\n> if so, perhaps the function should be explained with that word in\n> it, and the function name should also contain that word, no?\n\nThat's what I thought initially. However is_current_worktree() is \ndefined in terms of \"the_repository\" rather than \"wt->repo\". That means \nall the struct worktrees within a single process agree on the \"current\" \nworktree but it is suprising that if \"wt->path\" matches \n\"wt->repo->worktree\" it is not necessarily the \"current\" worktree. I'm \nnot sure if we want to change the definition of is_current_worktree() to \nuse \"wt->repo\" rather than \"the_repository\", but if we do I think we can \ndo that separately.\n\n>> +struct worktree *get_worktree_from_repository(struct repository *repo)\n>> +{\n>> +\tstruct worktree *wt = xcalloc(1, sizeof(*wt));\n>> +\tchar *gitdir = absolute_pathdup(repo->gitdir);\n>> +\tchar *commondir = absolute_pathdup(repo->commondir);\n>> +\n>> +\twt->repo = repo;\n>> +\tif (repo->worktree)\n>> +\t\twt->path = absolute_pathdup(repo->worktree);\n> \n> So, if the repository instance knows where the worktree is, we use\n> that to wt->path.  Otherwise wt->path is left NULL.\n\nThat's actually a bug, we should be using repo->gitdir when the \nrepository is bare.\n\n>> +\twt->is_bare = !!repo->worktree;\n> \n> I may be confused but don't we have one ! too many?  If we have a\n> worktree directory, \"git checkout\" would check the files there, and\n> that is not quite a \"bare\" repository, no?\n\nYes, it should be \"wt->is_bare = !repo->worktree;\"\n\n>> +\tif (fspathcmp(gitdir, commondir))\n>> +\t\twt->id = xstrdup(find_last_dir_sep(commondir) + 1);\n> \n> OK.  So gitdir and commondir would be the same for the primary and\n> for everybody else we'd have \"id\" as the last directory component of\n> the commondir.\n> \n>> +\twt->is_current = is_current_worktree(wt);\n> \n> Oh, so I guessed wrong and this is not about \"current\" worktree?\n> What does the directory pointed at by repo->worktree mean to the\n> callers of this function?  I somehow thought that is_current would\n> be always 1 here,.\n\nAs explained above \"repo->worktree\" means nothing to \nis_current_worktree() because it uses \"the_repository\" instead of \n\"wt->repo\".\n\nI'll re-roll with the fixes above and a bit more detail in the commit \nmessage about is_current_worktree() and the that the \"struct worktree\" \ninstance corresponds to worktree that uses repo->gitdir\n\nThanks\n\nPhillip\n\n"},{"id":"536291","messageId":"d881edeb-d73b-43ba-bdb3-1b664e1cb882@gmail.com","threadId":"64989","inReplyTo":"CAOLa=ZQKLqFn4w3s7PD87FZ_120gohoqKX5c3uLKo2vASsbxfA@mail.gmail.com","subject":"Re: [PATCH 1/2] wt-status: avoid passing NULL worktree","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-02-18T14:19:56Z","receivedAt":"2026-02-18T14:19:59Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 17/02/2026 17:46, Karthik Nayak wrote:\n> Phillip Wood <phillip.wood123@gmail.com> writes:\n>> From: Phillip Wood <phillip.wood@dunelm.org.uk>\n>>\n>> This updates the only callers that pass a NULL worktree to\n>> worktree_git_path().\n>>\n> \n> I was thinking surely there must be other places where we also pass NULL\n> for worktree, but doesn't seem like there are any such instances.\n\nYes, I was pleasantly surprised there weren't more sites to convert.\n\n>> +struct worktree *get_worktree_from_repository(struct repository *repo)\n>> +{\n>> +\tstruct worktree *wt = xcalloc(1, sizeof(*wt));\n>> +\tchar *gitdir = absolute_pathdup(repo->gitdir);\n>> +\tchar *commondir = absolute_pathdup(repo->commondir);\n>> +\n>> +\twt->repo = repo;\n>> +\tif (repo->worktree)\n>> +\t\twt->path = absolute_pathdup(repo->worktree);\n> \n> Shouldn't this always be set? I guess my question is, will\n> `repo->worktree` ever be NULL?\n\nOh, wt->path should never be NULL. repo->worktree is NULL in bare \nrepositories but then we should use repo->gitdir as the worktree path.\n\n>> +\twt->is_bare = !!repo->worktree;\n>> +\tif (fspathcmp(gitdir, commondir))\n>> +\t\twt->id = xstrdup(find_last_dir_sep(commondir) + 1);\n> \n> So here we continue to treat NULL as the main worktree. Okay.\n> \n>> +\twt->is_current = is_current_worktree(wt);\n> \n> Since we're getting the worktree from the repo, shouldn't this be\n> 'true'?\n\nThat's what I thought initially. However is_current_worktree() compares \n\"repo->gitdir\" to \"the_repository->gitdir\" so the \"current\" worktree is \nthe one that the process was started in, which is not necessarily the \nsame as the one matching \"wt->repo->gitdir\". It's possible that we will \nwant to change that definition in the future but I opted to make this \nfunction consistent with the status quo.\n\n>> -/*\n>> +/* Construct a struct worktree from a struct repository */\n>> +struct worktree *get_worktree_from_repository(struct repository *repo);\n>> +\n>> + /*\n> \n> Nit: extra space?\n\nGood spot I'll fix it\n\nThanks for the review\n\nPhillip\n\n"},{"id":"536407","messageId":"cover.1771511192.git.phillip.wood@dunelm.org.uk","threadId":"64989","inReplyTo":"cover.1771258688.git.phillip.wood@dunelm.org.uk","subject":"[PATCH v2 0/2] worktree_git_path(): remove repository argument","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-02-19T14:26:31Z","receivedAt":"2026-02-19T14:26:47Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nThese patches remove the repository argument from worktree_git_path()\nin favor of using the repository in the \"sturct worktree\" argument.\nThis enables us to remove some uses of \"the_repository\". The first\npatch adds a new function git_worktree_from_repository() to construct\na \"struct worktree\" based on the repository's worktree and uses it\nto avoid passing a NULL worktree to worktree_git_path(). The second\npatch then removes the repository argument from that function and\nalways uses the repository in the worktree argument instead.\n\nThanks to Karthik and Junio for their comments, here are the changes\nsince V1:\n - always set worktree path - for bare repositories the worktree path\n   is repo->gitdir\n - fix the worktree bareness (there were too many negations)\n - fix the wortkree id (it comes from repo->gitdir not repo->commondir)\n - add a test for \"git status\" on a rebase in a linked worktree.\n - expand the commit message to explain\n   (a) that we use the \"gitdir\" and \"worktree\" members of \"struct\n       repository\" to construct the \"struct worktree\"\n   (b) how the \"current\" worktree is determined\n\nBase-Commit: 852829b3dd2fe4e7c7fc4d8badde644cf1b66c74\nPublished-As: https://github.com/phillipwood/git/releases/tag/pw%2Fget-current-worktree%2Fv2\nView-Changes-At: https://github.com/phillipwood/git/compare/852829b3d...db9d519cb\nFetch-It-Via: git fetch https://github.com/phillipwood/git pw/get-current-worktree/v2\n\n\nPhillip Wood (2):\n  wt-status: avoid passing NULL worktree\n  path: remove repository argument from worktree_git_path()\n\n builtin/fsck.c         |  2 +-\n builtin/worktree.c     |  4 ++--\n path.c                 |  9 ++++-----\n path.h                 |  8 +++-----\n revision.c             |  2 +-\n t/t7512-status-help.sh |  9 +++++++++\n worktree.c             | 22 +++++++++++++++++++++-\n worktree.h             |  6 ++++++\n wt-status.c            | 29 +++++++++++++++++++----------\n 9 files changed, 66 insertions(+), 25 deletions(-)\n\nRange-diff against v1:\n1:  409871a7d52 ! 1:  902295b8714 wt-status: avoid passing NULL worktree\n    @@ Commit message\n     \n         In preparation for removing the repository argument from\n         worktree_git_path() add a function to construct a \"struct worktree\"\n    -    from a \"struct repository\" and use that to avoid passing a NULL\n    -    worktree to wt_status_check_bisect() and wt_status_check_rebase().\n    +    from a \"struct repository\" using its \"gitdir\" and \"worktree\"\n    +    members. This function is then used to avoid passing a NULL worktree to\n    +    wt_status_check_bisect() and wt_status_check_rebase(). In general the\n    +    \"struct worktree\" returned may not correspond to the \"current\" worktree\n    +    defined by is_current_worktree() as that function uses \"the_repository\"\n    +    rather than \"wt->repo\" when deciding which worktree is \"current\". In\n    +    practice the \"struct repository\" we pass corresponds to \"the_repository\"\n    +    as we only ever operate on a single repository at the moment.\n     \n         wt_status_check_bisect() and wt_status_check_rebase() have the following\n         callers:\n    @@ Commit message\n            NULL worktree by this patch.\n     \n         This updates the only callers that pass a NULL worktree to\n    -    worktree_git_path().\n    +    worktree_git_path(). A new test is added to check that \"git status\"\n    +    detects a rebase in a linked worktree.\n     \n         Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n     \n    + ## t/t7512-status-help.sh ##\n    +@@ t/t7512-status-help.sh: EOF\n    + \ttest_cmp expected actual\n    + '\n    + \n    ++test_expect_success 'rebase in a linked worktree' '\n    ++\ttest_might_fail git rebase --abort &&\n    ++\tgit worktree add wt &&\n    ++\ttest_when_finished \"test_might_fail git -C wt rebase --abort;\n    ++\t\t\t\tgit worktree remove wt\" &&\n    ++\tGIT_SEQUENCE_EDITOR=\"echo break >\" git -C wt rebase -i HEAD &&\n    ++\tgit -C wt status >actual &&\n    ++\ttest_grep \"interactive rebase in progress\" actual\n    ++'\n    + \n    + test_expect_success 'prepare am_session' '\n    + \tgit reset --hard main &&\n    +\n      ## worktree.c ##\n     @@ worktree.c: static int is_current_worktree(struct worktree *wt)\n      \treturn is_current;\n    @@ worktree.c: static int is_current_worktree(struct worktree *wt)\n     +\tchar *commondir = absolute_pathdup(repo->commondir);\n     +\n     +\twt->repo = repo;\n    -+\tif (repo->worktree)\n    -+\t\twt->path = absolute_pathdup(repo->worktree);\n    -+\twt->is_bare = !!repo->worktree;\n    ++\twt->path = absolute_pathdup(repo->worktree ? repo->worktree\n    ++\t\t\t\t\t\t   : repo->gitdir);\n    ++\twt->is_bare = !repo->worktree;\n     +\tif (fspathcmp(gitdir, commondir))\n    -+\t\twt->id = xstrdup(find_last_dir_sep(commondir) + 1);\n    ++\t\twt->id = xstrdup(find_last_dir_sep(gitdir) + 1);\n     +\twt->is_current = is_current_worktree(wt);\n     +\tadd_head_info(wt);\n     +\n    @@ worktree.h: struct worktree **get_worktrees(void);\n       */\n      struct worktree **get_worktrees_without_reading_head(void);\n      \n    --/*\n    -+/* Construct a struct worktree from a struct repository */\n    ++/*\n    ++ * Construct a struct worktree corresponding to repo->gitdir and\n    ++ * repo->worktree.\n    ++ */\n     +struct worktree *get_worktree_from_repository(struct repository *repo);\n     +\n    -+ /*\n    + /*\n       * Returns 1 if linked worktrees exist, 0 otherwise.\n       */\n    - int submodule_uses_worktrees(const char *path);\n     \n      ## wt-status.c ##\n     @@ wt-status.c: int wt_status_check_rebase(const struct worktree *wt,\n2:  23b8a355b41 = 2:  db9d519cbda path: remove repository argument from worktree_git_path()\n-- \n2.52.0.362.g884e03848a9\n\n"},{"id":"536408","messageId":"902295b87146e5cb5358cebab51f8d66701290a8.1771511192.git.phillip.wood@dunelm.org.uk","threadId":"64989","inReplyTo":"cover.1771511192.git.phillip.wood@dunelm.org.uk","subject":"[PATCH v2 1/2] wt-status: avoid passing NULL worktree","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-02-19T14:26:32Z","receivedAt":"2026-02-19T14:26:48Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nIn preparation for removing the repository argument from\nworktree_git_path() add a function to construct a \"struct worktree\"\nfrom a \"struct repository\" using its \"gitdir\" and \"worktree\"\nmembers. This function is then used to avoid passing a NULL worktree to\nwt_status_check_bisect() and wt_status_check_rebase(). In general the\n\"struct worktree\" returned may not correspond to the \"current\" worktree\ndefined by is_current_worktree() as that function uses \"the_repository\"\nrather than \"wt->repo\" when deciding which worktree is \"current\". In\npractice the \"struct repository\" we pass corresponds to \"the_repository\"\nas we only ever operate on a single repository at the moment.\n\nwt_status_check_bisect() and wt_status_check_rebase() have the following\ncallers:\n\n - branch.c:prepare_checked_out_branches() which loops over all\n   worktrees.\n\n - worktree.c:is_worktree_being_rebased() which is called from\n   builtin/branch.c:reject_rebase_or_bisect_branch() that loops over all\n   worktrees and worktree.c:is_shared_symref() which dereferences wt\n   earlier in the function.\n\n - wt-status:wt_status_get_state() which is updated to avoid passing a\n   NULL worktree by this patch.\n\nThis updates the only callers that pass a NULL worktree to\nworktree_git_path(). A new test is added to check that \"git status\"\ndetects a rebase in a linked worktree.\n\nSigned-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n---\n t/t7512-status-help.sh |  9 +++++++++\n worktree.c             | 20 ++++++++++++++++++++\n worktree.h             |  6 ++++++\n wt-status.c            | 15 ++++++++++++---\n 4 files changed, 47 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh\nindex 25e8e9711f8..08e82f79140 100755\n--- a/t/t7512-status-help.sh\n+++ b/t/t7512-status-help.sh\n@@ -594,6 +594,15 @@ EOF\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'rebase in a linked worktree' '\n+\ttest_might_fail git rebase --abort &&\n+\tgit worktree add wt &&\n+\ttest_when_finished \"test_might_fail git -C wt rebase --abort;\n+\t\t\t\tgit worktree remove wt\" &&\n+\tGIT_SEQUENCE_EDITOR=\"echo break >\" git -C wt rebase -i HEAD &&\n+\tgit -C wt status >actual &&\n+\ttest_grep \"interactive rebase in progress\" actual\n+'\n \n test_expect_success 'prepare am_session' '\n \tgit reset --hard main &&\ndiff --git a/worktree.c b/worktree.c\nindex 9308389cb6f..218c332a66d 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -66,6 +66,26 @@ static int is_current_worktree(struct worktree *wt)\n \treturn is_current;\n }\n \n+struct worktree *get_worktree_from_repository(struct repository *repo)\n+{\n+\tstruct worktree *wt = xcalloc(1, sizeof(*wt));\n+\tchar *gitdir = absolute_pathdup(repo->gitdir);\n+\tchar *commondir = absolute_pathdup(repo->commondir);\n+\n+\twt->repo = repo;\n+\twt->path = absolute_pathdup(repo->worktree ? repo->worktree\n+\t\t\t\t\t\t   : repo->gitdir);\n+\twt->is_bare = !repo->worktree;\n+\tif (fspathcmp(gitdir, commondir))\n+\t\twt->id = xstrdup(find_last_dir_sep(gitdir) + 1);\n+\twt->is_current = is_current_worktree(wt);\n+\tadd_head_info(wt);\n+\n+\tfree(gitdir);\n+\tfree(commondir);\n+\treturn wt;\n+}\n+\n /*\n * When in a secondary worktree, and when extensions.worktreeConfig\n * is true, only $commondir/config and $commondir/worktrees/<id>/\ndiff --git a/worktree.h b/worktree.h\nindex e4bcccdc0ae..06efe26b835 100644\n--- a/worktree.h\n+++ b/worktree.h\n@@ -38,6 +38,12 @@ struct worktree **get_worktrees(void);\n  */\n struct worktree **get_worktrees_without_reading_head(void);\n \n+/*\n+ * Construct a struct worktree corresponding to repo->gitdir and\n+ * repo->worktree.\n+ */\n+struct worktree *get_worktree_from_repository(struct repository *repo);\n+\n /*\n  * Returns 1 if linked worktrees exist, 0 otherwise.\n  */\ndiff --git a/wt-status.c b/wt-status.c\nindex 95942399f8c..2debda534c1 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1747,6 +1747,9 @@ int wt_status_check_rebase(const struct worktree *wt,\n {\n \tstruct stat st;\n \n+\tif (!wt)\n+\t\tBUG(\"wt_status_check_rebase() called with NULL worktree\");\n+\n \tif (!stat(worktree_git_path(the_repository, wt, \"rebase-apply\"), &st)) {\n \t\tif (!stat(worktree_git_path(the_repository, wt, \"rebase-apply/applying\"), &st)) {\n \t\t\tstate->am_in_progress = 1;\n@@ -1774,6 +1777,9 @@ int wt_status_check_bisect(const struct worktree *wt,\n {\n \tstruct stat st;\n \n+\tif (!wt)\n+\t\tBUG(\"wt_status_check_bisect() called with NULL worktree\");\n+\n \tif (!stat(worktree_git_path(the_repository, wt, \"BISECT_LOG\"), &st)) {\n \t\tstate->bisect_in_progress = 1;\n \t\tstate->bisecting_from = get_branch(wt, \"BISECT_START\");\n@@ -1819,18 +1825,19 @@ void wt_status_get_state(struct repository *r,\n \tstruct stat st;\n \tstruct object_id oid;\n \tenum replay_action action;\n+\tstruct worktree *wt = get_worktree_from_repository(r);\n \n \tif (!stat(git_path_merge_head(r), &st)) {\n-\t\twt_status_check_rebase(NULL, state);\n+\t\twt_status_check_rebase(wt, state);\n \t\tstate->merge_in_progress = 1;\n-\t} else if (wt_status_check_rebase(NULL, state)) {\n+\t} else if (wt_status_check_rebase(wt, state)) {\n \t\t;\t\t/* all set */\n \t} else if (refs_ref_exists(get_main_ref_store(r), \"CHERRY_PICK_HEAD\") &&\n \t\t   !repo_get_oid(r, \"CHERRY_PICK_HEAD\", &oid)) {\n \t\tstate->cherry_pick_in_progress = 1;\n \t\toidcpy(&state->cherry_pick_head_oid, &oid);\n \t}\n-\twt_status_check_bisect(NULL, state);\n+\twt_status_check_bisect(wt, state);\n \tif (refs_ref_exists(get_main_ref_store(r), \"REVERT_HEAD\") &&\n \t    !repo_get_oid(r, \"REVERT_HEAD\", &oid)) {\n \t\tstate->revert_in_progress = 1;\n@@ -1848,6 +1855,8 @@ void wt_status_get_state(struct repository *r,\n \tif (get_detached_from)\n \t\twt_status_get_detached_from(r, state);\n \twt_status_check_sparse_checkout(r, state);\n+\n+\tfree_worktree(wt);\n }\n \n static void wt_longstatus_print_state(struct wt_status *s)\n-- \n2.52.0.362.g884e03848a9\n\n"},{"id":"536409","messageId":"db9d519cbda44c46986e127e820b5b7b0ba31206.1771511192.git.phillip.wood@dunelm.org.uk","threadId":"64989","inReplyTo":"cover.1771511192.git.phillip.wood@dunelm.org.uk","subject":"[PATCH v2 2/2] path: remove repository argument from worktree_git_path()","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-02-19T14:26:33Z","receivedAt":"2026-02-19T14:26:49Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nworktree_git_path() takes a struct repository and a struct worktree\nwhich also contains a struct repository. The repository argument\nwas added by a973f60dc7c (path: stop relying on `the_repository` in\n`worktree_git_path()`, 2024-08-13) and exists because the worktree\nargument is optional. Having two ways of passing a repository is\na potential foot-gun as if the the worktree argument is present the\nrepository argument must match the worktree's repository member. Since\nthe last commit there are no callers that pass a NULL worktree so lets\nremove the repository argument. This removes the potential confusion\nand lets us delete a number of uses of \"the_repository\".\n\nworktree_git_path() has the following callers:\n\n - builtin/worktree.c:validate_no_submodules() which is called from\n   check_clean_worktree() and move_worktree(), both of which supply\n   a non-NULL worktree.\n\n - builtin/fsck.c:cmd_fsck() which loops over all worktrees.\n\n - revision.c:add_index_objects_to_pending() which loops over all\n   worktrees.\n\n - worktree.c:worktree_lock_reason() which dereferences wt before\n   calling worktree_git_path().\n\n - wt-status.c:wt_status_check_bisect() and wt_status_check_rebase()\n   which are always called with a non-NULL worktree after the last\n   commit.\n\n - wt-status.c:git_branch() which is only called by\n   wt_status_check_bisect() and wt_status_check_rebase().\n\nSigned-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n---\n builtin/fsck.c     |  2 +-\n builtin/worktree.c |  4 ++--\n path.c             |  9 ++++-----\n path.h             |  8 +++-----\n revision.c         |  2 +-\n worktree.c         |  2 +-\n wt-status.c        | 14 +++++++-------\n 7 files changed, 19 insertions(+), 22 deletions(-)\n\ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex 0512f78a87f..42ba0afb91a 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -1137,7 +1137,7 @@ int cmd_fsck(int argc,\n \t\t\t * and may get overwritten by other calls\n \t\t\t * while we're examining the index.\n \t\t\t */\n-\t\t\tpath = xstrdup(worktree_git_path(the_repository, wt, \"index\"));\n+\t\t\tpath = xstrdup(worktree_git_path(wt, \"index\"));\n \t\t\twt_gitdir = get_worktree_git_dir(wt);\n \n \t\t\tread_index_from(&istate, path, wt_gitdir);\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 3d6547c23b4..62fd4642e5d 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -1191,14 +1191,14 @@ static void validate_no_submodules(const struct worktree *wt)\n \n \twt_gitdir = get_worktree_git_dir(wt);\n \n-\tif (is_directory(worktree_git_path(the_repository, wt, \"modules\"))) {\n+\tif (is_directory(worktree_git_path(wt, \"modules\"))) {\n \t\t/*\n \t\t * There could be false positives, e.g. the \"modules\"\n \t\t * directory exists but is empty. But it's a rare case and\n \t\t * this simpler check is probably good enough for now.\n \t\t */\n \t\tfound_submodules = 1;\n-\t} else if (read_index_from(&istate, worktree_git_path(the_repository, wt, \"index\"),\n+\t} else if (read_index_from(&istate, worktree_git_path(wt, \"index\"),\n \t\t\t\t   wt_gitdir) > 0) {\n \t\tfor (i = 0; i < istate.cache_nr; i++) {\n \t\t\tstruct cache_entry *ce = istate.cache[i];\ndiff --git a/path.c b/path.c\nindex d726537622c..073f631b914 100644\n--- a/path.c\n+++ b/path.c\n@@ -486,17 +486,16 @@ const char *mkpath(const char *fmt, ...)\n \treturn cleanup_path(pathname->buf);\n }\n \n-const char *worktree_git_path(struct repository *r,\n-\t\t\t      const struct worktree *wt, const char *fmt, ...)\n+const char *worktree_git_path(const struct worktree *wt, const char *fmt, ...)\n {\n \tstruct strbuf *pathname = get_pathname();\n \tva_list args;\n \n-\tif (wt && wt->repo != r)\n-\t\tBUG(\"worktree not connected to expected repository\");\n+\tif (!wt)\n+\t\tBUG(\"%s() called with NULL worktree\", __func__);\n \n \tva_start(args, fmt);\n-\trepo_git_pathv(r, wt, pathname, fmt, args);\n+\trepo_git_pathv(wt->repo, wt, pathname, fmt, args);\n \tva_end(args);\n \treturn pathname->buf;\n }\ndiff --git a/path.h b/path.h\nindex 0ec95a0b079..cbcad254a0a 100644\n--- a/path.h\n+++ b/path.h\n@@ -66,13 +66,11 @@ const char *repo_git_path_replace(struct repository *repo,\n \n /*\n  * Similar to repo_git_path() but can produce paths for a specified\n- * worktree instead of current one. When no worktree is given, then the path is\n- * computed relative to main worktree of the given repository.\n+ * worktree instead of current one.\n  */\n-const char *worktree_git_path(struct repository *r,\n-\t\t\t      const struct worktree *wt,\n+const char *worktree_git_path(const struct worktree *wt,\n \t\t\t      const char *fmt, ...)\n-\t__attribute__((format (printf, 3, 4)));\n+\t__attribute__((format (printf, 2, 3)));\n \n /*\n  * The `repo_worktree_path` family of functions will construct a path into a\ndiff --git a/revision.c b/revision.c\nindex 29972c3a198..ca3481c1902 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1847,7 +1847,7 @@ void add_index_objects_to_pending(struct rev_info *revs, unsigned int flags)\n \t\twt_gitdir = get_worktree_git_dir(wt);\n \n \t\tif (read_index_from(&istate,\n-\t\t\t\t    worktree_git_path(the_repository, wt, \"index\"),\n+\t\t\t\t    worktree_git_path(wt, \"index\"),\n \t\t\t\t    wt_gitdir) > 0)\n \t\t\tdo_add_index_objects_to_pending(revs, &istate, flags);\n \ndiff --git a/worktree.c b/worktree.c\nindex 218c332a66d..6e2f0f78283 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -308,7 +308,7 @@ const char *worktree_lock_reason(struct worktree *wt)\n \tif (!wt->lock_reason_valid) {\n \t\tstruct strbuf path = STRBUF_INIT;\n \n-\t\tstrbuf_addstr(&path, worktree_git_path(the_repository, wt, \"locked\"));\n+\t\tstrbuf_addstr(&path, worktree_git_path(wt, \"locked\"));\n \t\tif (file_exists(path.buf)) {\n \t\t\tstruct strbuf lock_reason = STRBUF_INIT;\n \t\t\tif (strbuf_read_file(&lock_reason, path.buf, 0) < 0)\ndiff --git a/wt-status.c b/wt-status.c\nindex 2debda534c1..68257d6dfd2 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1648,7 +1648,7 @@ static char *get_branch(const struct worktree *wt, const char *path)\n \tstruct object_id oid;\n \tconst char *branch_name;\n \n-\tif (strbuf_read_file(&sb, worktree_git_path(the_repository, wt, \"%s\", path), 0) <= 0)\n+\tif (strbuf_read_file(&sb, worktree_git_path(wt, \"%s\", path), 0) <= 0)\n \t\tgoto got_nothing;\n \n \twhile (sb.len && sb.buf[sb.len - 1] == '\\n')\n@@ -1750,18 +1750,18 @@ int wt_status_check_rebase(const struct worktree *wt,\n \tif (!wt)\n \t\tBUG(\"wt_status_check_rebase() called with NULL worktree\");\n \n-\tif (!stat(worktree_git_path(the_repository, wt, \"rebase-apply\"), &st)) {\n-\t\tif (!stat(worktree_git_path(the_repository, wt, \"rebase-apply/applying\"), &st)) {\n+\tif (!stat(worktree_git_path(wt, \"rebase-apply\"), &st)) {\n+\t\tif (!stat(worktree_git_path(wt, \"rebase-apply/applying\"), &st)) {\n \t\t\tstate->am_in_progress = 1;\n-\t\t\tif (!stat(worktree_git_path(the_repository, wt, \"rebase-apply/patch\"), &st) && !st.st_size)\n+\t\t\tif (!stat(worktree_git_path(wt, \"rebase-apply/patch\"), &st) && !st.st_size)\n \t\t\t\tstate->am_empty_patch = 1;\n \t\t} else {\n \t\t\tstate->rebase_in_progress = 1;\n \t\t\tstate->branch = get_branch(wt, \"rebase-apply/head-name\");\n \t\t\tstate->onto = get_branch(wt, \"rebase-apply/onto\");\n \t\t}\n-\t} else if (!stat(worktree_git_path(the_repository, wt, \"rebase-merge\"), &st)) {\n-\t\tif (!stat(worktree_git_path(the_repository, wt, \"rebase-merge/interactive\"), &st))\n+\t} else if (!stat(worktree_git_path(wt, \"rebase-merge\"), &st)) {\n+\t\tif (!stat(worktree_git_path(wt, \"rebase-merge/interactive\"), &st))\n \t\t\tstate->rebase_interactive_in_progress = 1;\n \t\telse\n \t\t\tstate->rebase_in_progress = 1;\n@@ -1780,7 +1780,7 @@ int wt_status_check_bisect(const struct worktree *wt,\n \tif (!wt)\n \t\tBUG(\"wt_status_check_bisect() called with NULL worktree\");\n \n-\tif (!stat(worktree_git_path(the_repository, wt, \"BISECT_LOG\"), &st)) {\n+\tif (!stat(worktree_git_path(wt, \"BISECT_LOG\"), &st)) {\n \t\tstate->bisect_in_progress = 1;\n \t\tstate->bisecting_from = get_branch(wt, \"BISECT_START\");\n \t\treturn 1;\n-- \n2.52.0.362.g884e03848a9\n\n"},{"id":"536434","messageId":"xmqqv7fs4jlp.fsf@gitster.g","threadId":"64989","inReplyTo":"902295b87146e5cb5358cebab51f8d66701290a8.1771511192.git.phillip.wood@dunelm.org.uk","subject":"Re: [PATCH v2 1/2] wt-status: avoid passing NULL worktree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-19T19:30:10Z","receivedAt":"2026-02-19T19:30:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood <phillip.wood123@gmail.com> writes:\n\n> In general the \"struct worktree\" returned may not correspond to\n> the \"current\" worktree defined by is_current_worktree() as that\n> function uses \"the_repository\" rather than \"wt->repo\" when\n> deciding which worktree is \"current\".  In practice the \"struct\n> repository\" we pass corresponds to \"the_repository\" as we only\n> ever operate on a single repository at the moment.\n\nThis may technically be a correct description, but feels very\nunsatisfactory, as it fails to answer this very simple question:\n\n    what does it mean when is_current_worktree() says \"no\" to the\n    worktree instance returned by this function?  In other words,\n    what are the sample sequences that can lead to such a worktree?\n\nWe start a Git process in a directory which is part of a set of\nworktrees governed by a single repository.  That repository becomes\nthe_repository and the worktree instance that represents our\ndirectory would satisfy is_current_worktree().  Then we visit\nanother directory that is one of a set of worktrees goverend by a\nseparate and different repository.  We now have a repository\ninstance that is different from our the_repository.  Perhaps our\nin-core submodule code may do that, and that different repository is\nthe submodule in question.  Running this function will yield the\nworktree instance, whose path is a subdirectory of our current\ndirectory where the submodule is checked out?  It may be the current\nworktree if we asked is_current_worktree() about that worktree in\nthe context of the submodule, but it is not in the context of our\nsuperproject repository.  In fact, none of the worktrees governed by\nthe submodule repository can be \"current\", as they are not our\ncheckout, from the viewpoint of our superproject repository.\n\nIs that what is going on here?  \n\nA related question that is much more relevant is this:\n\n    What is the significance of the worktree, relative to our\n    process, returned by this function for a given repo?  What is so\n    special about this worktree, among others that are also linked\n    to the same repository?\n\n    What does it mean for a worktree to \"corresponds to\"\n    repo->{gitdir,worktree}?  Why does the currently running Git\n    process want to grab such a worktree?\n\nIf the answer were \"it is the current worktree\", then we would have\na nice and very understandable name \"get-current-worktree-for-repo\"\nfor the function, but because I do not think of a good answer to the\nquestion (and you already explained why it is not the \"current\"\nworktree), I cannot improve on \"get _A_ worktree from repository\",\nthe name given by the patch, which leaves the \"which one of the\nworktrees are we talking about?  Why did we pick that particular one\ninstead of other ones\" unanswered.\n\nOr perhaps \"the current worktree\" is not a per-Git-process concept,\nbut is a per-process-per-repo concept?\n\nIn other words, the function is_current_worktree(wt) may not take a\nrepository and always compute things relative to the_repository, but\nonce we wean ourselves off of the_repository, we would/should have\nrepo_is_current_worktree(repo, wt), making is_current_worktree(wt) a\nthin wrapper for repo_is_current_worktree(the_repository, wt)?\n\n> diff --git a/worktree.h b/worktree.h\n> index e4bcccdc0ae..06efe26b835 100644\n> --- a/worktree.h\n> +++ b/worktree.h\n> @@ -38,6 +38,12 @@ struct worktree **get_worktrees(void);\n>   */\n>  struct worktree **get_worktrees_without_reading_head(void);\n>  \n> +/*\n> + * Construct a struct worktree corresponding to repo->gitdir and\n> + * repo->worktree.\n> + */\n> +struct worktree *get_worktree_from_repository(struct repository *repo);\n> +\n"},{"id":"536435","messageId":"xmqqqzqg4jeq.fsf@gitster.g","threadId":"64989","inReplyTo":"db9d519cbda44c46986e127e820b5b7b0ba31206.1771511192.git.phillip.wood@dunelm.org.uk","subject":"Re: [PATCH v2 2/2] path: remove repository argument from worktree_git_path()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-19T19:34:21Z","receivedAt":"2026-02-19T19:34:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood <phillip.wood123@gmail.com> writes:\n\n> From: Phillip Wood <phillip.wood@dunelm.org.uk>\n>\n> worktree_git_path() takes a struct repository and a struct worktree\n> which also contains a struct repository. The repository argument\n> was added by a973f60dc7c (path: stop relying on `the_repository` in\n> `worktree_git_path()`, 2024-08-13) and exists because the worktree\n> argument is optional. Having two ways of passing a repository is\n> a potential foot-gun as if the the worktree argument is present the\n> repository argument must match the worktree's repository member. Since\n> the last commit there are no callers that pass a NULL worktree so lets\n> remove the repository argument. This removes the potential confusion\n> and lets us delete a number of uses of \"the_repository\".\n>\n> worktree_git_path() has the following callers:\n>\n>  - builtin/worktree.c:validate_no_submodules() which is called from\n>    check_clean_worktree() and move_worktree(), both of which supply\n>    a non-NULL worktree.\n>\n>  - builtin/fsck.c:cmd_fsck() which loops over all worktrees.\n>\n>  - revision.c:add_index_objects_to_pending() which loops over all\n>    worktrees.\n>\n>  - worktree.c:worktree_lock_reason() which dereferences wt before\n>    calling worktree_git_path().\n>\n>  - wt-status.c:wt_status_check_bisect() and wt_status_check_rebase()\n>    which are always called with a non-NULL worktree after the last\n>    commit.\n>\n>  - wt-status.c:git_branch() which is only called by\n>    wt_status_check_bisect() and wt_status_check_rebase().\n>\n> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n> ---\n>  builtin/fsck.c     |  2 +-\n>  builtin/worktree.c |  4 ++--\n>  path.c             |  9 ++++-----\n>  path.h             |  8 +++-----\n>  revision.c         |  2 +-\n>  worktree.c         |  2 +-\n>  wt-status.c        | 14 +++++++-------\n>  7 files changed, 19 insertions(+), 22 deletions(-)\n\nThank you for working on this clean-up.  Very well reasoned.\n"},{"id":"536439","messageId":"xmqq4inc4ghg.fsf@gitster.g","threadId":"64989","inReplyTo":"xmqqv7fs4jlp.fsf@gitster.g","subject":"Re: [PATCH v2 1/2] wt-status: avoid passing NULL worktree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-19T20:37:31Z","receivedAt":"2026-02-19T20:37:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> In other words, the function is_current_worktree(wt) may not take a\n> repository and always compute things relative to the_repository, but\n> once we wean ourselves off of the_repository, we would/should have\n> repo_is_current_worktree(repo, wt), making is_current_worktree(wt) a\n> thin wrapper for repo_is_current_worktree(the_repository, wt)?\n\nEh, in light of 2/2 of this series, since wt knows which repository\nit belongs to, what I wrote above does not make much sense.\nAllowing callers to give repo that is different from wt->repo to\nthat function is a potential foot-gun.  In other words, isn't\nis_current_worktree(wt) using the_worktree and not wt->repo a bug\nalready, I have to wonder?\n"},{"id":"537100","messageId":"8397f971-39dd-4a18-b520-3157ae15324f@gmail.com","threadId":"64989","inReplyTo":"xmqq4inc4ghg.fsf@gitster.g","subject":"Re: [PATCH v2 1/2] wt-status: avoid passing NULL worktree","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-02-25T16:39:05Z","receivedAt":"2026-02-25T16:39:08Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 19/02/2026 20:37, Junio C Hamano wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n> \n>> In other words, the function is_current_worktree(wt) may not take a\n>> repository and always compute things relative to the_repository, but\n>> once we wean ourselves off of the_repository, we would/should have\n>> repo_is_current_worktree(repo, wt), making is_current_worktree(wt) a\n>> thin wrapper for repo_is_current_worktree(the_repository, wt)?\n> \n> Eh, in light of 2/2 of this series, since wt knows which repository\n> it belongs to, what I wrote above does not make much sense.\n> Allowing callers to give repo that is different from wt->repo to\n> that function is a potential foot-gun.  In other words, isn't\n> is_current_worktree(wt) using the_worktree and not wt->repo a bug\n> already, I have to wonder?\n\nI wonder that too. You, Karthik and me all initially assumed that the \ncurrent worktree would be defined by wt->repo->worktree matching \nwt->path (the code actually compares the git_dir of the worktree and the \nrepository to accommodate bare repositories but the same principle \napplies). The use of \"the_repository\" in is_current_wortree() comes from \nreplacing get_git_dir() with repo_get_git_dir() in 246deeac951 \n(environment: make `get_git_dir()` accept a repository, 2024-09-12). In\nget_worktree_git_dir() it comes from replacing git_common_path() with \nrepo_common_path() in 07242c2a5af (path: drop `git_common_path()` in \nfavor of `repo_common_path()`, 2025-02-07). I suspect the replacements \nwere mechanical and not much thought went into considering whether, in a \nworld where there can be more than one repository per process, they \nshould use the local repository instance instead of \"the_repository\".\n\nThe current situation seems counter intuitive and I don't see what the \nbenefit is in defining the current worktree to be per-process rather \nthan per-struct-repository instance.\n\nThis series isn't in next yet - shall I re-roll with an extra \npreparatory patch changing is_current_worktree() and \ngit_worktree_git_dir() to use wt->repo, or are you happy to have that as \na separate follow up on top of these patches?\n\nThanks\n\nPhillip\n"},{"id":"537105","messageId":"xmqqjyw0bve6.fsf@gitster.g","threadId":"64989","inReplyTo":"8397f971-39dd-4a18-b520-3157ae15324f@gmail.com","subject":"Re: [PATCH v2 1/2] wt-status: avoid passing NULL worktree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-25T17:11:45Z","receivedAt":"2026-02-25T17:11:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood <phillip.wood123@gmail.com> writes:\n\n> On 19/02/2026 20:37, Junio C Hamano wrote:\n>> Junio C Hamano <gitster@pobox.com> writes:\n>> \n>>> In other words, the function is_current_worktree(wt) may not take a\n>>> repository and always compute things relative to the_repository, but\n>>> once we wean ourselves off of the_repository, we would/should have\n>>> repo_is_current_worktree(repo, wt), making is_current_worktree(wt) a\n>>> thin wrapper for repo_is_current_worktree(the_repository, wt)?\n>> \n>> Eh, in light of 2/2 of this series, since wt knows which repository\n>> it belongs to, what I wrote above does not make much sense.\n>> Allowing callers to give repo that is different from wt->repo to\n>> that function is a potential foot-gun.  In other words, isn't\n>> is_current_worktree(wt) using the_worktree and not wt->repo a bug\n>> already, I have to wonder?\n>\n> I wonder that too. You, Karthik and me all initially assumed that the \n> current worktree would be defined by wt->repo->worktree matching \n> wt->path (the code actually compares the git_dir of the worktree and the \n> repository to accommodate bare repositories but the same principle \n> applies). The use of \"the_repository\" in is_current_wortree() comes from \n> replacing get_git_dir() with repo_get_git_dir() in 246deeac951 \n> (environment: make `get_git_dir()` accept a repository, 2024-09-12). In\n> get_worktree_git_dir() it comes from replacing git_common_path() with \n> repo_common_path() in 07242c2a5af (path: drop `git_common_path()` in \n> favor of `repo_common_path()`, 2025-02-07). I suspect the replacements \n> were mechanical and not much thought went into considering whether, in a \n> world where there can be more than one repository per process, they \n> should use the local repository instance instead of \"the_repository\".\n>\n> The current situation seems counter intuitive and I don't see what the \n> benefit is in defining the current worktree to be per-process rather \n> than per-struct-repository instance.\n>\n> This series isn't in next yet - shall I re-roll with an extra \n> preparatory patch changing is_current_worktree() and \n> git_worktree_git_dir() to use wt->repo, or are you happy to have that as \n> a separate follow up on top of these patches?\n\nThanks for investigating how we got here.  I do not have strong\npreference in the order, as long as we eventually get there.\n\n"},{"id":"537201","messageId":"d5866041-3e2f-4f5e-a8d1-725fd3eac2e2@gmail.com","threadId":"64989","inReplyTo":"xmqqjyw0bve6.fsf@gitster.g","subject":"Re: [PATCH v2 1/2] wt-status: avoid passing NULL worktree","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-02-26T16:09:48Z","receivedAt":"2026-02-26T16:09:52Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 25/02/2026 17:11, Junio C Hamano wrote:\n> Phillip Wood <phillip.wood123@gmail.com> writes:\n>>\n>> This series isn't in next yet - shall I re-roll with an extra\n>> preparatory patch changing is_current_worktree() and\n>> git_worktree_git_dir() to use wt->repo, or are you happy to have that as\n>> a separate follow up on top of these patches?\n> \n> Thanks for investigating how we got here.  I do not have strong\n> preference in the order, as long as we eventually get there.\n\nIn that case I'll send a follow-up series next week.\n\nThanks\n\nPhillip\n\n"},{"id":"537202","messageId":"xmqqy0kf4h2x.fsf@gitster.g","threadId":"64989","inReplyTo":"d5866041-3e2f-4f5e-a8d1-725fd3eac2e2@gmail.com","subject":"Re: [PATCH v2 1/2] wt-status: avoid passing NULL worktree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-26T16:15:02Z","receivedAt":"2026-02-26T16:15:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood <phillip.wood123@gmail.com> writes:\n\n> On 25/02/2026 17:11, Junio C Hamano wrote:\n>> Phillip Wood <phillip.wood123@gmail.com> writes:\n>>>\n>>> This series isn't in next yet - shall I re-roll with an extra\n>>> preparatory patch changing is_current_worktree() and\n>>> git_worktree_git_dir() to use wt->repo, or are you happy to have that as\n>>> a separate follow up on top of these patches?\n>> \n>> Thanks for investigating how we got here.  I do not have strong\n>> preference in the order, as long as we eventually get there.\n>\n> In that case I'll send a follow-up series next week.\n\nOK, then let's merge what we have to 'next'.\n"}]}