2.37.2 can't "git pull" but 2.18.0 can

2.37.2 can't "git pull" but 2.18.0 can

[Lana Deere]

I'm testing an upgrade to git 2.37.2 from the current version we're
using of 2.18.0.  When I try to pull in my development tree, 2.37.2
gives me an error but 2.18.0 things all is fine:

$ /tools/linux-x86_64/git/2.37.2/bin/git pull
Your configuration specifies to merge with the ref
'refs/heads/feature/switch-to-qt5'
from the remote, but no such ref was fetched.

$ /tools/linux-x86_64/git/2.18.0/bin/git pull
From http://githost:7990/scm/dp/sw
 * branch                  feature/switch-to-qt5 -> FETCH_HEAD
Already up to date.

Anyone have any ideas about this?  All I could find on google was a
suggestion that the "no such ref" message indicates the remote branch
was deleted, but that's not the case here.


.. Lana (lana.deere@gmail.com)

Re: 2.37.2 can't "git pull" but 2.18.0 can

[brian m. carlson]

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

On 2022-09-02 at 19:27:55, Lana Deere wrote:
> I'm testing an upgrade to git 2.37.2 from the current version we're
> using of 2.18.0.  When I try to pull in my development tree, 2.37.2
> gives me an error but 2.18.0 things all is fine:
> 
> $ /tools/linux-x86_64/git/2.37.2/bin/git pull
> Your configuration specifies to merge with the ref
> 'refs/heads/feature/switch-to-qt5'
> from the remote, but no such ref was fetched.
> 
> $ /tools/linux-x86_64/git/2.18.0/bin/git pull
> From http://githost:7990/scm/dp/sw
>  * branch                  feature/switch-to-qt5 -> FETCH_HEAD
> Already up to date.
> 
> Anyone have any ideas about this?  All I could find on google was a
> suggestion that the "no such ref" message indicates the remote branch
> was deleted, but that's not the case here.

Can you provide the output of `git ls-remote origin` (assuming that's
the remote you're using) and `git config -l` (the latter with both
versions)?  I don't know of any reason why Git 2.37 should be broken in
this regard, but I suspect that there's a difference in configuration
between the two leading to this.
-- 
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA

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

Re: 2.37.2 can't "git pull" but 2.18.0 can

[Lana Deere]

The 'git ls-remote origin' command on both produces about 3600 lines
of output which appears to consist of all the branches in our repo.
The two outputs are identical according to diff.  Both include a
single mention of the switch-to-qt5 branch,

$ diff /tmp/ls-remote*
[no output here]
$ grep switch-to-qt5 /tmp/ls-remote.*
/tmp/ls-remote.2.18:6a9363081d05c313ba6a6ac59183193f1340bb1f
refs/heads/feature/switch-to-qt5
/tmp/ls-remote.2.37:6a9363081d05c313ba6a6ac59183193f1340bb1f
refs/heads/feature/switch-to-qt5

The 'git config -l' from both versions is almost the same - two lines
have moved around.
$ diff /tmp/config*
1,2d0
< filter.lfs.required=true
< filter.lfs.clean=git-lfs clean -- %f
4a3,4
> filter.lfs.required=true
> filter.lfs.clean=git-lfs clean -- %f

$ cat /tmp/config.2.37
filter.lfs.smudge=git-lfs smudge -- %f
filter.lfs.process=git-lfs filter-process
filter.lfs.required=true
filter.lfs.clean=git-lfs clean -- %f
user.name=Lana Deere
user.email=lana.deere@gmail.com
filter.lfs.clean=git-lfs clean -- %f
filter.lfs.smudge=git-lfs smudge -- %f
filter.lfs.process=git-lfs filter-process
filter.lfs.required=true
color.branch=false
color.diff=false
color.grep=false
color.interactive=false
color.pager=false
color.showbranch=false
color.status=false
color.ui=false
core.repositoryformatversion=0
core.filemode=true
core.bare=false
core.logallrefupdates=true
remote.origin.url=http://lana@githost:7990/scm/dp/sw.git
remote.origin.fetch=+refs/heads/master:refs/remotes/origin/master
branch.master.remote=origin
branch.master.merge=refs/heads/master
branch.feature/switch-to-qt5.remote=origin
branch.feature/switch-to-qt5.merge=refs/heads/feature/switch-to-qt5


.. Lana (lana.deere@gmail.com)

On Fri, Sep 2, 2022 at 4:16 PM brian m. carlson
<sandals@crustytoothpaste.net> wrote:
>
> On 2022-09-02 at 19:27:55, Lana Deere wrote:
> > I'm testing an upgrade to git 2.37.2 from the current version we're
> > using of 2.18.0.  When I try to pull in my development tree, 2.37.2
> > gives me an error but 2.18.0 things all is fine:
> >
> > $ /tools/linux-x86_64/git/2.37.2/bin/git pull
> > Your configuration specifies to merge with the ref
> > 'refs/heads/feature/switch-to-qt5'
> > from the remote, but no such ref was fetched.
> >
> > $ /tools/linux-x86_64/git/2.18.0/bin/git pull
> > From http://githost:7990/scm/dp/sw
> >  * branch                  feature/switch-to-qt5 -> FETCH_HEAD
> > Already up to date.
> >
> > Anyone have any ideas about this?  All I could find on google was a
> > suggestion that the "no such ref" message indicates the remote branch
> > was deleted, but that's not the case here.
>
> Can you provide the output of `git ls-remote origin` (assuming that's
> the remote you're using) and `git config -l` (the latter with both
> versions)?  I don't know of any reason why Git 2.37 should be broken in
> this regard, but I suspect that there's a difference in configuration
> between the two leading to this.
> --
> brian m. carlson (he/him or they/them)
> Toronto, Ontario, CA

Re: 2.37.2 can't "git pull" but 2.18.0 can

[Johannes Schindelin]

Hi Lana,

as per usual, replies inline.

On Tue, 6 Sep 2022, Lana Deere wrote:

> The 'git ls-remote origin' command on both produces about 3600 lines
> of output which appears to consist of all the branches in our repo.
> The two outputs are identical according to diff.  Both include a
> single mention of the switch-to-qt5 branch,
>
> $ diff /tmp/ls-remote*
> [no output here]
> $ grep switch-to-qt5 /tmp/ls-remote.*
> /tmp/ls-remote.2.18:6a9363081d05c313ba6a6ac59183193f1340bb1f
> refs/heads/feature/switch-to-qt5
> /tmp/ls-remote.2.37:6a9363081d05c313ba6a6ac59183193f1340bb1f
> refs/heads/feature/switch-to-qt5
>
> The 'git config -l' from both versions is almost the same - two lines
> have moved around.
> $ diff /tmp/config*
> 1,2d0
> < filter.lfs.required=true
> < filter.lfs.clean=git-lfs clean -- %f
> 4a3,4
> > filter.lfs.required=true
> > filter.lfs.clean=git-lfs clean -- %f
>
> $ cat /tmp/config.2.37
> filter.lfs.smudge=git-lfs smudge -- %f
> filter.lfs.process=git-lfs filter-process
> filter.lfs.required=true
> filter.lfs.clean=git-lfs clean -- %f
> user.name=Lana Deere
> user.email=lana.deere@gmail.com
> filter.lfs.clean=git-lfs clean -- %f
> filter.lfs.smudge=git-lfs smudge -- %f
> filter.lfs.process=git-lfs filter-process
> filter.lfs.required=true
> color.branch=false
> color.diff=false
> color.grep=false
> color.interactive=false
> color.pager=false
> color.showbranch=false
> color.status=false
> color.ui=false
> core.repositoryformatversion=0
> core.filemode=true
> core.bare=false
> core.logallrefupdates=true
> remote.origin.url=http://lana@githost:7990/scm/dp/sw.git
> remote.origin.fetch=+refs/heads/master:refs/remotes/origin/master

At first I thought that this would be the root cause:
`feature/switch-to-qt5` is not included in the refs to fetch.

But then I added a test case for that specific scenario:

-- snip --
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 081808009b2..6e6ddeb7e63 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -218,6 +218,17 @@ test_expect_success 'fail if upstream branch does not exist' '
 	test_cmp expect file
 '

+test_expect_success 'fetch upstream branch even if refspec excludes it' '
+	git branch tirili &&
+	git branch tirili2 &&
+	git init -b tirili downstream &&
+	git -C downstream remote add -t tirili origin "file://$(pwd)/.git" &&
+	git -C downstream config branch.tirili.remote origin &&
+	git -C downstream config branch.tirili.merge refs/heads/tirili2 &&
+	git -C downstream pull 2>err &&
+	! grep "configuration specifies to merge" err
+'
+
 test_expect_success 'fail if the index has unresolved entries' '
 	git checkout -b third second^ &&
 	test_when_finished "git checkout -f copy && git branch -D third" &&

-- snap --

And that test case passes!

The reason is that we specifically add the ref that needs to be merged to
the list of refs to be fetched:
https://github.com/git/git/blob/v2.37.2/builtin/fetch.c#L605-L614

