[BUG] cannot git clone with includeif onbranch

Hi,

below the bug report, not totally sure this is a bug btw.

---

Thank you for filling out a Git bug report! Please answer the following questions to help us understand your issue.

What did you do before the bug happened? (Steps to reproduce your issue)

Perform a git clone https with includeif onbranch in the .gitconfig

Create a .gitconfig
with
[includeIf "onbranch:wip/pippo/**"]
         path = ~/.gitconfig.pippo.inc

git clone https://github.com/analogdevicesinc/no-OS.git

Cloning into 'no-OS'...
BUG: refs.c:2083: reference backend is unknown
error: git-remote-https died of signal 6

What did you expect to happen? (Expected behavior) Proper clone

What happened instead? (Actual behavior) Error above

What's different between what you expected and what actually happened? I cannot clone

Anything else you want to add: I am assuming that cloning is outside from any gitdir so should ingnore onbranch.

Please review the rest of the bug report below. You can delete any lines you don't wish to share.

[System Info] git version: git version 2.44.0 cpu: x86_64 no commit associated with this build sizeof-long: 8 sizeof-size_t: 8 shell-path: /bin/sh uname: Linux 6.5.0-rc2-devel-00345-gc2782531397f #2 SMP PREEMPT_DYNAMIC Sun Jul 23 12:25:41 CEST 2023 x86_64 compiler info: gnuc: 13.2 libc info: glibc: 2.39 $SHELL (typically, interactive shell): /bin/zsh

[Enabled Hooks] not run from a git repository - no hooks to show

On 2024-03-08 at 19:25:52, Angelo Dureghello wrote:

Thanks for the report.

I can definitely confirm this with a local Git 2.44.0 built out of my working tree. It seems to trigger as long as there's a `path` entry whether the path exists or not. It _doesn't_ seem to trigger with a `gitdir` check, but does trigger for `onbranch`. v2.43.0 is not affected.

I do definitely think this is a bug. First of all, we should not trigger BUG conditions, even if the user has done something naughty, so we should fix it for that reason. Second of all, this seems like a completely reasonable thing to want to do, and considering it triggers for existing files, and that it worked just fine in v2.43.0, I don't see a reason we shouldn't have this work.

A bisection[0] leads us to 0fcc285c5e ("refs: refactor logic to look up storage backends", 2023-12-29). I've CCed the author of that commit, who hopefully can provide some more helpful context.

I have some guesses about what's going on here, but I haven't poked further into the situation, so I'll refrain from speculating for now.

[0] git bisect run sh -c 'make -j12 && cd $TMPDIR && rm -fr no-OS && PATH="$HOME/checkouts/git/bin-wrappers:$PATH" git clone https://github.com/analogdevicesinc/no-OS.git; RET=$?; [ "$RET" -eq 128 ] && RET=1; exit $RET'

Hi Brian,

On 08/03/24 9:10 PM, brian m. carlson wrote:

Thanks a lot.

Actually i can work with git clone git@github.com:.. that works, issue is only on http.

Regards, and if i can help, welcome. angelo

On Friday, March 8, 2024 3:11 PM, brian m. carlson wrote:

I was wondering whether this should only work on a clone with -b wip/pippo/... argument, although it should not fail the clone, just not apply the path. I can see how it would be somewhat confusing if the global config defaults to master and remote only serves up main - or vise-versa.

--Randall

On Fri, Mar 08, 2024 at 08:10:56PM +0000, brian m. carlson wrote:

Thanks for bisecting! My first hunch would've been that it's caused by the change that changed when we initialize the refdb. I would have thus hoped for 199f44cb2e (builtin/clone: allow remote helpers to detect repo, 2024-02-27) to have fixed the issue. But the bisected commit points into a different direction, so I'd be surprised if that was it.

Anyway, I didn't yet have the time to investigate this issue further, and am currently a bit short on time. I will have a deeper look next week at the latest.

Patrick

It was reported that git-clone(1) started to fail in Git v2.44 when cloning via HTTPS when the config contains an "includeIf.*.onbranch" condition:

    $ git clone https://example.com/repo.git
    Cloning into 'repo'...
    BUG: refs.c:2083: reference backend is unknown
    error: git-remote-https died of signal 6

This regression was bisected to 0fcc285c5e (refs: refactor logic to look up storage backends, 2023-12-29). This commit tightens the logic to look up ref backends such that we now die when the backend has not yet been detected by reading the gitconfig.

Now on its own, this commit wouldn't have caused the failure. But in 18c9cb7524 (builtin/clone: create the refdb with the correct object format, 2023-12-12) we have also changed how git-clone(1) initializes the refdb such that it happens after the remote helper is spawned, which is required so that we can first learn about the object format used by the remote repository before initializing the refdb. Starting with this change, the remote helper will be unable to detect the repository right from the start and thus have an unconfigured ref backend. Consequently, when we try to resolve the "includeIf.*.onbranch" condition, we will now fail to look up the refdb and die.

