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

Re: [PATCH] setup: fix bug with "includeIf.onbranch" when initializing dir

From
HRHeghedus Razvan <heghedus.razvan@protonmail.com>
Date
May 22, 2024, 10:58 UTC
Message-ID
<D1G4BWOMJD3E.2CNXHMRZQ12FW@protonmail.com>
In-Reply-To
<cf182bb9ee7d4a7eb46e5dbf4f3ef5deb198d823.1716374321.git.ps@pks.im>
On Wed May 22, 2024 at 1:38 PM EEST, Patrick Steinhardt wrote:
Show 48 quoted lines
> It was reported that git-init(1) can fail when initializing an existing
> directory in case the config contains an "includeIf.onbranch:"
> condition:
>
> ```shell
> $ mkdir repo
> $ git -c includeIf.onbranch:main.path=nonexistent init repo
> BUG: refs.c:2056: reference backend is unknown
> ```
>
> The same error can also be triggered when re-initializing an already
> existing repository.
>
> The bug has been introduced in 173761e21b (setup: start tracking ref
> storage format, 2023-12-29), which wired up the ref storage format. The
> root cause is in `init_db()`, which tries to read the config before we
> have initialized `the_repository` and most importantly its ref storage
> format. We eventually end up calling `include_by_branch()` and execute
> `refs_resolve_ref_unsafe()`, but because we have not initialized the ref
> storage format yet this will trigger the above bug.
>
> Interestingly, `include_by_branch()` has a mechanism that will only
> cause us to resolve the ref when `the_repository->gitdir` is set. This
> is also the reason why this only happens when we initialize an already
> existing directory or repository: `gitdir` is set in those cases, but
> not when creating a new directory.
>
> Now there are two ways to address the issue:
>
>   - We can adapt `include_by_branch()` to also make the code conditional
>     on whether `the_repository->ref_storage_format` is set.
>
>   - We can shift around code such that we initialize the repository
>     format before we read the config.
>
> While the first approach would be safe, it may also cause us to paper
> over issues where a ref store should have been set up. In our case for
> example, it may be reasonable to expect that re-initializing the repo
> will cause the "onbranch:" condition to trigger, but we would not do
> that if the ref storage format was not set up yet. This also used to
> work before the above commit that introduced this bug.
>
> Rearrange the code such that we set up the repository format before
> reading the config. This fixes the bug and ensures that "onbranch:"
> conditions can trigger.
>
> Reported-by: Heghedus Razvan <heghedus.razvan@protonmail.com>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
I can confirm it's fixing the issue. Feel free to add:
Tested-by: Heghedus Razvan <heghedus.razvan@protonmail.com>
Previous: Patrick SteinhardtNext: Junio C Hamano
Message 8 of 14 in “git init BUG when gitconfig has includeIf”
  1. Heghedus RazvanMay 21, 2024
  2. Junio C HamanoMay 21, 2024
  3. Heghedus RazvanMay 21, 2024
  4. Patrick SteinhardtMay 22, 2024
  5. Heghedus RazvanMay 22, 2024
  6. Patrick SteinhardtMay 22, 2024
  7. setup: fix bug with "includeIf.onbranch" when initializing dirPatrick Steinhardt, May 22, 2024
  8. Heghedus RazvanMay 22, 2024
  9. Junio C HamanoMay 22, 2024
  10. Junio C HamanoMay 23, 2024
  11. Patrick SteinhardtMay 23, 2024
  12. setup: fix bug with "includeIf.onbranch" when initializing dirJunio C Hamano, May 23, 2024
  13. [rPATCH v2] setup: fix bug with "includeIf.onbranch" when initializing dirJunio C Hamano, May 23, 2024
  14. Patrick SteinhardtMay 23, 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.