Now, clearly it is not quite working as intended in your scenario. The
message you pasted is produced by the code in
https://github.com/git/git/blob/v2.37.2/builtin/pull.c#L421-L494, which is
only entered if there are no entries in `.git/FETCH_HEAD` except
`not-for-merge` ones.

Lana, would you mind pasting the contents of `.git/FETCH_HEAD` just after
a failed `git pull`?

Ciao,
Johannes

> branch.master.remote=origin
> branch.master.merge=refs/heads/master
> branch.feature/switch-to-qt5.remote=origin
> branch.feature/switch-to-qt5.merge=refs/heads/feature/switch-to-qt5
>
>
> .. Lana (lana.deere@gmail.com)
>
> On Fri, Sep 2, 2022 at 4:16 PM brian m. carlson
> <sandals@crustytoothpaste.net> wrote:
> >
> > On 2022-09-02 at 19:27:55, Lana Deere wrote:
> > > I'm testing an upgrade to git 2.37.2 from the current version we're
> > > using of 2.18.0.  When I try to pull in my development tree, 2.37.2
> > > gives me an error but 2.18.0 things all is fine:
> > >
> > > $ /tools/linux-x86_64/git/2.37.2/bin/git pull
> > > Your configuration specifies to merge with the ref
> > > 'refs/heads/feature/switch-to-qt5'
> > > from the remote, but no such ref was fetched.
> > >
> > > $ /tools/linux-x86_64/git/2.18.0/bin/git pull
> > > From http://githost:7990/scm/dp/sw
> > >  * branch                  feature/switch-to-qt5 -> FETCH_HEAD
> > > Already up to date.
> > >
> > > Anyone have any ideas about this?  All I could find on google was a
> > > suggestion that the "no such ref" message indicates the remote branch
> > > was deleted, but that's not the case here.
> >
> > Can you provide the output of `git ls-remote origin` (assuming that's
> > the remote you're using) and `git config -l` (the latter with both
> > versions)?  I don't know of any reason why Git 2.37 should be broken in
> > this regard, but I suspect that there's a difference in configuration
> > between the two leading to this.
> > --
> > brian m. carlson (he/him or they/them)
> > Toronto, Ontario, CA
>

Re: 2.37.2 can't "git pull" but 2.18.0 can

[Lana Deere]

As you guessed, the only line is "not-for-merge".

$ cat .git/FETCH_HEAD
4a537c911d9b90f002b682badf9a9121f62622d7        not-for-merge   branch
'master' of http://githost:7990/scm/dp/sw

What does that mean?

.. Lana (lana.deere@gmail.com)


On Wed, Sep 7, 2022 at 8:59 AM Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
>
> Hi Lana,
>
> as per usual, replies inline.
>
> On Tue, 6 Sep 2022, Lana Deere wrote:
>
> > The 'git ls-remote origin' command on both produces about 3600 lines
> > of output which appears to consist of all the branches in our repo.
> > The two outputs are identical according to diff.  Both include a
> > single mention of the switch-to-qt5 branch,
> >
> > $ diff /tmp/ls-remote*
> > [no output here]
> > $ grep switch-to-qt5 /tmp/ls-remote.*
> > /tmp/ls-remote.2.18:6a9363081d05c313ba6a6ac59183193f1340bb1f
> > refs/heads/feature/switch-to-qt5
> > /tmp/ls-remote.2.37:6a9363081d05c313ba6a6ac59183193f1340bb1f
> > refs/heads/feature/switch-to-qt5
> >
> > The 'git config -l' from both versions is almost the same - two lines
> > have moved around.
> > $ diff /tmp/config*
> > 1,2d0
> > < filter.lfs.required=true
> > < filter.lfs.clean=git-lfs clean -- %f
> > 4a3,4
> > > filter.lfs.required=true
> > > filter.lfs.clean=git-lfs clean -- %f
> >
> > $ cat /tmp/config.2.37
> > filter.lfs.smudge=git-lfs smudge -- %f
> > filter.lfs.process=git-lfs filter-process
> > filter.lfs.required=true
> > filter.lfs.clean=git-lfs clean -- %f
> > user.name=Lana Deere
> > user.email=lana.deere@gmail.com
> > filter.lfs.clean=git-lfs clean -- %f
> > filter.lfs.smudge=git-lfs smudge -- %f
> > filter.lfs.process=git-lfs filter-process
> > filter.lfs.required=true
> > color.branch=false
> > color.diff=false
> > color.grep=false
> > color.interactive=false
> > color.pager=false
> > color.showbranch=false
> > color.status=false
> > color.ui=false
> > core.repositoryformatversion=0
> > core.filemode=true
> > core.bare=false
> > core.logallrefupdates=true
> > remote.origin.url=http://lana@githost:7990/scm/dp/sw.git
> > remote.origin.fetch=+refs/heads/master:refs/remotes/origin/master
>
> At first I thought that this would be the root cause:
> `feature/switch-to-qt5` is not included in the refs to fetch.
>
> But then I added a test case for that specific scenario:
>
> -- snip --
> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
> index 081808009b2..6e6ddeb7e63 100755
> --- a/t/t5520-pull.sh
> +++ b/t/t5520-pull.sh
> @@ -218,6 +218,17 @@ test_expect_success 'fail if upstream branch does not exist' '
>         test_cmp expect file
>  '
>
> +test_expect_success 'fetch upstream branch even if refspec excludes it' '
> +       git branch tirili &&
> +       git branch tirili2 &&
> +       git init -b tirili downstream &&
> +       git -C downstream remote add -t tirili origin "file://$(pwd)/.git" &&
> +       git -C downstream config branch.tirili.remote origin &&
> +       git -C downstream config branch.tirili.merge refs/heads/tirili2 &&
> +       git -C downstream pull 2>err &&
> +       ! grep "configuration specifies to merge" err
> +'
> +
>  test_expect_success 'fail if the index has unresolved entries' '
>         git checkout -b third second^ &&
>         test_when_finished "git checkout -f copy && git branch -D third" &&
>
> -- snap --
>
> And that test case passes!
>
> The reason is that we specifically add the ref that needs to be merged to
> the list of refs to be fetched:
> https://github.com/git/git/blob/v2.37.2/builtin/fetch.c#L605-L614
>
> Now, clearly it is not quite working as intended in your scenario. The
> message you pasted is produced by the code in
> https://github.com/git/git/blob/v2.37.2/builtin/pull.c#L421-L494, which is
> only entered if there are no entries in `.git/FETCH_HEAD` except
> `not-for-merge` ones.
>
> Lana, would you mind pasting the contents of `.git/FETCH_HEAD` just after
> a failed `git pull`?
>
> Ciao,
> Johannes
>
> > branch.master.remote=origin
> > branch.master.merge=refs/heads/master
> > branch.feature/switch-to-qt5.remote=origin
> > branch.feature/switch-to-qt5.merge=refs/heads/feature/switch-to-qt5
> >
> >
> > .. Lana (lana.deere@gmail.com)
> >
> > On Fri, Sep 2, 2022 at 4:16 PM brian m. carlson
> > <sandals@crustytoothpaste.net> wrote:
> > >
> > > On 2022-09-02 at 19:27:55, Lana Deere wrote:
> > > > I'm testing an upgrade to git 2.37.2 from the current version we're
> > > > using of 2.18.0.  When I try to pull in my development tree, 2.37.2
> > > > gives me an error but 2.18.0 things all is fine:
> > > >
> > > > $ /tools/linux-x86_64/git/2.37.2/bin/git pull
> > > > Your configuration specifies to merge with the ref
> > > > 'refs/heads/feature/switch-to-qt5'
> > > > from the remote, but no such ref was fetched.
> > > >
> > > > $ /tools/linux-x86_64/git/2.18.0/bin/git pull
> > > > From http://githost:7990/scm/dp/sw
> > > >  * branch                  feature/switch-to-qt5 -> FETCH_HEAD
> > > > Already up to date.
> > > >
> > > > Anyone have any ideas about this?  All I could find on google was a
> > > > suggestion that the "no such ref" message indicates the remote branch
> > > > was deleted, but that's not the case here.
> > >
> > > Can you provide the output of `git ls-remote origin` (assuming that's
> > > the remote you're using) and `git config -l` (the latter with both
> > > versions)?  I don't know of any reason why Git 2.37 should be broken in
> > > this regard, but I suspect that there's a difference in configuration
> > > between the two leading to this.
> > > --
> > > brian m. carlson (he/him or they/them)
> > > Toronto, Ontario, CA
> >

Re: 2.37.2 can't "git pull" but 2.18.0 can

[Jeff King]

On Wed, Sep 07, 2022 at 02:59:08PM +0200, Johannes Schindelin wrote:

