[BUG] cannot git clone with includeif onbranch

[BUG] cannot git clone with includeif onbranch

[Angelo Dureghello]

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

Re: [BUG] cannot git clone with includeif onbranch

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 2064 bytes --]

On 2024-03-08 at 19:25:52, Angelo Dureghello wrote:
> 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

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'
-- 
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [BUG] cannot git clone with includeif onbranch

[Angelo Dureghello]

Hi Brian,

On 08/03/24 9:10 PM, brian m. carlson wrote:
> On 2024-03-08 at 19:25:52, Angelo Dureghello wrote:
>> 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
> 
> 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'

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

RE: [BUG] cannot git clone with includeif onbranch

[rsbecker]

On Friday, March 8, 2024 3:11 PM, brian m. carlson wrote:
>On 2024-03-08 at 19:25:52, Angelo Dureghello wrote:
>> 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
>
>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'

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

Re: [BUG] cannot git clone with includeif onbranch

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 2691 bytes --]

On Fri, Mar 08, 2024 at 08:10:56PM +0000, brian m. carlson wrote:
> On 2024-03-08 at 19:25:52, Angelo Dureghello wrote:
> > 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
> 
> 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'

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH] t5601: exercise clones with "includeIf.*.onbranch"

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 2882 bytes --]

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(+)

diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index fb1b9c686d..ca43185681 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -776,6 +776,18 @@ test_expect_success 'batch missing blob request does not inadvertently try to fe
 . "$TEST_DIRECTORY"/lib-httpd.sh
 start_httpd
 
+test_expect_success 'clone with includeIf' '
+	test_when_finished "rm -rf repo \"$HTTPD_DOCUMENT_ROOT_PATH/repo.git\"" &&
+	git clone --bare --no-local src "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+
+	test_when_finished "rm \"$HOME\"/.gitconfig" &&
+	cat >"$HOME"/.gitconfig <<-EOF &&
+	[includeIf "onbranch:something"]
+		path = /does/not/exist.inc
+	EOF
+	git clone $HTTPD_URL/smart/repo.git repo
+'
+
 test_expect_success 'partial clone using HTTP' '
 	partial_clone "$HTTPD_DOCUMENT_ROOT_PATH/server" "$HTTPD_URL/smart/server"
 '
-- 
2.44.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] t5601: exercise clones with "includeIf.*.onbranch"

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> +test_expect_success 'clone with includeIf' '
> +	test_when_finished "rm -rf repo \"$HTTPD_DOCUMENT_ROOT_PATH/repo.git\"" &&
> +	git clone --bare --no-local src "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
> +
> +	test_when_finished "rm \"$HOME\"/.gitconfig" &&
> +	cat >"$HOME"/.gitconfig <<-EOF &&
> +	[includeIf "onbranch:something"]
> +		path = /does/not/exist.inc
> +	EOF
> +	git clone $HTTPD_URL/smart/repo.git repo
> +'

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.

Re: [PATCH] t5601: exercise clones with "includeIf.*.onbranch"

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 4927 bytes --]

On Tue, Mar 12, 2024 at 01:38:26PM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > +test_expect_success 'clone with includeIf' '
> > +	test_when_finished "rm -rf repo \"$HTTPD_DOCUMENT_ROOT_PATH/repo.git\"" &&
> > +	git clone --bare --no-local src "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
> > +
> > +	test_when_finished "rm \"$HOME\"/.gitconfig" &&
> > +	cat >"$HOME"/.gitconfig <<-EOF &&
> > +	[includeIf "onbranch:something"]
> > +		path = /does/not/exist.inc
> > +	EOF
> > +	git clone $HTTPD_URL/smart/repo.git repo
> > +'
> 
> 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?

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] t5601: exercise clones with "includeIf.*.onbranch"

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> 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.

;-)

> 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.

OK.  That is a sensible position to take.

Thanks.

Re: [PATCH] t5601: exercise clones with "includeIf.*.onbranch"

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 643 bytes --]

On Fri, Mar 22, 2024 at 03:01:25AM +0100, Patrick Steinhardt wrote:
> On Tue, Mar 12, 2024 at 01:38:26PM -0700, Junio C Hamano wrote:
> > Patrick Steinhardt <ps@pks.im> writes:
[snip]
> 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.

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]