Re: [PATCH] [RFC][GSoC 2026] builtin/repo: avoid global state in get_layout_bare
From: Junio C Hamano <hidden>
Date: 2026-02-06 18:03:17
Ayush Jha [off-list ref] writes:
The get_layout_bare() function accepts a struct repository *repo argument but marks it UNUSED and instead relies on is_bare_repository(), which depends on global state. As bareness is a per-repository property, this causes the function to always report the status of the global repository, even when a specific repository instance is provided. This change computes the bare status using the passed-in repository instance (based on core.bare and the absence of a worktree), thereby removing the dependency on global state. This patch is sent as an RFC to solicit feedback on whether using repository-local state here is the preferred approach. Signed-off-by: Ayush Jha <redacted> ---
I'll let others to suggest improvements above the three-dash line.
quoted hunk
diff --git a/builtin/repo.c b/builtin/repo.c index 0ea045abc1..b2619cc77c 100644 --- a/builtin/repo.c +++ b/builtin/repo.c@@ -35,9 +35,12 @@ struct field { get_value_fn *get_value; }; -static int get_layout_bare(struct repository *repo UNUSED, struct strbuf *buf) +static int get_layout_bare(struct repository *repo, struct strbuf *buf) { - strbuf_addstr(buf, is_bare_repository() ? "true" : "false"); + int is_bare_cfg = -1; + repo_config_get_bool(repo, "core.bare", &is_bare_cfg); + + strbuf_addstr(buf, is_bare_cfg && !repo_get_work_tree(repo) ? "true" : "false"); return 0; }
As git.c:commands[] lists "repo" with RUN_SETUP, we know that in cmd_repo(), after passing parse_options(), repo is guaranteed to be a non-NULL pointer that is the_repository. So this new version of the function that assumes repo can be safely passed to repo_config_get_bool() and repo_get_work_tree() would probably be OK. I didn't dig to find out is_bare_repository() does exactly the same thing as what the new code does when the_repository is given to the "repo" parameter, though. Having said all that. In general, anything in builtin/soething.c is an implementation of "git something" command and cannot be reused outside the context of that "something" command, so it is much less interesting to get rid of dependencies on the_repository from these files. The design choice to work with a single repository at a time is with the "something" command, and as long as that design choice stays, there isn't point in hiding the fact that we work only on the_repository. If we were moving the logic implemented by the helper functions that are used by cmd_repo() function to a shared and reusable library-ish part of the system (perhaps in repo.c at the top-level), that is when it becomes much more interesting and releavnt to make sure that a function that takes "repo" as its parameter does work on that repository and not the_repository. But until then, ... Thanks.