{"thread":{"id":"64934","subject":"[PATCH] [RFC][GSoC 2026] builtin/repo: avoid global state in get_layout_bare","startedAt":"2026-02-06T15:20:16Z","lastAt":"2026-02-06T18:03:17Z","messageCount":2,"participants":["Ayush Jha","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"535348","messageId":"20260206152002.1244-1-kumarayushjha123@gmail.com","threadId":"64934","inReplyTo":null,"subject":"[PATCH] [RFC][GSoC 2026] builtin/repo: avoid global state in get_layout_bare","fromName":"Ayush Jha","fromEmail":"kumarayushjha123@gmail.com","sentAt":"2026-02-06T15:20:02Z","receivedAt":"2026-02-06T15:20:16Z","isPatch":true,"sender":{"key":"kumarayushjha123@gmail.com","avatar":null},"body":"The get_layout_bare() function accepts a struct repository *repo\nargument but marks it UNUSED and instead relies on\nis_bare_repository(), which depends on global state.\n\nAs bareness is a per-repository property, this causes the function\nto always report the status of the global repository, even when a\nspecific repository instance is provided.\n\nThis change computes the bare status using the passed-in repository\ninstance (based on core.bare and the absence of a worktree),\nthereby removing the dependency on global state.\n\nThis patch is sent as an RFC to solicit feedback on whether using\nrepository-local state here is the preferred approach.\n\nSigned-off-by: Ayush Jha <kumarayushjha123@gmail.com>\n---\n builtin/repo.c | 7 +++++--\n 1 file changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/repo.c b/builtin/repo.c\nindex 0ea045abc1..b2619cc77c 100644\n--- a/builtin/repo.c\n+++ b/builtin/repo.c\n@@ -35,9 +35,12 @@ struct field {\n \tget_value_fn *get_value;\n };\n \n-static int get_layout_bare(struct repository *repo UNUSED, struct strbuf *buf)\n+static int get_layout_bare(struct repository *repo, struct strbuf *buf)\n {\n-\tstrbuf_addstr(buf, is_bare_repository() ? \"true\" : \"false\");\n+\tint is_bare_cfg = -1;\n+\trepo_config_get_bool(repo, \"core.bare\", &is_bare_cfg);\n+\n+\tstrbuf_addstr(buf, is_bare_cfg && !repo_get_work_tree(repo) ? \"true\" : \"false\");\n \treturn 0;\n }\n \n-- \n2.53.0.windows.1\n\n"},{"id":"535375","messageId":"xmqqfr7dg36k.fsf@gitster.g","threadId":"64934","inReplyTo":"20260206152002.1244-1-kumarayushjha123@gmail.com","subject":"Re: [PATCH] [RFC][GSoC 2026] builtin/repo: avoid global state in get_layout_bare","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-06T18:03:15Z","receivedAt":"2026-02-06T18:03:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ayush Jha <kumarayushjha123@gmail.com> writes:\n\n> The get_layout_bare() function accepts a struct repository *repo\n> argument but marks it UNUSED and instead relies on\n> is_bare_repository(), which depends on global state.\n>\n> As bareness is a per-repository property, this causes the function\n> to always report the status of the global repository, even when a\n> specific repository instance is provided.\n>\n> This change computes the bare status using the passed-in repository\n> instance (based on core.bare and the absence of a worktree),\n> thereby removing the dependency on global state.\n>\n> This patch is sent as an RFC to solicit feedback on whether using\n> repository-local state here is the preferred approach.\n>\n> Signed-off-by: Ayush Jha <kumarayushjha123@gmail.com>\n> ---\n\nI'll let others to suggest improvements above the three-dash line.\n\n> diff --git a/builtin/repo.c b/builtin/repo.c\n> index 0ea045abc1..b2619cc77c 100644\n> --- a/builtin/repo.c\n> +++ b/builtin/repo.c\n> @@ -35,9 +35,12 @@ struct field {\n>  \tget_value_fn *get_value;\n>  };\n>  \n> -static int get_layout_bare(struct repository *repo UNUSED, struct strbuf *buf)\n> +static int get_layout_bare(struct repository *repo, struct strbuf *buf)\n>  {\n> -\tstrbuf_addstr(buf, is_bare_repository() ? \"true\" : \"false\");\n> +\tint is_bare_cfg = -1;\n> +\trepo_config_get_bool(repo, \"core.bare\", &is_bare_cfg);\n> +\n> +\tstrbuf_addstr(buf, is_bare_cfg && !repo_get_work_tree(repo) ? \"true\" : \"false\");\n>  \treturn 0;\n>  }\n\n\nAs git.c:commands[] lists \"repo\" with RUN_SETUP, we know that in\ncmd_repo(), after passing parse_options(), repo is guaranteed to\nbe a non-NULL pointer that is the_repository.  So this new version\nof the function that assumes repo can be safely passed to\nrepo_config_get_bool() and repo_get_work_tree() would probably be\nOK.  I didn't dig to find out is_bare_repository() does exactly the\nsame thing as what the new code does when the_repository is given to\nthe \"repo\" parameter, though.\n\nHaving said all that.\n\nIn general, anything in builtin/soething.c is an implementation of\n\"git something\" command and cannot be reused outside the context of\nthat \"something\" command, so it is much less interesting to get rid\nof dependencies on the_repository from these files.  The design\nchoice to work with a single repository at a time is with the\n\"something\" command, and as long as that design choice stays, there\nisn't point in hiding the fact that we work only on the_repository.\n\nIf we were moving the logic implemented by the helper functions that\nare used by cmd_repo() function to a shared and reusable library-ish\npart of the system (perhaps in repo.c at the top-level), that is when\nit becomes much more interesting and releavnt to make sure that a\nfunction that takes \"repo\" as its parameter does work on that\nrepository and not the_repository.\n\nBut until then, ...\n\nThanks.\n\n"}]}