Re: [PATCH 1/3] git: remove is_bare_repository_cfg global variable

[Junio C Hamano <gitster@pobox.com>]

shejialuo <shejialuo@gmail.com> writes:

> I also want to ask this question. Actually, I feel quite strange about
> why we need to add a new parameter `is_bare` to `repo_init` function.
>
> For this call:
>
>     repo_init(the_repository, git_dir, work_tree, -1);
>
> We add a new field "is_bare_cfg" to the "struct repository". So, at now,
> `the_repository` variable should contain the information about whether
> the repo is bare(1), is not bare(0) or unknown(-1). However, in this
> call, we pass "-1" to the parameter `is_bare` for "repo_init" function.

Isn't this merely trying to be faithful to the original to avoid
unintended behaviour change?  We initialize the global variable
is_bare_repository_cfg to unspecified(-1) in the original, and
for a rewrite to move the global to a member in the singleton
instance of the_repo, it would need to be able to do the same.

And for callers of repo_init() that prepares _another_ in-core
repository instance, which is different from the_repository, because
the original has a process-wide singleton global variable, copying
the value from the_repository->is_bare to a newly initialized one
would hopefully give us the most faithful rewrite to avoid
unintended behaviour change.

At least, that is how I understood why the patch does it this way.
As you noticed, too, there are ...

> When I first look at this code, I have thought that we will set
> "repo->is_bare_cfg = -1" to indicate that we cannot tell whether the
> repo is bare or not. But it just sets the "repo->is_bare_cfg = is_bare"
> if `bare > 0`. Junio has already commented on this.

... places in the updated code that makes it unclear what the
is_bare member really means.  The corresponding global variable used
to be "this is what we were told by config or env or command line",
but it is unclear, with conditional assignments like the above, what
it means in the updated code.

Thanks.