Re: [PATCHv3] transport: Catch non positive --depth option value

On Fri, Nov 22, 2013 at 3:18 AM, Junio C Hamano [off-list ref] wrote:

"Andrés G. Aragoneses" [off-list ref] writes:

quoted

From 99e387151594572dc136bf1fae45593ee710e817 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andr=C3=A9s=20G=2E=20Aragoneses?= <redacted>
Date: Wed, 13 Nov 2013 16:55:08 +0100
Subject: [PATCH] transport: Catch non positive --depth option value

Instead of simply ignoring the value passed to --depth
option when it is zero or negative, now it is caught
and reported.

This will let people know that they were using the
option incorrectly (as depth<0 should be simply invalid,
and under the hood depth==0 didn't have any effect).

Signed-off-by: Andres G. Aragoneses <redacted>
Reviewed-by: Duy Nguyen <redacted>
Reviewed-by: Junio C Hamano <redacted>

I didn't exactly "review" this.

Have you run the tests with this patch?  It seems that it breaks
quite a lot of them, including t5500, t5503, t5510, among others.

I guess it's caused by builtin/fetch.c:backfill_tags(). And the call
could be replaced with

transport_set_option(transport, TRANS_OPT_DEPTH, NULL);

quoted

---
 transport.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/transport.c b/transport.c
index 7202b77..edd63eb 100644
--- a/transport.c
+++ b/transport.c

@@ -483,6 +483,8 @@ static int set_git_option(struct git_transport_options *opts,
                      opts->depth = strtol(value, &end, 0);
                      if (*end)
                              die("transport: invalid depth option '%s'", value);
+                     if (opts->depth < 1)
+                             die("transport: invalid depth option '%s' (must be positive)", value);
              }
              return 0;
      }

-- 
Duy