git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Ghanshyam Thakkar <shyamthakkar001@gmail.com>
Date
Jan 6, 2024, 12:07 UTC
Message-ID
<CY7M09XT547N.2OOTI5APX9RIX@gmail.com>
In-Reply-To
<xmqqjzonpy9l.fsf@gitster.g>
On Fri Jan 5, 2024 at 9:29 PM IST, Junio C Hamano wrote:
Show 43 quoted lines
> 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.

Show 14 quoted lines
> 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.
Previous: Junio C HamanoNext: Junio C Hamano
Message 6 of 15 in “[GSOC][RFC] Heed core.bare from template config file when no command line override given, as a microproject.”
  1. Ghanshyam ThakkarJan 2, 2024
  2. Christian CouderJan 4, 2024
  3. Ghanshyam ThakkarJan 4, 2024
  4. Elijah NewrenJan 5, 2024
  5. Junio C HamanoJan 5, 2024
  6. Ghanshyam ThakkarJan 6, 2024
  7. Junio C HamanoJan 8, 2024
  8. Elijah NewrenJan 19, 2024
  9. setup: clarify TODO comment about ignoring core.bareGhanshyam Thakkar, Feb 29, 2024
  10. Junio C HamanoFeb 29, 2024
  11. Ghanshyam ThakkarFeb 29, 2024
  12. setup: remove unnecessary variableGhanshyam Thakkar, Mar 4, 2024
  13. Junio C HamanoMar 4, 2024
  14. Ghanshyam ThakkarMar 4, 2024
  15. Junio C HamanoMar 4, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.