Re: [PATCH] [RFC][GSoC 2026] builtin/repo: avoid global state in get_layout_bare
Ayush Jha <kumarayushjha123@gmail.com> writes:
Show 17 quoted lines
> 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 <kumarayushjha123@gmail.com>
> ---
I'll let others to suggest improvements above the three-dash line.
Show 18 quoted lines
> 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.