> At first I thought that this would be the root cause:
> `feature/switch-to-qt5` is not included in the refs to fetch.
> 
> But then I added a test case for that specific scenario:
> 
> -- snip --
> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
> index 081808009b2..6e6ddeb7e63 100755
> --- a/t/t5520-pull.sh
> +++ b/t/t5520-pull.sh
> @@ -218,6 +218,17 @@ test_expect_success 'fail if upstream branch does not exist' '
>  	test_cmp expect file
>  '
> 
> +test_expect_success 'fetch upstream branch even if refspec excludes it' '
> +	git branch tirili &&
> +	git branch tirili2 &&
> +	git init -b tirili downstream &&
> +	git -C downstream remote add -t tirili origin "file://$(pwd)/.git" &&
> +	git -C downstream config branch.tirili.remote origin &&
> +	git -C downstream config branch.tirili.merge refs/heads/tirili2 &&
> +	git -C downstream pull 2>err &&
> +	! grep "configuration specifies to merge" err
> +'
> +
>  test_expect_success 'fail if the index has unresolved entries' '
>  	git checkout -b third second^ &&
>  	test_when_finished "git checkout -f copy && git branch -D third" &&
> 
> -- snap --
> 
> And that test case passes!
> 
> The reason is that we specifically add the ref that needs to be merged to
> the list of refs to be fetched:
> https://github.com/git/git/blob/v2.37.2/builtin/fetch.c#L605-L614

I just sent another message with more details. But the reason this test
passes is that "tirili" is a prefix of "tirili2". The v2 feature to
limit advertisements works on string prefixes, so asking for "tirili"
will let the server advertise both branches. Switching the first branch
name like so:

diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 6e6ddeb7e6..7e9ed436d3 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -219,12 +219,12 @@ test_expect_success 'fail if upstream branch does not exist' '
 '
 
 test_expect_success 'fetch upstream branch even if refspec excludes it' '
-	git branch tirili &&
+	git branch tirili1 &&
 	git branch tirili2 &&
-	git init -b tirili downstream &&
-	git -C downstream remote add -t tirili origin "file://$(pwd)/.git" &&
-	git -C downstream config branch.tirili.remote origin &&
-	git -C downstream config branch.tirili.merge refs/heads/tirili2 &&
+	git init -b tirili1 downstream &&
+	git -C downstream remote add -t tirili1 origin "file://$(pwd)/.git" &&
+	git -C downstream config branch.tirili1.remote origin &&
+	git -C downstream config branch.tirili1.merge refs/heads/tirili2 &&
 	git -C downstream pull 2>err &&
 	! grep "configuration specifies to merge" err
 '

causes it to fail just like Lana's case.

-Peff

Re: 2.37.2 can't "git pull" but 2.18.0 can

[Jeff King]

On Fri, Sep 02, 2022 at 03:27:55PM -0400, Lana Deere wrote:

> I'm testing an upgrade to git 2.37.2 from the current version we're
> using of 2.18.0.  When I try to pull in my development tree, 2.37.2
> gives me an error but 2.18.0 things all is fine:
> 
> $ /tools/linux-x86_64/git/2.37.2/bin/git pull
> Your configuration specifies to merge with the ref
> 'refs/heads/feature/switch-to-qt5'
> from the remote, but no such ref was fetched.
> 
> $ /tools/linux-x86_64/git/2.18.0/bin/git pull
> From http://githost:7990/scm/dp/sw
>  * branch                  feature/switch-to-qt5 -> FETCH_HEAD
> Already up to date.
> 
> Anyone have any ideas about this?  All I could find on google was a
> suggestion that the "no such ref" message indicates the remote branch
> was deleted, but that's not the case here.

It's curious that the older version shows us fetching into FETCH_HEAD,
but the new one doesn't. I wonder if you have some unusual refspecs. Or
perhaps a branch.*.remote config option which fetches from a different
remote. The "git config" output brian asked for may be instructive
there.

If it's possible for you to build Git from source, it may also be
interesting to bisect to find the commit that caused the change.

-Peff

Re: 2.37.2 can't "git pull" but 2.18.0 can

[Lana Deere]

This is the final output from git bisect:

$ git bisect good
Bisecting: 0 revisions left to test after this (roughly 0 steps)
[d8d3d632f4165955da49032d50279c20cfbde2e5] hooks--update.sample: use
hash-agnostic zero OID

Does that offer any hint about what is going on?

Incidentally, some but not all of the pulls produced additional
output.  Maybe it's a clue?

$ ~/tmp/git/install/bin/git pull
warning: Pulling without specifying how to reconcile divergent branches is
discouraged. You can squelch this message by running one of the following
commands sometime before your next pull:

  git config pull.rebase false  # merge (the default strategy)
  git config pull.rebase true   # rebase
  git config pull.ff only       # fast-forward only

You can replace "git config" with "git config --global" to set a default
preference for all repositories. You can also pass --rebase, --no-rebase,
or --ff-only on the command line to override the configured default per
invocation.

From http://githost:7990/scm/dp/d2s_sw
 * branch                  feature/switch-to-qt5 -> FETCH_HEAD
Already up to date.


.. Lana (lana.deere@gmail.com)

On Fri, Sep 2, 2022 at 9:07 PM Jeff King <peff@peff.net> wrote:
>
> On Fri, Sep 02, 2022 at 03:27:55PM -0400, Lana Deere wrote:
>
> > I'm testing an upgrade to git 2.37.2 from the current version we're
> > using of 2.18.0.  When I try to pull in my development tree, 2.37.2
> > gives me an error but 2.18.0 things all is fine:
> >
> > $ /tools/linux-x86_64/git/2.37.2/bin/git pull
> > Your configuration specifies to merge with the ref
> > 'refs/heads/feature/switch-to-qt5'
> > from the remote, but no such ref was fetched.
> >
> > $ /tools/linux-x86_64/git/2.18.0/bin/git pull
> > From http://githost:7990/scm/dp/sw
> >  * branch                  feature/switch-to-qt5 -> FETCH_HEAD
> > Already up to date.
> >
> > Anyone have any ideas about this?  All I could find on google was a
> > suggestion that the "no such ref" message indicates the remote branch
> > was deleted, but that's not the case here.
>
> It's curious that the older version shows us fetching into FETCH_HEAD,
> but the new one doesn't. I wonder if you have some unusual refspecs. Or
> perhaps a branch.*.remote config option which fetches from a different
> remote. The "git config" output brian asked for may be instructive
> there.
>
> If it's possible for you to build Git from source, it may also be
> interesting to bisect to find the commit that caused the change.
>
> -Peff

Re: 2.37.2 can't "git pull" but 2.18.0 can

[Đoàn Trần Công Danh]

On 2022-09-06 15:37:45-0400, Lana Deere <lana.deere@gmail.com> wrote:
> This is the final output from git bisect:
> 
> $ git bisect good
> Bisecting: 0 revisions left to test after this (roughly 0 steps)
> [d8d3d632f4165955da49032d50279c20cfbde2e5] hooks--update.sample: use
> hash-agnostic zero OID
> 
> Does that offer any hint about what is going on?

It is still bisecting, can you continue to bisect until it says
something like:

	first bad commit is ...


> 
> Incidentally, some but not all of the pulls produced additional
> output.  Maybe it's a clue?
> 
> $ ~/tmp/git/install/bin/git pull
> warning: Pulling without specifying how to reconcile divergent branches is
> discouraged. You can squelch this message by running one of the following
> commands sometime before your next pull:
> 
>   git config pull.rebase false  # merge (the default strategy)
>   git config pull.rebase true   # rebase
>   git config pull.ff only       # fast-forward only
> 
> You can replace "git config" with "git config --global" to set a default
> preference for all repositories. You can also pass --rebase, --no-rebase,
> or --ff-only on the command line to override the configured default per
> invocation.

This is a hint in some version of git for 2 modes of pull, you can
ignore it.

-- 
Danh

Re: 2.37.2 can't "git pull" but 2.18.0 can

[Lana Deere]

Sorry, I was confused by the "0 left".  With one more besect it says

9f489ac6bbb755fa4c83289e44cad12f3b765d69 is the first bad commit

That appears to be
 [9f489ac6bbb755fa4c83289e44cad12f3b765d69] Merge branch 'dl/zero-oid-in-hooks'

.. Lana (lana.deere@gmail.com)

On Tue, Sep 6, 2022 at 10:11 PM Đoàn Trần Công Danh
<congdanhqx@gmail.com> wrote:
>
> On 2022-09-06 15:37:45-0400, Lana Deere <lana.deere@gmail.com> wrote:
> > This is the final output from git bisect:
> >
> > $ git bisect good
> > Bisecting: 0 revisions left to test after this (roughly 0 steps)
> > [d8d3d632f4165955da49032d50279c20cfbde2e5] hooks--update.sample: use
> > hash-agnostic zero OID
> >
> > Does that offer any hint about what is going on?
>
> It is still bisecting, can you continue to bisect until it says
> something like:
>
>         first bad commit is ...
>
>
> >
> > Incidentally, some but not all of the pulls produced additional
> > output.  Maybe it's a clue?
> >
> > $ ~/tmp/git/install/bin/git pull
> > warning: Pulling without specifying how to reconcile divergent branches is
> > discouraged. You can squelch this message by running one of the following
> > commands sometime before your next pull:
> >
> >   git config pull.rebase false  # merge (the default strategy)
> >   git config pull.rebase true   # rebase
> >   git config pull.ff only       # fast-forward only
> >
> > You can replace "git config" with "git config --global" to set a default
> > preference for all repositories. You can also pass --rebase, --no-rebase,
> > or --ff-only on the command line to override the configured default per
> > invocation.
>
> This is a hint in some version of git for 2 modes of pull, you can
> ignore it.
>
> --
> Danh

