[PATCH v2] fetch-pack: check for shallow if depth given

[PATCH v2] fetch-pack: check for shallow if depth given

[Mike Edgar]

When a repository is first fetched as a shallow clone, either by
git-clone or by fetching into an empty repo, the server's capabilities
are not currently consulted. The client will send shallow requests even
if the server does not understand them, and the resulting error may be
unhelpful to the user. This change pre-emptively checks so we can exit
with a helpful error if necessary.

Signed-off-by: Mike Edgar <adgar@google.com>
Acked-by: Jeff King <peff@peff.net>
---
 fetch-pack.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fetch-pack.c b/fetch-pack.c
index 48526aa..849a9d6 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -790,7 +790,7 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,
 	sort_ref_list(&ref, ref_compare_name);
 	qsort(sought, nr_sought, sizeof(*sought), cmp_ref_by_name);
 
-	if (is_repository_shallow() && !server_supports("shallow"))
+	if ((args->depth > 0 || is_repository_shallow()) && !server_supports("shallow"))
 		die("Server does not support shallow clients");
 	if (server_supports("multi_ack_detailed")) {
 		if (args->verbose)
-- 
2.4.3.573.g4eafbef

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

[Junio C Hamano]

Michael Edgar <adgar@google.com> writes:

> On Thu, Jun 11, 2015 at 10:32 AM, Jeff King <peff@peff.net> wrote:
>> 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.
>
> Shall I send that out as a revised patch for review? (This is my first
> stab at a Git
> patch)

Surely, and thanks.  Hopefully one or both of them will Ack it and
all there is left for me to do will be to pick it up ;-)

[PATCH v2] fetch-pack: check for shallow if depth given

[Mike Edgar]

When a repository is first fetched as a shallow clone, either by
git-clone or by fetching into an empty repo, the server's capabilities
are not currently consulted. The client will send shallow requests even
if the server does not understand them, and the resulting error may be
unhelpful to the user. This change pre-emptively checks so we can exit
with a helpful error if necessary.

Signed-off-by: Mike Edgar <adgar@google.com>
---
 fetch-pack.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fetch-pack.c b/fetch-pack.c
index a912935..a136772 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -809,7 +809,7 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,
 	sort_ref_list(&ref, ref_compare_name);
 	qsort(sought, nr_sought, sizeof(*sought), cmp_ref_by_name);
 
-	if (is_repository_shallow() && !server_supports("shallow"))
+	if ((args->depth > 0 || is_repository_shallow()) && !server_supports("shallow"))
 		die("Server does not support shallow clients");
 	if (server_supports("multi_ack_detailed")) {
 		if (args->verbose)
-- 
2.2.0.rc0.207.ga3a616c

Re: [PATCH v2] fetch-pack: check for shallow if depth given

[Jeff King]

On Wed, Jun 17, 2015 at 07:48:14AM -0400, Mike Edgar wrote:

> When a repository is first fetched as a shallow clone, either by
> git-clone or by fetching into an empty repo, the server's capabilities
> are not currently consulted. The client will send shallow requests even
> if the server does not understand them, and the resulting error may be
> unhelpful to the user. This change pre-emptively checks so we can exit
> with a helpful error if necessary.
> 
> Signed-off-by: Mike Edgar <adgar@google.com>

Looks good to me.

-Peff