Re: [PATCH] transport: Catch non positive --depth option value
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:59:16
"Andrés G. Aragoneses" [off-list ref] writes:
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 mean 'no depth' or 'no history' but 'full depth' instead).
My initial knee-jerk reaction was: doesn't this change break existing use to unplug a shallow repository and bring it to a repository with an unshallow one to disallow depth=0, though? I somehow thought that the code supports unshallowing with --depth=0 even though since 4dcb167f (fetch: add --unshallow for turning shallow repo into complete one, 2013-01-11), the officially supported way to tell Git to unshallow is with that option. But apparently that is not the case; I do not think depth==0 meant 'full depth' (i.e. "git fetch --depth=0" did not unshallow); it was simply ignored in fetch_pack.c::find_common() and friends. So I think it should be a safe change to disallow non-positive depth like this patch does, but the proposed commit log message may need polishing. Thanks.
quoted hunk
Signed-off-by: Andres G. Aragoneses <redacted> --- 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(structgit_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' (non positive)", value); } return 0; }