Re: 2.37.2 can't "git pull" but 2.18.0 can

[Jeff King]

On Wed, Sep 07, 2022 at 11:56:27AM -0400, Lana Deere wrote:

> Sorry, I was confused by the "0 left".  With one more besect it says
> 
> 9f489ac6bbb755fa4c83289e44cad12f3b765d69 is the first bad commit
> 
> That appears to be
>  [9f489ac6bbb755fa4c83289e44cad12f3b765d69] Merge branch 'dl/zero-oid-in-hooks'

That seems unlikely to be the real culprit. I wonder if something went
wrong during the bisect.

A common gotcha when building Git from source is to directly run:

  /path/to/git-clone/git pull

Under the hood git-pull will run git-fetch, which it will look for in
the installed libexec dir. But of course if you didn't run "make
install", then what is there may be some old version installed
previously. Instead, you want to run:

  /path/to/git-clone/bin-wrappers/git pull

which will set up the environment so that we'll find any other git
commands inside the build directory.

That's all a wild guess, of course, but if you think that might be the
problem it's worth trying the bisect again.

-Peff

Re: 2.37.2 can't "git pull" but 2.18.0 can

[Lana Deere]

OK, I tried the bisect again.  I used the bin-wrappers/git from my git
source each time I did a pull and each time I did a bisect.  This time
the final result was indeed different:

$ ~/tmp/git/git/bin-wrappers/git bisect bad
eb049759fb6b739310af52ee0e13ce6cd0c86be7 is the first bad commit
commit eb049759fb6b739310af52ee0e13ce6cd0c86be7
Author: Jeff King <peff@peff.net>
Date:   Fri Sep 25 14:34:36 2020 -0400

    protocol: re-enable v2 protocol by default

    Protocol v2 became the default in v2.26.0 via 684ceae32d (fetch: default
    to protocol version 2, 2019-12-23). More widespread use turned up a
    regression in negotiation. That was fixed in v2.27.0 via 4fa3f00abb
    (fetch-pack: in protocol v2, in_vain only after ACK, 2020-04-27), but we
    also reverted the default to v0 as a precuation in 11c7f2a30b (Revert
    "fetch: default to protocol version 2", 2020-04-22).

    In v2.28.0, we re-enabled it for experimental users with 3697caf4b9
    (config: let feature.experimental imply protocol.version=2, 2020-05-20)
    and haven't heard any complaints. v2.28 has only been out for 2 months,
    but I'd generally expect people turning on feature.experimental to also
    stay pretty up-to-date. So we're not likely to collect much more data by
    waiting. In addition, we have no further reports from people running
    v2.26.0, and of course some people have been setting protocol.version
    manually for ages.

    Let's move forward with v2 as the default again. It's possible there are
    still lurking bugs, but we won't know until it gets more widespread use.
    And we can find and squash them just like any other bug at this point.

    Signed-off-by: Jeff King <peff@peff.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>

 Documentation/config/feature.txt  | 4 ----
 Documentation/config/protocol.txt | 3 +--
 protocol.c                        | 6 +-----
 3 files changed, 2 insertions(+), 11 deletions(-)


.. Lana (lana.deere@gmail.com)


On Wed, Sep 7, 2022 at 2:21 PM Jeff King <peff@peff.net> wrote:
>
> On Wed, Sep 07, 2022 at 11:56:27AM -0400, Lana Deere wrote:
>
> > Sorry, I was confused by the "0 left".  With one more besect it says
> >
> > 9f489ac6bbb755fa4c83289e44cad12f3b765d69 is the first bad commit
> >
> > That appears to be
> >  [9f489ac6bbb755fa4c83289e44cad12f3b765d69] Merge branch 'dl/zero-oid-in-hooks'
>
> That seems unlikely to be the real culprit. I wonder if something went
> wrong during the bisect.
>
> A common gotcha when building Git from source is to directly run:
>
>   /path/to/git-clone/git pull
>
> Under the hood git-pull will run git-fetch, which it will look for in
> the installed libexec dir. But of course if you didn't run "make
> install", then what is there may be some old version installed
> previously. Instead, you want to run:
>
>   /path/to/git-clone/bin-wrappers/git pull
>
> which will set up the environment so that we'll find any other git
> commands inside the build directory.
>
> That's all a wild guess, of course, but if you think that might be the
> problem it's worth trying the bisect again.
>
> -Peff

Re: 2.37.2 can't "git pull" but 2.18.0 can

[Jeff King]

On Wed, Sep 07, 2022 at 02:53:37PM -0400, Lana Deere wrote:

> OK, I tried the bisect again.  I used the bin-wrappers/git from my git
> source each time I did a pull and each time I did a bisect.  This time
> the final result was indeed different:
> 
> $ ~/tmp/git/git/bin-wrappers/git bisect bad
> eb049759fb6b739310af52ee0e13ce6cd0c86be7 is the first bad commit
> commit eb049759fb6b739310af52ee0e13ce6cd0c86be7
> Author: Jeff King <peff@peff.net>

Drat, I shouldn't have helped you figure out I was the culprit. ;)

>     protocol: re-enable v2 protocol by default

OK, so this is definitely a plausible bisection result. Things shouldn't
behave any different between the two protocols, but there could be a
bug.

The first thing I'd try is whether:

  .../git/2.37.2/bin/git -c protocol.version=0 pull

works like 2.18.0 does. If so, then that confirms that protocol v2 is
the problem. At that point I might try capturing packet dumps with:

  GIT_TRACE_PACKET=/tmp/v0.trace \
  .../git/2.37.2/bin/git -c protocol.version=0 pull

  GIT_TRACE_PACKET=/tmp/v2.trace \
  .../git/2.37.2/bin/git -c protocol.version=2 pull

They'll be sufficiently different that you can't just diff them, but if
you're able to share them, one of us familiar with the protocol might be
able to notice something.

I don't _think_ there should be a problem with the server side of your
connection speaking the v2 protocol. After all, you found that the
ls-remote output was the same for both versions.

-Peff

Re: 2.37.2 can't "git pull" but 2.18.0 can

[Lana Deere]

With an explicit -c protocol.version=0 on the 2.37.2 git command line,
the pull is successful.  For what it's worth, the server git is still
2.18.0 in all of these cases.  Only the client side is being tested so
far.  I will try to gather the packet traces and see if there's a
problem sharing them.  Will this mailing list allow attachments?

.. Lana (lana.deere@gmail.com)



On Wed, Sep 7, 2022 at 5:10 PM Jeff King <peff@peff.net> wrote:
>
> On Wed, Sep 07, 2022 at 02:53:37PM -0400, Lana Deere wrote:
>
> > OK, I tried the bisect again.  I used the bin-wrappers/git from my git
> > source each time I did a pull and each time I did a bisect.  This time
> > the final result was indeed different:
> >
> > $ ~/tmp/git/git/bin-wrappers/git bisect bad
> > eb049759fb6b739310af52ee0e13ce6cd0c86be7 is the first bad commit
> > commit eb049759fb6b739310af52ee0e13ce6cd0c86be7
> > Author: Jeff King <peff@peff.net>
>
> Drat, I shouldn't have helped you figure out I was the culprit. ;)
>
> >     protocol: re-enable v2 protocol by default
>
> OK, so this is definitely a plausible bisection result. Things shouldn't
> behave any different between the two protocols, but there could be a
> bug.
>
> The first thing I'd try is whether:
>
>   .../git/2.37.2/bin/git -c protocol.version=0 pull
>
> works like 2.18.0 does. If so, then that confirms that protocol v2 is
> the problem. At that point I might try capturing packet dumps with:
>
>   GIT_TRACE_PACKET=/tmp/v0.trace \
>   .../git/2.37.2/bin/git -c protocol.version=0 pull
>
>   GIT_TRACE_PACKET=/tmp/v2.trace \
>   .../git/2.37.2/bin/git -c protocol.version=2 pull
>
> They'll be sufficiently different that you can't just diff them, but if
> you're able to share them, one of us familiar with the protocol might be
> able to notice something.
>
> I don't _think_ there should be a problem with the server side of your
> connection speaking the v2 protocol. After all, you found that the
> ls-remote output was the same for both versions.
>
> -Peff

Re: 2.37.2 can't "git pull" but 2.18.0 can

[Jeff King]

On Thu, Sep 08, 2022 at 12:46:14PM -0400, Lana Deere wrote:

