Re: [PATCH] clone: check if server supports shallow clones
- From
Michael Edgar <adgar@google.com>
- Date
- Jun 11, 2015, 18:18 UTC
- Message-ID
- <CALkbG6+vRqKwmBjK9jwSph4iUUbqPjiuNdHEeT3nvFhFJ0VpEg@mail.gmail.com>
- In-Reply-To
- <20150611143204.GA3343@peff.net>
On Thu, Jun 11, 2015 at 10:32 AM, Jeff King <peff@peff.net> wrote:
Show 21 quoted lines
> On Thu, Jun 11, 2015 at 08:02:33PM +0700, Duy Nguyen wrote:
>
>> > I see that do_fetch_pack checks server_supports("shallow"). Is that
>> > enough to cover all fetch cases? And if it is, why does it not cover the
>> > matching clone cases?
>>
>> I think this replacement check would do
>>
>> if ((args->depth > 0 || is_repository_shallow()) && !server_supports("shallow"))
>> die("Server does not support shallow clients");
>
> Oh, indeed, there is the depth flag I was looking for. :)
>
> And from some rudimentary testing, I believe that:
>
> git init
> git fetch --depth=1 ...
>
> is currently broken in the same way as clone (we are not shallow yet, so
> it does not complain when the server does not support it). I think the
> patch above fixes both that and the clone case.Great point! Duy's proposed fix worked for me testing against the Git server that led me to the original bug (code.google.com), for both clones and init/fetch.
Shall I send that out as a revised patch for review? (This is my first stab at a Git patch)
> Of course it's hard to add to the test suite, since we do not have a way > of hitting a server that does not understand shallow (I simply fudged > server_supports() to return false on the client).
I noticed this, and figured it would be more dangerous for users to include a testing backdoor (eg a flag overriding the advertised capabilities). Is that an accurate assessment, or is there a safer way to test these sorts of interop cases?
Cheers!
-- Michael Edgar | Software Engineer | adgar@google.com