Re: [PATCH/GSoC/RFC] fetch: git fetch --deepen
On Mon, Mar 16, 2015 at 11:10 PM, Dongcan Jiang dongcan.ji...@gmail.com wrote: Hi Duy, Sorry for my late response. we need to make sure that upload-pack barf if some client sends both deepen and depth. Actually, in my current design, when client just wants depth, it sends depth N; when it want deepen, it sends depth N as well as depth_deepen. For the latter case, it tells upload-pack to collect objects for deepen N. Thus, I'm not so sure whether upload-pack needs to check their exclusion. C Git is not the only client that can talk to upload-pack. A buggy client may send both. The least we need is make sure it would not crash or break the server security in any way. But if we have to consider that, we may as well reject with a clear message, which would help the client writer. And speaking of clients.. On Mon, Mar 16, 2015 at 11:08 PM, Dongcan Jiang dongcan.ji...@gmail.com wrote: - if (starts_with(line, deepen )) { + if (starts_with(line, depth )) { char *end; - depth = strtol(line + 7, end, 0); - if (end == line + 7 || depth = 0) - die(Invalid deepen: %s, line); + depth = strtol(line + 6, end, 0); + if (end == line + 6 || depth = 0) + die(Invalid depth: %s, line); continue; } if (!starts_with(line, want ) || I do not quite understand this hunk. What happens when this program is talking to an existing fetch-pack that requested deepen? In this hunk, I actually just changed the one command name in upload-pack service from deepen to depth. I made this change because the name deepen here is quite misleading, as it just means depth. Of course, I've changed the caller's sent pack from deepen to depth too (See below). You missed Junio's keyword existing. Your new client will work well with your new server. But it breaks git clone --depth (or git fetch --depth) for all existing git installations. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/GSoC/RFC] fetch: git fetch --deepen
C Git is not the only client that can talk to upload-pack. A buggy client may send both. The least we need is make sure it would not crash or break the server security in any way. But if we have to consider that, we may as well reject with a clear message, which would help the client writer. And speaking of clients.. Actually, I mean that the upload-pack in this patch will not crash, whatever it receives. e.g. depth N, depth_deepen, depth N+depth_deepen Because depth_deepen is just a boolean signal to tell the upload-pack that depth N means deepen N, it takes effect only when depth N is given. Otherwise, depth_deepen will be ignored. You missed Junio's keyword existing. Your new client will work well with your new server. But it breaks git clone --depth (or git fetch --depth) for all existing git installations. Oh, thank you for pointing out this. And sorry Junio. I misunderstood what you said. I haven't realized the problem of existing git server. You've reminded me of the importance of compatibility. Thank you! 2015-03-17 18:44 GMT+08:00 Duy Nguyen pclo...@gmail.com: On Mon, Mar 16, 2015 at 11:10 PM, Dongcan Jiang dongcan.ji...@gmail.com wrote: Hi Duy, Sorry for my late response. we need to make sure that upload-pack barf if some client sends both deepen and depth. Actually, in my current design, when client just wants depth, it sends depth N; when it want deepen, it sends depth N as well as depth_deepen. For the latter case, it tells upload-pack to collect objects for deepen N. Thus, I'm not so sure whether upload-pack needs to check their exclusion. C Git is not the only client that can talk to upload-pack. A buggy client may send both. The least we need is make sure it would not crash or break the server security in any way. But if we have to consider that, we may as well reject with a clear message, which would help the client writer. And speaking of clients.. On Mon, Mar 16, 2015 at 11:08 PM, Dongcan Jiang dongcan.ji...@gmail.com wrote: - if (starts_with(line, deepen )) { + if (starts_with(line, depth )) { char *end; - depth = strtol(line + 7, end, 0); - if (end == line + 7 || depth = 0) - die(Invalid deepen: %s, line); + depth = strtol(line + 6, end, 0); + if (end == line + 6 || depth = 0) + die(Invalid depth: %s, line); continue; } if (!starts_with(line, want ) || I do not quite understand this hunk. What happens when this program is talking to an existing fetch-pack that requested deepen? In this hunk, I actually just changed the one command name in upload-pack service from deepen to depth. I made this change because the name deepen here is quite misleading, as it just means depth. Of course, I've changed the caller's sent pack from deepen to depth too (See below). You missed Junio's keyword existing. Your new client will work well with your new server. But it breaks git clone --depth (or git fetch --depth) for all existing git installations. -- Duy -- 江东灿(Dongcan Jiang) Team of Search Engine Web Mining School of Electronic Engineering Computer Science Peking University, Beijing, 100871, P.R.China -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/GSoC/RFC] fetch: git fetch --deepen
Hi Duy, Sorry for my late response. we need to make sure that upload-pack barf if some client sends both deepen and depth. Actually, in my current design, when client just wants depth, it sends depth N; when it want deepen, it sends depth N as well as depth_deepen. For the latter case, it tells upload-pack to collect objects for deepen N. Thus, I'm not so sure whether upload-pack needs to check their exclusion. 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. This suggestion looks neat! However, I'm afraid it may involves quite a bit changes as you've mentioned, which would be better to take in next stage. Thanks, Dongcan 2015-03-14 18:56 GMT+08:00 Duy Nguyen pclo...@gmail.com: On Fri, Mar 13, 2015 at 8:04 PM, Dongcan Jiang dongcan.ji...@gmail.com 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 -- 江东灿(Dongcan Jiang) Team of Search Engine Web Mining School of Electronic Engineering Computer Science Peking University, Beijing, 100871, P.R.China -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/GSoC/RFC] fetch: git fetch --deepen
Hi Junio, Sorry for my late response. As this operation is not about moving _any_ refs, whether local branches or remote-tracking branches, any ref that used to point at commit B before you executed fetch --deepen would point at the same commit after the command finishes. Thanks for clarifying the definition of you. In this patch, it actually would update the remote-tracking branches to the newest history. I will keep working on it to figure out the reason. It looks that it would be more efficient if the new history is not fetched, as it transports less objects. Is this your main consideration on not updating any refs? - if (starts_with(line, deepen )) { + if (starts_with(line, depth )) { char *end; - depth = strtol(line + 7, end, 0); - if (end == line + 7 || depth = 0) - die(Invalid deepen: %s, line); + depth = strtol(line + 6, end, 0); + if (end == line + 6 || depth = 0) + die(Invalid depth: %s, line); continue; } if (!starts_with(line, want ) || I do not quite understand this hunk. What happens when this program is talking to an existing fetch-pack that requested deepen? In this hunk, I actually just changed the one command name in upload-pack service from deepen to depth. I made this change because the name deepen here is quite misleading, as it just means depth. Of course, I've changed the caller's sent pack from deepen to depth too (See below). @@ -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; If the user wants deepen, he should send a depth_deepen signal as well as depth N. When the upload-pack service in this patch receive depth_deepen and depth N, it collects N more commits ahead of the shallow commit and send back to the caller. Thanks, Dongcan 2015-03-14 13:35 GMT+08:00 Junio C Hamano gits...@pobox.com: Dongcan Jiang dongcan.ji...@gmail.com writes: This patch is just for discusstion. An option --deepen is added to 'git fetch'. When it comes to '--deepen', git should fetch N more commits ahead the local shallow commit, where N is indicated by '--depth=N'. [1] e.g. (upstream) ---o---o---o---A---B (you) A---B After excuting git fetch --depth=1 --deepen, (you) get one more tip and it becomes (you) o---A---B '--deepen' is designed to be a boolean option in this patch, which is a little different from [1]. It's designed in this way, because it can reuse '--depth' in the program, and just costs one more bit in some data structure, such as fetch_pack_args, git_transport_options. Of course, as a patch for discussion, it remains a long way to go before being complete. 1) Documents should be completed. 2) More test cases, expecially corner cases, should be added. 3) No need to get remote refs when it comes to '--deepen' option. 4) Validity on options combination should be checked. 5) smart-http protocol remains to be supported. [2] Besides, I still have one question: What does (you) exactly mean in [1]? The local branch or the local remote ref? As this operation is not about moving _any_ refs, whether local branches or remote-tracking branches, any ref that used to point at commit B before you executed fetch --deepen would point at the same commit after the command finishes. The you does not explicitly depict any ref, because the picture is meant to illustrate _everything_ the repository at the receiving end of fetch has. It used to have two commits, A and B (and the tree and blob objects necessary to complete these two commits). After deepening the history by one commit, it then will have commit A^ and its trees and blobs. diff --git a/upload-pack.c b/upload-pack.c index b531a32..8004f2f 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -31,6 +31,7 @@ static const char upload_pack_usage[] = git upload-pack [--strict] [--timeout= static unsigned long oldest_have; +static int depth_deepen; static int multi_ack; static int no_done; static int use_thin_pack, use_ofs_delta, use_include_tag; @@ -563,11 +564,11 @@ static void receive_needs(void) } continue; } - if (starts_with(line, deepen )) { + if (starts_with(line, depth )) { char *end; - depth = strtol(line + 7, end, 0); - if (end == line + 7 || depth = 0) -
Re: [PATCH/GSoC/RFC] fetch: git fetch --deepen
Hi Eric, Sorry for my late response. Thank you for your suggestions! I will try to use them in my next patch version. Best Regards, Dongcan 2015-03-14 3:42 GMT+08:00 Eric Sunshine sunsh...@sunshineco.com: On Fri, Mar 13, 2015 at 9:04 AM, Dongcan Jiang dongcan.ji...@gmail.com wrote: This patch is just for discusstion. An option --deepen is added to 'git fetch'. When it comes to '--deepen', git should fetch N more commits ahead the local shallow commit, where N is indicated by '--depth=N'. [1] [...] Of course, as a patch for discussion, it remains a long way to go before being complete. [...] Signed-off-by: Dongcan Jiang dongcan.ji...@gmail.com --- diff --git a/fetch-pack.c b/fetch-pack.c index 655ee64..6f4adca 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -295,6 +295,7 @@ static int find_common(struct fetch_pack_args *args, if (no_done)strbuf_addstr(c, no-done); if (use_sideband == 2) strbuf_addstr(c, side-band-64k); if (use_sideband == 1) strbuf_addstr(c, side-band); + if (args-depth_deepen) strbuf_addstr(c, depth_deepen); For consistency, should this be depth-deepen rather than depth_deepen? if (args-use_thin_pack) strbuf_addstr(c, thin-pack); if (args-no_progress) strbuf_addstr(c, no-progress); if (args-include_tag) strbuf_addstr(c, include-tag); diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index d78f320..6738006 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -708,4 +708,15 @@ test_expect_success 'fetching a one-level ref works' ' ) ' +test_expect_success 'fetching deepen' ' + git clone . deepen --depth=1 + cd deepen When this tests ends, the working directory will still be 'deepen', which will likely break tests added after this one. If you wrap the 'cd' and following statements in a subshell via '(' and ')', then the 'cd' will affect the subshell but leave the parent shell's working directory alone, and thus not negatively impact subsequent tests. + git fetch .. foo --depth=1 + git show foo + test_must_fail git show foo~ + git fetch .. foo --depth=1 --deepen + git show foo~ + test_must_fail git show foo~2 Broken -chain throughout. +' + test_done -- 江东灿(Dongcan Jiang) Team of Search Engine Web Mining School of Electronic Engineering Computer Science Peking University, Beijing, 100871, P.R.China -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/GSoC/RFC] fetch: git fetch --deepen
Duy Nguyen pclo...@gmail.com writes: On Sat, Mar 14, 2015 at 12:35 PM, Junio C Hamano gits...@pobox.com wrote: Dongcan Jiang dongcan.ji...@gmail.com writes: What does (you) exactly mean in [1]? The local branch or the local remote ref? As this operation is not about moving _any_ refs, whether local branches or remote-tracking branches, any ref that used to point at commit B before you executed fetch --deepen would point at the same commit after the command finishes. That would make it harder to implement fetch --deepen than the version that moves refs if they are updated. The comment you are responding to was in the context of What does the illustrated 'your' history mean? Is it a history of a single ref (if so in which repository)? to clarify that the example was not fetching _new_ history (and with the RFC/POC design that was posted, my understanding is that deepen was meant to only deepen). The paragraph you are reacting to is not an endorsement for that only deepen, never advance design. It merely is a clarification of the explanation that was in the paragraph that follows. And I think what Dongcan implemented moves refs. From the user point of view, I think it's ok with either version, so the one that's easier to implement wins. The you does not explicitly depict any ref, because the picture is meant to illustrate _everything_ the repository at the receiving end of fetch has. It used to have two commits, A and B (and the tree and blob objects necessary to complete these two commits). After deepening the history by one commit, it then will have commit A^ and its trees and blobs. [1] http://article.gmane.org/gmane.comp.version-control.git/212950 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/GSoC/RFC] fetch: git fetch --deepen
On Sat, Mar 14, 2015 at 12:35 PM, Junio C Hamano gits...@pobox.com wrote: Dongcan Jiang dongcan.ji...@gmail.com writes: What does (you) exactly mean in [1]? The local branch or the local remote ref? As this operation is not about moving _any_ refs, whether local branches or remote-tracking branches, any ref that used to point at commit B before you executed fetch --deepen would point at the same commit after the command finishes. That would make it harder to implement fetch --deepen than the version that moves refs if they are updated. And I think what Dongcan implemented moves refs. From the user point of view, I think it's ok with either version, so the one that's easier to implement wins. The you does not explicitly depict any ref, because the picture is meant to illustrate _everything_ the repository at the receiving end of fetch has. It used to have two commits, A and B (and the tree and blob objects necessary to complete these two commits). After deepening the history by one commit, it then will have commit A^ and its trees and blobs. [1] http://article.gmane.org/gmane.comp.version-control.git/212950 -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/GSoC/RFC] fetch: git fetch --deepen
On Fri, Mar 13, 2015 at 8:04 PM, Dongcan Jiang dongcan.ji...@gmail.com 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/GSoC/RFC] fetch: git fetch --deepen
On Fri, Mar 13, 2015 at 9:04 AM, Dongcan Jiang dongcan.ji...@gmail.com wrote: This patch is just for discusstion. An option --deepen is added to 'git fetch'. When it comes to '--deepen', git should fetch N more commits ahead the local shallow commit, where N is indicated by '--depth=N'. [1] [...] Of course, as a patch for discussion, it remains a long way to go before being complete. [...] Signed-off-by: Dongcan Jiang dongcan.ji...@gmail.com --- diff --git a/fetch-pack.c b/fetch-pack.c index 655ee64..6f4adca 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -295,6 +295,7 @@ static int find_common(struct fetch_pack_args *args, if (no_done)strbuf_addstr(c, no-done); if (use_sideband == 2) strbuf_addstr(c, side-band-64k); if (use_sideband == 1) strbuf_addstr(c, side-band); + if (args-depth_deepen) strbuf_addstr(c, depth_deepen); For consistency, should this be depth-deepen rather than depth_deepen? if (args-use_thin_pack) strbuf_addstr(c, thin-pack); if (args-no_progress) strbuf_addstr(c, no-progress); if (args-include_tag) strbuf_addstr(c, include-tag); diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index d78f320..6738006 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -708,4 +708,15 @@ test_expect_success 'fetching a one-level ref works' ' ) ' +test_expect_success 'fetching deepen' ' + git clone . deepen --depth=1 + cd deepen When this tests ends, the working directory will still be 'deepen', which will likely break tests added after this one. If you wrap the 'cd' and following statements in a subshell via '(' and ')', then the 'cd' will affect the subshell but leave the parent shell's working directory alone, and thus not negatively impact subsequent tests. + git fetch .. foo --depth=1 + git show foo + test_must_fail git show foo~ + git fetch .. foo --depth=1 --deepen + git show foo~ + test_must_fail git show foo~2 Broken -chain throughout. +' + test_done -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/GSoC/RFC] fetch: git fetch --deepen
Dongcan Jiang dongcan.ji...@gmail.com writes: This patch is just for discusstion. An option --deepen is added to 'git fetch'. When it comes to '--deepen', git should fetch N more commits ahead the local shallow commit, where N is indicated by '--depth=N'. [1] e.g. (upstream) ---o---o---o---A---B (you) A---B After excuting git fetch --depth=1 --deepen, (you) get one more tip and it becomes (you) o---A---B '--deepen' is designed to be a boolean option in this patch, which is a little different from [1]. It's designed in this way, because it can reuse '--depth' in the program, and just costs one more bit in some data structure, such as fetch_pack_args, git_transport_options. Of course, as a patch for discussion, it remains a long way to go before being complete. 1) Documents should be completed. 2) More test cases, expecially corner cases, should be added. 3) No need to get remote refs when it comes to '--deepen' option. 4) Validity on options combination should be checked. 5) smart-http protocol remains to be supported. [2] Besides, I still have one question: What does (you) exactly mean in [1]? The local branch or the local remote ref? As this operation is not about moving _any_ refs, whether local branches or remote-tracking branches, any ref that used to point at commit B before you executed fetch --deepen would point at the same commit after the command finishes. The you does not explicitly depict any ref, because the picture is meant to illustrate _everything_ the repository at the receiving end of fetch has. It used to have two commits, A and B (and the tree and blob objects necessary to complete these two commits). After deepening the history by one commit, it then will have commit A^ and its trees and blobs. diff --git a/upload-pack.c b/upload-pack.c index b531a32..8004f2f 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -31,6 +31,7 @@ static const char upload_pack_usage[] = git upload-pack [--strict] [--timeout= static unsigned long oldest_have; +static int depth_deepen; static int multi_ack; static int no_done; static int use_thin_pack, use_ofs_delta, use_include_tag; @@ -563,11 +564,11 @@ static void receive_needs(void) } continue; } - if (starts_with(line, deepen )) { + if (starts_with(line, depth )) { char *end; - depth = strtol(line + 7, end, 0); - if (end == line + 7 || depth = 0) - die(Invalid deepen: %s, line); + depth = strtol(line + 6, end, 0); + if (end == line + 6 || depth = 0) + die(Invalid depth: %s, line); continue; } if (!starts_with(line, want ) || I do not quite understand this hunk. What happens when this program is talking to an existing fetch-pack that requested deepen? @@ -577,6 +578,8 @@ static void receive_needs(void) features = line + 45; + if (parse_feature_request(features, depth_deepen)) + depth_deepen = 1; if (parse_feature_request(features, multi_ack_detailed)) multi_ack = 2; else if (parse_feature_request(features, multi_ack)) @@ -631,6 +634,10 @@ static void receive_needs(void) struct object *object = shallows.objects[i].item; object-flags |= NOT_SHALLOW; } + else if (depth_deepen) + backup = result = + get_shallow_commits(shallows, depth + 1, + SHALLOW, NOT_SHALLOW); else backup = result = get_shallow_commits(want_obj, depth, -- 2.3.1.253.gb3fd89e -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/GSoC/RFC] fetch: git fetch --deepen
This patch is just for discusstion. An option --deepen is added to 'git fetch'. When it comes to '--deepen', git should fetch N more commits ahead the local shallow commit, where N is indicated by '--depth=N'. [1] e.g. (upstream) ---o---o---o---A---B (you) A---B After excuting git fetch --depth=1 --deepen, (you) get one more tip and it becomes (you) o---A---B '--deepen' is designed to be a boolean option in this patch, which is a little different from [1]. It's designed in this way, because it can reuse '--depth' in the program, and just costs one more bit in some data structure, such as fetch_pack_args, git_transport_options. Of course, as a patch for discussion, it remains a long way to go before being complete. 1) Documents should be completed. 2) More test cases, expecially corner cases, should be added. 3) No need to get remote refs when it comes to '--deepen' option. 4) Validity on options combination should be checked. 5) smart-http protocol remains to be supported. [2] Besides, I still have one question: What does (you) exactly mean in [1]? The local branch or the local remote ref? I hope you could give me some comments and suggestions on this patch. I would like to know whether I'm off the right way. [1] http://article.gmane.org/gmane.comp.version-control.git/212950 [2] http://article.gmane.org/gmane.comp.version-control.git/265050 Helped-by: Duy Nguyen pclo...@gmail.com Signed-off-by: Dongcan Jiang dongcan.ji...@gmail.com --- builtin/fetch.c | 5 - fetch-pack.c | 3 ++- fetch-pack.h | 1 + t/t5510-fetch.sh | 11 +++ transport.c | 4 transport.h | 4 upload-pack.c| 15 +++ 7 files changed, 37 insertions(+), 6 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index f951265..6861207 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -33,7 +33,7 @@ static int fetch_prune_config = -1; /* unspecified */ static int prune = -1; /* unspecified */ #define PRUNE_BY_DEFAULT 0 /* do we prune by default? */ -static int all, append, dry_run, force, keep, multiple, update_head_ok, verbosity; +static int all, append, dry_run, force, keep, multiple, update_head_ok, verbosity, deepen; static int progress = -1, recurse_submodules = RECURSE_SUBMODULES_DEFAULT; static int tags = TAGS_DEFAULT, unshallow, update_shallow; static const char *depth; @@ -111,6 +111,7 @@ static struct option builtin_fetch_options[] = { OPT_BOOL(0, progress, progress, N_(force progress reporting)), OPT_STRING(0, depth, depth, N_(depth), N_(deepen history of shallow clone)), + OPT_BOOL(0, deepen, deepen, N_(deepen)), { OPTION_SET_INT, 0, unshallow, unshallow, NULL, N_(convert to a complete repository), PARSE_OPT_NONEG | PARSE_OPT_NOARG, NULL, 1 }, @@ -853,6 +854,8 @@ static struct transport *prepare_transport(struct remote *remote) set_option(transport, TRANS_OPT_KEEP, yes); if (depth) set_option(transport, TRANS_OPT_DEPTH, depth); + if (deepen) + set_option(transport, TRANS_OPT_DEEPEN, yes); if (update_shallow) set_option(transport, TRANS_OPT_UPDATE_SHALLOW, yes); return transport; diff --git a/fetch-pack.c b/fetch-pack.c index 655ee64..6f4adca 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -295,6 +295,7 @@ static int find_common(struct fetch_pack_args *args, if (no_done)strbuf_addstr(c, no-done); if (use_sideband == 2) strbuf_addstr(c, side-band-64k); if (use_sideband == 1) strbuf_addstr(c, side-band); + if (args-depth_deepen) strbuf_addstr(c, depth_deepen); if (args-use_thin_pack) strbuf_addstr(c, thin-pack); if (args-no_progress) strbuf_addstr(c, no-progress); if (args-include_tag) strbuf_addstr(c, include-tag); @@ -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; diff --git a/fetch-pack.h b/fetch-pack.h index bb7fd76..200ac78 100644 --- a/fetch-pack.h +++ b/fetch-pack.h @@ -10,6 +10,7 @@ struct fetch_pack_args { const char *uploadpack; int unpacklimit; int depth; + unsigned depth_deepen:1; unsigned quiet:1; unsigned keep_pack:1; unsigned lock_pack:1; diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index d78f320..6738006 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -708,4 +708,15 @@