> With an explicit -c protocol.version=0 on the 2.37.2 git command line,
> the pull is successful.  For what it's worth, the server git is still
> 2.18.0 in all of these cases.  Only the client side is being tested so
> far.  I will try to gather the packet traces and see if there's a
> problem sharing them.  Will this mailing list allow attachments?

You can send attachments to the list as long as the total mail size is
under 100kb. But to keep the list in the loop: Lana sent me the traces
off-list, because naturally they have a bunch of semi-private ref names.

I was able to see the problem from the traces: the v2 protocol has an
extension to tell the server to limit the advertisement only to branches
we're interested in. And it does so based on the configured refspec. As
Dscho noted earlier in the thread, the upstream branch you want isn't in
the refspec. We try to add that branch explicitly to what we're
fetching, but I think that happens too late to affect the ref-prefix
limiting. So the server is asked not to advertise the ref, and from the
client's perspective, it looks like the branch does not exist on the
server.

Here's a minimal reproduction:

  # a server with two branches
  git init server
  (
    cd server
    git checkout -b branch1
    git commit --allow-empty -m foo
    git branch branch2
  )

  # and a client which points its origin there,
  # and has local copies of both branches, tracking
  # the upstream versions
  git clone server client
  cd client
  git checkout branch1
  git checkout branch2

  # but afterwards, the client narrows its refspec to only fetch branch1
  git config remote.origin.fetch +refs/heads/branch1:refs/remotes/origin/branch1

  # pulling branch2 with v0 works
  git -c protocol.version=0 pull

  # but does not with v2, because the ref-prefix extension tells the
  # server not to advertise anything outside of branch1
  git -c protocol.version=2 pull

This is a bug which we should fix. But in the meantime the obvious
workaround is to expand the default refspec to cover both branches.
Obviously the default of fetching "refs/heads/*" would work, but if you
want to keep it limited for some reason, you can add the second branch
explicitly. In the example above, it would be:

  git config --add remote.origin.fetch +refs/heads/branch2:refs/remotes/origin/branch2

-Peff

[PATCH 0/2] v2 protocol can't "git pull" with restricted refspec

[Jeff King]

On Thu, Sep 08, 2022 at 02:14:07PM -0400, Jeff King wrote:

> This is a bug which we should fix. But in the meantime the obvious
> workaround is to expand the default refspec to cover both branches.
> Obviously the default of fetching "refs/heads/*" would work, but if you
> want to keep it limited for some reason, you can add the second branch
> explicitly. In the example above, it would be:
> 
>   git config --add remote.origin.fetch +refs/heads/branch2:refs/remotes/origin/branch2

And here's the patch to fix it. There was a small cleanup needed, hence
the 2-patch series.

  [1/2]: fetch: stop checking for NULL transport->remote in do_fetch()
  [2/2]: fetch: add branch.*.merge to default ref-prefix extension

 builtin/fetch.c | 18 +++++++++++++++---
 t/t5520-pull.sh | 17 +++++++++++++++++
 2 files changed, 32 insertions(+), 3 deletions(-)

-Peff

[PATCH 1/2] fetch: stop checking for NULL transport->remote in do_fetch()

[Jeff King]

This field will never be NULL; if it were, we'd segfault earlier in the
function when we unconditionally check transport->remote->fetch_tags.
Likewise, many other functions dereference it unconditionally.

This is a small simplification, but it will make things easier as we
extend this conditional in the next patch.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/fetch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 368a0f5329..f78146ca81 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1616,7 +1616,7 @@ static int do_fetch(struct transport *transport,
 				break;
 			}
 		}
-	} else if (transport->remote && transport->remote->fetch.nr)
+	} else if (transport->remote->fetch.nr)
 		refspec_ref_prefixes(&transport->remote->fetch,
 				     &transport_ls_refs_options.ref_prefixes);
 
-- 
2.37.3.1164.gb600acaa9f

[PATCH 2/2] fetch: add branch.*.merge to default ref-prefix extension

[Jeff King]

When running "git pull" with no arguments, we'll do a default "git
fetch" and then try to merge the branch specified by the branch.*.merge
config. There's code in get_ref_map() to treat that "merge" branch as
something we want to fetch, even if it is not otherwise covered by the
default refspec.

This works fine with the v0 protocol, as the server tells us about all
of the refs, and get_ref_map() is the ultimate decider of what we fetch.

But in the v2 protocol, we send the ref-prefix extension to the server,
asking it to limit the ref advertisement. And we only tell it about the
default refspec for the remote; we don't mention the branch.*.merge
config at all.

This usually doesn't matter, because the default refspec matches
"refs/heads/*", which covers all branches. But if you explicitly use a
narrow refspec, then "git pull" on some branches may fail. The server
doesn't advertise the branch, so we don't fetch it, and "git pull"
thinks that it went away upstream.

We can fix this by including any branch.*.merge entries for the current
branch in the list of ref-prefixes we pass to the server. This only
needs to happen when using the default configured refspec (since
command-line refspecs are already added, and take precedence in deciding
what we fetch). We don't otherwise need to replicate any of the "what to
fetch" logic in get_ref_map(). These ref-prefixes are an optimization,
so it's OK if we tell the server to advertise the branch.*.merge ref,
even if we're not going to pull it. We'll just choose not to fetch it.

The test here is based on one constructed by Johannes. I modified the
branch names to trigger the ref-prefix issue (and be more descriptive),
and to confirm that "git pull" actually updated the local ref, which
should be more robust than just checking stderr.

Reported-by: Lana Deere <lana.deere@gmail.com>
Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/fetch.c | 18 +++++++++++++++---
 t/t5520-pull.sh | 17 +++++++++++++++++
 2 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index f78146ca81..d4bbef6695 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1616,9 +1616,21 @@ static int do_fetch(struct transport *transport,
 				break;
 			}
 		}
-	} else if (transport->remote->fetch.nr)
-		refspec_ref_prefixes(&transport->remote->fetch,
-				     &transport_ls_refs_options.ref_prefixes);
+	} else {
+		struct branch *branch = branch_get(NULL);
+
+		if (transport->remote->fetch.nr)
+			refspec_ref_prefixes(&transport->remote->fetch,
+					     &transport_ls_refs_options.ref_prefixes);
+		if (branch_has_merge_config(branch) &&
+		    !strcmp(branch->remote_name, transport->remote->name)) {
+			int i;
+			for (i = 0; i < branch->merge_nr; i++) {
+				strvec_push(&transport_ls_refs_options.ref_prefixes,
+					    branch->merge[i]->src);
+			}
+		}
+	}
 
 	if (tags == TAGS_SET || tags == TAGS_DEFAULT) {
 		must_list_refs = 1;
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 081808009b..0b72112fb1 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -218,6 +218,23 @@ test_expect_success 'fail if upstream branch does not exist' '
 	test_cmp expect file
 '
 
+test_expect_success 'fetch upstream branch even if refspec excludes it' '
+	# the branch names are not important here except that
+	# the first one must not be a prefix of the second,
+	# since otherwise the ref-prefix protocol extension
+	# would match both
+	git branch in-refspec HEAD^ &&
+	git branch not-in-refspec HEAD &&
+	git init -b in-refspec downstream &&
+	git -C downstream remote add -t in-refspec origin "file://$(pwd)/.git" &&
+	git -C downstream config branch.in-refspec.remote origin &&
+	git -C downstream config branch.in-refspec.merge refs/heads/not-in-refspec &&
+	git -C downstream pull &&
+	git rev-parse --verify not-in-refspec >expect &&
+	git -C downstream rev-parse --verify HEAD >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'fail if the index has unresolved entries' '
 	git checkout -b third second^ &&
 	test_when_finished "git checkout -f copy && git branch -D third" &&
-- 
2.37.3.1164.gb600acaa9f

Re: [PATCH 2/2] fetch: add branch.*.merge to default ref-prefix extension

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> When running "git pull" with no arguments, we'll do a default "git
> fetch" and then try to merge the branch specified by the branch.*.merge
> config. There's code in get_ref_map() to treat that "merge" branch as
> something we want to fetch, even if it is not otherwise covered by the
> default refspec.
>
> This works fine with the v0 protocol, as the server tells us about all
> of the refs, and get_ref_map() is the ultimate decider of what we fetch.

Correct.

> But in the v2 protocol, we send the ref-prefix extension to the server,
> asking it to limit the ref advertisement. And we only tell it about the
> default refspec for the remote; we don't mention the branch.*.merge
> config at all.

Yikes.  But unfortunately it is not at all surprising that v2 is
still buggy like this.  It tries to do things in much fancier way to
"optimize" and this is an example of such.

> This usually doesn't matter, because the default refspec matches
> "refs/heads/*", which covers all branches. But if you explicitly use a
> narrow refspec, then "git pull" on some branches may fail. The server
> doesn't advertise the branch, so we don't fetch it, and "git pull"
> thinks that it went away upstream.

Nicely analysed.

> We can fix this by including any branch.*.merge entries for the current
> branch in the list of ref-prefixes we pass to the server. This only
> needs to happen when using the default configured refspec (since
> command-line refspecs are already added, and take precedence in deciding
> what we fetch). We don't otherwise need to replicate any of the "what to
> fetch" logic in get_ref_map(). These ref-prefixes are an optimization,
> so it's OK if we tell the server to advertise the branch.*.merge ref,
> even if we're not going to pull it. We'll just choose not to fetch it.

The solution does make sense.

> -	} else if (transport->remote->fetch.nr)
> -		refspec_ref_prefixes(&transport->remote->fetch,
> -				     &transport_ls_refs_options.ref_prefixes);
> +	} else {
> +		struct branch *branch = branch_get(NULL);
> +
> +		if (transport->remote->fetch.nr)
> +			refspec_ref_prefixes(&transport->remote->fetch,
> +					     &transport_ls_refs_options.ref_prefixes);

OK, this is what we add from remote.$there.fetch, just as before.

> +		if (branch_has_merge_config(branch) &&
> +		    !strcmp(branch->remote_name, transport->remote->name)) {
> +			int i;
> +			for (i = 0; i < branch->merge_nr; i++) {
> +				strvec_push(&transport_ls_refs_options.ref_prefixes,
> +					    branch->merge[i]->src);
> +			}
> +		}
> +	}

