[BUG] shallow clones over http

[BUG] shallow clones over http

[Jeff King]

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

I'm trying to track down a protocol bug that happens with shallow clones
over smart-http. As far as I can tell, the bug has existed in all
versions.

You can reproduce it using the attached repository, which is a shallow
clone of https://github.com/mileszs/ack.vim.git, like:

  $ tar xzf repo.tar.gz
  $ cd repo.git
  $ git fetch --depth=10
  fatal: git fetch-pack: expected shallow list

In that test my fetch actually hit github.com as the upstream full repo,
but you can also clone it down locally and demonstrate it with purely
local copies of git (but it's more of a pain, because you have to set up
a smart http server).

The last part of the conversation looks like this:

  packet:   fetch-pack< 0000
  packet:   fetch-pack< ACK f183a345a0c10caed7684d07dabae33e007c7590 common
  packet:   fetch-pack> have f183a345a0c10caed7684d07dabae33e007c7590
  packet:   fetch-pack< ACK 33312d4db4e91468957b1b41dd039c5d88e85fda common
  packet:   fetch-pack< ACK 34d0b2fbc182b31d926632d170bc07d6a6fc3f9b common
  packet:   fetch-pack< ACK 45c802e07c60686986474b6b05b2c7048330b6b5 common
  packet:   fetch-pack< ACK e93f693fd2a9940d6421bf9e4ddd1f535994eaa5 common
  packet:   fetch-pack< ACK 132ee41e8e2c8c545b3aed120171e1596c9211a4 common
  packet:   fetch-pack< ACK 973deb3145a99992638b2301cfd654721cf35d68 common
  packet:   fetch-pack< ACK e53a88a4e72d84562493313e8911ada4def787da common
  packet:   fetch-pack< ACK 90be0bf3eee6f7a0cb9c2377a50610f4ce738da3 common
  packet:   fetch-pack< ACK aeab88ccf41bf216fde37983bd403d9b913391e7 common
  packet:   fetch-pack< ACK 5f480935d3ce431c393657c3000337bcbdbd5535 common
  packet:   fetch-pack< ACK db81e01b433501b159983ea38690aeb01eea1e6b common
  packet:   fetch-pack< ACK 06c44b8cab93e780a29ff7f7b5b1dd41dba4b2d5 common
  packet:   fetch-pack< ACK 65f3966becdb2d931d5afbdcc6a28008d154668a common
  packet:   fetch-pack< ACK 10e8caef9f2ed308231ce1abc326c512e86a5d4c common
  packet:   fetch-pack< ACK 6b55dd91f2e7fc64c23eea57e85171cb958f9cd2 common
  packet:   fetch-pack< ACK 6b55dd91f2e7fc64c23eea57e85171cb958f9cd2 ready
  packet:   fetch-pack< NAK
  packet:   fetch-pack< ACK 6b55dd91f2e7fc64c23eea57e85171cb958f9cd2
  fatal: git fetch-pack: expected shallow list

So we see that upload-pack sends a bunch of detailed ACKs, followed by a
NAK, and then it sends another ACK.

Fetch-pack is inside find_common, reading these acks. At the beginning
of each stateless-rpc response, it expects to consume any
shallow/unshallow lines up to a flush packet (the call to
consume_shallow_list). And then it reads the acks in a loop. After it
sees the NAK, it assumes that the server is done sending the packet, and
loops again, expecting another set of shallow/unshallow lines. But we
get the next ACK instead, and die.

So who is wrong? Is upload-pack wrong to send an ACK after the NAK? From
the protocol description, it seems like it should wait after sending a
NAK. Or is fetch-pack wrong not to handle the extra ACK without
receiving another shallow/unshallow segment?

-Peff

