Re: [PATCH] clone: check if server supports shallow clones

From: Jeff King <peff@peff.net>
Date: Jun 10, 2015, 20:56 UTC
Message-ID: <20150610205607.GA25842@peff.net>
In-Reply-To: <CALkbG6+Xqw+giW3msUKYx4odFv+2GAi=C2t2d=6VjuFp0xpO0w@mail.gmail.com>

On Wed, Jun 10, 2015 at 04:25:45PM -0400, Michael Edgar wrote:

Show 14 quoted lines

> > On Wed, Jun 10, 2015 at 3:05 PM, Jeff King <peff@peff.net> 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?
> > -Peff
> 
> Great question. I determined that the do_fetch_pack logic doesn't work for
> clones because it also checks is_repository_shallow(), which looks for the
> .git/shallow file (or the alternate file).
> 
> I considered changing clone to create an empty .git/shallow file or alternate,
> but it turns out that an empty shallow file is treated as no shallow file at
> all. Since at this stage in cloning we have nothing to put in the shallow file,
> it seemed like any other fix would require more substantial changes.

Hmm. Can we improve the check in do_fetch_pack? That is, do we have a way of checking whether the _current_ fetch is going to make us shallow? I don't see anything obvious (like a "depth" flag) passed into do_fetch_pack, so I guess it is relying solely on global shallow state that we've already set up? Or is it the case that "git fetch --depth=" in a non-shallow repository does not properly check this flag?

If it turns out that "fetch --depth" is fine, and only "clone --depth" is broken, I can certainly live with the patch you provided. But I think we need to clarify that a bit in the commit message.

-Peff