I am surprised strvec_push() is used here, not expand_ref_prefix().

refspec_ref_prefixes() takes refspec, inspects each item in it, and
munges the source side (i.e. the name the server side calls it) to
chomp at the first '*' and strvec_push the result for a pattern refspec,
or calls expand_ref_prefix(), to prefix all the possible rev-parse
dwim prefixes to given string.  So "remote.origin.fetch = a:something"
is turned into "a", which is not a pattern, and refs/a, refs/tags/a,
refs/heads/a, refs/remotes/a, and refs/remotes/a/HEAD are asked to
be advertised.

Here, branch->merge[i]->src is branch.<name>.merge for the branch
currently checked out, which is?  'master'?  'refs/heads/master'?

remote.c::branch_merge_matches() uses refname_match() on its value,
so it seems that we expect the branch name proper, without refs/heads
prefix, as its value.

So, is strvec_push() a correct thing to use here?  ref_prefixes will
receive something like 'master' here, without 'refs/heads/master'
getting pushed, when "branch.*.merge = master"?  Given that the
advertisement restriction is merely an optimization, I wouldn't be
surprised if 'master' in .ref_prefixes strvec is further expanded
by an unnecessary extra call to expand_ref_prefix() later to cause
the server side to advertise refs/heads/master and refs/tags/master
etc., but it smells, eh, bad.