This regression has already been fixed via 199f44cb2e (builtin/clone: allow remote helpers to detect repo, 2024-02-27), where we now pre-initialize a partial refdb so that the remote helper can detect the repository right from the start. But it's clear that we're lacking test coverage of this functionality.

Add a test to avoid regressing in the future. Note that this test stops short of defining the desired behaviour for the "onbranch" condition during a clone. It's not quite clear how exactly it should behave, so this is a leftover bit for the future.

Reported-by: Angelo Dureghello <angelo@kernel-space.org>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t5601-clone.sh | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Patrick Steinhardt <ps@pks.im> writes:

Hmph, isn't the end-user expectation more like if you clone with "git clone -b something" then the configuration stored in the named file to take effect, while "git clone" that would never place you on that something branch would ignore that missing file? Is this only the latter half of the pair?

Thanks.

On Tue, Mar 12, 2024 at 01:38:26PM -0700, Junio C Hamano wrote:

It probably is, but I'm not really sure to be honest. That's why I punted on it and just assert that it doesn't die.

In any case I would claim that the current behaviour is really quite broken in Git v2.43:

```
$ cat >$HOME/.gitconfig <<EOF
[includeIf "onbranch:master"]
    path = $HOME/include
EOF

$ cat >$HOME/include <<EOF garbage EOF

# Init source with a different branch name than our condition. $ git init source --initial-branch=main $ git -C source commit --allow-empty -m initial

$ git clone source target Cloning into 'target'... error: key does not contain a section: garbage error: key does not contain a section: garbage error: key does not contain a section: garbage error: key does not contain a section: garbage remote: Enumerating objects: 2, done. remote: Counting objects: 100% (2/2), done. remote: Total 2 (delta 0), reused 0 (delta 0), pack-reused 0 Receiving objects: 100% (2/2), done. error: key does not contain a section: garbage

$ git clone -b main source target Cloning into 'target'... error: key does not contain a section: garbage error: key does not contain a section: garbage error: key does not contain a section: garbage error: key does not contain a section: garbage remote: Enumerating objects: 2, done. remote: Counting objects: 100% (2/2), done. remote: Total 2 (delta 0), reused 0 (delta 0), pack-reused 0 Receiving objects: 100% (2/2), done. error: key does not contain a section: garbage

$ git clone -b other source target Cloning into 'target'... error: key does not contain a section: garbage error: key does not contain a section: garbage error: key does not contain a section: garbage error: key does not contain a section: garbage remote: Enumerating objects: 2, done. remote: Counting objects: 100% (2/2), done. remote: Total 2 (delta 0), reused 0 (delta 0), pack-reused 0 Receiving objects: 100% (2/2), done. error: key does not contain a section: garbage

$ cat >$HOME/.gitconfig <<EOF
[includeIf "onbranch:other"]
    path = $HOME/include
EOF
$ git -C source branch other

$ git clone -b other source target Cloning into 'target'... remote: Enumerating objects: 2, done. remote: Counting objects: 100% (2/2), done. remote: Total 2 (delta 0), reused 0 (delta 0), pack-reused 0 Receiving objects: 100% (2/2), done. error: key does not contain a section: garbage ```

So the behaviour is inconsistent already and does not make a lot of sense. Part of the problem is likely that we pre-initialize "HEAD" to "refs/heads/master", which is why you can see some of the includes being triggered.

With Git v2.44 this behaviour did indeed change, and arguably for the better. This is because we now pre-init "HEAD" to "refs/heads/.invalid" instead of using the default branch name. Thus, we do not match "master" anymore, which is likely the correct thing to do.

```
$ cat >$HOME/.gitconfig <<EOF
[includeIf "onbranch:other"]
    path = $HOME/include
EOF

$ git clone -b main source target Cloning into 'target'... remote: Enumerating objects: 2, done. remote: Counting objects: 100% (2/2), done. remote: Total 2 (delta 0), reused 0 (delta 0), pack-reused 0 (from 0) Receiving objects: 100% (2/2), done.

$ git clone -b other source target Cloning into 'target'... remote: Enumerating objects: 2, done. remote: Counting objects: 100% (2/2), done. remote: Total 2 (delta 0), reused 0 (delta 0), pack-reused 0 (from 0) Receiving objects: 100% (2/2), done. error: key does not contain a section: garbage ```

So arguably, the ".invalid" hack actually fixed this case somewhat and made it behave saner. But I'm just not sure whether I feel comfortable enough with all of this incidental behaviour to actually put it into a test and cast it into stone.

Thus the current version of my test that simply asserts that this does something successfully instead of crashing. In my opinion, we need to put more thought into how this is supposed to work before adding more tests.

Patrick

Patrick Steinhardt <ps@pks.im> writes:

;-)

OK.  That is a sensible position to take.

Thanks.

On Fri, Mar 22, 2024 at 03:01:25AM +0100, Patrick Steinhardt wrote:

[snip]

This is wrong by the way. I didn't mean to say Git v2.44, but the upcoming Git v2.45 starting with 199f44cb2e (builtin/clone: allow remote helpers to detect repo, 2024-02-27).

Patrick