Git wire protocol v2 fails fetching shallow changes with `pack has XXX unresolved deltas` on large repos with lots of tags Inbox x

Hello,

On large repositories with lots of tags - git wire protocol v2 fails
to fetch shallow changes, it fails with error `pack has XXX unresolved
deltas`. Unfortunately I didn't find easy way to reproduce it except
cloning+fetching chromium repository, the way jenkins does.
Reproduction steps:
```
$ git clone --mirror https://chromium.googlesource.com/chromium/src chromium
Cloning into bare repository 'chromium'...
remote: Sending approximately 12.66 GiB ...
remote: Counting objects: 8523271, done
remote: Finding sources: 100% (8399287/8399287)
remote: Total 19829104 (delta 11897257), reused 19828780 (delta 11897257)
Receiving objects: 100% (19829104/19829104), 20.45 GiB | 29.17 MiB/s, done.
Resolving deltas: 100% (11897257/11897257), done
$ mkdir chromium-test
$ cd chromium-test
$ git init
Initialized empty Git repository in /home/arturas/.git/
$ git -c protocol.version=2 fetch --tags file://$(pwd)/../chromium/
+refs/heads/*:refs/remotes/origin/* --depth=1
remote: Enumerating objects: 4614683, done.
remote: Counting objects: 100% (4614683/4614683), done.
remote: Compressing objects: 100% (1556691/1556691), done.
Receiving objects: 100% (4614683/4614683), 8.29 GiB | 24.71 MiB/s, done.
remote: Total 4614683 (delta 3470249), reused 3989191 (delta 2923800)
Resolving deltas: 100% (3470249/3470249), done.
From file:///home/arturas/chromium-test/../chromium
 * [new branch]                ignore/bar      -> origin/ignore/bar
 * [new branch]                ignore/foo      -> origin/ignore/foo
 * [new branch]                infra/config    -> origin/infra/config
 * [new branch]                lkgr            -> origin/lkgr
 * [new branch]                master          -> origin/master
 * [new tag]                   10.0.601.0      -> 10.0.601.0
...
$ git -c protocol.version=2 fetch --tags file://$(pwd)/../chromium/
+refs/heads/*:refs/remotes/origin/* --depth=1
remote: Enumerating objects: 642969, done.
remote: Counting objects: 100% (588314/588314), done.
remote: Compressing objects: 100% (240564/240564), done.
Receiving objects: 100% (571353/571353), 967.83 MiB | 10.52 MiB/s, done.
remote: Total 571353 (delta 372206), reused 497895 (delta 310113)
Resolving deltas:  99% (371726/372206), completed with 5582 local objects.
fatal: pack has 480 unresolved deltas
fatal: index-pack failed
$ git --version
git version 2.19.1
```
No changes are needed in mirrored repository. Crash happens both with
2.18.0 and 2.19.1 git versions. Having repository locally is not
required but reduces test runtime, you can quite reliably reproduce
issue when fetching over net directly from chromium.orgbypassing
mirroring step.

-- Arturas Moskvinas

[snip]

Thanks for your bug report and reproduction steps. I managed to reproduce your issue and took a look.

The main issue seems to be that with v2, upload-pack doesn't pass "--shallow X" to pack-objects (the write_one_shallow() callback is never called, even if I change the "if (shallow_nr)" to "if (1)"), so pack-objects probably doesn't know that some objects cannot be used as delta bases. (With v0, write_one_shallow() is indeed called.) The issue probably lies in how v0 and v2 handle client-provided shallows differently.

There also seems to be another issue in that negotiation occurs differently in these 2 protocols - I see the full list of "have" lines being sent in the final request to the server in v0, but a very limited list in v2. This might be because of the ref prefix limiting in v2, but I haven't fully investigated it.

I'll look some more into this.

Do you have the reproduction steps for this? If I run

  git -c protocol.version=2 fetch --tags \
    https://chromium.googlesource.com/chromium/src \
    +refs/heads/*:refs/remotes/origin/* --depth=1

repeatedly in the same repository, it succeeds. (And I've checked with GIT_TRACE_PACKET that it uses protocol v2.)

Hello,

Unfortunately I do not have for such step. On one of our internal repositories it is failing but unreliably when fetching from remote.

-- Arturas Moskvinas

On Wed, Oct 17, 2018 at 11:53 PM Jonathan Tan <jonathantanmy@google.com> wrote: