Re: [PATCH/GSoC/RFC] fetch: git fetch --deepen

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Mar 13, 2015 at 8:04 PM, Dongcan Jiang <dongcan.jiang@xxxxxxxxx> wrote:
> @@ -317,7 +318,7 @@ static int find_common(struct fetch_pack_args *args,
>         if (is_repository_shallow())
>                 write_shallow_commits(&req_buf, 1, NULL);
>         if (args->depth > 0)
> -               packet_buf_write(&req_buf, "deepen %d", args->depth);
> +               packet_buf_write(&req_buf, "depth %d", args->depth);
>         packet_buf_flush(&req_buf);
>         state_len = req_buf.len;

Junio already questioned about your replacing starts_with("deepen "..)
with starts_with("depth "..), this is part of that question. If you
run "make test" you should find some breakages, or we need to improve
our test suite.

I think you just need to keep the old "if (args->depth > 0) send
"depth" unchanged and add two new statements that check
args->depth_deepen and sends "depth ". BTW, I think "deepen-more" or
"deepen-relative" would be better than "depth" (*), which is a noun.
But that's a minor point at this stage.

And because --depth and --deepen are mutually exclusive, you need to
make sure that the user must not specify that. The "user" includes the
"client" from the server perspective, we need to make sure that
upload-pack barf if some client sends both "deepen" and "depth".

(*) I was about to suggest you use "deepen" for both --depth and
--deepen: --depth sends "deepen NUM" while --deepen sends "deepen
+NUM". The protocol as described in pack-protocol.txt may allow this
extension. Unfortunately the current implementation is too relaxed. We
use strtol() to parse "NUM" and it would accept the leading "+"
instead of barfing out. So old clients would mistakes --deepen as
--depth, not good. And it even accepts base 8 and 16!! We should fix
this.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]