[-- Attachment #2: repo.tar.gz --]
[-- Type: application/octet-stream, Size: 16245 bytes --]

Re: [BUG] shallow clones over http

[Tom]

Jeff King <peff <at> peff.net> writes:

> 
> I'm trying to track down a protocol bug that happens with shallow clones
> over smart-http. As far as I can tell, the bug has existed in all
> versions.
> 
> You can reproduce it using the attached repository, which is a shallow
> clone of https://github.com/mileszs/ack.vim.git, like:
> 
>   $ tar xzf repo.tar.gz
>   $ cd repo.git
>   $ git fetch --depth=10
>   fatal: git fetch-pack: expected shallow list

The problem occurs to me also when I want to "deepen" a shallow clone of
MediaWiki via https://

 git clone --depth 1 https://gerrit.wikimedia.org/r/p/mediawiki/core.git
 git pull --depth=99999

fatal: git fetch-pack: expected shallow list.

Perhaps it helps someone to find the reason.

Re: [BUG] shallow clones over http

[Ramkumar Ramachandra]

Tom wrote:
> git pull --depth=99999

Unrelated nit: we now have git fetch --unshallow.

Re: [BUG] shallow clones over http

[Tom]

The problem occurs to me also when I want to "deepen" a shallow clone of
MediaWiki via https://

 git clone --depth 1 https://gerrit.wikimedia.org/r/p/mediawiki/core.git
 git pull --depth=99999

fatal: git fetch-pack: expected shallow list.

Perhaps it helps someone to find the reason.

UPDATE:

This however works

 git clone --depth=2 https://gerrit.wikimedia.org/r/p/mediawiki/core.git
 git fetch --depth=5

Re: [BUG] shallow clones over http

[Jeff King]

On Wed, Apr 10, 2013 at 12:48:51PM +0000, Tom wrote:

> The problem occurs to me also when I want to "deepen" a shallow clone of
> MediaWiki via https://
> 
>  git clone --depth 1 https://gerrit.wikimedia.org/r/p/mediawiki/core.git
>  git pull --depth=99999
> 
> fatal: git fetch-pack: expected shallow list.
> 
> Perhaps it helps someone to find the reason.
> 
> UPDATE:
> 
> This however works
> 
>  git clone --depth=2 https://gerrit.wikimedia.org/r/p/mediawiki/core.git
>  git fetch --depth=5

Yes, I experienced the same thing. I don't think the problem is related
to the depth per se, but rather the particular pattern of ACKs that
upload-pack sends based on which commits are being selected.

-Peff

Re: [BUG] shallow clones over http

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> I'm trying to track down a protocol bug that happens with shallow clones
> over smart-http. As far as I can tell, the bug has existed in all
> versions.
>
> You can reproduce it using the attached repository, which is a shallow
> clone of https://github.com/mileszs/ack.vim.git, like:
>
>   $ tar xzf repo.tar.gz
>   $ cd repo.git
>   $ git fetch --depth=10
>   fatal: git fetch-pack: expected shallow list
>
> In that test my fetch actually hit github.com as the upstream full repo,
> but you can also clone it down locally and demonstrate it with purely
> local copies of git (but it's more of a pain, because you have to set up
> a smart http server).
>
> The last part of the conversation looks like this:
>
>   packet:   fetch-pack< 0000
>   packet:   fetch-pack< ACK f183a345a0c10caed7684d07dabae33e007c7590 common
>   packet:   fetch-pack> have f183a345a0c10caed7684d07dabae33e007c7590
>   packet:   fetch-pack< ACK 33312d4db4e91468957b1b41dd039c5d88e85fda common
>   packet:   fetch-pack< ACK 34d0b2fbc182b31d926632d170bc07d6a6fc3f9b common
>   packet:   fetch-pack< ACK 45c802e07c60686986474b6b05b2c7048330b6b5 common
>   packet:   fetch-pack< ACK e93f693fd2a9940d6421bf9e4ddd1f535994eaa5 common
>   packet:   fetch-pack< ACK 132ee41e8e2c8c545b3aed120171e1596c9211a4 common
>   packet:   fetch-pack< ACK 973deb3145a99992638b2301cfd654721cf35d68 common
>   packet:   fetch-pack< ACK e53a88a4e72d84562493313e8911ada4def787da common
>   packet:   fetch-pack< ACK 90be0bf3eee6f7a0cb9c2377a50610f4ce738da3 common
>   packet:   fetch-pack< ACK aeab88ccf41bf216fde37983bd403d9b913391e7 common
>   packet:   fetch-pack< ACK 5f480935d3ce431c393657c3000337bcbdbd5535 common
>   packet:   fetch-pack< ACK db81e01b433501b159983ea38690aeb01eea1e6b common
>   packet:   fetch-pack< ACK 06c44b8cab93e780a29ff7f7b5b1dd41dba4b2d5 common
>   packet:   fetch-pack< ACK 65f3966becdb2d931d5afbdcc6a28008d154668a common
>   packet:   fetch-pack< ACK 10e8caef9f2ed308231ce1abc326c512e86a5d4c common
>   packet:   fetch-pack< ACK 6b55dd91f2e7fc64c23eea57e85171cb958f9cd2 common
>   packet:   fetch-pack< ACK 6b55dd91f2e7fc64c23eea57e85171cb958f9cd2 ready
>   packet:   fetch-pack< NAK
>   packet:   fetch-pack< ACK 6b55dd91f2e7fc64c23eea57e85171cb958f9cd2
>   fatal: git fetch-pack: expected shallow list
>
> So we see that upload-pack sends a bunch of detailed ACKs, followed by a
> NAK, and then it sends another ACK.
>
> Fetch-pack is inside find_common, reading these acks. At the beginning
> of each stateless-rpc response, it expects to consume any
> shallow/unshallow lines up to a flush packet (the call to
> consume_shallow_list). And then it reads the acks in a loop. After it
> sees the NAK, it assumes that the server is done sending the packet, and
> loops again, expecting another set of shallow/unshallow lines. But we
> get the next ACK instead, and die.
>
> So who is wrong? Is upload-pack wrong to send an ACK after the NAK?

3e63b21aced1 (upload-pack: Implement no-done capability, 2011-03-14)
claims that the above sequence of acks and naks is what upload-pack
wants to show.

What happens when you disable no-done extension handling on the
server end, I wonder?

Re: [BUG] shallow clones over http

[Duy Nguyen]

(Digging back an old topic after Jeff mentioned it)

On Thu, May 9, 2013 at 2:12 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Jeff King <peff@peff.net> writes:
>
>> I'm trying to track down a protocol bug that happens with shallow clones
>> over smart-http. As far as I can tell, the bug has existed in all
>> versions.
>>
>> You can reproduce it using the attached repository, which is a shallow
>> clone of https://github.com/mileszs/ack.vim.git, like:
>>
>>   $ tar xzf repo.tar.gz
>>   $ cd repo.git
>>   $ git fetch --depth=10
>>   fatal: git fetch-pack: expected shallow list
>>
>> In that test my fetch actually hit github.com as the upstream full repo,
>> but you can also clone it down locally and demonstrate it with purely
>> local copies of git (but it's more of a pain, because you have to set up
>> a smart http server).
>>
>> The last part of the conversation looks like this:
>>
>>   packet:   fetch-pack< 0000
>>   packet:   fetch-pack< ACK f183a345a0c10caed7684d07dabae33e007c7590 common
>>   packet:   fetch-pack> have f183a345a0c10caed7684d07dabae33e007c7590
>>   packet:   fetch-pack< ACK 33312d4db4e91468957b1b41dd039c5d88e85fda common
>>   packet:   fetch-pack< ACK 34d0b2fbc182b31d926632d170bc07d6a6fc3f9b common
>>   packet:   fetch-pack< ACK 45c802e07c60686986474b6b05b2c7048330b6b5 common
>>   packet:   fetch-pack< ACK e93f693fd2a9940d6421bf9e4ddd1f535994eaa5 common
>>   packet:   fetch-pack< ACK 132ee41e8e2c8c545b3aed120171e1596c9211a4 common
>>   packet:   fetch-pack< ACK 973deb3145a99992638b2301cfd654721cf35d68 common
>>   packet:   fetch-pack< ACK e53a88a4e72d84562493313e8911ada4def787da common
>>   packet:   fetch-pack< ACK 90be0bf3eee6f7a0cb9c2377a50610f4ce738da3 common
>>   packet:   fetch-pack< ACK aeab88ccf41bf216fde37983bd403d9b913391e7 common
>>   packet:   fetch-pack< ACK 5f480935d3ce431c393657c3000337bcbdbd5535 common
>>   packet:   fetch-pack< ACK db81e01b433501b159983ea38690aeb01eea1e6b common
>>   packet:   fetch-pack< ACK 06c44b8cab93e780a29ff7f7b5b1dd41dba4b2d5 common
>>   packet:   fetch-pack< ACK 65f3966becdb2d931d5afbdcc6a28008d154668a common
>>   packet:   fetch-pack< ACK 10e8caef9f2ed308231ce1abc326c512e86a5d4c common
>>   packet:   fetch-pack< ACK 6b55dd91f2e7fc64c23eea57e85171cb958f9cd2 common
>>   packet:   fetch-pack< ACK 6b55dd91f2e7fc64c23eea57e85171cb958f9cd2 ready
>>   packet:   fetch-pack< NAK
>>   packet:   fetch-pack< ACK 6b55dd91f2e7fc64c23eea57e85171cb958f9cd2
>>   fatal: git fetch-pack: expected shallow list
>>
>> So we see that upload-pack sends a bunch of detailed ACKs, followed by a
>> NAK, and then it sends another ACK.
>>
>> Fetch-pack is inside find_common, reading these acks. At the beginning
>> of each stateless-rpc response, it expects to consume any
>> shallow/unshallow lines up to a flush packet (the call to
>> consume_shallow_list). And then it reads the acks in a loop. After it
>> sees the NAK, it assumes that the server is done sending the packet, and
>> loops again, expecting another set of shallow/unshallow lines. But we
>> get the next ACK instead, and die.
>>
>> So who is wrong? Is upload-pack wrong to send an ACK after the NAK?
>
> 3e63b21aced1 (upload-pack: Implement no-done capability, 2011-03-14)
> claims that the above sequence of acks and naks is what upload-pack
> wants to show.
>
> What happens when you disable no-done extension handling on the
> server end, I wonder?

fetch succeeded when no-done was disabled. An immediate workaround
would be disable no-done in fetch-pack.c in a shallow repo but maybe
we can do better..
-- 
Duy