>  	if (tags == TAGS_SET || tags == TAGS_DEFAULT) {
>  		must_list_refs = 1;
> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
> index 081808009b..0b72112fb1 100755
> --- a/t/t5520-pull.sh
> +++ b/t/t5520-pull.sh
> @@ -218,6 +218,23 @@ test_expect_success 'fail if upstream branch does not exist' '
>  	test_cmp expect file
>  '
>  
> +test_expect_success 'fetch upstream branch even if refspec excludes it' '
> +	# the branch names are not important here except that
> +	# the first one must not be a prefix of the second,
> +	# since otherwise the ref-prefix protocol extension
> +	# would match both
> +	git branch in-refspec HEAD^ &&
> +	git branch not-in-refspec HEAD &&
> +	git init -b in-refspec downstream &&
> +	git -C downstream remote add -t in-refspec origin "file://$(pwd)/.git" &&
> +	git -C downstream config branch.in-refspec.remote origin &&
> +	git -C downstream config branch.in-refspec.merge refs/heads/not-in-refspec &&



> +	git -C downstream pull &&
> +	git rev-parse --verify not-in-refspec >expect &&
> +	git -C downstream rev-parse --verify HEAD >actual &&
> +	test_cmp expect actual
> +'
> +
>  test_expect_success 'fail if the index has unresolved entries' '
>  	git checkout -b third second^ &&
>  	test_when_finished "git checkout -f copy && git branch -D third" &&

Re: [PATCH 2/2] fetch: add branch.*.merge to default ref-prefix extension

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> So, is strvec_push() a correct thing to use here?  ref_prefixes will
> receive something like 'master' here, without 'refs/heads/master'
> getting pushed, when "branch.*.merge = master"?  Given that the
> advertisement restriction is merely an optimization, I wouldn't be
> surprised if 'master' in .ref_prefixes strvec is further expanded
> by an unnecessary extra call to expand_ref_prefix() later to cause
> the server side to advertise refs/heads/master and refs/tags/master
> etc., but it smells, eh, bad.
>
>>  	if (tags == TAGS_SET || tags == TAGS_DEFAULT) {
>>  		must_list_refs = 1;
>> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
>> index 081808009b..0b72112fb1 100755
>> --- a/t/t5520-pull.sh
>> +++ b/t/t5520-pull.sh
>> @@ -218,6 +218,23 @@ test_expect_success 'fail if upstream branch does not exist' '
>>  	test_cmp expect file
>>  '
>>  
>> +test_expect_success 'fetch upstream branch even if refspec excludes it' '
>> +	# the branch names are not important here except that
>> +	# the first one must not be a prefix of the second,
>> +	# since otherwise the ref-prefix protocol extension
>> +	# would match both
>> +	git branch in-refspec HEAD^ &&
>> +	git branch not-in-refspec HEAD &&
>> +	git init -b in-refspec downstream &&
>> +	git -C downstream remote add -t in-refspec origin "file://$(pwd)/.git" &&
>> +	git -C downstream config branch.in-refspec.remote origin &&
>> +	git -C downstream config branch.in-refspec.merge refs/heads/not-in-refspec &&

Ah, OK, so the breakage may be the other way around.

The new code assumes that branch.<name>.merge is a full refname, and
strvec_push() is the right thing to do, when we add the knowledge
that the current branch we are on by default merges with their
refs/heads/frotz.  We just ask them to advertise refs/heads/frotz
and they do not need to advertise refs/tags/frotz etc. let alone
refs/tags/refs/heads/frotz so using expand_ref_prefix() here is
wrong.

It means that the patch claims that remote.c::branch_merge_matches()
assume that branch->merge[i]->src may not be a full refname by
calling refname_match() on it, which is incorrect and may need to be
corrected.  But that is totally outside the scope of this fix.

Thanks.

Re: [PATCH 2/2] fetch: add branch.*.merge to default ref-prefix extension

[Jeff King]

On Thu, Sep 08, 2022 at 01:48:38PM -0700, Junio C Hamano wrote:

> The new code assumes that branch.<name>.merge is a full refname, and
> strvec_push() is the right thing to do, when we add the knowledge
> that the current branch we are on by default merges with their
> refs/heads/frotz.  We just ask them to advertise refs/heads/frotz
> and they do not need to advertise refs/tags/frotz etc. let alone
> refs/tags/refs/heads/frotz so using expand_ref_prefix() here is
> wrong.

Right. When I was writing the patch I had no inkling that branch.*.merge
could ever be anything but a fully qualified ref. I don't think I've
ever seen one that isn't, and the documentation is vague. It says:

  [...]The value is handled like the remote part of a refspec, and must match
  a ref which is fetched from the remote[...]

I took "match" to mean a full string match. That text comes from
b888d61c83 (Make fetch a builtin, 2007-09-10); before that it said "the
value has exactly to match a remote part of one of the refspecs...".

But documentation aside, if we've been allowing:

  git config branch.master.merge master

to work forever, then perhaps we need to continue to support it. I
dunno.

> It means that the patch claims that remote.c::branch_merge_matches()
> assume that branch->merge[i]->src may not be a full refname by
> calling refname_match() on it, which is incorrect and may need to be
> corrected.  But that is totally outside the scope of this fix.

I make no claims. ;) I just didn't even consider a non-qualified ref to
be a possibility.

The code in fetch's add_merge_config() that does branch_merge_matches()
comes from 85682c1903 (Correct handling of branch.$name.merge in
builtin-fetch, 2007-09-18), but I don't see any indication there that
non-qualified refs were intended.

So I could either way: non-qualified refs in branch.*.merge has always
worked, and we should continue to support it. Or it was never intended
to work, and we are not obligated to continue supporting random things.

I do think "continue supporting" would probably just mean using
expand_ref_prefix() here as you suggest. It does increase the size of
our request, and the work the server has to do when it matches the
prefixes (which is inherently linear on the number of prefixes we give
it).

One thing we could do, but my patch doesn't, is skip sending this prefix
when it is a subset of the default refspec (i.e., in the default config
refs/heads/foo is already part of refs/heads/, so there is no need to
specify it separately). That doesn't really help if we expand the
prefix, though. The default refspec doesn't include:

  refs/remotes/refs/heads/foo/HEAD

which is exactly the kind of thing we'd ask for. Maybe that is all
premature optimization, though.

-Peff

Re: [PATCH 2/2] fetch: add branch.*.merge to default ref-prefix extension

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> The code in fetch's add_merge_config() that does branch_merge_matches()
> comes from 85682c1903 (Correct handling of branch.$name.merge in
> builtin-fetch, 2007-09-18), but I don't see any indication there that
> non-qualified refs were intended.
>
> So I could either way: non-qualified refs in branch.*.merge has always
> worked, and we should continue to support it. Or it was never intended
> to work, and we are not obligated to continue supporting random things.

Yeah, it looks like it was working by accident.  I do not care too
deeply about folks who edit their configuration files to futz with
branch.<name>.merge, and "checkout -t -b" and "branch -t" commands
have been recording only full refs, so it is tempting to tighten
branch_merge_matches() to only allow full refname.  The only thing
that makes me hesitate to start writing code to do so is that some
third-party tools might have taken advantage of the fact that using
a branch-name was "working" by accident.

> I do think "continue supporting" would probably just mean using
> expand_ref_prefix() here as you suggest. It does increase the size of
> our request, and the work the server has to do when it matches the
> prefixes (which is inherently linear on the number of prefixes we give
> it).

Giving extra garbage to the set of prefixes does not hurt the
correctness, but we didn't add the extra prefix for
branch.<name>.merge before this fix, so not using
expand_ref_prefix() is not breaking anybody who weren't broken
before.  So I think it may be OK to support only the full refs at
first.  It's just that folks who didn't have full refname as the
value is not helped by our fix.

If enough folks complain that they have handcrafted (or prepared by
third-party tools) branch.<name>.merge that is not a full refname,
we could switch to expand_ref_prefix() and as long as the refnames
on the remote side is not ambiguous, things will still work
correctly, but I'd prefer to keep it tight until we actually hear
complaints.

Thanks.

Re: [PATCH 2/2] fetch: add branch.*.merge to default ref-prefix extension

[Jeff King]

On Thu, Sep 08, 2022 at 10:23:43PM -0700, Junio C Hamano wrote:

> Giving extra garbage to the set of prefixes does not hurt the
> correctness, but we didn't add the extra prefix for
> branch.<name>.merge before this fix, so not using
> expand_ref_prefix() is not breaking anybody who weren't broken
> before.  So I think it may be OK to support only the full refs at
> first.  It's just that folks who didn't have full refname as the
> value is not helped by our fix.

Right. My patch is a strict improvement. I just wasn't sure if we should
go further while we are here.

> If enough folks complain that they have handcrafted (or prepared by
> third-party tools) branch.<name>.merge that is not a full refname,
> we could switch to expand_ref_prefix() and as long as the refnames
> on the remote side is not ambiguous, things will still work
> correctly, but I'd prefer to keep it tight until we actually hear
> complaints.

OK, that matches my feeling, too. So I think the series as-is should be
fine.

-Peff

Re: 2.37.2 can't "git pull" but 2.18.0 can

[Lana Deere]

I can use a workaround to continue testing 2.37.2, but I saw in a
different mail that there has already been a patch for this problem.
I'm guessing that will be in 2.37.4.  When would that be likely to be
available?

Thanks for the quick patch, by the way.

.. Lana (lana.deere@gmail.com)



On Thu, Sep 8, 2022 at 2:14 PM Jeff King <peff@peff.net> wrote:
>
> On Thu, Sep 08, 2022 at 12:46:14PM -0400, Lana Deere wrote:
>
> > With an explicit -c protocol.version=0 on the 2.37.2 git command line,
> > the pull is successful.  For what it's worth, the server git is still
> > 2.18.0 in all of these cases.  Only the client side is being tested so
> > far.  I will try to gather the packet traces and see if there's a
> > problem sharing them.  Will this mailing list allow attachments?
>
> You can send attachments to the list as long as the total mail size is
> under 100kb. But to keep the list in the loop: Lana sent me the traces
> off-list, because naturally they have a bunch of semi-private ref names.
>
> I was able to see the problem from the traces: the v2 protocol has an
> extension to tell the server to limit the advertisement only to branches
> we're interested in. And it does so based on the configured refspec. As
> Dscho noted earlier in the thread, the upstream branch you want isn't in
> the refspec. We try to add that branch explicitly to what we're
> fetching, but I think that happens too late to affect the ref-prefix
> limiting. So the server is asked not to advertise the ref, and from the
> client's perspective, it looks like the branch does not exist on the
> server.
>
> Here's a minimal reproduction:
>
>   # a server with two branches
>   git init server
>   (
>     cd server
>     git checkout -b branch1
>     git commit --allow-empty -m foo
>     git branch branch2
>   )
>
>   # and a client which points its origin there,
>   # and has local copies of both branches, tracking
>   # the upstream versions
>   git clone server client
>   cd client
>   git checkout branch1
>   git checkout branch2
>
>   # but afterwards, the client narrows its refspec to only fetch branch1
>   git config remote.origin.fetch +refs/heads/branch1:refs/remotes/origin/branch1
>
>   # pulling branch2 with v0 works
>   git -c protocol.version=0 pull
>
>   # but does not with v2, because the ref-prefix extension tells the
>   # server not to advertise anything outside of branch1
>   git -c protocol.version=2 pull
>
> This is a bug which we should fix. But in the meantime the obvious
> workaround is to expand the default refspec to cover both branches.
> Obviously the default of fetching "refs/heads/*" would work, but if you
> want to keep it limited for some reason, you can add the second branch
> explicitly. In the example above, it would be:
>
>   git config --add remote.origin.fetch +refs/heads/branch2:refs/remotes/origin/branch2
>
> -Peff

Re: 2.37.2 can't "git pull" but 2.18.0 can

[Junio C Hamano]

Lana Deere <lana.deere@gmail.com> writes:

> I can use a workaround to continue testing 2.37.2, but I saw in a
> different mail that there has already been a patch for this problem.
> I'm guessing that will be in 2.37.4.  When would that be likely to be
> available?

It is unlikely we would have 2.37.4 in the first place.

The patch was already discussed and I expect it would take a few
more days before we agree it is a good approach to take, at which
time it will be merged to 'next'.  And then it will be cooked for
about a week, before graduating to 'master', to be part of Git 2.38.

Usually topics for fixing bugs are downmerged to 'maint' and lower
after they get merged and spent at least a week or so on the
'master' branch.

But by that time, the tip of the master branch would be already
tagged as 2.38-rc0 (see https://tinyurl.com/gitCal) and it is
unlikely that we would issue more maintenance releases for the 2.37
track at that point, especially given that there are not that many
other topics to downmerge to 'maint' remaining on the master front.

You can see when Git 2.38 is scheduled to become available from the
same calendar.

Thanks.

Re: 2.37.2 can't "git pull" but 2.18.0 can

[Lana Deere]

Thanks for the info.  That sounds kind of like this fix, even if
accepted, would miss rc0 but would be not unlikely to make rc1.  Is
there any way to "watch" the issue so I can know which git release
gets the fix?  I would like to be an early adopter of that release
since I would want to test the change works for me.

Thanks.

.. Lana (lana.deere@gmail.com)



On Fri, Sep 9, 2022 at 2:27 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Lana Deere <lana.deere@gmail.com> writes:
>
> > I can use a workaround to continue testing 2.37.2, but I saw in a
> > different mail that there has already been a patch for this problem.
> > I'm guessing that will be in 2.37.4.  When would that be likely to be
> > available?
>
> It is unlikely we would have 2.37.4 in the first place.
>
> The patch was already discussed and I expect it would take a few
> more days before we agree it is a good approach to take, at which
> time it will be merged to 'next'.  And then it will be cooked for
> about a week, before graduating to 'master', to be part of Git 2.38.
>
> Usually topics for fixing bugs are downmerged to 'maint' and lower
> after they get merged and spent at least a week or so on the
> 'master' branch.
>
> But by that time, the tip of the master branch would be already
> tagged as 2.38-rc0 (see https://tinyurl.com/gitCal) and it is
> unlikely that we would issue more maintenance releases for the 2.37
> track at that point, especially given that there are not that many
> other topics to downmerge to 'maint' remaining on the master front.
>
> You can see when Git 2.38 is scheduled to become available from the
> same calendar.
>
> Thanks.

Re: 2.37.2 can't "git pull" but 2.18.0 can

[Jeff King]

On Mon, Sep 12, 2022 at 10:58:53AM -0400, Lana Deere wrote:

> Thanks for the info.  That sounds kind of like this fix, even if
> accepted, would miss rc0 but would be not unlikely to make rc1.  Is
> there any way to "watch" the issue so I can know which git release
> gets the fix?  I would like to be an early adopter of that release
> since I would want to test the change works for me.

Thanks. Early testing, especially of release candidates, is very
welcome. There's no tracking for a specific issue, but you can check the
status of the commit. The fix has been merged to 'next' now, so the
commit id will remain stable. You can periodically fetch any of the
usual Git mirrors[1] and check:

  git tag --contains 49ca2fba393fa277ab70253337c53c7831597c3a

Likewise, you can see when it hits master with "git branch -a
--contains".

You can also see the progress of topics in Junio's "What's cooking"
emails. If you're not a regular list reader, you can hit the recent ones
here:

  https://lore.kernel.org/git/?q=f%3Ajunio+s%3Acooking

Likewise, when -rc0, etc, are released, the release notes should mention
(or not) this fix. You can see those announcements in the archive like
this:

  https://lore.kernel.org/git/?q=f%3Ajunio+t%3Avger+s%3Aannounce

-Peff

[1] e.g., https://git.kernel.org/pub/scm/git/git.git/

Re: 2.37.2 can't "git pull" but 2.18.0 can

[Johannes Schindelin]

Hi Lana,

On Fri, 2 Sep 2022, Lana Deere wrote:

> I'm testing an upgrade to git 2.37.2 from the current version we're
> using of 2.18.0.  When I try to pull in my development tree, 2.37.2
> gives me an error but 2.18.0 things all is fine:
>
> $ /tools/linux-x86_64/git/2.37.2/bin/git pull
> Your configuration specifies to merge with the ref
> 'refs/heads/feature/switch-to-qt5'
> from the remote, but no such ref was fetched.

I bet this means that that `switch-to-qt5` branch was deleted on the
remote side, in which case...

> $ /tools/linux-x86_64/git/2.18.0/bin/git pull
> From http://githost:7990/scm/dp/sw
>  * branch                  feature/switch-to-qt5 -> FETCH_HEAD
> Already up to date.

... Git would lie here and simply use the locally-cached version of the
last successfully-fetched `switch-to-qt5` branch.

To test this, you could pass the `--prune` option to `pull` (see
https://git-scm.com/docs/git-pull#Documentation/git-pull.txt---prune for
details), then try both pulls, and I bet _both_ will now fail.

Ciao,
Johannes

> Anyone have any ideas about this?  All I could find on google was a
> suggestion that the "no such ref" message indicates the remote branch
> was deleted, but that's not the case here.
>
>
> .. Lana (lana.deere@gmail.com)
>

Re: 2.37.2 can't "git pull" but 2.18.0 can

[Lana Deere]

With --prune set, the 2.18.0 pull still works but the 2.37.2 pull
still fails.  There are several of us sharing this branch and we
aren't having any issues pushing or pulling with 2.18.0.

.. Lana (lana.deere@gmail.com)



On Mon, Sep 5, 2022 at 6:25 AM Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
>
> Hi Lana,
>
> On Fri, 2 Sep 2022, Lana Deere wrote:
>
> > I'm testing an upgrade to git 2.37.2 from the current version we're
> > using of 2.18.0.  When I try to pull in my development tree, 2.37.2
> > gives me an error but 2.18.0 things all is fine:
> >
> > $ /tools/linux-x86_64/git/2.37.2/bin/git pull
> > Your configuration specifies to merge with the ref
> > 'refs/heads/feature/switch-to-qt5'
> > from the remote, but no such ref was fetched.
>
> I bet this means that that `switch-to-qt5` branch was deleted on the
> remote side, in which case...
>
> > $ /tools/linux-x86_64/git/2.18.0/bin/git pull
> > From http://githost:7990/scm/dp/sw
> >  * branch                  feature/switch-to-qt5 -> FETCH_HEAD
> > Already up to date.
>
> ... Git would lie here and simply use the locally-cached version of the
> last successfully-fetched `switch-to-qt5` branch.
>
> To test this, you could pass the `--prune` option to `pull` (see
> https://git-scm.com/docs/git-pull#Documentation/git-pull.txt---prune for
> details), then try both pulls, and I bet _both_ will now fail.
>
> Ciao,
> Johannes
>
> > Anyone have any ideas about this?  All I could find on google was a
> > suggestion that the "no such ref" message indicates the remote branch
> > was deleted, but that's not the case here.
> >
> >
> > .. Lana (lana.deere@gmail.com)
> >

Re: 2.37.2 can't "git pull" but 2.18.0 can

[Johannes Schindelin]

Hi Lana,

Please find my reply inline.

On Tue, 6 Sep 2022, Lana Deere wrote:

> With --prune set, the 2.18.0 pull still works but the 2.37.2 pull
> still fails.  There are several of us sharing this branch and we
> aren't having any issues pushing or pulling with 2.18.0.

Oh, so there _is_ a remote branch called `feature/switch-to-qt5`? What
happens if you call `git pull origin feature/switch-to-qt5` explicitly?

Ciao,
Johannes

>
> .. Lana (lana.deere@gmail.com)
>
>
>
> On Mon, Sep 5, 2022 at 6:25 AM Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> >
> > Hi Lana,
> >
> > On Fri, 2 Sep 2022, Lana Deere wrote:
> >
> > > I'm testing an upgrade to git 2.37.2 from the current version we're
> > > using of 2.18.0.  When I try to pull in my development tree, 2.37.2
> > > gives me an error but 2.18.0 things all is fine:
> > >
> > > $ /tools/linux-x86_64/git/2.37.2/bin/git pull
> > > Your configuration specifies to merge with the ref
> > > 'refs/heads/feature/switch-to-qt5'
> > > from the remote, but no such ref was fetched.
> >
> > I bet this means that that `switch-to-qt5` branch was deleted on the
> > remote side, in which case...
> >
> > > $ /tools/linux-x86_64/git/2.18.0/bin/git pull
> > > From http://githost:7990/scm/dp/sw
> > >  * branch                  feature/switch-to-qt5 -> FETCH_HEAD
> > > Already up to date.
> >
> > ... Git would lie here and simply use the locally-cached version of the
> > last successfully-fetched `switch-to-qt5` branch.
> >
> > To test this, you could pass the `--prune` option to `pull` (see
> > https://git-scm.com/docs/git-pull#Documentation/git-pull.txt---prune for
> > details), then try both pulls, and I bet _both_ will now fail.
> >
> > Ciao,
> > Johannes
> >
> > > Anyone have any ideas about this?  All I could find on google was a
> > > suggestion that the "no such ref" message indicates the remote branch
> > > was deleted, but that's not the case here.
> > >
> > >
> > > .. Lana (lana.deere@gmail.com)
> > >
>

Re: 2.37.2 can't "git pull" but 2.18.0 can

[Lana Deere]

> What
> happens if you call `git pull origin feature/switch-to-qt5` explicitly?

$ /re/tools/linux-x86_64/opensrc/git/2.37.2/bin/git pull origin
feature/switch-to-qt5
From http://githost:7990/scm/dp/d2s_sw
 * branch                  feature/switch-to-qt5 -> FETCH_HEAD
Already up to date.

In that case, it works.  But subsequently trying without the explicit
specification still fails.

.. Lana (lana.deere@gmail.com)


On Wed, Sep 7, 2022 at 6:21 AM Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
>
> Hi Lana,
>
> Please find my reply inline.
>
> On Tue, 6 Sep 2022, Lana Deere wrote:
>
> > With --prune set, the 2.18.0 pull still works but the 2.37.2 pull
> > still fails.  There are several of us sharing this branch and we
> > aren't having any issues pushing or pulling with 2.18.0.
>
> Oh, so there _is_ a remote branch called `feature/switch-to-qt5`? What
> happens if you call `git pull origin feature/switch-to-qt5` explicitly?
>
> Ciao,
> Johannes
>
> >
> > .. Lana (lana.deere@gmail.com)
> >
> >
> >
> > On Mon, Sep 5, 2022 at 6:25 AM Johannes Schindelin
> > <Johannes.Schindelin@gmx.de> wrote:
> > >
> > > Hi Lana,
> > >
> > > On Fri, 2 Sep 2022, Lana Deere wrote:
> > >
> > > > I'm testing an upgrade to git 2.37.2 from the current version we're
> > > > using of 2.18.0.  When I try to pull in my development tree, 2.37.2
> > > > gives me an error but 2.18.0 things all is fine:
> > > >
> > > > $ /tools/linux-x86_64/git/2.37.2/bin/git pull
> > > > Your configuration specifies to merge with the ref
> > > > 'refs/heads/feature/switch-to-qt5'
> > > > from the remote, but no such ref was fetched.
> > >
> > > I bet this means that that `switch-to-qt5` branch was deleted on the
> > > remote side, in which case...
> > >
> > > > $ /tools/linux-x86_64/git/2.18.0/bin/git pull
> > > > From http://githost:7990/scm/dp/sw
> > > >  * branch                  feature/switch-to-qt5 -> FETCH_HEAD
> > > > Already up to date.
> > >
> > > ... Git would lie here and simply use the locally-cached version of the
> > > last successfully-fetched `switch-to-qt5` branch.
> > >
> > > To test this, you could pass the `--prune` option to `pull` (see
> > > https://git-scm.com/docs/git-pull#Documentation/git-pull.txt---prune for
> > > details), then try both pulls, and I bet _both_ will now fail.
> > >
> > > Ciao,
> > > Johannes
> > >
> > > > Anyone have any ideas about this?  All I could find on google was a
> > > > suggestion that the "no such ref" message indicates the remote branch
> > > > was deleted, but that's not the case here.
> > > >
> > > >
> > > > .. Lana (lana.deere@gmail.com)
> > > >
> >