Re: [GSOC][RFC] Heed core.bare from template config file when no command line override given, as a microproject.

["Ghanshyam Thakkar" <shyamthakkar001@gmail.com>]

On Fri Jan 5, 2024 at 9:29 PM IST, Junio C Hamano wrote:
> Elijah Newren <newren@gmail.com> writes:
>
> > If you look back at the mailing list discussion on the series that
> > introduced this TODO comment you are trying to address, you'll note
> > that both Glen and I dug into the code and attempted to explain it to
> > each other, and we both got it wrong on our first try.
>
> I think you meant 0f7443bd (init-db: document existing bug with
> core.bare in template config, 2023-05-16), where it says:
>
>     The comments in create_default_files() talks about reading config from
>     the config file in the specified `--templates` directory, which leads to
>     the question of whether core.bare could be set in such a config file and
>     thus whether the code is doing the right thing.
>
> But I suspect the all of the above comes from a misunderstanding.
> The comment the above commit log message talks about is:
>
>  /*
>   * First copy the templates -- we might have the default
>   * config file there, in which case we would want to read
>   * from it after installing.
>   *
>   * Before reading that config, we also need to clear out any cached
>   * values (since we've just potentially changed what's available on
>   * disk).
>   */
>
> This primarily comes from my 4f629539 (init-db: check template and
> repository format., 2005-11-25), whose focus was to control the way
> HEAD symref is created, but care was taken to avoid propagating
> values from the configuration variables in the template that do not
> make sense for the repository being initialized.  The most important
> thing being the repository format version, but the intent to avoid
> nonsense combination between the characteristic the new repository
> has and the configuration values copied from the template was not
> limited to the format version.
>
> Specifically, the commit that introduced the comment never wanted to
> honor core.bare in the template.  I do not think I has core.bare in
> mind when I wrote the comment, but I would have described it as the
> same category as the repository format version, i.e. something you
> would not want to copy, if I were pressed to clarify back then.

Then I suppose this warrants updating the TODO comment in
create_default_files(), which currently can be interpreted as this 
being a unwanted behavior. And also amending the testcases which
currently display this as knwon breakage.

> Besides, create_default_files() is way too late, even if we wanted
> to create a bare repository when the template config file says
> core.bare = true, as the caller would already have created before
> passing $DIR (when "git --bare init $DIR" was run) or $DIR/.git
> (when "git init $DIR" was run) to the function.
>
> If somebody wants to always create a bare repository by having
> core.bare=true in their template and if we wanted to honor it (which
> I am dubious of the value of, by the way), I would think the right
> place to do so would be way before create_default_files() is called.
> When running "git init [$DIR]", long before calling init_db(), we
> decide if we are about to create a bare repository and either create
> $DIR or $DIR/.git.  What is in the template, if we really wanted to
> do so, should be read before that happens, no?

That is what I proposed in my original email, after which I had a
working solution which passed all the tests. That solution was indeed to
check for core.bare in the template before we set GIT_DIR_ENVIRONMENT, 
which subsequently creates either $DIR or $DIR/.git as you described 
above. 

Regardless, I can send the patch with updated comments to clarify
that ignoring core.bare from template files is the intended behavior and 
amend the test_expect_failure testcases, with Elijah's consensus.

Thanks.