{"thread":{"id":"65236","subject":"[PATCH 0/3] worktree: stop using \"the_repository\" in is_current_worktree()","startedAt":"2026-03-13T14:20:04Z","lastAt":"2026-04-02T15:10:27Z","messageCount":27,"participants":["Phillip Wood","Junio C Hamano","Patrick Steinhardt","Shreyansh Paliwal"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"538895","messageId":"cover.1773411586.git.phillip.wood@dunelm.org.uk","threadId":"65236","inReplyTo":null,"subject":"[PATCH 0/3] worktree: stop using \"the_repository\" in is_current_worktree()","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-03-13T14:19:47Z","receivedAt":"2026-03-13T14:20:04Z","isPatch":true,"body":"From: Phillip Wood \n\nThis is a follow up to pw/no-more-NULL-means-current-worktree that removes\n\"the_repository\" from is_current_worktree() and get_worktree_git_dir().\nThe first patch removes the use of \"the_repository\" when determining\nif a worktree is current. Patches 2 & 3 require a non-NULL worktree\nwhen calling get_worktree_git_dir() to remove the last use of\n\"the_repository\" in that function.\n\nBase-Commit: 7f19e4e1b6a3ad259e2ed66033e01e03b8b74c5e\nPublished-As: https://github.com/phillipwood/git/releases/tag/pw%2Fworktree-is-current-use-repo%2Fv1\nView-Changes-At: https://github.com/phillipwood/git/compare/7f19e4e1b...1151b5b30\nFetch-It-Via: git fetch https://github.com/phillipwood/git pw/worktree-is-current-use-repo/v1\n\n\nPhillip Wood (3):\n worktree: remove \"the_repository\" from is_current_worktree()\n worktree add: stop reading \".git/HEAD\"\n worktree: reject NULL worktree in get_worktree_git_dir()\n\n builtin/worktree.c | 21 ++-------------------\n t/t2400-worktree-add.sh | 28 ++++++++++++----------------\n worktree.c | 10 +++++-----\n 3 files changed, 19 insertions(+), 40 deletions(-)\n\n-- \n2.52.0.362.g884e03848a9\n\n"},{"id":"538896","messageId":"075700a22568913988c9fa8e1ff49db1a1a5b606.1773411586.git.phillip.wood@dunelm.org.uk","threadId":"65236","inReplyTo":"cover.1773411586.git.phillip.wood@dunelm.org.uk","subject":"[PATCH 1/3] worktree: remove \"the_repository\" from is_current_worktree()","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-03-13T14:19:48Z","receivedAt":"2026-03-13T14:20:05Z","isPatch":true,"body":"From: Phillip Wood \n\nis_current_worktree() compares the gitdir of the worktree to the gitdir\nof \"the_repository\" and returns true when they match. To get the gitdir\nof the worktree it calls get_workree_git_dir() which also depends on\n\"the_repository\". This has the effect that even if \"wt->path\" matches\n\"wt->repo->worktree\" is_current_worktree(wt) will return false when\n\"wt->repo\" is not \"the_repository\" which is confusing.\n\nThe 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()`\nin favor of `repo_common_path()`, 2025-02-07). In both cases we have\na repository instance available so use that instead. This means\nthat a worktree \"wt\" is always considered current when \"wt->path\"\nmatches \"wt->repo->worktree\" and so the worktree returned by\nget_worktree_from_repository() is always considered current.\n\nSigned-off-by: Phillip Wood \n---\n worktree.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/worktree.c b/worktree.c\nindex e9ff6e6ef2e..344ad0c031b 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -58,7 +58,7 @@ static void add_head_info(struct worktree *wt)\n \n static int is_current_worktree(struct worktree *wt)\n {\n-\tchar *git_dir = absolute_pathdup(repo_get_git_dir(the_repository));\n+\tchar *git_dir = absolute_pathdup(repo_get_git_dir(wt->repo));\n \tchar *wt_git_dir = get_worktree_git_dir(wt);\n \tint is_current = !fspathcmp(git_dir, absolute_path(wt_git_dir));\n \tfree(wt_git_dir);\n@@ -78,7 +78,7 @@ struct worktree *get_worktree_from_repository(struct repository *repo)\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+\twt->is_current = true;\n \tadd_head_info(wt);\n \n \tfree(gitdir);\n@@ -229,9 +229,9 @@ char *get_worktree_git_dir(const struct worktree *wt)\n \tif (!wt)\n \t\treturn xstrdup(repo_get_git_dir(the_repository));\n \telse if (!wt->id)\n-\t\treturn xstrdup(repo_get_common_dir(the_repository));\n+\t\treturn xstrdup(repo_get_common_dir(wt->repo));\n \telse\n-\t\treturn repo_common_path(the_repository, \"worktrees/%s\", wt->id);\n+\t\treturn repo_common_path(wt->repo, \"worktrees/%s\", wt->id);\n }\n \n static struct worktree *find_worktree_by_suffix(struct worktree **list,\n-- \n2.52.0.362.g884e03848a9\n\n"},{"id":"538897","messageId":"ae2a368e7e783bfe9dd038bbb2e986e6d8540900.1773411586.git.phillip.wood@dunelm.org.uk","threadId":"65236","inReplyTo":"cover.1773411586.git.phillip.wood@dunelm.org.uk","subject":"[PATCH 2/3] worktree add: stop reading \".git/HEAD\"","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-03-13T14:19:49Z","receivedAt":"2026-03-13T14:20:07Z","isPatch":true,"body":"From: Phillip Wood \n\nThe function can_use_local_refs() prints a warning if there are no local\nbranches and HEAD is invalid or points to an unborn branch. As part of\nthe warning it prints the contents of \".git/HEAD\". In a repository using\nthe reftable backend HEAD is not stored in the filesystem so reading\nthat file is pointless. In a repository using the files backend it is\nunclear how useful printing it is - it would be better to diagnose the\nproblem for the user. For now, simplify the warning by not printing\nthe file contents and adjust the relevant test case accordingly. Also\nfixup the test case to use test_grep so that anyone trying to debug a\ntest failure in the future is not met by a wall of silence.\n\nSigned-off-by: Phillip Wood \n---\n builtin/worktree.c | 21 ++-------------------\n t/t2400-worktree-add.sh | 28 ++++++++++++----------------\n 2 files changed, 14 insertions(+), 35 deletions(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex bc2d0d645ba..70410b53df3 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -692,25 +692,8 @@ static int can_use_local_refs(const struct add_opts *opts)\n \tif (refs_head_ref(get_main_ref_store(the_repository), first_valid_ref, NULL)) {\n \t\treturn 1;\n \t} else if (refs_for_each_branch_ref(get_main_ref_store(the_repository), first_valid_ref, NULL)) {\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-\n-\t\t\tstrbuf_add_real_path(&path, wt_gitdir);\n-\t\t\tstrbuf_addstr(&path, \"/HEAD\");\n-\t\t\tstrbuf_read_file(&contents, path.buf, 64);\n-\t\t\tstrbuf_stripspace(&contents, NULL);\n-\t\t\tstrbuf_strip_suffix(&contents, \"\\n\");\n-\n-\t\t\twarning(_(\"HEAD points to an invalid (or orphaned) reference.\\n\"\n-\t\t\t\t \"HEAD path: '%s'\\n\"\n-\t\t\t\t \"HEAD contents: '%s'\"),\n-\t\t\t\t path.buf, contents.buf);\n-\t\t\tstrbuf_release(&path);\n-\t\t\tstrbuf_release(&contents);\n-\t\t\tfree(wt_gitdir);\n-\t\t}\n+\t\tif (!opts->quiet)\n+\t\t\t\twarning(_(\"HEAD points to an invalid (or orphaned) reference.\\n\"));\n \t\treturn 1;\n \t}\n \treturn 0;\ndiff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh\nindex 023e1301c8e..58b4445cc44 100755\n--- a/t/t2400-worktree-add.sh\n+++ b/t/t2400-worktree-add.sh\n@@ -987,7 +987,7 @@ test_dwim_orphan () {\n \t\t\t\tthen\n \t\t\t\t\ttest_must_be_empty actual\n \t\t\t\telse\n-\t\t\t\t\tgrep \"$info_text\" actual\n+\t\t\t\t\ttest_grep \"$info_text\" actual\n \t\t\t\tfi\n \t\t\telif [ \"$outcome\" = \"no_infer\" ]\n \t\t\tthen\n@@ -996,39 +996,35 @@ test_dwim_orphan () {\n \t\t\t\tthen\n \t\t\t\t\ttest_must_be_empty actual\n \t\t\t\telse\n-\t\t\t\t\t! grep \"$info_text\" actual\n+\t\t\t\t\ttest_grep ! \"$info_text\" actual\n \t\t\t\tfi\n \t\t\telif [ \"$outcome\" = \"fetch_error\" ]\n \t\t\tthen\n \t\t\t\ttest_must_fail git $dashc_args worktree add $args 2>actual &&\n-\t\t\t\tgrep \"$fetch_error_text\" actual\n+\t\t\t\ttest_grep \"$fetch_error_text\" actual\n \t\t\telif [ \"$outcome\" = \"fatal_orphan_bad_combo\" ]\n \t\t\tthen\n \t\t\t\ttest_must_fail git $dashc_args worktree add $args 2>actual &&\n \t\t\t\tif [ $use_quiet -eq 1 ]\n \t\t\t\tthen\n-\t\t\t\t\t! grep \"$info_text\" actual\n+\t\t\t\t\ttest_grep ! \"$info_text\" actual\n \t\t\t\telse\n-\t\t\t\t\tgrep \"$info_text\" actual\n+\t\t\t\t\ttest_grep \"$info_text\" actual\n \t\t\t\tfi &&\n-\t\t\t\tgrep \"$bad_combo_regex\" actual\n+\t\t\t\ttest_grep \"$bad_combo_regex\" actual\n \t\t\telif [ \"$outcome\" = \"warn_bad_head\" ]\n \t\t\tthen\n \t\t\t\ttest_must_fail git $dashc_args worktree add $args 2>actual &&\n \t\t\t\tif [ $use_quiet -eq 1 ]\n \t\t\t\tthen\n-\t\t\t\t\tgrep \"$invalid_ref_regex\" actual &&\n-\t\t\t\t\t! grep \"$orphan_hint\" actual\n+\t\t\t\t\ttest_grep \"$invalid_ref_regex\" actual &&\n+\t\t\t\t\ttest_grep ! \"$orphan_hint\" actual\n \t\t\t\telse\n-\t\t\t\t\theadpath=$(git $dashc_args rev-parse --path-format=absolute --git-path HEAD) &&\n-\t\t\t\t\theadcontents=$(cat \"$headpath\") &&\n-\t\t\t\t\tgrep \"HEAD points to an invalid (or orphaned) reference\" actual &&\n-\t\t\t\t\tgrep \"HEAD path: .$headpath.\" actual &&\n-\t\t\t\t\tgrep \"HEAD contents: .$headcontents.\" actual &&\n-\t\t\t\t\tgrep \"$orphan_hint\" actual &&\n-\t\t\t\t\t! grep \"$info_text\" actual\n+\t\t\t\t\ttest_grep \"HEAD points to an invalid (or orphaned) reference\" actual &&\n+\t\t\t\t\ttest_grep \"$orphan_hint\" actual &&\n+\t\t\t\t\ttest_grep ! \"$info_text\" actual\n \t\t\t\tfi &&\n-\t\t\t\tgrep \"$invalid_ref_regex\" actual\n+\t\t\t\ttest_grep \"$invalid_ref_regex\" actual\n \t\t\telse\n \t\t\t\t# Unreachable\n \t\t\t\tfalse\n-- \n2.52.0.362.g884e03848a9\n\n"},{"id":"538898","messageId":"1151b5b302069b4f3414a37e3be4bdbbc7e40686.1773411586.git.phillip.wood@dunelm.org.uk","threadId":"65236","inReplyTo":"cover.1773411586.git.phillip.wood@dunelm.org.uk","subject":"[PATCH 3/3] worktree: reject NULL worktree in get_worktree_git_dir()","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-03-13T14:19:50Z","receivedAt":"2026-03-13T14:20:07Z","isPatch":true,"body":"From: Phillip Wood \n\nThis removes the final dependence on \"the_repository\" in\nget_worktree_git_dir(). The last commit removed only caller that\npassed a NULL worktree.\n\nget_worktree_git_dir() has the following callers:\n\n - branch.c:prepare_checked_out_branches() which loops over all\n worktrees.\n\n - builtin/fsck.c:cmd_fsck() which loops over all worktrees.\n\n - builtin/receive-pack.c:update_worktree() which is called from\n update() only when \"worktree\" is non-NULL.\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 - reachable.c:add_rebase_files() which loops over all worktrees.\n\n - revision.c:add_index_objects_to_pending() which loops over all\n worktrees.\n\n - worktree.c:is_current_worktree() which expects a non-NULL worktree.\n\nSigned-off-by: Phillip Wood \n---\n worktree.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/worktree.c b/worktree.c\nindex 344ad0c031b..1ed5e8c3cd2 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -227,7 +227,7 @@ struct worktree **get_worktrees_without_reading_head(void)\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+\t\tBUG(\"%s() called with NULL worktree\", __func__);\n \telse if (!wt->id)\n \t\treturn xstrdup(repo_get_common_dir(wt->repo));\n \telse\n-- \n2.52.0.362.g884e03848a9\n\n"},{"id":"538936","messageId":"xmqqjyvf2yrj.fsf@gitster.g","threadId":"65236","inReplyTo":"ae2a368e7e783bfe9dd038bbb2e986e6d8540900.1773411586.git.phillip.wood@dunelm.org.uk","subject":"Re: [PATCH 2/3] worktree add: stop reading \".git/HEAD\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-13T21:41:20Z","receivedAt":"2026-03-13T21:41:23Z","isPatch":true,"body":"Phillip Wood writes:\n\n> From: Phillip Wood \n>\n> The function can_use_local_refs() prints a warning if there are no local\n> branches and HEAD is invalid or points to an unborn branch. As part of\n> the warning it prints the contents of \".git/HEAD\". In a repository using\n> the reftable backend HEAD is not stored in the filesystem so reading\n> that file is pointless. In a repository using the files backend it is\n> unclear how useful printing it is - it would be better to diagnose the\n> problem for the user. For now, simplify the warning by not printing\n> the file contents and adjust the relevant test case accordingly. Also\n> fixup the test case to use test_grep so that anyone trying to debug a\n> test failure in the future is not met by a wall of silence.\n>\n> Signed-off-by: Phillip Wood \n> ---\n> builtin/worktree.c | 21 ++-------------------\n> t/t2400-worktree-add.sh | 28 ++++++++++++----------------\n> 2 files changed, 14 insertions(+), 35 deletions(-)\n>\n> diff --git a/builtin/worktree.c b/builtin/worktree.c\n> index bc2d0d645ba..70410b53df3 100644\n> --- a/builtin/worktree.c\n> +++ b/builtin/worktree.c\n> @@ -692,25 +692,8 @@ static int can_use_local_refs(const struct add_opts *opts)\n> \tif (refs_head_ref(get_main_ref_store(the_repository), first_valid_ref, NULL)) {\n> \t\treturn 1;\n> \t} else if (refs_for_each_branch_ref(get_main_ref_store(the_repository), first_valid_ref, NULL)) {\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> -\n> -\t\t\tstrbuf_add_real_path(&path, wt_gitdir);\n> -\t\t\tstrbuf_addstr(&path, \"/HEAD\");\n> -\t\t\tstrbuf_read_file(&contents, path.buf, 64);\n> -\t\t\tstrbuf_stripspace(&contents, NULL);\n> -\t\t\tstrbuf_strip_suffix(&contents, \"\\n\");\n> -\n> -\t\t\twarning(_(\"HEAD points to an invalid (or orphaned) reference.\\n\"\n> -\t\t\t\t \"HEAD path: '%s'\\n\"\n> -\t\t\t\t \"HEAD contents: '%s'\"),\n> -\t\t\t\t path.buf, contents.buf);\n> -\t\t\tstrbuf_release(&path);\n> -\t\t\tstrbuf_release(&contents);\n> -\t\t\tfree(wt_gitdir);\n> -\t\t}\n> +\t\tif (!opts->quiet)\n> +\t\t\t\twarning(_(\"HEAD points to an invalid (or orphaned) reference.\\n\"));\n\nThis is indented one level too deep, it seems.\n\nOther than that, I fully agree with the reasoning of the removal\nexplained in the proposed log message, and the updated test to use\ntest_grep does look much better.\n\nWe seem to use \"[ a = b ]\" instead of \"test a = b\" in this test\nfile, unlike everybody else, which I didn't notice before. Of\ncourse, this series has no need to touch them; it is just a tangent\nI happened to have noticed.\n\nThanks.\n\n> diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh\n> index 023e1301c8e..58b4445cc44 100755\n> --- a/t/t2400-worktree-add.sh\n> +++ b/t/t2400-worktree-add.sh\n> @@ -987,7 +987,7 @@ test_dwim_orphan () {\n> \t\t\t\tthen\n> \t\t\t\t\ttest_must_be_empty actual\n> \t\t\t\telse\n> -\t\t\t\t\tgrep \"$info_text\" actual\n> +\t\t\t\t\ttest_grep \"$info_text\" actual\n> \t\t\t\tfi\n> \t\t\telif [ \"$outcome\" = \"no_infer\" ]\n> \t\t\tthen\n> @@ -996,39 +996,35 @@ test_dwim_orphan () {\n> \t\t\t\tthen\n> \t\t\t\t\ttest_must_be_empty actual\n> \t\t\t\telse\n> -\t\t\t\t\t! grep \"$info_text\" actual\n> +\t\t\t\t\ttest_grep ! \"$info_text\" actual\n> \t\t\t\tfi\n> \t\t\telif [ \"$outcome\" = \"fetch_error\" ]\n> \t\t\tthen\n> \t\t\t\ttest_must_fail git $dashc_args worktree add $args 2>actual &&\n> -\t\t\t\tgrep \"$fetch_error_text\" actual\n> +\t\t\t\ttest_grep \"$fetch_error_text\" actual\n> \t\t\telif [ \"$outcome\" = \"fatal_orphan_bad_combo\" ]\n> \t\t\tthen\n> \t\t\t\ttest_must_fail git $dashc_args worktree add $args 2>actual &&\n> \t\t\t\tif [ $use_quiet -eq 1 ]\n> \t\t\t\tthen\n> -\t\t\t\t\t! grep \"$info_text\" actual\n> +\t\t\t\t\ttest_grep ! \"$info_text\" actual\n> \t\t\t\telse\n> -\t\t\t\t\tgrep \"$info_text\" actual\n> +\t\t\t\t\ttest_grep \"$info_text\" actual\n> \t\t\t\tfi &&\n> -\t\t\t\tgrep \"$bad_combo_regex\" actual\n> +\t\t\t\ttest_grep \"$bad_combo_regex\" actual\n> \t\t\telif [ \"$outcome\" = \"warn_bad_head\" ]\n> \t\t\tthen\n> \t\t\t\ttest_must_fail git $dashc_args worktree add $args 2>actual &&\n> \t\t\t\tif [ $use_quiet -eq 1 ]\n> \t\t\t\tthen\n> -\t\t\t\t\tgrep \"$invalid_ref_regex\" actual &&\n> -\t\t\t\t\t! grep \"$orphan_hint\" actual\n> +\t\t\t\t\ttest_grep \"$invalid_ref_regex\" actual &&\n> +\t\t\t\t\ttest_grep ! \"$orphan_hint\" actual\n> \t\t\t\telse\n> -\t\t\t\t\theadpath=$(git $dashc_args rev-parse --path-format=absolute --git-path HEAD) &&\n> -\t\t\t\t\theadcontents=$(cat \"$headpath\") &&\n> -\t\t\t\t\tgrep \"HEAD points to an invalid (or orphaned) reference\" actual &&\n> -\t\t\t\t\tgrep \"HEAD path: .$headpath.\" actual &&\n> -\t\t\t\t\tgrep \"HEAD contents: .$headcontents.\" actual &&\n> -\t\t\t\t\tgrep \"$orphan_hint\" actual &&\n> -\t\t\t\t\t! grep \"$info_text\" actual\n> +\t\t\t\t\ttest_grep \"HEAD points to an invalid (or orphaned) reference\" actual &&\n> +\t\t\t\t\ttest_grep \"$orphan_hint\" actual &&\n> +\t\t\t\t\ttest_grep ! \"$info_text\" actual\n> \t\t\t\tfi &&\n> -\t\t\t\tgrep \"$invalid_ref_regex\" actual\n> +\t\t\t\ttest_grep \"$invalid_ref_regex\" actual\n> \t\t\telse\n> \t\t\t\t# Unreachable\n> \t\t\t\tfalse\n"},{"id":"538937","messageId":"xmqqfr632yq8.fsf@gitster.g","threadId":"65236","inReplyTo":"1151b5b302069b4f3414a37e3be4bdbbc7e40686.1773411586.git.phillip.wood@dunelm.org.uk","subject":"Re: [PATCH 3/3] worktree: reject NULL worktree in get_worktree_git_dir()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-13T21:42:07Z","receivedAt":"2026-03-13T21:42:08Z","isPatch":true,"body":"Phillip Wood writes:\n\n> From: Phillip Wood \n>\n> This removes the final dependence on \"the_repository\" in\n> get_worktree_git_dir(). The last commit removed only caller that\n> passed a NULL worktree.\n>\n> get_worktree_git_dir() has the following callers:\n>\n> - branch.c:prepare_checked_out_branches() which loops over all\n> worktrees.\n>\n> - builtin/fsck.c:cmd_fsck() which loops over all worktrees.\n>\n> - builtin/receive-pack.c:update_worktree() which is called from\n> update() only when \"worktree\" is non-NULL.\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> - reachable.c:add_rebase_files() which loops over all worktrees.\n>\n> - revision.c:add_index_objects_to_pending() which loops over all\n> worktrees.\n>\n> - worktree.c:is_current_worktree() which expects a non-NULL worktree.\n>\n> Signed-off-by: Phillip Wood \n> ---\n> worktree.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/worktree.c b/worktree.c\n> index 344ad0c031b..1ed5e8c3cd2 100644\n> --- a/worktree.c\n> +++ b/worktree.c\n> @@ -227,7 +227,7 @@ struct worktree **get_worktrees_without_reading_head(void)\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> +\t\tBUG(\"%s() called with NULL worktree\", __func__);\n> \telse if (!wt->id)\n> \t\treturn xstrdup(repo_get_common_dir(wt->repo));\n> \telse\n\n still has\n\n /*\n * Return git dir of the worktree. Note that the path may be relative.\n * If wt is NULL, git dir of current worktree is returned.\n */\n char *get_worktree_git_dir(const struct worktree *wt);\n\nwhich needs a matching adjustment.\n"},{"id":"538991","messageId":"4649d374-59ad-4019-aacc-259245e18587@gmail.com","threadId":"65236","inReplyTo":"xmqqfr632yq8.fsf@gitster.g","subject":"Re: [PATCH 3/3] worktree: reject NULL worktree in get_worktree_git_dir()","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-03-14T20:09:34Z","receivedAt":"2026-03-14T20:09:37Z","isPatch":true,"body":"On 13/03/2026 21:42, Junio C Hamano wrote:\n> \n> still has\n> \n> /*\n> * Return git dir of the worktree. Note that the path may be relative.\n> * If wt is NULL, git dir of current worktree is returned.\n> */\n> char *get_worktree_git_dir(const struct worktree *wt);\n\nGood catch, I'll fix that and send a re-roll\n\nThanks\n\nPhillip\n\n"},{"id":"539037","messageId":"cover.1773591528.git.phillip.wood@dunelm.org.uk","threadId":"65236","inReplyTo":"cover.1773411586.git.phillip.wood@dunelm.org.uk","subject":"[PATCH v2 0/3] worktree: stop using \"the_repository\" in is_current_worktree()","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-03-15T16:18:49Z","receivedAt":"2026-03-15T16:19:08Z","isPatch":true,"body":"From: Phillip Wood \n\nThis is a follow up to pw/no-more-NULL-means-current-worktree that removes\n\"the_repository\" from is_current_worktree() and get_worktree_git_dir().\nThe first patch removes the use of \"the_repository\" when determining\nif a worktree is current. Patches 2 & 3 require a non-NULL worktree\nwhen calling get_worktree_git_dir() to remove the last use of\n\"the_repository\" in that function.\n\nChanges since V1\n\n - Patch 2: fixed indentation (thanks to Junio)\n - Patch 3: removed stale comment (thanks to Junio)\n\nBase-Commit: 7f19e4e1b6a3ad259e2ed66033e01e03b8b74c5e\nPublished-As: https://github.com/phillipwood/git/releases/tag/pw%2Fworktree-is-current-use-repo%2Fv2\nView-Changes-At: https://github.com/phillipwood/git/compare/7f19e4e1b...75eecc849\nFetch-It-Via: git fetch https://github.com/phillipwood/git pw/worktree-is-current-use-repo/v2\n\n\nPhillip Wood (3):\n worktree: remove \"the_repository\" from is_current_worktree()\n worktree add: stop reading \".git/HEAD\"\n worktree: reject NULL worktree in get_worktree_git_dir()\n\n builtin/worktree.c | 21 ++-------------------\n t/t2400-worktree-add.sh | 28 ++++++++++++----------------\n worktree.c | 10 +++++-----\n worktree.h | 1 -\n 4 files changed, 19 insertions(+), 41 deletions(-)\n\nRange-diff against v1:\n1: 075700a2256 = 1: 075700a2256 worktree: remove \"the_repository\" from is_current_worktree()\n2: ae2a368e7e7 ! 2: c3c5767725d worktree add: stop reading \".git/HEAD\"\n @@ builtin/worktree.c: static int can_use_local_refs(const struct add_opts *opts)\n -\t\t\tfree(wt_gitdir);\n -\t\t}\n +\t\tif (!opts->quiet)\n -+\t\t\t\twarning(_(\"HEAD points to an invalid (or orphaned) reference.\\n\"));\n ++\t\t\twarning(_(\"HEAD points to an invalid (or orphaned) reference.\\n\"));\n \t\treturn 1;\n \t}\n \treturn 0;\n3: 1151b5b3020 ! 3: 75eecc8492e worktree: reject NULL worktree in get_worktree_git_dir()\n @@ worktree.c: struct worktree **get_worktrees_without_reading_head(void)\n \telse if (!wt->id)\n \t\treturn xstrdup(repo_get_common_dir(wt->repo));\n \telse\n +\n + ## worktree.h ##\n +@@ worktree.h: int submodule_uses_worktrees(const char *path);\n + \n + /*\n + * Return git dir of the worktree. Note that the path may be relative.\n +- * If wt is NULL, git dir of current worktree is returned.\n + */\n + char *get_worktree_git_dir(const struct worktree *wt);\n + \n-- \n2.52.0.362.g884e03848a9\n\n"},{"id":"539038","messageId":"075700a22568913988c9fa8e1ff49db1a1a5b606.1773591528.git.phillip.wood@dunelm.org.uk","threadId":"65236","inReplyTo":"cover.1773591528.git.phillip.wood@dunelm.org.uk","subject":"[PATCH v2 1/3] worktree: remove \"the_repository\" from is_current_worktree()","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-03-15T16:18:50Z","receivedAt":"2026-03-15T16:19:10Z","isPatch":true,"body":"From: Phillip Wood \n\nis_current_worktree() compares the gitdir of the worktree to the gitdir\nof \"the_repository\" and returns true when they match. To get the gitdir\nof the worktree it calls get_workree_git_dir() which also depends on\n\"the_repository\". This has the effect that even if \"wt->path\" matches\n\"wt->repo->worktree\" is_current_worktree(wt) will return false when\n\"wt->repo\" is not \"the_repository\" which is confusing.\n\nThe 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()`\nin favor of `repo_common_path()`, 2025-02-07). In both cases we have\na repository instance available so use that instead. This means\nthat a worktree \"wt\" is always considered current when \"wt->path\"\nmatches \"wt->repo->worktree\" and so the worktree returned by\nget_worktree_from_repository() is always considered current.\n\nSigned-off-by: Phillip Wood \n---\n worktree.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/worktree.c b/worktree.c\nindex e9ff6e6ef2e..344ad0c031b 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -58,7 +58,7 @@ static void add_head_info(struct worktree *wt)\n \n static int is_current_worktree(struct worktree *wt)\n {\n-\tchar *git_dir = absolute_pathdup(repo_get_git_dir(the_repository));\n+\tchar *git_dir = absolute_pathdup(repo_get_git_dir(wt->repo));\n \tchar *wt_git_dir = get_worktree_git_dir(wt);\n \tint is_current = !fspathcmp(git_dir, absolute_path(wt_git_dir));\n \tfree(wt_git_dir);\n@@ -78,7 +78,7 @@ struct worktree *get_worktree_from_repository(struct repository *repo)\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+\twt->is_current = true;\n \tadd_head_info(wt);\n \n \tfree(gitdir);\n@@ -229,9 +229,9 @@ char *get_worktree_git_dir(const struct worktree *wt)\n \tif (!wt)\n \t\treturn xstrdup(repo_get_git_dir(the_repository));\n \telse if (!wt->id)\n-\t\treturn xstrdup(repo_get_common_dir(the_repository));\n+\t\treturn xstrdup(repo_get_common_dir(wt->repo));\n \telse\n-\t\treturn repo_common_path(the_repository, \"worktrees/%s\", wt->id);\n+\t\treturn repo_common_path(wt->repo, \"worktrees/%s\", wt->id);\n }\n \n static struct worktree *find_worktree_by_suffix(struct worktree **list,\n-- \n2.52.0.362.g884e03848a9\n\n"},{"id":"539039","messageId":"c3c5767725d6d3b31604fbd0dd29486b70bc18a1.1773591528.git.phillip.wood@dunelm.org.uk","threadId":"65236","inReplyTo":"cover.1773591528.git.phillip.wood@dunelm.org.uk","subject":"[PATCH v2 2/3] worktree add: stop reading \".git/HEAD\"","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-03-15T16:18:51Z","receivedAt":"2026-03-15T16:19:11Z","isPatch":true,"body":"From: Phillip Wood \n\nThe function can_use_local_refs() prints a warning if there are no local\nbranches and HEAD is invalid or points to an unborn branch. As part of\nthe warning it prints the contents of \".git/HEAD\". In a repository using\nthe reftable backend HEAD is not stored in the filesystem so reading\nthat file is pointless. In a repository using the files backend it is\nunclear how useful printing it is - it would be better to diagnose the\nproblem for the user. For now, simplify the warning by not printing\nthe file contents and adjust the relevant test case accordingly. Also\nfixup the test case to use test_grep so that anyone trying to debug a\ntest failure in the future is not met by a wall of silence.\n\nSigned-off-by: Phillip Wood \n---\n builtin/worktree.c | 21 ++-------------------\n t/t2400-worktree-add.sh | 28 ++++++++++++----------------\n 2 files changed, 14 insertions(+), 35 deletions(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex bc2d0d645ba..9170b2e8981 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -692,25 +692,8 @@ static int can_use_local_refs(const struct add_opts *opts)\n \tif (refs_head_ref(get_main_ref_store(the_repository), first_valid_ref, NULL)) {\n \t\treturn 1;\n \t} else if (refs_for_each_branch_ref(get_main_ref_store(the_repository), first_valid_ref, NULL)) {\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-\n-\t\t\tstrbuf_add_real_path(&path, wt_gitdir);\n-\t\t\tstrbuf_addstr(&path, \"/HEAD\");\n-\t\t\tstrbuf_read_file(&contents, path.buf, 64);\n-\t\t\tstrbuf_stripspace(&contents, NULL);\n-\t\t\tstrbuf_strip_suffix(&contents, \"\\n\");\n-\n-\t\t\twarning(_(\"HEAD points to an invalid (or orphaned) reference.\\n\"\n-\t\t\t\t \"HEAD path: '%s'\\n\"\n-\t\t\t\t \"HEAD contents: '%s'\"),\n-\t\t\t\t path.buf, contents.buf);\n-\t\t\tstrbuf_release(&path);\n-\t\t\tstrbuf_release(&contents);\n-\t\t\tfree(wt_gitdir);\n-\t\t}\n+\t\tif (!opts->quiet)\n+\t\t\twarning(_(\"HEAD points to an invalid (or orphaned) reference.\\n\"));\n \t\treturn 1;\n \t}\n \treturn 0;\ndiff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh\nindex 023e1301c8e..58b4445cc44 100755\n--- a/t/t2400-worktree-add.sh\n+++ b/t/t2400-worktree-add.sh\n@@ -987,7 +987,7 @@ test_dwim_orphan () {\n \t\t\t\tthen\n \t\t\t\t\ttest_must_be_empty actual\n \t\t\t\telse\n-\t\t\t\t\tgrep \"$info_text\" actual\n+\t\t\t\t\ttest_grep \"$info_text\" actual\n \t\t\t\tfi\n \t\t\telif [ \"$outcome\" = \"no_infer\" ]\n \t\t\tthen\n@@ -996,39 +996,35 @@ test_dwim_orphan () {\n \t\t\t\tthen\n \t\t\t\t\ttest_must_be_empty actual\n \t\t\t\telse\n-\t\t\t\t\t! grep \"$info_text\" actual\n+\t\t\t\t\ttest_grep ! \"$info_text\" actual\n \t\t\t\tfi\n \t\t\telif [ \"$outcome\" = \"fetch_error\" ]\n \t\t\tthen\n \t\t\t\ttest_must_fail git $dashc_args worktree add $args 2>actual &&\n-\t\t\t\tgrep \"$fetch_error_text\" actual\n+\t\t\t\ttest_grep \"$fetch_error_text\" actual\n \t\t\telif [ \"$outcome\" = \"fatal_orphan_bad_combo\" ]\n \t\t\tthen\n \t\t\t\ttest_must_fail git $dashc_args worktree add $args 2>actual &&\n \t\t\t\tif [ $use_quiet -eq 1 ]\n \t\t\t\tthen\n-\t\t\t\t\t! grep \"$info_text\" actual\n+\t\t\t\t\ttest_grep ! \"$info_text\" actual\n \t\t\t\telse\n-\t\t\t\t\tgrep \"$info_text\" actual\n+\t\t\t\t\ttest_grep \"$info_text\" actual\n \t\t\t\tfi &&\n-\t\t\t\tgrep \"$bad_combo_regex\" actual\n+\t\t\t\ttest_grep \"$bad_combo_regex\" actual\n \t\t\telif [ \"$outcome\" = \"warn_bad_head\" ]\n \t\t\tthen\n \t\t\t\ttest_must_fail git $dashc_args worktree add $args 2>actual &&\n \t\t\t\tif [ $use_quiet -eq 1 ]\n \t\t\t\tthen\n-\t\t\t\t\tgrep \"$invalid_ref_regex\" actual &&\n-\t\t\t\t\t! grep \"$orphan_hint\" actual\n+\t\t\t\t\ttest_grep \"$invalid_ref_regex\" actual &&\n+\t\t\t\t\ttest_grep ! \"$orphan_hint\" actual\n \t\t\t\telse\n-\t\t\t\t\theadpath=$(git $dashc_args rev-parse --path-format=absolute --git-path HEAD) &&\n-\t\t\t\t\theadcontents=$(cat \"$headpath\") &&\n-\t\t\t\t\tgrep \"HEAD points to an invalid (or orphaned) reference\" actual &&\n-\t\t\t\t\tgrep \"HEAD path: .$headpath.\" actual &&\n-\t\t\t\t\tgrep \"HEAD contents: .$headcontents.\" actual &&\n-\t\t\t\t\tgrep \"$orphan_hint\" actual &&\n-\t\t\t\t\t! grep \"$info_text\" actual\n+\t\t\t\t\ttest_grep \"HEAD points to an invalid (or orphaned) reference\" actual &&\n+\t\t\t\t\ttest_grep \"$orphan_hint\" actual &&\n+\t\t\t\t\ttest_grep ! \"$info_text\" actual\n \t\t\t\tfi &&\n-\t\t\t\tgrep \"$invalid_ref_regex\" actual\n+\t\t\t\ttest_grep \"$invalid_ref_regex\" actual\n \t\t\telse\n \t\t\t\t# Unreachable\n \t\t\t\tfalse\n-- \n2.52.0.362.g884e03848a9\n\n"},{"id":"539040","messageId":"75eecc8492e3fae70c3f11edfc29417937459dd0.1773591528.git.phillip.wood@dunelm.org.uk","threadId":"65236","inReplyTo":"cover.1773591528.git.phillip.wood@dunelm.org.uk","subject":"[PATCH v2 3/3] worktree: reject NULL worktree in get_worktree_git_dir()","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-03-15T16:18:52Z","receivedAt":"2026-03-15T16:19:12Z","isPatch":true,"body":"From: Phillip Wood \n\nThis removes the final dependence on \"the_repository\" in\nget_worktree_git_dir(). The last commit removed only caller that\npassed a NULL worktree.\n\nget_worktree_git_dir() has the following callers:\n\n - branch.c:prepare_checked_out_branches() which loops over all\n worktrees.\n\n - builtin/fsck.c:cmd_fsck() which loops over all worktrees.\n\n - builtin/receive-pack.c:update_worktree() which is called from\n update() only when \"worktree\" is non-NULL.\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 - reachable.c:add_rebase_files() which loops over all worktrees.\n\n - revision.c:add_index_objects_to_pending() which loops over all\n worktrees.\n\n - worktree.c:is_current_worktree() which expects a non-NULL worktree.\n\nSigned-off-by: Phillip Wood \n---\n worktree.c | 2 +-\n worktree.h | 1 -\n 2 files changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/worktree.c b/worktree.c\nindex 344ad0c031b..1ed5e8c3cd2 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -227,7 +227,7 @@ struct worktree **get_worktrees_without_reading_head(void)\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+\t\tBUG(\"%s() called with NULL worktree\", __func__);\n \telse if (!wt->id)\n \t\treturn xstrdup(repo_get_common_dir(wt->repo));\n \telse\ndiff --git a/worktree.h b/worktree.h\nindex e450d1a3317..85d634c36c0 100644\n--- a/worktree.h\n+++ b/worktree.h\n@@ -51,7 +51,6 @@ int submodule_uses_worktrees(const char *path);\n \n /*\n * Return git dir of the worktree. Note that the path may be relative.\n- * If wt is NULL, git dir of current worktree is returned.\n */\n char *get_worktree_git_dir(const struct worktree *wt);\n \n-- \n2.52.0.362.g884e03848a9\n\n"},{"id":"539049","messageId":"xmqqzf48rdvr.fsf@gitster.g","threadId":"65236","inReplyTo":"cover.1773591528.git.phillip.wood@dunelm.org.uk","subject":"Re: [PATCH v2 0/3] worktree: stop using \"the_repository\" in is_current_worktree()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-15T21:17:44Z","receivedAt":"2026-03-15T21:17:46Z","isPatch":true,"body":"Phillip Wood writes:\n\n> Range-diff against v1:\n> 1: 075700a2256 = 1: 075700a2256 worktree: remove \"the_repository\" from is_current_worktree()\n> 2: ae2a368e7e7 ! 2: c3c5767725d worktree add: stop reading \".git/HEAD\"\n> @@ builtin/worktree.c: static int can_use_local_refs(const struct add_opts *opts)\n> -\t\t\tfree(wt_gitdir);\n> -\t\t}\n> +\t\tif (!opts->quiet)\n> -+\t\t\t\twarning(_(\"HEAD points to an invalid (or orphaned) reference.\\n\"));\n> ++\t\t\twarning(_(\"HEAD points to an invalid (or orphaned) reference.\\n\"));\n> \t\treturn 1;\n> \t}\n> \treturn 0;\n> 3: 1151b5b3020 ! 3: 75eecc8492e worktree: reject NULL worktree in get_worktree_git_dir()\n> @@ worktree.c: struct worktree **get_worktrees_without_reading_head(void)\n> \telse if (!wt->id)\n> \t\treturn xstrdup(repo_get_common_dir(wt->repo));\n> \telse\n> +\n> + ## worktree.h ##\n> +@@ worktree.h: int submodule_uses_worktrees(const char *path);\n> + \n> + /*\n> + * Return git dir of the worktree. Note that the path may be relative.\n> +- * If wt is NULL, git dir of current worktree is returned.\n> + */\n> + char *get_worktree_git_dir(const struct worktree *wt);\n> + \n\nAh, I somehow expected to see that we say \"passing NULL to wt is an\nerror\", but that is misleading. When something expects a worktree\ninstance, and it accepts NULL as a special case, then that is worth\ncommenting, but otherwise, it is not worth mentioning.\n\nAll look good. Will replace. Thanks.\n"},{"id":"539072","messageId":"abezeNELL9SU8v82@pks.im","threadId":"65236","inReplyTo":"075700a22568913988c9fa8e1ff49db1a1a5b606.1773591528.git.phillip.wood@dunelm.org.uk","subject":"Re: [PATCH v2 1/3] worktree: remove \"the_repository\" from is_current_worktree()","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-16T07:38:32Z","receivedAt":"2026-03-16T07:38:38Z","isPatch":true,"body":"On Sun, Mar 15, 2026 at 04:18:50PM +0000, Phillip Wood wrote:\n> From: Phillip Wood \n> \n> is_current_worktree() compares the gitdir of the worktree to the gitdir\n> of \"the_repository\" and returns true when they match. To get the gitdir\n> of the worktree it calls get_workree_git_dir() which also depends on\n> \"the_repository\". This has the effect that even if \"wt->path\" matches\n> \"wt->repo->worktree\" is_current_worktree(wt) will return false when\n> \"wt->repo\" is not \"the_repository\" which is confusing.\n> \n> 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()`\n> in favor of `repo_common_path()`, 2025-02-07). In both cases we have\n> a repository instance available so use that instead. This means\n> that a worktree \"wt\" is always considered current when \"wt->path\"\n> matches \"wt->repo->worktree\" and so the worktree returned by\n> get_worktree_from_repository() is always considered current.\n> \n> Signed-off-by: Phillip Wood \n> ---\n> worktree.c | 8 ++++----\n> 1 file changed, 4 insertions(+), 4 deletions(-)\n> \n> diff --git a/worktree.c b/worktree.c\n> index e9ff6e6ef2e..344ad0c031b 100644\n> --- a/worktree.c\n> +++ b/worktree.c\n> @@ -58,7 +58,7 @@ static void add_head_info(struct worktree *wt)\n> \n> static int is_current_worktree(struct worktree *wt)\n> {\n> -\tchar *git_dir = absolute_pathdup(repo_get_git_dir(the_repository));\n> +\tchar *git_dir = absolute_pathdup(repo_get_git_dir(wt->repo));\n> \tchar *wt_git_dir = get_worktree_git_dir(wt);\n> \tint is_current = !fspathcmp(git_dir, absolute_path(wt_git_dir));\n> \tfree(wt_git_dir);\n\nHm, okay.\n\n> @@ -78,7 +78,7 @@ struct worktree *get_worktree_from_repository(struct repository *repo)\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> +\twt->is_current = true;\n> \tadd_head_info(wt);\n> \n> \tfree(gitdir);\n\nI have been staring at this code for longer than I want to admit, and I\nstill haven't convinced myself that this is not a change in behaviour.\nI think what I'm wondering about is what `is_current_worktree()` is\nactually intended to do. In other words, what _do_ we consider to be\n\"current\"?\n\nNaively, I would consider the worktree \"current\" that the Git process\nhas been invoked in. So if I pass a repo other than `the_repository`, or\nif I pass a worktree that is not the one that Git has been started in,\nthen I would expect the function to return `false`. With that naive\nassumption your change would be breaking the existing logic.\n\nBut I have no idea whether my assumption is correct or not, as\nthere is not really any documentation of what the function or of the\n`struct worktree::is_current` field. And having a look at a couple of\ncallers doesn't really make me all the wiser.\n\nIt would be great if you could shine some light on this and then also\nadd a bit of documentation to either the function, the field, or both :)\n\nThanks!\n\nPatrick\n"},{"id":"539073","messageId":"abeztWLCxdWADCJ8@pks.im","threadId":"65236","inReplyTo":"c3c5767725d6d3b31604fbd0dd29486b70bc18a1.1773591528.git.phillip.wood@dunelm.org.uk","subject":"Re: [PATCH v2 2/3] worktree add: stop reading \".git/HEAD\"","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-16T07:39:33Z","receivedAt":"2026-03-16T07:39:38Z","isPatch":true,"body":"On Sun, Mar 15, 2026 at 04:18:51PM +0000, Phillip Wood wrote:\n> From: Phillip Wood \n> \n> The function can_use_local_refs() prints a warning if there are no local\n> branches and HEAD is invalid or points to an unborn branch. As part of\n> the warning it prints the contents of \".git/HEAD\". In a repository using\n> the reftable backend HEAD is not stored in the filesystem so reading\n> that file is pointless. In a repository using the files backend it is\n> unclear how useful printing it is - it would be better to diagnose the\n> problem for the user. For now, simplify the warning by not printing\n> the file contents and adjust the relevant test case accordingly. Also\n> fixup the test case to use test_grep so that anyone trying to debug a\n> test failure in the future is not met by a wall of silence.\n\nOh, interesting, and good catch. I fully agree that removing this makes\nsense.\n\nPatrick\n"},{"id":"539128","messageId":"9e3b59d1-58f3-476c-9c7a-3ceffbb71810@gmail.com","threadId":"65236","inReplyTo":"abezeNELL9SU8v82@pks.im","subject":"Re: [PATCH v2 1/3] worktree: remove \"the_repository\" from is_current_worktree()","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-03-16T16:22:11Z","receivedAt":"2026-03-16T16:22:16Z","isPatch":true,"body":"Hi Patrick\n\nOn 16/03/2026 07:38, Patrick Steinhardt wrote:\n> On Sun, Mar 15, 2026 at 04:18:50PM +0000, Phillip Wood wrote:\n>> From: Phillip Wood \n>>\n>> is_current_worktree() compares the gitdir of the worktree to the gitdir\n>> of \"the_repository\" and returns true when they match. To get the gitdir\n>> of the worktree it calls get_workree_git_dir() which also depends on\n>> \"the_repository\". This has the effect that even if \"wt->path\" matches\n>> \"wt->repo->worktree\" is_current_worktree(wt) will return false when\n>> \"wt->repo\" is not \"the_repository\" which is confusing.\n>>\n>> 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()`\n>> in favor of `repo_common_path()`, 2025-02-07). In both cases we have\n>> a repository instance available so use that instead. This means\n>> that a worktree \"wt\" is always considered current when \"wt->path\"\n>> matches \"wt->repo->worktree\" and so the worktree returned by\n>> get_worktree_from_repository() is always considered current.\n>>\n>> Signed-off-by: Phillip Wood \n>> ---\n>> worktree.c | 8 ++++----\n>> 1 file changed, 4 insertions(+), 4 deletions(-)\n>>\n>> diff --git a/worktree.c b/worktree.c\n>> index e9ff6e6ef2e..344ad0c031b 100644\n>> --- a/worktree.c\n>> +++ b/worktree.c\n>> @@ -58,7 +58,7 @@ static void add_head_info(struct worktree *wt)\n>> \n>> static int is_current_worktree(struct worktree *wt)\n>> {\n>> -\tchar *git_dir = absolute_pathdup(repo_get_git_dir(the_repository));\n>> +\tchar *git_dir = absolute_pathdup(repo_get_git_dir(wt->repo));\n>> \tchar *wt_git_dir = get_worktree_git_dir(wt);\n>> \tint is_current = !fspathcmp(git_dir, absolute_path(wt_git_dir));\n>> \tfree(wt_git_dir);\n> \n> Hm, okay.\n> \n>> @@ -78,7 +78,7 @@ struct worktree *get_worktree_from_repository(struct repository *repo)\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>> +\twt->is_current = true;\n>> \tadd_head_info(wt);\n>> \n>> \tfree(gitdir);\n> \n> I have been staring at this code for longer than I want to admit, and I\n> still haven't convinced myself that this is not a change in behaviour.\n> I think what I'm wondering about is what `is_current_worktree()` is\n> actually intended to do. In other words, what _do_ we consider to be\n> \"current\"?\n\nThere was some discussion about that in [1] where reviewers were \nsurprised that we needed to call is_current_worktree() here. This change \nis consistent with the rest of the patch but you're right that it is a \nchange in behavior as at the moment the \"current\" worktree is set by the \nworktree that the process was started in. In practice we only ever have \na single struct repository instance per process so there is no practical \nchange in behavior here. At the moment, if you visit a set of submodules \nfrom the superproject by forking one process per submodule each \nsubmodule worktree is considered \"current\", but if you visit them by \nspinning up some threads in the current process they are not considered \n\"current\". That seems to me to be inconsistent as the process started by \nthe user is in the superproject in both cases.\n\nThe \"is_current\" field was added in [1] without any discussion in the \ncommit message. It seems to have been added to stop the same branch \nbeing checked out in multiple worktrees [2].\n\nThanks\n\nPhillip\n\n[1] 750e8a60d69 (worktree.c: mark current worktree, 2016-04-22)\n[2] \nhttps://lore.kernel.org/git/CAJZYdzhG8h3s=Ep1fuGbam1cWhYkv0tW6tQ7pBGGj+fj6=Nrsw@mail.gmail.com/\n\n\n> I would consider the worktree \"current\" that the Git process\n> has been invoked in. So if I pass a repo other than `the_repository`, or\n> if I pass a worktree that is not the one that Git has been started in,\n> then I would expect the function to return `false`. With that naive\n> assumption your change would be breaking the existing logic.\n> \n> But I have no idea whether my assumption is correct or not, as\n> there is not really any documentation of what the function or of the\n> `struct worktree::is_current` field. And having a look at a couple of\n> callers doesn't really make me all the wiser.\n> \n> It would be great if you could shine some light on this and then also\n> add a bit of documentation to either the function, the field, or both :)\n> \n> Thanks!\n> \n> Patrick\n> \n\n"},{"id":"539211","messageId":"9c915043-02da-4823-b4e7-d2a340c0373d@gmail.com","threadId":"65236","inReplyTo":"9e3b59d1-58f3-476c-9c7a-3ceffbb71810@gmail.com","subject":"Re: [PATCH v2 1/3] worktree: remove \"the_repository\" from is_current_worktree()","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-03-17T10:24:36Z","receivedAt":"2026-03-17T10:24:39Z","isPatch":true,"body":"On 16/03/2026 16:22, Phillip Wood wrote:\n> \n>> I have been staring at this code for longer than I want to admit, and I\n>> still haven't convinced myself that this is not a change in behaviour.\n>> I think what I'm wondering about is what `is_current_worktree()` is\n>> actually intended to do. In other words, what _do_ we consider to be\n>> \"current\"?\n> \n> There was some discussion about that in [1] where reviewers were \n> surprised that we needed to call is_current_worktree() here. This change \n> is consistent with the rest of the patch but you're right that it is a \n> change in behavior as at the moment the \"current\" worktree is set by the \n> worktree that the process was started in. In practice we only ever have \n> a single struct repository instance per process so there is no practical \n> change in behavior here. At the moment, if you visit a set of submodules \n> from the superproject by forking one process per submodule each \n> submodule worktree is considered \"current\", but if you visit them by \n> spinning up some threads in the current process they are not considered \n> \"current\". That seems to me to be inconsistent as the process started by \n> the user is in the superproject in both cases.\n\nTo add another example get_worktree_ref_store() checks `wt->is_current` \nto see if it should use the ref store in `wt->repo` or create a new \nstore. That means that if we use `the_repository` to define the current \nworktree we'll end up opening a copy of the ref store in `wt->repo` when \n`repo != the_repository` and `wt->path` matches `wt->repo->worktree` \nwhen we could be using the ref store that's already open. It is a bit \nlike the bug fixed by 1339cb3c47a (worktree: don't store main worktree \ntwice, 2024-06-06) but for multiple repositories.\n\nI'll re-roll with a bit more description in the commit message but I'm \ngoing to be off the list for most of the rest of this week so it will \nprobably be next week before I post a new version.\n\nThanks\n\nPhillip\n\n> The \"is_current\" field was added in [1] without any discussion in the \n> commit message. It seems to have been added to stop the same branch \n> being checked out in multiple worktrees [2].\n> \n> Thanks\n> \n> Phillip\n> \n> [1] 750e8a60d69 (worktree.c: mark current worktree, 2016-04-22)\n> [2] https://lore.kernel.org/git/ \n> CAJZYdzhG8h3s=Ep1fuGbam1cWhYkv0tW6tQ7pBGGj+fj6=Nrsw@mail.gmail.com/\n> \n> \n>> I would consider the worktree \"current\" that the Git process\n>> has been invoked in. So if I pass a repo other than `the_repository`, or\n>> if I pass a worktree that is not the one that Git has been started in,\n>> then I would expect the function to return `false`. With that naive\n>> assumption your change would be breaking the existing logic.\n>>\n>> But I have no idea whether my assumption is correct or not, as\n>> there is not really any documentation of what the function or of the\n>> `struct worktree::is_current` field. And having a look at a couple of\n>> callers doesn't really make me all the wiser.\n>>\n>> It would be great if you could shine some light on this and then also\n>> add a bit of documentation to either the function, the field, or both :)\n>>\n>> Thanks!\n>>\n>> Patrick\n>>\n> \n\n"},{"id":"539720","messageId":"20260323094341.880375-1-shreyanshpaliwalcmsmn@gmail.com","threadId":"65236","inReplyTo":"9c915043-02da-4823-b4e7-d2a340c0373d@gmail.com","subject":"Re: [PATCH v2 1/3] worktree: remove \"the_repository\" from is_current_worktree()","fromName":"Shreyansh Paliwal","fromEmail":"shreyanshpaliwalcmsmn@gmail.com","sentAt":"2026-03-23T09:41:38Z","receivedAt":"2026-03-23T09:43:53Z","isPatch":true,"body":"> On 16/03/2026 16:22, Phillip Wood wrote:\n> >\n> >> I have been staring at this code for longer than I want to admit, and I\n> >> still haven't convinced myself that this is not a change in behaviour.\n> >> I think what I'm wondering about is what `is_current_worktree()` is\n> >> actually intended to do. In other words, what _do_ we consider to be\n> >> \"current\"?\n> >\n> > There was some discussion about that in [1] where reviewers were\n> > surprised that we needed to call is_current_worktree() here. This change\n> > is consistent with the rest of the patch but you're right that it is a\n> > change in behavior as at the moment the \"current\" worktree is set by the\n> > worktree that the process was started in. In practice we only ever have\n> > a single struct repository instance per process so there is no practical\n> > change in behavior here. At the moment, if you visit a set of submodules\n> > from the superproject by forking one process per submodule each\n> > submodule worktree is considered \"current\", but if you visit them by\n> > spinning up some threads in the current process they are not considered\n> > \"current\". That seems to me to be inconsistent as the process started by\n> > the user is in the superproject in both cases.\n>\n> To add another example get_worktree_ref_store() checks `wt->is_current`\n> to see if it should use the ref store in `wt->repo` or create a new\n> store. That means that if we use `the_repository` to define the current\n> worktree we'll end up opening a copy of the ref store in `wt->repo` when\n> `repo != the_repository` and `wt->path` matches `wt->repo->worktree`\n> when we could be using the ref store that's already open. It is a bit\n> like the bug fixed by 1339cb3c47a (worktree: don't store main worktree\n> twice, 2024-06-06) but for multiple repositories.\n>\n> I'll re-roll with a bit more description in the commit message but I'm\n> going to be off the list for most of the rest of this week so it will\n> probably be next week before I post a new version.\n>\n> Thanks\n>\n> Phillip\n>\n> > The \"is_current\" field was added in [1] without any discussion in the\n> > commit message. It seems to have been added to stop the same branch\n> > being checked out in multiple worktrees [2].\n> >\n> > Thanks\n> >\n> > Phillip\n> >\n> > [1] 750e8a60d69 (worktree.c: mark current worktree, 2016-04-22)\n> > [2] https://lore.kernel.org/git/\n> > CAJZYdzhG8h3s=Ep1fuGbam1cWhYkv0tW6tQ7pBGGj+fj6=Nrsw@mail.gmail.com/\n> >\n> >\n> >> I would consider the worktree \"current\" that the Git process\n> >> has been invoked in. So if I pass a repo other than `the_repository`, or\n> >> if I pass a worktree that is not the one that Git has been started in,\n> >> then I would expect the function to return `false`. With that naive\n> >> assumption your change would be breaking the existing logic.\n> >>\n> >> But I have no idea whether my assumption is correct or not, as\n> >> there is not really any documentation of what the function or of the\n> >> `struct worktree::is_current` field. And having a look at a couple of\n> >> callers doesn't really make me all the wiser.\n> >>\n> >> It would be great if you could shine some light on this and then also\n> >> add a bit of documentation to either the function, the field, or both :)\n> >>\n> >> Thanks!\n> >>\n> >> Patrick\n> >>\n> >\n\nHi Phillip,\n\nThis may be slightly out of scope for this series. My understanding so far\nhas been that originally wt == NULL is used to represent the 'current worktree',\nwhich eventually meant following the process-wide state (the_repository).\nWith the ongoing multi-repository work, the meaning is being changed to be\ninterpreted as 'the worktree associated with the repository that we are working in'.\nHowever, in path.c there are some callers of repo_git_pathv() passing wt as 'NULL',\nI know that there is not involvement of the_repository state but it would be create\nless confusion if the semantics of worktrees are same everywhere. So if we replace\nthose NULL callers with the current worktree and update the checks of (!wt) to\n(is_current_worktree(wt)), some tests are failing mostly related to refs of linked\nworktrees, and I think the error is originating from this,\n\n if (!wt)\n adjust_git_path(repo, buf, gitdir_len);\n\nSo I am a bit confused to whether wt being NULL here could mean something else\nbehaviour wise ?\n\nBest,\nShreyansh\n"},{"id":"539733","messageId":"532616a4-d410-4a38-8038-1fd22e39217f@gmail.com","threadId":"65236","inReplyTo":"20260323094341.880375-1-shreyanshpaliwalcmsmn@gmail.com","subject":"Re: [PATCH v2 1/3] worktree: remove \"the_repository\" from is_current_worktree()","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-03-23T14:37:51Z","receivedAt":"2026-03-23T14:37:55Z","isPatch":true,"body":"On 23/03/2026 09:41, Shreyansh Paliwal wrote:\n> \n> This may be slightly out of scope for this series. My understanding so far\n> has been that originally wt == NULL is used to represent the 'current worktree',\n> which eventually meant following the process-wide state (the_repository).\n> With the ongoing multi-repository work, the meaning is being changed to be\n> interpreted as 'the worktree associated with the repository that we are working in'.\n> However, in path.c there are some callers of repo_git_pathv() passing wt as 'NULL',\n> I know that there is not involvement of the_repository state but it would be create\n> less confusion if the semantics of worktrees are same everywhere. So if we replace\n> those NULL callers with the current worktree and update the checks of (!wt) to\n> (is_current_worktree(wt)), some tests are failing mostly related to refs of linked\n> worktrees, and I think the error is originating from this,\n> \n> if (!wt)\n> adjust_git_path(repo, buf, gitdir_len);\n> \n> So I am a bit confused to whether wt being NULL here could mean something else\n> behaviour wise ?\n\nThat line comes from 543107333b3 (path: worktree_git_path() should not \nuse file relocation, 2017-06-22) which explains why we don't adjust the \npatch when wt is non-NULL. worktree_git_path() is called from \nbuiltin/fsck.c in a loop over all worktrees so changing 'if (!wt)' 'if \n(!wt->is_current)' will change the behavior for the current worktree. \nWhile it might be nice to clean this up in the future, it is an internal \nhelper function so I'm less worried it than if it were a public function.\n\nThanks\n\nPhillip\n"},{"id":"539770","messageId":"20260323170650.938396-1-shreyanshpaliwalcmsmn@gmail.com","threadId":"65236","inReplyTo":"532616a4-d410-4a38-8038-1fd22e39217f@gmail.com","subject":"Re: [PATCH v2 1/3] worktree: remove \"the_repository\" from is_current_worktree()","fromName":"Shreyansh Paliwal","fromEmail":"shreyanshpaliwalcmsmn@gmail.com","sentAt":"2026-03-23T17:05:32Z","receivedAt":"2026-03-23T17:07:45Z","isPatch":true,"body":"> On 23/03/2026 09:41, Shreyansh Paliwal wrote:\n> >\n> > This may be slightly out of scope for this series. My understanding so far\n> > has been that originally wt == NULL is used to represent the 'current worktree',\n> > which eventually meant following the process-wide state (the_repository).\n> > With the ongoing multi-repository work, the meaning is being changed to be\n> > interpreted as 'the worktree associated with the repository that we are working in'.\n> > However, in path.c there are some callers of repo_git_pathv() passing wt as 'NULL',\n> > I know that there is not involvement of the_repository state but it would be create\n> > less confusion if the semantics of worktrees are same everywhere. So if we replace\n> > those NULL callers with the current worktree and update the checks of (!wt) to\n> > (is_current_worktree(wt)), some tests are failing mostly related to refs of linked\n> > worktrees, and I think the error is originating from this,\n> >\n> > if (!wt)\n> > adjust_git_path(repo, buf, gitdir_len);\n> >\n> > So I am a bit confused to whether wt being NULL here could mean something else\n> > behaviour wise ?\n>\n> That line comes from 543107333b3 (path: worktree_git_path() should not\n> use file relocation, 2017-06-22) which explains why we don't adjust the\n> patch when wt is non-NULL. worktree_git_path() is called from\n> builtin/fsck.c in a loop over all worktrees so changing 'if (!wt)' 'if\n> (!wt->is_current)' will change the behavior for the current worktree.\n> While it might be nice to clean this up in the future, it is an internal\n> helper function so I'm less worried it than if it were a public function.\n\nHmm, I see. That would end up calling adjust_git_path() for the current worktree\nin that list of worktrees, which wasn’t happening before, that makes sense.\nThanks.\n"},{"id":"540078","messageId":"cover.1774534617.git.phillip.wood@dunelm.org.uk","threadId":"65236","inReplyTo":"cover.1773411586.git.phillip.wood@dunelm.org.uk","subject":"[PATCH v3 0/3] worktree: stop using \"the_repository\" in is_current_worktree()","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-03-26T14:16:56Z","receivedAt":"2026-03-26T14:17:28Z","isPatch":true,"body":"From: Phillip Wood \n\nThis is a follow up to pw/no-more-NULL-means-current-worktree that removes\n\"the_repository\" from is_current_worktree() and get_worktree_git_dir().\nThe first patch removes the use of \"the_repository\" when determining\nif a worktree is current. Patches 2 & 3 require a non-NULL worktree\nwhen calling get_worktree_git_dir() to remove the last use of\n\"the_repository\" in that function.\n\nChanges since V2\n\n - Patch 1: expanded commit message and added a comment to is_current\n member of struct worktree. (thanks to Patrick)\n\nChanges since V1\n\n - Patch 2: fixed indentation (thanks to Junio)\n - Patch 3: removed stale comment (thanks to Junio)\n\nBase-Commit: 7f19e4e1b6a3ad259e2ed66033e01e03b8b74c5e\nPublished-As: https://github.com/phillipwood/git/releases/tag/pw%2Fworktree-is-current-use-repo%2Fv3\nView-Changes-At: https://github.com/phillipwood/git/compare/7f19e4e1b...c33290280\nFetch-It-Via: git fetch https://github.com/phillipwood/git pw/worktree-is-current-use-repo/v3\n\n\nPhillip Wood (3):\n worktree: remove \"the_repository\" from is_current_worktree()\n worktree add: stop reading \".git/HEAD\"\n worktree: reject NULL worktree in get_worktree_git_dir()\n\n builtin/worktree.c | 21 ++-------------------\n t/t2400-worktree-add.sh | 28 ++++++++++++----------------\n worktree.c | 10 +++++-----\n worktree.h | 3 +--\n 4 files changed, 20 insertions(+), 42 deletions(-)\n\nRange-diff against v2:\n1: 075700a2256 ! 1: 5357c0dd53e worktree: remove \"the_repository\" from is_current_worktree()\n @@ Metadata\n ## Commit message ##\n worktree: remove \"the_repository\" from is_current_worktree()\n \n - is_current_worktree() compares the gitdir of the worktree to the gitdir\n - of \"the_repository\" and returns true when they match. To get the gitdir\n - of the worktree it calls get_workree_git_dir() which also depends on\n - \"the_repository\". This has the effect that even if \"wt->path\" matches\n + The \"is_current\" member of struct worktree was added in 750e8a60d69\n + (worktree.c: mark current worktree, 2016-04-22) and was used in\n + 8d9fdd7087d (worktree.c: check whether branch is rebased in another\n + worktree, 2016-04-22) to optionally skip the current worktree when\n + seeing if a branch is already checked out in die_if_checked_out().\n +\n + To determine if a worktree is \"current\" is_current_worktree() compares\n + the gitdir of the worktree to the gitdir of \"the_repository\"\n + and returns true when they match. To get the gitdir of the\n + worktree it calls get_workree_git_dir() which also depends on\n + \"the_repository\". This means that even if \"wt->path\" matches\n \"wt->repo->worktree\" is_current_worktree(wt) will return false when\n - \"wt->repo\" is not \"the_repository\" which is confusing.\n + \"wt->repo\" is not \"the_repository\". Consequently die_if_checked_out()\n + will fail to skip such a worktree when checking if a branch is already\n + checked out and may die errounously. Fix this by using the worktree's\n + repository instance instead of \"the_repository\" when comparing gitdirs.\n \n 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()`\n - in favor of `repo_common_path()`, 2025-02-07). In both cases we have\n - a repository instance available so use that instead. This means\n - that a worktree \"wt\" is always considered current when \"wt->path\"\n - matches \"wt->repo->worktree\" and so the worktree returned by\n - get_worktree_from_repository() is always considered current.\n + in favor of `repo_common_path()`, 2025-02-07). In both cases the\n + replacements appear to have been mechanical.\n \n Signed-off-by: Phillip Wood \n \n @@ worktree.c: char *get_worktree_git_dir(const struct worktree *wt)\n }\n \n static struct worktree *find_worktree_by_suffix(struct worktree **list,\n +\n + ## worktree.h ##\n +@@ worktree.h: struct worktree {\n + \tstruct object_id head_oid;\n + \tint is_detached;\n + \tint is_bare;\n +-\tint is_current;\n ++\tint is_current;\t\t/* does `path` match `repo->worktree` */\n + \tint lock_reason_valid; /* private */\n + \tint prune_reason_valid; /* private */\n + };\n2: c3c5767725d = 2: 4d50e6bcb2e worktree add: stop reading \".git/HEAD\"\n3: 75eecc8492e = 3: c3329028010 worktree: reject NULL worktree in get_worktree_git_dir()\n-- \n2.52.0.362.g884e03848a9.dirty\n\n"},{"id":"540079","messageId":"5357c0dd53ee123a4ea064412c83983b0be5e400.1774534617.git.phillip.wood@dunelm.org.uk","threadId":"65236","inReplyTo":"cover.1774534617.git.phillip.wood@dunelm.org.uk","subject":"[PATCH v3 1/3] worktree: remove \"the_repository\" from is_current_worktree()","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-03-26T14:16:57Z","receivedAt":"2026-03-26T14:17:29Z","isPatch":true,"body":"From: Phillip Wood \n\nThe \"is_current\" member of struct worktree was added in 750e8a60d69\n(worktree.c: mark current worktree, 2016-04-22) and was used in\n8d9fdd7087d (worktree.c: check whether branch is rebased in another\nworktree, 2016-04-22) to optionally skip the current worktree when\nseeing if a branch is already checked out in die_if_checked_out().\n\nTo determine if a worktree is \"current\" is_current_worktree() compares\nthe gitdir of the worktree to the gitdir of \"the_repository\"\nand returns true when they match. To get the gitdir of the\nworktree it calls get_workree_git_dir() which also depends on\n\"the_repository\". This means that even if \"wt->path\" matches\n\"wt->repo->worktree\" is_current_worktree(wt) will return false when\n\"wt->repo\" is not \"the_repository\". Consequently die_if_checked_out()\nwill fail to skip such a worktree when checking if a branch is already\nchecked out and may die errounously. Fix this by using the worktree's\nrepository instance instead of \"the_repository\" when comparing gitdirs.\n\nThe 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()`\nin favor of `repo_common_path()`, 2025-02-07). In both cases the\nreplacements appear to have been mechanical.\n\nSigned-off-by: Phillip Wood \n---\n worktree.c | 8 ++++----\n worktree.h | 2 +-\n 2 files changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/worktree.c b/worktree.c\nindex e9ff6e6ef2e..344ad0c031b 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -58,7 +58,7 @@ static void add_head_info(struct worktree *wt)\n \n static int is_current_worktree(struct worktree *wt)\n {\n-\tchar *git_dir = absolute_pathdup(repo_get_git_dir(the_repository));\n+\tchar *git_dir = absolute_pathdup(repo_get_git_dir(wt->repo));\n \tchar *wt_git_dir = get_worktree_git_dir(wt);\n \tint is_current = !fspathcmp(git_dir, absolute_path(wt_git_dir));\n \tfree(wt_git_dir);\n@@ -78,7 +78,7 @@ struct worktree *get_worktree_from_repository(struct repository *repo)\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+\twt->is_current = true;\n \tadd_head_info(wt);\n \n \tfree(gitdir);\n@@ -229,9 +229,9 @@ char *get_worktree_git_dir(const struct worktree *wt)\n \tif (!wt)\n \t\treturn xstrdup(repo_get_git_dir(the_repository));\n \telse if (!wt->id)\n-\t\treturn xstrdup(repo_get_common_dir(the_repository));\n+\t\treturn xstrdup(repo_get_common_dir(wt->repo));\n \telse\n-\t\treturn repo_common_path(the_repository, \"worktrees/%s\", wt->id);\n+\t\treturn repo_common_path(wt->repo, \"worktrees/%s\", wt->id);\n }\n \n static struct worktree *find_worktree_by_suffix(struct worktree **list,\ndiff --git a/worktree.h b/worktree.h\nindex e450d1a3317..94ae58db973 100644\n--- a/worktree.h\n+++ b/worktree.h\n@@ -16,7 +16,7 @@ struct worktree {\n \tstruct object_id head_oid;\n \tint is_detached;\n \tint is_bare;\n-\tint is_current;\n+\tint is_current;\t\t/* does `path` match `repo->worktree` */\n \tint lock_reason_valid; /* private */\n \tint prune_reason_valid; /* private */\n };\n-- \n2.52.0.362.g884e03848a9.dirty\n\n"},{"id":"540080","messageId":"4d50e6bcb2e5899f9f14b878bb9cca1c12cc5119.1774534617.git.phillip.wood@dunelm.org.uk","threadId":"65236","inReplyTo":"cover.1774534617.git.phillip.wood@dunelm.org.uk","subject":"[PATCH v3 2/3] worktree add: stop reading \".git/HEAD\"","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-03-26T14:16:58Z","receivedAt":"2026-03-26T14:17:30Z","isPatch":true,"body":"From: Phillip Wood \n\nThe function can_use_local_refs() prints a warning if there are no local\nbranches and HEAD is invalid or points to an unborn branch. As part of\nthe warning it prints the contents of \".git/HEAD\". In a repository using\nthe reftable backend HEAD is not stored in the filesystem so reading\nthat file is pointless. In a repository using the files backend it is\nunclear how useful printing it is - it would be better to diagnose the\nproblem for the user. For now, simplify the warning by not printing\nthe file contents and adjust the relevant test case accordingly. Also\nfixup the test case to use test_grep so that anyone trying to debug a\ntest failure in the future is not met by a wall of silence.\n\nSigned-off-by: Phillip Wood \n---\n builtin/worktree.c | 21 ++-------------------\n t/t2400-worktree-add.sh | 28 ++++++++++++----------------\n 2 files changed, 14 insertions(+), 35 deletions(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex bc2d0d645ba..9170b2e8981 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -692,25 +692,8 @@ static int can_use_local_refs(const struct add_opts *opts)\n \tif (refs_head_ref(get_main_ref_store(the_repository), first_valid_ref, NULL)) {\n \t\treturn 1;\n \t} else if (refs_for_each_branch_ref(get_main_ref_store(the_repository), first_valid_ref, NULL)) {\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-\n-\t\t\tstrbuf_add_real_path(&path, wt_gitdir);\n-\t\t\tstrbuf_addstr(&path, \"/HEAD\");\n-\t\t\tstrbuf_read_file(&contents, path.buf, 64);\n-\t\t\tstrbuf_stripspace(&contents, NULL);\n-\t\t\tstrbuf_strip_suffix(&contents, \"\\n\");\n-\n-\t\t\twarning(_(\"HEAD points to an invalid (or orphaned) reference.\\n\"\n-\t\t\t\t \"HEAD path: '%s'\\n\"\n-\t\t\t\t \"HEAD contents: '%s'\"),\n-\t\t\t\t path.buf, contents.buf);\n-\t\t\tstrbuf_release(&path);\n-\t\t\tstrbuf_release(&contents);\n-\t\t\tfree(wt_gitdir);\n-\t\t}\n+\t\tif (!opts->quiet)\n+\t\t\twarning(_(\"HEAD points to an invalid (or orphaned) reference.\\n\"));\n \t\treturn 1;\n \t}\n \treturn 0;\ndiff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh\nindex 023e1301c8e..58b4445cc44 100755\n--- a/t/t2400-worktree-add.sh\n+++ b/t/t2400-worktree-add.sh\n@@ -987,7 +987,7 @@ test_dwim_orphan () {\n \t\t\t\tthen\n \t\t\t\t\ttest_must_be_empty actual\n \t\t\t\telse\n-\t\t\t\t\tgrep \"$info_text\" actual\n+\t\t\t\t\ttest_grep \"$info_text\" actual\n \t\t\t\tfi\n \t\t\telif [ \"$outcome\" = \"no_infer\" ]\n \t\t\tthen\n@@ -996,39 +996,35 @@ test_dwim_orphan () {\n \t\t\t\tthen\n \t\t\t\t\ttest_must_be_empty actual\n \t\t\t\telse\n-\t\t\t\t\t! grep \"$info_text\" actual\n+\t\t\t\t\ttest_grep ! \"$info_text\" actual\n \t\t\t\tfi\n \t\t\telif [ \"$outcome\" = \"fetch_error\" ]\n \t\t\tthen\n \t\t\t\ttest_must_fail git $dashc_args worktree add $args 2>actual &&\n-\t\t\t\tgrep \"$fetch_error_text\" actual\n+\t\t\t\ttest_grep \"$fetch_error_text\" actual\n \t\t\telif [ \"$outcome\" = \"fatal_orphan_bad_combo\" ]\n \t\t\tthen\n \t\t\t\ttest_must_fail git $dashc_args worktree add $args 2>actual &&\n \t\t\t\tif [ $use_quiet -eq 1 ]\n \t\t\t\tthen\n-\t\t\t\t\t! grep \"$info_text\" actual\n+\t\t\t\t\ttest_grep ! \"$info_text\" actual\n \t\t\t\telse\n-\t\t\t\t\tgrep \"$info_text\" actual\n+\t\t\t\t\ttest_grep \"$info_text\" actual\n \t\t\t\tfi &&\n-\t\t\t\tgrep \"$bad_combo_regex\" actual\n+\t\t\t\ttest_grep \"$bad_combo_regex\" actual\n \t\t\telif [ \"$outcome\" = \"warn_bad_head\" ]\n \t\t\tthen\n \t\t\t\ttest_must_fail git $dashc_args worktree add $args 2>actual &&\n \t\t\t\tif [ $use_quiet -eq 1 ]\n \t\t\t\tthen\n-\t\t\t\t\tgrep \"$invalid_ref_regex\" actual &&\n-\t\t\t\t\t! grep \"$orphan_hint\" actual\n+\t\t\t\t\ttest_grep \"$invalid_ref_regex\" actual &&\n+\t\t\t\t\ttest_grep ! \"$orphan_hint\" actual\n \t\t\t\telse\n-\t\t\t\t\theadpath=$(git $dashc_args rev-parse --path-format=absolute --git-path HEAD) &&\n-\t\t\t\t\theadcontents=$(cat \"$headpath\") &&\n-\t\t\t\t\tgrep \"HEAD points to an invalid (or orphaned) reference\" actual &&\n-\t\t\t\t\tgrep \"HEAD path: .$headpath.\" actual &&\n-\t\t\t\t\tgrep \"HEAD contents: .$headcontents.\" actual &&\n-\t\t\t\t\tgrep \"$orphan_hint\" actual &&\n-\t\t\t\t\t! grep \"$info_text\" actual\n+\t\t\t\t\ttest_grep \"HEAD points to an invalid (or orphaned) reference\" actual &&\n+\t\t\t\t\ttest_grep \"$orphan_hint\" actual &&\n+\t\t\t\t\ttest_grep ! \"$info_text\" actual\n \t\t\t\tfi &&\n-\t\t\t\tgrep \"$invalid_ref_regex\" actual\n+\t\t\t\ttest_grep \"$invalid_ref_regex\" actual\n \t\t\telse\n \t\t\t\t# Unreachable\n \t\t\t\tfalse\n-- \n2.52.0.362.g884e03848a9.dirty\n\n"},{"id":"540081","messageId":"c3329028010269995008d92653ba6dc4a5322118.1774534617.git.phillip.wood@dunelm.org.uk","threadId":"65236","inReplyTo":"cover.1774534617.git.phillip.wood@dunelm.org.uk","subject":"[PATCH v3 3/3] worktree: reject NULL worktree in get_worktree_git_dir()","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-03-26T14:16:59Z","receivedAt":"2026-03-26T14:17:32Z","isPatch":true,"body":"From: Phillip Wood \n\nThis removes the final dependence on \"the_repository\" in\nget_worktree_git_dir(). The last commit removed only caller that\npassed a NULL worktree.\n\nget_worktree_git_dir() has the following callers:\n\n - branch.c:prepare_checked_out_branches() which loops over all\n worktrees.\n\n - builtin/fsck.c:cmd_fsck() which loops over all worktrees.\n\n - builtin/receive-pack.c:update_worktree() which is called from\n update() only when \"worktree\" is non-NULL.\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 - reachable.c:add_rebase_files() which loops over all worktrees.\n\n - revision.c:add_index_objects_to_pending() which loops over all\n worktrees.\n\n - worktree.c:is_current_worktree() which expects a non-NULL worktree.\n\nSigned-off-by: Phillip Wood \n---\n worktree.c | 2 +-\n worktree.h | 1 -\n 2 files changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/worktree.c b/worktree.c\nindex 344ad0c031b..1ed5e8c3cd2 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -227,7 +227,7 @@ struct worktree **get_worktrees_without_reading_head(void)\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+\t\tBUG(\"%s() called with NULL worktree\", __func__);\n \telse if (!wt->id)\n \t\treturn xstrdup(repo_get_common_dir(wt->repo));\n \telse\ndiff --git a/worktree.h b/worktree.h\nindex 94ae58db973..400b614f133 100644\n--- a/worktree.h\n+++ b/worktree.h\n@@ -51,7 +51,6 @@ int submodule_uses_worktrees(const char *path);\n \n /*\n * Return git dir of the worktree. Note that the path may be relative.\n- * If wt is NULL, git dir of current worktree is returned.\n */\n char *get_worktree_git_dir(const struct worktree *wt);\n \n-- \n2.52.0.362.g884e03848a9.dirty\n\n"},{"id":"540100","messageId":"xmqqy0jer3qu.fsf@gitster.g","threadId":"65236","inReplyTo":"5357c0dd53ee123a4ea064412c83983b0be5e400.1774534617.git.phillip.wood@dunelm.org.uk","subject":"Re: [PATCH v3 1/3] worktree: remove \"the_repository\" from is_current_worktree()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-26T15:48:25Z","receivedAt":"2026-03-26T15:48:28Z","isPatch":true,"body":"Phillip Wood writes:\n\n> ... it calls get_workree_git_dir() which also depends on\n> \"the_repository\". This means that even if \"wt->path\" matches\n> \"wt->repo->worktree\" is_current_worktree(wt) will return false when\n> \"wt->repo\" is not \"the_repository\". Consequently die_if_checked_out()\n> will fail to skip such a worktree when checking if a branch is already\n> checked out and may die errounously. Fix this by using the worktree's\n> repository instance instead of \"the_repository\" when comparing gitdirs.\n\nIt sounds like the above is something we can write in a test to make\nsure the code with this fix won't regress in the future. Can we add\none?\n\n> 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()`\n> in favor of `repo_common_path()`, 2025-02-07). In both cases the\n> replacements appear to have been mechanical.\n>\n> Signed-off-by: Phillip Wood \n> ---\n> worktree.c | 8 ++++----\n> worktree.h | 2 +-\n> 2 files changed, 5 insertions(+), 5 deletions(-)\n>\n> diff --git a/worktree.c b/worktree.c\n> index e9ff6e6ef2e..344ad0c031b 100644\n> --- a/worktree.c\n> +++ b/worktree.c\n> @@ -58,7 +58,7 @@ static void add_head_info(struct worktree *wt)\n> \n> static int is_current_worktree(struct worktree *wt)\n> {\n> -\tchar *git_dir = absolute_pathdup(repo_get_git_dir(the_repository));\n> +\tchar *git_dir = absolute_pathdup(repo_get_git_dir(wt->repo));\n> \tchar *wt_git_dir = get_worktree_git_dir(wt);\n> \tint is_current = !fspathcmp(git_dir, absolute_path(wt_git_dir));\n> \tfree(wt_git_dir);\n> @@ -78,7 +78,7 @@ struct worktree *get_worktree_from_repository(struct repository *repo)\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> +\twt->is_current = true;\n> \tadd_head_info(wt);\n> \n> \tfree(gitdir);\n\nI think I found the semantics of get_worktree_from_repository()\nunclear when we discussed a different patch series, so I may have\nasked the same question already, but what exactly does it mean to\n\"get worktree from repository\", i.e., this function does? A\nrepository can have one or more worktrees attached to it (that was\nthe whole point of introducing the worktree mechanism), so \"I have\nthis repository, give me the worktree for it\" is not a sensible\nrequest. The function's comment in talks only at the\nimplementation level \"construct a worktree struct from repo->gitdir\nand repo->worktree\" as if it is so obvious what the resulting\nworktree struct means at a higher layer's point of view, which does\nnot help, either.\n\nI am asking the above because I find this unconditional assignment\nof \"true\" utterly confusing. Is it because \"we created a brand new\nworktree structure that by definition is current out of a repository\nstructure, so as far as the caller is concerned by definition it is\nwhat it is currently working on\"?\n\nStepping back a bit, how do \"is_current_worktree(wt)\" and\n\"wt->is_current\" relate to each other? Here is what happens in the\nformer:\n\n static int is_current_worktree(struct worktree *wt)\n {\n char *git_dir = absolute_pathdup(repo_get_git_dir(wt->repo));\n char *wt_git_dir = get_worktree_git_dir(wt);\n int is_current = !fspathcmp(git_dir, absolute_path(wt_git_dir));\n free(wt_git_dir);\n free(git_dir);\n return is_current;\n }\n\nand given the definition of get_worktree_git_dir(), which gives\neither the \"common\" .git directory for the primary worktree, or the\nworktree specific .git directory for the secondaries, the is_current\nbeing computed here sounds more like \"is wt the primary worktree\namong the ones that are attached to the same repository?\"\n\nBut given that there are API functions with \"main\" in their names,\nlike is_main_worktree() and internal function get_main_worktree(),\nwhat I said apparently is not the intention of whoever built the\nworktree subsystem. I am still confused... X-<.\n\n> @@ -229,9 +229,9 @@ char *get_worktree_git_dir(const struct worktree *wt)\n> \tif (!wt)\n> \t\treturn xstrdup(repo_get_git_dir(the_repository));\n> \telse if (!wt->id)\n> -\t\treturn xstrdup(repo_get_common_dir(the_repository));\n> +\t\treturn xstrdup(repo_get_common_dir(wt->repo));\n> \telse\n> -\t\treturn repo_common_path(the_repository, \"worktrees/%s\", wt->id);\n> +\t\treturn repo_common_path(wt->repo, \"worktrees/%s\", wt->id);\n> }\n> \n> static struct worktree *find_worktree_by_suffix(struct worktree **list,\n> diff --git a/worktree.h b/worktree.h\n> index e450d1a3317..94ae58db973 100644\n> --- a/worktree.h\n> +++ b/worktree.h\n> @@ -16,7 +16,7 @@ struct worktree {\n> \tstruct object_id head_oid;\n> \tint is_detached;\n> \tint is_bare;\n> -\tint is_current;\n> +\tint is_current;\t\t/* does `path` match `repo->worktree` */\n\nThis new comment also talks only at the lowest implementation level.\nWhat significance does it have to the upper layer that calls into\nthe API if `path` matches or does not match `repo->worktree` is\ntotally unclear, at least to me.\n\n> \tint lock_reason_valid; /* private */\n> \tint prune_reason_valid; /* private */\n> };\n\nThanks.\n"},{"id":"540199","messageId":"317ed4f7-f88d-4415-bd25-b62b4a076728@gmail.com","threadId":"65236","inReplyTo":"xmqqy0jer3qu.fsf@gitster.g","subject":"Re: [PATCH v3 1/3] worktree: remove \"the_repository\" from is_current_worktree()","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-03-27T16:40:15Z","receivedAt":"2026-03-27T16:40:18Z","isPatch":true,"body":"On 26/03/2026 15:48, Junio C Hamano wrote:\n> Phillip Wood writes:\n> \n>> ... it calls get_workree_git_dir() which also depends on\n>> \"the_repository\". This means that even if \"wt->path\" matches\n>> \"wt->repo->worktree\" is_current_worktree(wt) will return false when\n>> \"wt->repo\" is not \"the_repository\". Consequently die_if_checked_out()\n>> will fail to skip such a worktree when checking if a branch is already\n>> checked out and may die errounously. Fix this by using the worktree's\n>> repository instance instead of \"the_repository\" when comparing gitdirs.\n> \n> It sounds like the above is something we can write in a test to make\n> sure the code with this fix won't regress in the future. Can we add\n> one?\n\nWe'd need to create a struct worktree instance with a repository \ninstance that is not \"the_repository\" - I'm not sure we can do that in a \nscript.\n\n>> -\twt->is_current = is_current_worktree(wt);\n>> +\twt->is_current = true;\n>> \tadd_head_info(wt);\n>> \n>> \tfree(gitdir);\n> \n> I think I found the semantics of get_worktree_from_repository()\n> unclear when we discussed a different patch series, so I may have\n> asked the same question already, but what exactly does it mean to\n> \"get worktree from repository\", i.e., this function does? A\n> repository can have one or more worktrees attached to it (that was\n> the whole point of introducing the worktree mechanism), so \"I have\n> this repository, give me the worktree for it\" is not a sensible\n> request.\n\nA repository can have more that one worktree but a \"struct repository\" \ninstance has \"gitdir\" and \"worktree\" members that point to a specific \nworktree within that repository. For example\n\n\trepo_get_oid(repo, \"HEAD\", &oid);\n\nreads \"HEAD\" from a specific worktree within the repository.\n\n> The function's comment in talks only at the\n> implementation level \"construct a worktree struct from repo->gitdir\n> and repo->worktree\" as if it is so obvious what the resulting\n> worktree struct means at a higher layer's point of view, which does\n> not help, either.\n\nThat comes from me thinking of a struct repository as referring to a \nspecific worktree - would calling it \n\"get_worktree_from_repository_instance\" be clearer?\n\n> I am asking the above because I find this unconditional assignment\n> of \"true\" utterly confusing. Is it because \"we created a brand new\n> worktree structure that by definition is current out of a repository\n> structure, so as far as the caller is concerned by definition it is\n> what it is currently working on\"?\n\nYes\n\n> Stepping back a bit, how do \"is_current_worktree(wt)\" and\n> \"wt->is_current\" relate to each other?\n\n\"wt->current\" is the cached return value of is_current_worktree() (see \nthe implementation of get_main_worktree() and get_linked_worktree())\n\n> Here is what happens in the\n> former:\n> \n> static int is_current_worktree(struct worktree *wt)\n> {\n> char *git_dir = absolute_pathdup(repo_get_git_dir(wt->repo));\n> char *wt_git_dir = get_worktree_git_dir(wt);\n> int is_current = !fspathcmp(git_dir, absolute_path(wt_git_dir));\n> free(wt_git_dir);\n> free(git_dir);\n> return is_current;\n> }\n> \n> and given the definition of get_worktree_git_dir(), which gives\n> either the \"common\" .git directory for the primary worktree, or the\n> worktree specific .git directory for the secondaries, the is_current\n> being computed here sounds more like \"is wt the primary worktree\n> among the ones that are attached to the same repository?\"\n\nNo, the primary worktree (which the code calls the \"main\" worktree) is \northogonal to the \"current\" worktree. The main worktree is the one whose \ngitdir is $GIT_COMMON_DIR i.e. the worktree created by \"git init\". The \n\"current\" worktree is the one whose gitdir is wt->repo->gitdir and may \nor may not be the \"main\" worktree.\n\n> But given that there are API functions with \"main\" in their names,\n> like is_main_worktree() and internal function get_main_worktree(),\n> what I said apparently is not the intention of whoever built the\n> worktree subsystem. I am still confused... X-<.\n> \n>> @@ -229,9 +229,9 @@ char *get_worktree_git_dir(const struct worktree *wt)\n>> \tif (!wt)\n>> \t\treturn xstrdup(repo_get_git_dir(the_repository));\n>> \telse if (!wt->id)\n>> -\t\treturn xstrdup(repo_get_common_dir(the_repository));\n>> +\t\treturn xstrdup(repo_get_common_dir(wt->repo));\n>> \telse\n>> -\t\treturn repo_common_path(the_repository, \"worktrees/%s\", wt->id);\n>> +\t\treturn repo_common_path(wt->repo, \"worktrees/%s\", wt->id);\n>> }\n>> \n>> static struct worktree *find_worktree_by_suffix(struct worktree **list,\n>> diff --git a/worktree.h b/worktree.h\n>> index e450d1a3317..94ae58db973 100644\n>> --- a/worktree.h\n>> +++ b/worktree.h\n>> @@ -16,7 +16,7 @@ struct worktree {\n>> \tstruct object_id head_oid;\n>> \tint is_detached;\n>> \tint is_bare;\n>> -\tint is_current;\n>> +\tint is_current;\t\t/* does `path` match `repo->worktree` */\n> \n> This new comment also talks only at the lowest implementation level.\n> What significance does it have to the upper layer that calls into\n> the API if `path` matches or does not match `repo->worktree` is\n> totally unclear, at least to me.\n\nDoes it help to think about how \"is_current\" is used by functions like \nget_worktree_ref_store()? For the current worktree it can use the \nrefstore from wt->repo, if it is not it needs to open the store for that \nworktree.\n\nI feel I'm struggling to explain this clearly - I find this whole \ndiscussion gets confusing because we have \"struct worktree\" and also a \n\"worktree\" member of \"struct repository\" which means a \"struct \nrepository\" instance is tied to a specific worktree within the \nrepository. If \"struct repository\" only had a \"commondir\" member and no \n\"gitdir\" or \"worktree\" members and we instead used \"sturct worktree\" to \nrefer to a specific worktree within a repository with functions like\n\n\tworktree_get_oid(wt, \"HEAD\", &oid);\n\ninstead of\n\n\trepo_get_oid(repo, \"HEAD\", &oid);\n\nit might be clearer but that would be a very big change.\n\nThanks\n\nPhillip\n\n>> \tint lock_reason_valid; /* private */\n>> \tint prune_reason_valid; /* private */\n>> };\n> \n> Thanks.\n\n"},{"id":"540206","messageId":"xmqq1ph5kxpx.fsf@gitster.g","threadId":"65236","inReplyTo":"317ed4f7-f88d-4415-bd25-b62b4a076728@gmail.com","subject":"Re: [PATCH v3 1/3] worktree: remove \"the_repository\" from is_current_worktree()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-27T17:07:22Z","receivedAt":"2026-03-27T17:07:25Z","isPatch":true,"body":"Phillip Wood writes:\n\n> On 26/03/2026 15:48, Junio C Hamano wrote:\n>> Phillip Wood writes:\n>> \n>>> ... it calls get_workree_git_dir() which also depends on\n>>> \"the_repository\". This means that even if \"wt->path\" matches\n>>> \"wt->repo->worktree\" is_current_worktree(wt) will return false when\n>>> \"wt->repo\" is not \"the_repository\". Consequently die_if_checked_out()\n>>> will fail to skip such a worktree when checking if a branch is already\n>>> checked out and may die errounously. Fix this by using the worktree's\n>>> repository instance instead of \"the_repository\" when comparing gitdirs.\n>> \n>> It sounds like the above is something we can write in a test to make\n>> sure the code with this fix won't regress in the future. Can we add\n>> one?\n>\n> We'd need to create a struct worktree instance with a repository \n> instance that is not \"the_repository\" - I'm not sure we can do that in a \n> script.\n\nAh, so this is more of \"futureproofing\" than \"fix\"? That is fine.\nThanks for clarifying.\n\n>>> -\twt->is_current = is_current_worktree(wt);\n>>> +\twt->is_current = true;\n>>> \tadd_head_info(wt);\n>>> \n>>> \tfree(gitdir);\n>> \n>> I think I found the semantics of get_worktree_from_repository()\n>> unclear when we discussed a different patch series, so I may have\n>> asked the same question already, but what exactly does it mean to\n>> \"get worktree from repository\", i.e., this function does? A\n>> repository can have one or more worktrees attached to it (that was\n>> the whole point of introducing the worktree mechanism), so \"I have\n>> this repository, give me the worktree for it\" is not a sensible\n>> request.\n>\n> A repository can have more that one worktree but a \"struct repository\" \n> instance has \"gitdir\" and \"worktree\" members that point to a specific \n> worktree within that repository. For example\n>\n> \trepo_get_oid(repo, \"HEAD\", &oid);\n>\n> reads \"HEAD\" from a specific worktree within the repository.\n\nAnd...?\n\nI _think_ what I am frustrated about is the lack of description on\n\"what it means to be the worktree among many that is pointed by via\nthe .worktree member in the repository struct\". Does it correspond to\nthe worktree being \"the current worktree the codepath is working on?\"\n\n>> The function's comment in talks only at the\n>> implementation level \"construct a worktree struct from repo->gitdir\n>> and repo->worktree\" as if it is so obvious what the resulting\n>> worktree struct means at a higher layer's point of view, which does\n>> not help, either.\n>\n> That comes from me thinking of a struct repository as referring to a \n> specific worktree - would calling it \n> \"get_worktree_from_repository_instance\" be clearer?\n\nIt does not change the descriptive value of the name in any\nmeaningful way, so let's not do that. If the answer to my \"what\nfrustrates me\" comment above is \"yeah, we are getting the current\nworktree\", then renaming the function to include \"current\" in its\nname would add descriptive value vastly, though.\n\n> I feel I'm struggling to explain this clearly - I find this whole \n> discussion gets confusing because we have \"struct worktree\" and also a \n> \"worktree\" member of \"struct repository\" which means a \"struct \n> repository\" instance is tied to a specific worktree within the \n> repository. If \"struct repository\" only had a \"commondir\" member and no \n> \"gitdir\" or \"worktree\" members and we instead used \"sturct worktree\" to \n> refer to a specific worktree within a repository with functions like\n>\n> \tworktree_get_oid(wt, \"HEAD\", &oid);\n>\n> instead of\n>\n> \trepo_get_oid(repo, \"HEAD\", &oid);\n>\n> it might be clearer but that would be a very big change.\n\nIn short, am I hearing the worktree subsystem is not conceptually\nclean and it would be a huge undertaking to clean it up?\n"},{"id":"540768","messageId":"eadbe64e-b43f-42a8-852d-1e0824f5a9be@gmail.com","threadId":"65236","inReplyTo":"xmqq1ph5kxpx.fsf@gitster.g","subject":"Re: [PATCH v3 1/3] worktree: remove \"the_repository\" from is_current_worktree()","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-04-02T15:10:24Z","receivedAt":"2026-04-02T15:10:27Z","isPatch":true,"body":"On 27/03/2026 17:07, Junio C Hamano wrote:\n> Phillip Wood writes> \n>> On 26/03/2026 15:48, Junio C Hamano wrote:\n> \n> I _think_ what I am frustrated about is the lack of description on\n> \"what it means to be the worktree among many that is pointed by via\n> the .worktree member in the repository struct\". Does it correspond to\n> the worktree being \"the current worktree the codepath is working on?\"\n\nYes\n\n>>> The function's comment in talks only at the\n>>> implementation level \"construct a worktree struct from repo->gitdir\n>>> and repo->worktree\" as if it is so obvious what the resulting\n>>> worktree struct means at a higher layer's point of view, which does\n>>> not help, either.\n>>\n>> That comes from me thinking of a struct repository as referring to a\n>> specific worktree - would calling it\n>> \"get_worktree_from_repository_instance\" be clearer?\n> \n> It does not change the descriptive value of the name in any\n> meaningful way, so let's not do that. If the answer to my \"what\n> frustrates me\" comment above is \"yeah, we are getting the current\n> worktree\", then renaming the function to include \"current\" in its\n> name would add descriptive value vastly, though.\n\nThat's a good idea, I'll send a patch to rename \nget_worktree_from_repository() to get_current_worktree()\n\n>> I feel I'm struggling to explain this clearly - I find this whole\n>> discussion gets confusing because we have \"struct worktree\" and also a\n>> \"worktree\" member of \"struct repository\" which means a \"struct\n>> repository\" instance is tied to a specific worktree within the\n>> repository. If \"struct repository\" only had a \"commondir\" member and no\n>> \"gitdir\" or \"worktree\" members and we instead used \"sturct worktree\" to\n>> refer to a specific worktree within a repository with functions like\n>>\n>> \tworktree_get_oid(wt, \"HEAD\", &oid);\n>>\n>> instead of\n>>\n>> \trepo_get_oid(repo, \"HEAD\", &oid);\n>>\n>> it might be clearer but that would be a very big change.\n> \n> In short, am I hearing the worktree subsystem is not conceptually\n> clean and it would be a huge undertaking to clean it up?\n\nYes, we mostly use \"struct repository\" to operate on the current \nworktree but sometimes we need a \"struct worktree\" instead.\n\nThanks\n\nPhillip\n\n"}]}