{"thread":{"id":"38795","subject":"[PATCH/GSoC/RFC] fetch: git fetch --deepen","startedAt":"2015-03-13T13:04:06Z","lastAt":"2015-03-22T23:23:28Z","messageCount":14,"participants":["Dongcan Jiang","Eric Sunshine","Junio C Hamano","Duy Nguyen"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"257642","messageId":"1426251846-1604-1-git-send-email-dongcan.jiang@gmail.com","threadId":"38795","inReplyTo":null,"subject":"[PATCH/GSoC/RFC] fetch: git fetch --deepen","fromName":"Dongcan Jiang","fromEmail":"dongcan.jiang@gmail.com","sentAt":"2015-03-13T13:04:06Z","receivedAt":"2015-03-13T13:04:06Z","isPatch":true,"sender":{"key":"dongcan.jiang@gmail.com","avatar":"https://avatars.githubusercontent.com/u/10103762?v=4"},"body":"This patch is just for discusstion. An option --deepen is added to\n'git fetch'. When it comes to '--deepen', git should fetch N more\ncommits ahead the local shallow commit, where N is indicated by\n'--depth=N'. [1]\n\ne.g.\n\n>  (upstream)\n>   ---o---o---o---A---B\n>\n>  (you)\n>                  A---B\n\nAfter excuting \"git fetch --depth=1 --deepen\", (you) get one more\ntip and it becomes\n\n>  (you)\n>              o---A---B\n\n'--deepen' is designed to be a boolean option in this patch, which\nis a little different from [1]. It's designed in this way, because\nit can reuse '--depth' in the program, and just costs one more bit\nin some data structure, such as fetch_pack_args,\ngit_transport_options.\n\nOf course, as a patch for discussion, it remains a long way to go\nbefore being complete.\n\n\t1) Documents should be completed.\n\t2) More test cases, expecially corner cases, should be added.\n\t3) No need to get remote refs when it comes to '--deepen' option.\n\t4) Validity on options combination should be checked.\n\t5) smart-http protocol remains to be supported. [2]\n\nBesides, I still have one question:\nWhat does (you) exactly mean in [1]? The local branch or the local\nremote ref?\n\nI hope you could give me some comments and suggestions on this\npatch. I would like to know whether I'm off the right way.\n\n[1] http://article.gmane.org/gmane.comp.version-control.git/212950\n[2] http://article.gmane.org/gmane.comp.version-control.git/265050\n\nHelped-by: Duy Nguyen <pclouds@gmail.com>\nSigned-off-by: Dongcan Jiang <dongcan.jiang@gmail.com>\n---\n builtin/fetch.c  |  5 ++++-\n fetch-pack.c     |  3 ++-\n fetch-pack.h     |  1 +\n t/t5510-fetch.sh | 11 +++++++++++\n transport.c      |  4 ++++\n transport.h      |  4 ++++\n upload-pack.c    | 15 +++++++++++----\n 7 files changed, 37 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex f951265..6861207 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -33,7 +33,7 @@ static int fetch_prune_config = -1; /* unspecified */\n static int prune = -1; /* unspecified */\n #define PRUNE_BY_DEFAULT 0 /* do we prune by default? */\n\n-static int all, append, dry_run, force, keep, multiple, update_head_ok, verbosity;\n+static int all, append, dry_run, force, keep, multiple, update_head_ok, verbosity, deepen;\n static int progress = -1, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;\n static int tags = TAGS_DEFAULT, unshallow, update_shallow;\n static const char *depth;\n@@ -111,6 +111,7 @@ static struct option builtin_fetch_options[] = {\n \tOPT_BOOL(0, \"progress\", &progress, N_(\"force progress reporting\")),\n \tOPT_STRING(0, \"depth\", &depth, N_(\"depth\"),\n \t\t   N_(\"deepen history of shallow clone\")),\n+\tOPT_BOOL(0, \"deepen\", &deepen, N_(\"deepen\")),\n \t{ OPTION_SET_INT, 0, \"unshallow\", &unshallow, NULL,\n \t\t   N_(\"convert to a complete repository\"),\n \t\t   PARSE_OPT_NONEG | PARSE_OPT_NOARG, NULL, 1 },\n@@ -853,6 +854,8 @@ static struct transport *prepare_transport(struct remote *remote)\n \t\tset_option(transport, TRANS_OPT_KEEP, \"yes\");\n \tif (depth)\n \t\tset_option(transport, TRANS_OPT_DEPTH, depth);\n+\tif (deepen)\n+\t\tset_option(transport, TRANS_OPT_DEEPEN, \"yes\");\n \tif (update_shallow)\n \t\tset_option(transport, TRANS_OPT_UPDATE_SHALLOW, \"yes\");\n \treturn transport;\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex 655ee64..6f4adca 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -295,6 +295,7 @@ static int find_common(struct fetch_pack_args *args,\n \t\t\tif (no_done)            strbuf_addstr(&c, \" no-done\");\n \t\t\tif (use_sideband == 2)  strbuf_addstr(&c, \" side-band-64k\");\n \t\t\tif (use_sideband == 1)  strbuf_addstr(&c, \" side-band\");\n+\t\t\tif (args->depth_deepen)  strbuf_addstr(&c, \" depth_deepen\");\n \t\t\tif (args->use_thin_pack) strbuf_addstr(&c, \" thin-pack\");\n \t\t\tif (args->no_progress)   strbuf_addstr(&c, \" no-progress\");\n \t\t\tif (args->include_tag)   strbuf_addstr(&c, \" include-tag\");\n@@ -317,7 +318,7 @@ static int find_common(struct fetch_pack_args *args,\n \tif (is_repository_shallow())\n \t\twrite_shallow_commits(&req_buf, 1, NULL);\n \tif (args->depth > 0)\n-\t\tpacket_buf_write(&req_buf, \"deepen %d\", args->depth);\n+\t\tpacket_buf_write(&req_buf, \"depth %d\", args->depth);\n \tpacket_buf_flush(&req_buf);\n \tstate_len = req_buf.len;\n\ndiff --git a/fetch-pack.h b/fetch-pack.h\nindex bb7fd76..200ac78 100644\n--- a/fetch-pack.h\n+++ b/fetch-pack.h\n@@ -10,6 +10,7 @@ struct fetch_pack_args {\n \tconst char *uploadpack;\n \tint unpacklimit;\n \tint depth;\n+\tunsigned depth_deepen:1;\n \tunsigned quiet:1;\n \tunsigned keep_pack:1;\n \tunsigned lock_pack:1;\ndiff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\nindex d78f320..6738006 100755\n--- a/t/t5510-fetch.sh\n+++ b/t/t5510-fetch.sh\n@@ -708,4 +708,15 @@ test_expect_success 'fetching a one-level ref works' '\n \t)\n '\n\n+test_expect_success 'fetching deepen' '\n+\tgit clone . deepen --depth=1 &&\n+\tcd deepen &&\n+\tgit fetch .. foo --depth=1\n+\tgit show foo\n+\ttest_must_fail git show foo~\n+\tgit fetch .. foo --depth=1 --deepen\n+\tgit show foo~\n+\ttest_must_fail git show foo~2\n+'\n+\n test_done\ndiff --git a/transport.c b/transport.c\nindex 0694a7c..53ee7b3 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -478,6 +478,9 @@ static int set_git_option(struct git_transport_options *opts,\n \t\t\t\tdie(\"transport: invalid depth option '%s'\", value);\n \t\t}\n \t\treturn 0;\n+\t} else if (!strcmp(name, TRANS_OPT_DEEPEN)) {\n+\t\topts->depth_deepen = !!value;\n+\t\treturn 0;\n \t} else if (!strcmp(name, TRANS_OPT_PUSH_CERT)) {\n \t\topts->push_cert = !!value;\n \t\treturn 0;\n@@ -534,6 +537,7 @@ static int fetch_refs_via_pack(struct transport *transport,\n \targs.quiet = (transport->verbose < 0);\n \targs.no_progress = !transport->progress;\n \targs.depth = data->options.depth;\n+\targs.depth_deepen = data->options.depth_deepen;\n \targs.check_self_contained_and_connected =\n \t\tdata->options.check_self_contained_and_connected;\n \targs.cloning = transport->cloning;\ndiff --git a/transport.h b/transport.h\nindex 18d2cf8..4f432fd 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -13,6 +13,7 @@ struct git_transport_options {\n \tunsigned self_contained_and_connected : 1;\n \tunsigned update_shallow : 1;\n \tunsigned push_cert : 1;\n+\tunsigned depth_deepen : 1;\n \tint depth;\n \tconst char *uploadpack;\n \tconst char *receivepack;\n@@ -153,6 +154,9 @@ struct transport *transport_get(struct remote *, const char *);\n /* Limit the depth of the fetch if not null */\n #define TRANS_OPT_DEPTH \"depth\"\n\n+/* Limit the deepen of the fetch if not null */\n+#define TRANS_OPT_DEEPEN \"deepen\"\n+\n /* Aggressively fetch annotated tags if possible */\n #define TRANS_OPT_FOLLOWTAGS \"followtags\"\n\ndiff --git a/upload-pack.c b/upload-pack.c\nindex b531a32..8004f2f 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -31,6 +31,7 @@ static const char upload_pack_usage[] = \"git upload-pack [--strict] [--timeout=<\n\n static unsigned long oldest_have;\n\n+static int depth_deepen;\n static int multi_ack;\n static int no_done;\n static int use_thin_pack, use_ofs_delta, use_include_tag;\n@@ -563,11 +564,11 @@ static void receive_needs(void)\n \t\t\t}\n \t\t\tcontinue;\n \t\t}\n-\t\tif (starts_with(line, \"deepen \")) {\n+\t\tif (starts_with(line, \"depth \")) {\n \t\t\tchar *end;\n-\t\t\tdepth = strtol(line + 7, &end, 0);\n-\t\t\tif (end == line + 7 || depth <= 0)\n-\t\t\t\tdie(\"Invalid deepen: %s\", line);\n+\t\t\tdepth = strtol(line + 6, &end, 0);\n+\t\t\tif (end == line + 6 || depth <= 0)\n+\t\t\t\tdie(\"Invalid depth: %s\", line);\n \t\t\tcontinue;\n \t\t}\n \t\tif (!starts_with(line, \"want \") ||\n@@ -577,6 +578,8 @@ static void receive_needs(void)\n\n \t\tfeatures = line + 45;\n\n+\t\tif (parse_feature_request(features, \"depth_deepen\"))\n+\t\t\tdepth_deepen = 1;\n \t\tif (parse_feature_request(features, \"multi_ack_detailed\"))\n \t\t\tmulti_ack = 2;\n \t\telse if (parse_feature_request(features, \"multi_ack\"))\n@@ -631,6 +634,10 @@ static void receive_needs(void)\n \t\t\t\tstruct object *object = shallows.objects[i].item;\n \t\t\t\tobject->flags |= NOT_SHALLOW;\n \t\t\t}\n+\t\telse if (depth_deepen)\n+\t\t\tbackup = result =\n+\t\t\t\tget_shallow_commits(&shallows, depth + 1,\n+\t\t\t\t\t\t    SHALLOW, NOT_SHALLOW);\n \t\telse\n \t\t\tbackup = result =\n \t\t\t\tget_shallow_commits(&want_obj, depth,\n--\n2.3.1.253.gb3fd89e\n"},{"id":"257647","messageId":"CAPig+cS07W6gtW8L5wbQwZuxSLxb-r0s6KNO4eDO5BYdBDbWjw@mail.gmail.com","threadId":"38795","inReplyTo":"1426251846-1604-1-git-send-email-dongcan.jiang@gmail.com","subject":"Re: [PATCH/GSoC/RFC] fetch: git fetch --deepen","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-03-13T19:42:06Z","receivedAt":"2015-03-13T19:42:06Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Mar 13, 2015 at 9:04 AM, Dongcan Jiang <dongcan.jiang@gmail.com> wrote:\n> This patch is just for discusstion. An option --deepen is added to\n> 'git fetch'. When it comes to '--deepen', git should fetch N more\n> commits ahead the local shallow commit, where N is indicated by\n> '--depth=N'. [1]\n> [...]\n> Of course, as a patch for discussion, it remains a long way to go\n> before being complete.\n> [...]\n> Signed-off-by: Dongcan Jiang <dongcan.jiang@gmail.com>\n> ---\n> diff --git a/fetch-pack.c b/fetch-pack.c\n> index 655ee64..6f4adca 100644\n> --- a/fetch-pack.c\n> +++ b/fetch-pack.c\n> @@ -295,6 +295,7 @@ static int find_common(struct fetch_pack_args *args,\n>                         if (no_done)            strbuf_addstr(&c, \" no-done\");\n>                         if (use_sideband == 2)  strbuf_addstr(&c, \" side-band-64k\");\n>                         if (use_sideband == 1)  strbuf_addstr(&c, \" side-band\");\n> +                       if (args->depth_deepen)  strbuf_addstr(&c, \" depth_deepen\");\n\nFor consistency, should this be \"depth-deepen\" rather than \"depth_deepen\"?\n\n>                         if (args->use_thin_pack) strbuf_addstr(&c, \" thin-pack\");\n>                         if (args->no_progress)   strbuf_addstr(&c, \" no-progress\");\n>                         if (args->include_tag)   strbuf_addstr(&c, \" include-tag\");\n> diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\n> index d78f320..6738006 100755\n> --- a/t/t5510-fetch.sh\n> +++ b/t/t5510-fetch.sh\n> @@ -708,4 +708,15 @@ test_expect_success 'fetching a one-level ref works' '\n>         )\n>  '\n>\n> +test_expect_success 'fetching deepen' '\n> +       git clone . deepen --depth=1 &&\n> +       cd deepen &&\n\nWhen this tests ends, the working directory will still be 'deepen',\nwhich will likely break tests added after this one. If you wrap the\n'cd' and following statements in a subshell via '(' and ')', then the\n'cd' will affect the subshell but leave the parent shell's working\ndirectory alone, and thus not negatively impact subsequent tests.\n\n> +       git fetch .. foo --depth=1\n> +       git show foo\n> +       test_must_fail git show foo~\n> +       git fetch .. foo --depth=1 --deepen\n> +       git show foo~\n> +       test_must_fail git show foo~2\n\nBroken &&-chain throughout.\n\n> +'\n> +\n>  test_done\n"},{"id":"257664","messageId":"xmqqlhj0xhsl.fsf@gitster.dls.corp.google.com","threadId":"38795","inReplyTo":"1426251846-1604-1-git-send-email-dongcan.jiang@gmail.com","subject":"Re: [PATCH/GSoC/RFC] fetch: git fetch --deepen","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-14T05:35:38Z","receivedAt":"2015-03-14T05:35:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dongcan Jiang <dongcan.jiang@gmail.com> writes:\n\n> This patch is just for discusstion. An option --deepen is added to\n> 'git fetch'. When it comes to '--deepen', git should fetch N more\n> commits ahead the local shallow commit, where N is indicated by\n> '--depth=N'. [1]\n>\n> e.g.\n>\n>>  (upstream)\n>>   ---o---o---o---A---B\n>>\n>>  (you)\n>>                  A---B\n>\n> After excuting \"git fetch --depth=1 --deepen\", (you) get one more\n> tip and it becomes\n>\n>>  (you)\n>>              o---A---B\n>\n> '--deepen' is designed to be a boolean option in this patch, which\n> is a little different from [1]. It's designed in this way, because\n> it can reuse '--depth' in the program, and just costs one more bit\n> in some data structure, such as fetch_pack_args,\n> git_transport_options.\n>\n> Of course, as a patch for discussion, it remains a long way to go\n> before being complete.\n>\n> \t1) Documents should be completed.\n> \t2) More test cases, expecially corner cases, should be added.\n> \t3) No need to get remote refs when it comes to '--deepen' option.\n> \t4) Validity on options combination should be checked.\n> \t5) smart-http protocol remains to be supported. [2]\n>\n> Besides, I still have one question:\n> What does (you) exactly mean in [1]? The local branch or the local\n> remote ref?\n\nAs this operation is not about moving _any_ refs, whether local\nbranches or remote-tracking branches, any ref that used to point at\ncommit B before you executed \"fetch --deepen\" would point at the\nsame commit after the command finishes.\n\nThe \"you\" does not explicitly depict any ref, because the picture is\nmeant to illustrate _everything_ the repository at the receiving end\nof \"fetch\" has.  It used to have two commits, A and B (and the tree\nand blob objects necessary to complete these two commits).  After\ndeepening the history by one commit, it then will have commit A^ and\nits trees and blobs.\n\n> diff --git a/upload-pack.c b/upload-pack.c\n> index b531a32..8004f2f 100644\n> --- a/upload-pack.c\n> +++ b/upload-pack.c\n> @@ -31,6 +31,7 @@ static const char upload_pack_usage[] = \"git upload-pack [--strict] [--timeout=<\n>\n>  static unsigned long oldest_have;\n>\n> +static int depth_deepen;\n>  static int multi_ack;\n>  static int no_done;\n>  static int use_thin_pack, use_ofs_delta, use_include_tag;\n> @@ -563,11 +564,11 @@ static void receive_needs(void)\n>  \t\t\t}\n>  \t\t\tcontinue;\n>  \t\t}\n> -\t\tif (starts_with(line, \"deepen \")) {\n> +\t\tif (starts_with(line, \"depth \")) {\n>  \t\t\tchar *end;\n> -\t\t\tdepth = strtol(line + 7, &end, 0);\n> -\t\t\tif (end == line + 7 || depth <= 0)\n> -\t\t\t\tdie(\"Invalid deepen: %s\", line);\n> +\t\t\tdepth = strtol(line + 6, &end, 0);\n> +\t\t\tif (end == line + 6 || depth <= 0)\n> +\t\t\t\tdie(\"Invalid depth: %s\", line);\n>  \t\t\tcontinue;\n>  \t\t}\n>  \t\tif (!starts_with(line, \"want \") ||\n\nI do not quite understand this hunk.  What happens when this program\nis talking to an existing fetch-pack that requested \"deepen\"?\n\n> @@ -577,6 +578,8 @@ static void receive_needs(void)\n>\n>  \t\tfeatures = line + 45;\n>\n> +\t\tif (parse_feature_request(features, \"depth_deepen\"))\n> +\t\t\tdepth_deepen = 1;\n>  \t\tif (parse_feature_request(features, \"multi_ack_detailed\"))\n>  \t\t\tmulti_ack = 2;\n>  \t\telse if (parse_feature_request(features, \"multi_ack\"))\n> @@ -631,6 +634,10 @@ static void receive_needs(void)\n>  \t\t\t\tstruct object *object = shallows.objects[i].item;\n>  \t\t\t\tobject->flags |= NOT_SHALLOW;\n>  \t\t\t}\n> +\t\telse if (depth_deepen)\n> +\t\t\tbackup = result =\n> +\t\t\t\tget_shallow_commits(&shallows, depth + 1,\n> +\t\t\t\t\t\t    SHALLOW, NOT_SHALLOW);\n>  \t\telse\n>  \t\t\tbackup = result =\n>  \t\t\t\tget_shallow_commits(&want_obj, depth,\n> --\n> 2.3.1.253.gb3fd89e\n"},{"id":"257669","messageId":"CACsJy8C1nHCv0XTDYu+R8QFbG9LqjiUU7kTM1RqnzomtcoVCfw@mail.gmail.com","threadId":"38795","inReplyTo":"xmqqlhj0xhsl.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH/GSoC/RFC] fetch: git fetch --deepen","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-03-14T10:38:28Z","receivedAt":"2015-03-14T10:38:28Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sat, Mar 14, 2015 at 12:35 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Dongcan Jiang <dongcan.jiang@gmail.com> writes:\n>> What does (you) exactly mean in [1]? The local branch or the local\n>> remote ref?\n>\n> As this operation is not about moving _any_ refs, whether local\n> branches or remote-tracking branches, any ref that used to point at\n> commit B before you executed \"fetch --deepen\" would point at the\n> same commit after the command finishes.\n\nThat would make it harder to implement \"fetch --deepen\" than the\nversion that moves refs if they are updated. And I think what Dongcan\nimplemented moves refs. From the user point of view, I think it's ok\nwith either version, so the one that's easier to implement wins.\n\n> The \"you\" does not explicitly depict any ref, because the picture is\n> meant to illustrate _everything_ the repository at the receiving end\n> of \"fetch\" has.  It used to have two commits, A and B (and the tree\n> and blob objects necessary to complete these two commits).  After\n> deepening the history by one commit, it then will have commit A^ and\n> its trees and blobs.\n>\n>> [1] http://article.gmane.org/gmane.comp.version-control.git/212950\n-- \nDuy\n"},{"id":"257670","messageId":"CACsJy8AAsAO6gjWg0nsdiOXFEsZ4583pH9mkUx1Js3Yop5KHHQ@mail.gmail.com","threadId":"38795","inReplyTo":"1426251846-1604-1-git-send-email-dongcan.jiang@gmail.com","subject":"Re: [PATCH/GSoC/RFC] fetch: git fetch --deepen","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-03-14T10:56:31Z","receivedAt":"2015-03-14T10:56:31Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Mar 13, 2015 at 8:04 PM, Dongcan Jiang <dongcan.jiang@gmail.com> wrote:\n> @@ -317,7 +318,7 @@ static int find_common(struct fetch_pack_args *args,\n>         if (is_repository_shallow())\n>                 write_shallow_commits(&req_buf, 1, NULL);\n>         if (args->depth > 0)\n> -               packet_buf_write(&req_buf, \"deepen %d\", args->depth);\n> +               packet_buf_write(&req_buf, \"depth %d\", args->depth);\n>         packet_buf_flush(&req_buf);\n>         state_len = req_buf.len;\n\nJunio already questioned about your replacing starts_with(\"deepen \"..)\nwith starts_with(\"depth \"..), this is part of that question. If you\nrun \"make test\" you should find some breakages, or we need to improve\nour test suite.\n\nI think you just need to keep the old \"if (args->depth > 0) send\n\"depth\" unchanged and add two new statements that check\nargs->depth_deepen and sends \"depth \". BTW, I think \"deepen-more\" or\n\"deepen-relative\" would be better than \"depth\" (*), which is a noun.\nBut that's a minor point at this stage.\n\nAnd because --depth and --deepen are mutually exclusive, you need to\nmake sure that the user must not specify that. The \"user\" includes the\n\"client\" from the server perspective, we need to make sure that\nupload-pack barf if some client sends both \"deepen\" and \"depth\".\n\n(*) I was about to suggest you use \"deepen\" for both --depth and\n--deepen: --depth sends \"deepen NUM\" while --deepen sends \"deepen\n+NUM\". The protocol as described in pack-protocol.txt may allow this\nextension. Unfortunately the current implementation is too relaxed. We\nuse strtol() to parse \"NUM\" and it would accept the leading \"+\"\ninstead of barfing out. So old clients would mistakes --deepen as\n--depth, not good. And it even accepts base 8 and 16!! We should fix\nthis.\n-- \nDuy\n"},{"id":"257688","messageId":"xmqqoanvw7v4.fsf@gitster.dls.corp.google.com","threadId":"38795","inReplyTo":"CACsJy8C1nHCv0XTDYu+R8QFbG9LqjiUU7kTM1RqnzomtcoVCfw@mail.gmail.com","subject":"Re: [PATCH/GSoC/RFC] fetch: git fetch --deepen","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-14T22:07:43Z","receivedAt":"2015-03-14T22:07:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen <pclouds@gmail.com> writes:\n\n> On Sat, Mar 14, 2015 at 12:35 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Dongcan Jiang <dongcan.jiang@gmail.com> writes:\n>>> What does (you) exactly mean in [1]? The local branch or the local\n>>> remote ref?\n>>\n>> As this operation is not about moving _any_ refs, whether local\n>> branches or remote-tracking branches, any ref that used to point at\n>> commit B before you executed \"fetch --deepen\" would point at the\n>> same commit after the command finishes.\n>\n> That would make it harder to implement \"fetch --deepen\" than the\n> version that moves refs if they are updated.\n\nThe comment you are responding to was in the context of \"What does\nthe illustrated 'your' history mean?  Is it a history of a single\nref (if so in which repository)?\" to clarify that the example was\nnot fetching _new_ history (and with the RFC/POC design that was\nposted, my understanding is that deepen was meant to only deepen).\n\nThe paragraph you are reacting to is not an endorsement for that\n\"only deepen, never advance\" design. It merely is a clarification of\nthe explanation that was in the paragraph that follows.\n\n> And I think what Dongcan\n> implemented moves refs. From the user point of view, I think it's ok\n> with either version, so the one that's easier to implement wins.\n>\n>> The \"you\" does not explicitly depict any ref, because the picture is\n>> meant to illustrate _everything_ the repository at the receiving end\n>> of \"fetch\" has.  It used to have two commits, A and B (and the tree\n>> and blob objects necessary to complete these two commits).  After\n>> deepening the history by one commit, it then will have commit A^ and\n>> its trees and blobs.\n>>\n>>> [1] http://article.gmane.org/gmane.comp.version-control.git/212950\n"},{"id":"257792","messageId":"CABwkPcoRK9SGx=euPy3wjk58_TcOgN_uQYhvWvPyEn3JyrogGw@mail.gmail.com","threadId":"38795","inReplyTo":"CAPig+cS07W6gtW8L5wbQwZuxSLxb-r0s6KNO4eDO5BYdBDbWjw@mail.gmail.com","subject":"Re: [PATCH/GSoC/RFC] fetch: git fetch --deepen","fromName":"Dongcan Jiang","fromEmail":"dongcan.jiang@gmail.com","sentAt":"2015-03-16T16:04:40Z","receivedAt":"2015-03-16T16:04:40Z","isPatch":true,"sender":{"key":"dongcan.jiang@gmail.com","avatar":"https://avatars.githubusercontent.com/u/10103762?v=4"},"body":"Hi Eric,\n\nSorry for my late response. Thank you for your suggestions! I will try\nto use them in my next patch version.\n\nBest Regards,\nDongcan\n\n2015-03-14 3:42 GMT+08:00 Eric Sunshine <sunshine@sunshineco.com>:\n> On Fri, Mar 13, 2015 at 9:04 AM, Dongcan Jiang <dongcan.jiang@gmail.com> wrote:\n>> This patch is just for discusstion. An option --deepen is added to\n>> 'git fetch'. When it comes to '--deepen', git should fetch N more\n>> commits ahead the local shallow commit, where N is indicated by\n>> '--depth=N'. [1]\n>> [...]\n>> Of course, as a patch for discussion, it remains a long way to go\n>> before being complete.\n>> [...]\n>> Signed-off-by: Dongcan Jiang <dongcan.jiang@gmail.com>\n>> ---\n>> diff --git a/fetch-pack.c b/fetch-pack.c\n>> index 655ee64..6f4adca 100644\n>> --- a/fetch-pack.c\n>> +++ b/fetch-pack.c\n>> @@ -295,6 +295,7 @@ static int find_common(struct fetch_pack_args *args,\n>>                         if (no_done)            strbuf_addstr(&c, \" no-done\");\n>>                         if (use_sideband == 2)  strbuf_addstr(&c, \" side-band-64k\");\n>>                         if (use_sideband == 1)  strbuf_addstr(&c, \" side-band\");\n>> +                       if (args->depth_deepen)  strbuf_addstr(&c, \" depth_deepen\");\n>\n> For consistency, should this be \"depth-deepen\" rather than \"depth_deepen\"?\n>\n>>                         if (args->use_thin_pack) strbuf_addstr(&c, \" thin-pack\");\n>>                         if (args->no_progress)   strbuf_addstr(&c, \" no-progress\");\n>>                         if (args->include_tag)   strbuf_addstr(&c, \" include-tag\");\n>> diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\n>> index d78f320..6738006 100755\n>> --- a/t/t5510-fetch.sh\n>> +++ b/t/t5510-fetch.sh\n>> @@ -708,4 +708,15 @@ test_expect_success 'fetching a one-level ref works' '\n>>         )\n>>  '\n>>\n>> +test_expect_success 'fetching deepen' '\n>> +       git clone . deepen --depth=1 &&\n>> +       cd deepen &&\n>\n> When this tests ends, the working directory will still be 'deepen',\n> which will likely break tests added after this one. If you wrap the\n> 'cd' and following statements in a subshell via '(' and ')', then the\n> 'cd' will affect the subshell but leave the parent shell's working\n> directory alone, and thus not negatively impact subsequent tests.\n>\n>> +       git fetch .. foo --depth=1\n>> +       git show foo\n>> +       test_must_fail git show foo~\n>> +       git fetch .. foo --depth=1 --deepen\n>> +       git show foo~\n>> +       test_must_fail git show foo~2\n>\n> Broken &&-chain throughout.\n>\n>> +'\n>> +\n>>  test_done\n\n\n\n-- \n江东灿（Dongcan Jiang）\nTeam of Search Engine & Web Mining\nSchool of Electronic Engineering & Computer Science\nPeking University, Beijing, 100871, P.R.China\n"},{"id":"257795","messageId":"CABwkPcrCBAbGf6ToYTC5TWctvphbQsn3vsxETitgfZicLnijjw@mail.gmail.com","threadId":"38795","inReplyTo":"xmqqlhj0xhsl.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH/GSoC/RFC] fetch: git fetch --deepen","fromName":"Dongcan Jiang","fromEmail":"dongcan.jiang@gmail.com","sentAt":"2015-03-16T16:08:06Z","receivedAt":"2015-03-16T16:08:06Z","isPatch":true,"sender":{"key":"dongcan.jiang@gmail.com","avatar":"https://avatars.githubusercontent.com/u/10103762?v=4"},"body":"Hi Junio,\n\nSorry for my late response.\n\n> As this operation is not about moving _any_ refs, whether local\n> branches or remote-tracking branches, any ref that used to point at\n> commit B before you executed \"fetch --deepen\" would point at the\n> same commit after the command finishes.\n\nThanks for clarifying the definition of \"you\". In this patch, it\nactually would update the remote-tracking branches to the newest history.\nI will keep working on it to figure out the reason.\n\nIt looks that it would be more efficient if the new history is not fetched,\nas it transports less objects. Is this your main consideration on not\nupdating any refs?\n\n>> -             if (starts_with(line, \"deepen \")) {\n>> +             if (starts_with(line, \"depth \")) {\n>>                       char *end;\n>> -                     depth = strtol(line + 7, &end, 0);\n>> -                     if (end == line + 7 || depth <= 0)\n>> -                             die(\"Invalid deepen: %s\", line);\n>> +                     depth = strtol(line + 6, &end, 0);\n>> +                     if (end == line + 6 || depth <= 0)\n>> +                             die(\"Invalid depth: %s\", line);\n>>                       continue;\n>>               }\n>>               if (!starts_with(line, \"want \") ||\n>\n> I do not quite understand this hunk.  What happens when this program\n> is talking to an existing fetch-pack that requested \"deepen\"?\n\nIn this hunk, I actually just changed the one command name in upload-pack\nservice from \"deepen\" to \"depth\". I made this change because the name\n\"deepen\" here is quite misleading, as it just means \"depth\". Of course,\nI've changed the caller's sent pack from \"deepen\" to \"depth\" too (See below).\n\n> @@ -317,7 +318,7 @@ static int find_common(struct fetch_pack_args *args,\n>         if (is_repository_shallow())\n>                 write_shallow_commits(&req_buf, 1, NULL);\n>         if (args->depth > 0)\n> -               packet_buf_write(&req_buf, \"deepen %d\", args->depth);\n> +               packet_buf_write(&req_buf, \"depth %d\", args->depth);\n>         packet_buf_flush(&req_buf);\n>         state_len = req_buf.len;\n\nIf the user wants \"deepen\", he should send a \"depth_deepen\" signal as well as\n\"depth N\". When the upload-pack service in this patch receive \"depth_deepen\"\nand \"depth N\", it collects N more commits ahead of the shallow commit and send\nback to the caller.\n\nThanks,\nDongcan\n\n2015-03-14 13:35 GMT+08:00 Junio C Hamano <gitster@pobox.com>:\n> Dongcan Jiang <dongcan.jiang@gmail.com> writes:\n>\n>> This patch is just for discusstion. An option --deepen is added to\n>> 'git fetch'. When it comes to '--deepen', git should fetch N more\n>> commits ahead the local shallow commit, where N is indicated by\n>> '--depth=N'. [1]\n>>\n>> e.g.\n>>\n>>>  (upstream)\n>>>   ---o---o---o---A---B\n>>>\n>>>  (you)\n>>>                  A---B\n>>\n>> After excuting \"git fetch --depth=1 --deepen\", (you) get one more\n>> tip and it becomes\n>>\n>>>  (you)\n>>>              o---A---B\n>>\n>> '--deepen' is designed to be a boolean option in this patch, which\n>> is a little different from [1]. It's designed in this way, because\n>> it can reuse '--depth' in the program, and just costs one more bit\n>> in some data structure, such as fetch_pack_args,\n>> git_transport_options.\n>>\n>> Of course, as a patch for discussion, it remains a long way to go\n>> before being complete.\n>>\n>>       1) Documents should be completed.\n>>       2) More test cases, expecially corner cases, should be added.\n>>       3) No need to get remote refs when it comes to '--deepen' option.\n>>       4) Validity on options combination should be checked.\n>>       5) smart-http protocol remains to be supported. [2]\n>>\n>> Besides, I still have one question:\n>> What does (you) exactly mean in [1]? The local branch or the local\n>> remote ref?\n>\n> As this operation is not about moving _any_ refs, whether local\n> branches or remote-tracking branches, any ref that used to point at\n> commit B before you executed \"fetch --deepen\" would point at the\n> same commit after the command finishes.\n>\n> The \"you\" does not explicitly depict any ref, because the picture is\n> meant to illustrate _everything_ the repository at the receiving end\n> of \"fetch\" has.  It used to have two commits, A and B (and the tree\n> and blob objects necessary to complete these two commits).  After\n> deepening the history by one commit, it then will have commit A^ and\n> its trees and blobs.\n>\n>> diff --git a/upload-pack.c b/upload-pack.c\n>> index b531a32..8004f2f 100644\n>> --- a/upload-pack.c\n>> +++ b/upload-pack.c\n>> @@ -31,6 +31,7 @@ static const char upload_pack_usage[] = \"git upload-pack [--strict] [--timeout=<\n>>\n>>  static unsigned long oldest_have;\n>>\n>> +static int depth_deepen;\n>>  static int multi_ack;\n>>  static int no_done;\n>>  static int use_thin_pack, use_ofs_delta, use_include_tag;\n>> @@ -563,11 +564,11 @@ static void receive_needs(void)\n>>                       }\n>>                       continue;\n>>               }\n>> -             if (starts_with(line, \"deepen \")) {\n>> +             if (starts_with(line, \"depth \")) {\n>>                       char *end;\n>> -                     depth = strtol(line + 7, &end, 0);\n>> -                     if (end == line + 7 || depth <= 0)\n>> -                             die(\"Invalid deepen: %s\", line);\n>> +                     depth = strtol(line + 6, &end, 0);\n>> +                     if (end == line + 6 || depth <= 0)\n>> +                             die(\"Invalid depth: %s\", line);\n>>                       continue;\n>>               }\n>>               if (!starts_with(line, \"want \") ||\n>\n> I do not quite understand this hunk.  What happens when this program\n> is talking to an existing fetch-pack that requested \"deepen\"?\n>\n>> @@ -577,6 +578,8 @@ static void receive_needs(void)\n>>\n>>               features = line + 45;\n>>\n>> +             if (parse_feature_request(features, \"depth_deepen\"))\n>> +                     depth_deepen = 1;\n>>               if (parse_feature_request(features, \"multi_ack_detailed\"))\n>>                       multi_ack = 2;\n>>               else if (parse_feature_request(features, \"multi_ack\"))\n>> @@ -631,6 +634,10 @@ static void receive_needs(void)\n>>                               struct object *object = shallows.objects[i].item;\n>>                               object->flags |= NOT_SHALLOW;\n>>                       }\n>> +             else if (depth_deepen)\n>> +                     backup = result =\n>> +                             get_shallow_commits(&shallows, depth + 1,\n>> +                                                 SHALLOW, NOT_SHALLOW);\n>>               else\n>>                       backup = result =\n>>                               get_shallow_commits(&want_obj, depth,\n>> --\n>> 2.3.1.253.gb3fd89e\n\n\n\n-- \n江东灿（Dongcan Jiang）\nTeam of Search Engine & Web Mining\nSchool of Electronic Engineering & Computer Science\nPeking University, Beijing, 100871, P.R.China\n"},{"id":"257796","messageId":"CABwkPcrF9oFvNkbp6rFV28U3w-szyV9k4LFviYyFkoJvp8pjbA@mail.gmail.com","threadId":"38795","inReplyTo":"CACsJy8AAsAO6gjWg0nsdiOXFEsZ4583pH9mkUx1Js3Yop5KHHQ@mail.gmail.com","subject":"Re: [PATCH/GSoC/RFC] fetch: git fetch --deepen","fromName":"Dongcan Jiang","fromEmail":"dongcan.jiang@gmail.com","sentAt":"2015-03-16T16:10:08Z","receivedAt":"2015-03-16T16:10:08Z","isPatch":true,"sender":{"key":"dongcan.jiang@gmail.com","avatar":"https://avatars.githubusercontent.com/u/10103762?v=4"},"body":"Hi Duy,\n\nSorry for my late response.\n\n> we need to make sure that upload-pack barf if some client sends both \"deepen\" and\n> \"depth\".\n\nActually, in my current design, when client just wants \"depth\", it\nsends \"depth N\";\nwhen it want \"deepen\", it sends \"depth N\" as well as \"depth_deepen\".\nFor the latter\ncase, it tells upload-pack to collect objects for \"deepen N\".\n\nThus, I'm not so sure whether upload-pack needs to check their exclusion.\n\n> I was about to suggest you use \"deepen\" for both --depth and\n> --deepen: --depth sends \"deepen NUM\" while --deepen sends \"deepen\n> +NUM\". The protocol as described in pack-protocol.txt may allow this\n> extension.\n\nThis suggestion looks neat! However, I'm afraid it may involves quite\na bit changes\nas you've mentioned, which would be better to take in next stage.\n\nThanks,\nDongcan\n\n2015-03-14 18:56 GMT+08:00 Duy Nguyen <pclouds@gmail.com>:\n> On Fri, Mar 13, 2015 at 8:04 PM, Dongcan Jiang <dongcan.jiang@gmail.com> wrote:\n>> @@ -317,7 +318,7 @@ static int find_common(struct fetch_pack_args *args,\n>>         if (is_repository_shallow())\n>>                 write_shallow_commits(&req_buf, 1, NULL);\n>>         if (args->depth > 0)\n>> -               packet_buf_write(&req_buf, \"deepen %d\", args->depth);\n>> +               packet_buf_write(&req_buf, \"depth %d\", args->depth);\n>>         packet_buf_flush(&req_buf);\n>>         state_len = req_buf.len;\n>\n> Junio already questioned about your replacing starts_with(\"deepen \"..)\n> with starts_with(\"depth \"..), this is part of that question. If you\n> run \"make test\" you should find some breakages, or we need to improve\n> our test suite.\n>\n> I think you just need to keep the old \"if (args->depth > 0) send\n> \"depth\" unchanged and add two new statements that check\n> args->depth_deepen and sends \"depth \". BTW, I think \"deepen-more\" or\n> \"deepen-relative\" would be better than \"depth\" (*), which is a noun.\n> But that's a minor point at this stage.\n>\n> And because --depth and --deepen are mutually exclusive, you need to\n> make sure that the user must not specify that. The \"user\" includes the\n> \"client\" from the server perspective, we need to make sure that\n> upload-pack barf if some client sends both \"deepen\" and \"depth\".\n>\n> (*) I was about to suggest you use \"deepen\" for both --depth and\n> --deepen: --depth sends \"deepen NUM\" while --deepen sends \"deepen\n> +NUM\". The protocol as described in pack-protocol.txt may allow this\n> extension. Unfortunately the current implementation is too relaxed. We\n> use strtol() to parse \"NUM\" and it would accept the leading \"+\"\n> instead of barfing out. So old clients would mistakes --deepen as\n> --depth, not good. And it even accepts base 8 and 16!! We should fix\n> this.\n> --\n> Duy\n\n\n\n-- \n江东灿（Dongcan Jiang）\nTeam of Search Engine & Web Mining\nSchool of Electronic Engineering & Computer Science\nPeking University, Beijing, 100871, P.R.China\n"},{"id":"257849","messageId":"CACsJy8AO1w0tYmYkOLjB6osw50gYpc01_6iUt5JZLqL0xtDPrw@mail.gmail.com","threadId":"38795","inReplyTo":"CABwkPcrF9oFvNkbp6rFV28U3w-szyV9k4LFviYyFkoJvp8pjbA@mail.gmail.com","subject":"Re: [PATCH/GSoC/RFC] fetch: git fetch --deepen","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-03-17T10:44:18Z","receivedAt":"2015-03-17T10:44:18Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Mar 16, 2015 at 11:10 PM, Dongcan Jiang <dongcan.jiang@gmail.com> wrote:\n> Hi Duy,\n>\n> Sorry for my late response.\n>\n>> we need to make sure that upload-pack barf if some client sends both \"deepen\" and\n>> \"depth\".\n>\n> Actually, in my current design, when client just wants \"depth\", it\n> sends \"depth N\";\n> when it want \"deepen\", it sends \"depth N\" as well as \"depth_deepen\".\n> For the latter\n> case, it tells upload-pack to collect objects for \"deepen N\".\n>\n> Thus, I'm not so sure whether upload-pack needs to check their exclusion.\n\nC Git is not the only client that can talk to upload-pack. A buggy\nclient may send both. The least we need is make sure it would not\ncrash or break the server security in any way. But if we have to\nconsider that, we may as well reject with a clear message, which would\nhelp the client writer. And speaking of clients..\n\nOn Mon, Mar 16, 2015 at 11:08 PM, Dongcan Jiang <dongcan.jiang@gmail.com> wrote:\n>>> -             if (starts_with(line, \"deepen \")) {\n>>> +             if (starts_with(line, \"depth \")) {\n>>>                       char *end;\n>>> -                     depth = strtol(line + 7, &end, 0);\n>>> -                     if (end == line + 7 || depth <= 0)\n>>> -                             die(\"Invalid deepen: %s\", line);\n>>> +                     depth = strtol(line + 6, &end, 0);\n>>> +                     if (end == line + 6 || depth <= 0)\n>>> +                             die(\"Invalid depth: %s\", line);\n>>>                       continue;\n>>>               }\n>>>               if (!starts_with(line, \"want \") ||\n>>\n>> I do not quite understand this hunk.  What happens when this program\n>> is talking to an existing fetch-pack that requested \"deepen\"?\n>\n> In this hunk, I actually just changed the one command name in upload-pack\n> service from \"deepen\" to \"depth\". I made this change because the name\n> \"deepen\" here is quite misleading, as it just means \"depth\". Of course,\n> I've changed the caller's sent pack from \"deepen\" to \"depth\" too (See below).\n\nYou missed Junio's keyword \"existing\". Your new client will work well\nwith your new server. But it breaks \"git clone --depth\" (or \"git fetch\n--depth\") for all existing git installations.\n-- \nDuy\n"},{"id":"257851","messageId":"CABwkPcq2AawTCQLwfvyR8gbx4Z7BhA8vKd_ON3LSCVcxFX9b4w@mail.gmail.com","threadId":"38795","inReplyTo":"CACsJy8AO1w0tYmYkOLjB6osw50gYpc01_6iUt5JZLqL0xtDPrw@mail.gmail.com","subject":"Re: [PATCH/GSoC/RFC] fetch: git fetch --deepen","fromName":"Dongcan Jiang","fromEmail":"dongcan.jiang@gmail.com","sentAt":"2015-03-17T12:00:41Z","receivedAt":"2015-03-17T12:00:41Z","isPatch":true,"sender":{"key":"dongcan.jiang@gmail.com","avatar":"https://avatars.githubusercontent.com/u/10103762?v=4"},"body":"> C Git is not the only client that can talk to upload-pack. A buggy\n> client may send both. The least we need is make sure it would not\n> crash or break the server security in any way. But if we have to\n> consider that, we may as well reject with a clear message, which would\n> help the client writer. And speaking of clients..\n\nActually, I mean that the upload-pack in this patch will not crash,\nwhatever it receives.\ne.g. \"depth N\", \"depth_deepen\", \"depth N\"+\"depth_deepen\"\n\nBecause \"depth_deepen\" is just a boolean signal to tell the upload-pack\nthat \"depth N\" means \"deepen N\", it takes effect only when \"depth N\"\nis given. Otherwise, \"depth_deepen\" will be ignored.\n\n\n> You missed Junio's keyword \"existing\". Your new client will work well\n> with your new server. But it breaks \"git clone --depth\" (or \"git fetch\n> --depth\") for all existing git installations.\n\nOh, thank you for pointing out this. And sorry Junio. I misunderstood\nwhat you said. I haven't realized the problem of existing git server.\nYou've reminded me of the importance of compatibility. Thank you!\n\n2015-03-17 18:44 GMT+08:00 Duy Nguyen <pclouds@gmail.com>:\n> On Mon, Mar 16, 2015 at 11:10 PM, Dongcan Jiang <dongcan.jiang@gmail.com> wrote:\n>> Hi Duy,\n>>\n>> Sorry for my late response.\n>>\n>>> we need to make sure that upload-pack barf if some client sends both \"deepen\" and\n>>> \"depth\".\n>>\n>> Actually, in my current design, when client just wants \"depth\", it\n>> sends \"depth N\";\n>> when it want \"deepen\", it sends \"depth N\" as well as \"depth_deepen\".\n>> For the latter\n>> case, it tells upload-pack to collect objects for \"deepen N\".\n>>\n>> Thus, I'm not so sure whether upload-pack needs to check their exclusion.\n>\n> C Git is not the only client that can talk to upload-pack. A buggy\n> client may send both. The least we need is make sure it would not\n> crash or break the server security in any way. But if we have to\n> consider that, we may as well reject with a clear message, which would\n> help the client writer. And speaking of clients..\n>\n> On Mon, Mar 16, 2015 at 11:08 PM, Dongcan Jiang <dongcan.jiang@gmail.com> wrote:\n>>>> -             if (starts_with(line, \"deepen \")) {\n>>>> +             if (starts_with(line, \"depth \")) {\n>>>>                       char *end;\n>>>> -                     depth = strtol(line + 7, &end, 0);\n>>>> -                     if (end == line + 7 || depth <= 0)\n>>>> -                             die(\"Invalid deepen: %s\", line);\n>>>> +                     depth = strtol(line + 6, &end, 0);\n>>>> +                     if (end == line + 6 || depth <= 0)\n>>>> +                             die(\"Invalid depth: %s\", line);\n>>>>                       continue;\n>>>>               }\n>>>>               if (!starts_with(line, \"want \") ||\n>>>\n>>> I do not quite understand this hunk.  What happens when this program\n>>> is talking to an existing fetch-pack that requested \"deepen\"?\n>>\n>> In this hunk, I actually just changed the one command name in upload-pack\n>> service from \"deepen\" to \"depth\". I made this change because the name\n>> \"deepen\" here is quite misleading, as it just means \"depth\". Of course,\n>> I've changed the caller's sent pack from \"deepen\" to \"depth\" too (See below).\n>\n> You missed Junio's keyword \"existing\". Your new client will work well\n> with your new server. But it breaks \"git clone --depth\" (or \"git fetch\n> --depth\") for all existing git installations.\n> --\n> Duy\n\n\n\n-- \n江东灿（Dongcan Jiang）\nTeam of Search Engine & Web Mining\nSchool of Electronic Engineering & Computer Science\nPeking University, Beijing, 100871, P.R.China\n"},{"id":"258294","messageId":"39ef84113d77ee6fa371ac2f23bbb0ef321323ba.1427031746.git.dongcan.jiang@gmail.com","threadId":"38795","inReplyTo":"1426251846-1604-1-git-send-email-dongcan.jiang@gmail.com","subject":"[PATCH v2/GSoC/RFC] fetch: git fetch --deepen","fromName":"Dongcan Jiang","fromEmail":"dongcan.jiang@gmail.com","sentAt":"2015-03-22T15:24:35Z","receivedAt":"2015-03-22T15:24:35Z","isPatch":true,"sender":{"key":"dongcan.jiang@gmail.com","avatar":"https://avatars.githubusercontent.com/u/10103762?v=4"},"body":"This patch is just for discusstion. An option --deepen is added to\n'git fetch'. When it comes to '--deepen', git should fetch N more\ncommits ahead the local shallow commit, where N is indicated by\n'--depth=N'. [1]\n\ne.g.\n\n>  (upstream)\n>   ---o---o---o---A---B\n>\n>  (you)\n>                  A---B\n\nAfter excuting \"git fetch --depth=1 --deepen\", (you) get one more\ntip and it becomes\n\n>  (you)\n>              o---A---B\n\n'--deepen' is designed to be a boolean option in this patch, which\nis a little different from [1]. It's designed in this way, because\nit can reuse '--depth' in the program, and just costs one more bit\nin some data structure, such as fetch_pack_args,\ngit_transport_options.\n\nOf course, as a patch for discussion, it remains a long way to go\nbefore being complete.\n\n\t1) Documents should be completed.\n\t2) More test cases, expecially corner cases, should be added.\n\t3) No need to get remote refs when it comes to '--deepen' option.\n\t4) Validity on options combination should be checked.\n\t5) smart-http protocol remains to be supported. [2]\n\n[1] http://article.gmane.org/gmane.comp.version-control.git/212950\n[2] http://article.gmane.org/gmane.comp.version-control.git/265050\n\nHelped-by: Duy Nguyen <pclouds@gmail.com>\nHelped-By: Eric Sunshine <sunshine@sunshineco.com>\nHelped-By: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Dongcan Jiang <dongcan.jiang@gmail.com>\n---\n\n-Background-\n\nAt present the command \"git-fetch\" has one option \"depth\" which deepens or shortens the history of a shallow repository created by git clone with --depth=<depth> option (see git-clone[1]) to the specified number of commits from the tip of each remote branch history [2].\n\nThe drawback is that the starting point for deepening history is limited to the tip of each remote branch history. However, it's not able to satisfy the needs of users in some cases:\n1) If the user wants to know the process of how a function was created and developed. What he expects is that he goes to the point of latest modification about the function and deepens the history further back from this current cut-off point by N commit directly, rather than having to guess what the depth is from tip of remote branch.\n2) What's more, if the user only cares about the revisions of this function, the other commits after the latest modification are redundant for him, but he has to fetch them concomitantly, which wastes unnecessary time and space.\n\n\nFor the convenience of users, a new option should be added as \"deepen\" to allow users to just get history commits deepened by N commits from current cut-off point, irrespective of location where the tips are [3].\n\n\n-Goals-\n\n1) Supports for command \"git fetch --deepen\";\n2) Conflict Detection for this option to guarantee robustness;\n3) Sufficient tests;\n4) Guarantee for compatibility;\n5) Sufficient documents;\n\nMore Details are shown in next section.\n\n\n-Details-\n\n--1. Current Workflow of git-fetch--\n\n         +-+-+-+-+-+-+-+-+-+                      +-+-+-+-+-+-+-+-+-+-+-+\n         | git-upload-pack |                      | git-unpack-objects  |\n         +-+-+-+-+-+-+-+-+-+                      +-+-+-+-+-+-+-+-+-+-+-+\n                |   ^                                      |   ^\n                v   |                                      v   |\n fetch   +-+-+-+-+-+-+-+-+-+      +-+-+-+-+-+-+-+      +-+-+-+-+-+-+\n-------> | get remote refs | ---> | fetch refs  | ---> | get_pack  | -+\n         +-+-+-+-+-+-+-+-+-+      +-+-+-+-+-+-+-+      +-+-+-+-+-+-+  |\n                                       |     ^                        |\n                                       v     |                        v\n+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+  +-+-+-+-+-+-+-+\n| git-upload-pack                                       |  |   update    |\n+  +-+-+-+-+-+-+-+-+-+-+-+-+      +-+-+-+-+-+-+-+-+-+-+ +  | local refs  |\n|  | generate wanted objs  | ---> | create_pack_file  | |  +-+-+-+-+-+-+-+\n+  |    update shallows    |      +-+-+-+-+-+-+-+-+-+-+ +\n|  +-+-+-+-+-+-+-+-+-+-+-+-+            |     ^         |\n+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+\n                                        |     |\n                                        v     |\n                                  +-+-+-+-+-+-+-+-+-+-+\n                                  | git-pack-objects  |\n                                  +-+-+-+-+-+-+-+-+-+-+\n  Figure 1. Workflow of git-fetch (Please view it in fixed-width fonts)\n\nFigure 1 illustrates the workflow of git-fetch. When it comes to git-fetch, a series of operations are performed in cooperation with the server side via `transport` module.\n\nAt first, it gets remote refs from git-upload-pack, and then it fetches refs, i.e. generating wanted objs, updating shallows and creating pack file. Finally, it unpacks objects from the pack file and updates the local refs.\n\nActually, there are several protocols running the workflow above, which is showed in Table 1. It is determined in transport.c/transport_get().\n\nTable 1. Protocols supporting `fetch` (Please view it in fixed-width fonts)\n+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+\n| *protocol*  |   *get remote refs*   | *fetch refs / get_pack* |\n+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+\n|   rsync     |  get_refs_via_rsync   |  fetch_objs_via_rsync   |\n+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+\n|   bundle    | get_refs_from_bundle  | fetch_refs_from_bundle  |\n+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+\n|   normal    | get_refs_via_connect  |  fetch_refs_via_pack    |\n+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+\n| foreign_vsc |     get_refs_list     |          fetch          |\n| and others  |  in transport-helper  |   in transport-helper   |\n+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+\n\nIn this project, \"normal\" and \"foreign_vsc and others\" protocols will be mainly concerned, because they can support fetching with \"--depth\".\n\nMore details will be discussed in following parts.\n\n\n--2. How to Support \"fetch --deepen\" for Normal Protocol--\n\nNormal protocol will be used when the remote address is not a url or when it starts with \"files://\", \"git://\", \"ssh://\", \"git+ssh://\" or \"ssh+git://\".\n\nAn intuitive way to implement this feature is as follows,\n1) Add \"--deepen\" option throughout the workflow shown in Figure 1;\n2) Generate wanted objs and update shallows according to local shallows and \"--deepen\" value, but not the newest tips;\n3) Make git not to update local refs when it's processing with \"--deepen\".\n\nActually, my current patch on this issue is implemented in this intuitive way. However, it has several drawbacks:\n1) It gets remote refs, which are useless in its workflow;\n2) For each ref, it interacts with git-upload-pack, while only one interaction is needed;\n2) It's not able to fetch refs, when the useless \"get remote refs\" step is forced to skip.\n\nTherefore, several stuffs have to be improved:\n1) Not to get remote refs when processing with \"--deepen\";\n2) Just send shallows to git-upload-pack without \"want\" objects;\n3) Make git-upload-pack able to process \"--deepen\" without \"want\" objects;\n\n\n--3. How to Support \"fetch --deepen\" for foreign_vsc And Others Protocol--\n\nThis protocol is just a bit different from above. Actually it shares most logics with the normal one.\n\nThe biggest difference is that it does the whole process through git-remote-curl and git-fetch-pack [4], which is illustrated in Figure 2.\n\n                                   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+\n                                   | git-fetch-pack            |\n fetch   +-+-+-+-+-+-+-+-+-+       +           ...             +\n-------> | git-remote-curl | <---> |  +-+-+-+-+-+-+-+-+-+      |\n         +-+-+-+-+-+-+-+-+-+       +  | get remote refs | ...  +\n                                   |  +-+-+-+-+-+-+-+-+-+      |\n                                   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+\nFigure 2. fetch through git-remote-curl (Please view it in fixed-width fonts)\n\nThe key to make it work with \"--deepen\" is to add the option throughout remote-curl.c and builtin/fetch-pack.c.\n\n\n--4. Tests--\n\nAs git is a product demanding high robustness. Sufficient tests have to be done to make sure the new feature's correctness.\n\nCases needed to be tested are listed in Table 2.\n\nTable 2. Cases needed to be tested (Please view it in fixed-width fonts)\n+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+\n|  *module*   |                       *cases*                             |\n+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+\n|             | should fail when used with some options like --unshallow  |\n|   fetch,    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+\n|             |   work correctly when deepen exceeds the remote shallow   |\n|    pull,    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+\n|             |         the updated shallows by deepen are correct        |\n| fetch-pack  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+\n|             |          local remote refs should not be updated          |\n+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+\nEvery module on the left needs to test all cases on the right side.\n\n\n--5. Compatibility--\n\nIt's impossible to request users to install the software again for new changes, so all changes shouldn't go against the existing installations. For this case, neither existing options nor their behaviors in code should be changed to guarantee compatibility [5,6].\n\n\n[1] http://git-scm.com/docs/git-clone\n[2] http://git-scm.com/docs/git-fetch\n[3] http://article.gmane.org/gmane.comp.version-control.git/212950\n[4] http://marc.info/?l=git&m=142580732110033&w=2\n[5] http://marc.info/?l=git&m=142631134424268&w=2\n[6] http://marc.info/?l=git&m=142658909625290&w=2\n\nAbove is my draft proposal for \"git fetch --deepen\".\n\nHowever, I'm still not quite following when would it go through the git-remote-curl workflow shown in Figure 2. Are there any examples I could follow in the test set?\n\nHave I missed anything else in my draft? Look forward to your comments and suggestions.\n\n\n builtin/fetch.c  |  7 +++++--\n fetch-pack.c     |  1 +\n fetch-pack.h     |  1 +\n t/t5510-fetch.sh | 12 ++++++++++++\n transport.c      |  4 ++++\n transport.h      |  4 ++++\n upload-pack.c    | 27 ++++++++++++++++++---------\n 7 files changed, 45 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex f951265..d8b6504 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -33,7 +33,7 @@ static int fetch_prune_config = -1; /* unspecified */\n static int prune = -1; /* unspecified */\n #define PRUNE_BY_DEFAULT 0 /* do we prune by default? */\n\n-static int all, append, dry_run, force, keep, multiple, update_head_ok, verbosity;\n+static int all, append, dry_run, force, keep, multiple, update_head_ok, verbosity, deepen;\n static int progress = -1, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;\n static int tags = TAGS_DEFAULT, unshallow, update_shallow;\n static const char *depth;\n@@ -111,6 +111,7 @@ static struct option builtin_fetch_options[] = {\n \tOPT_BOOL(0, \"progress\", &progress, N_(\"force progress reporting\")),\n \tOPT_STRING(0, \"depth\", &depth, N_(\"depth\"),\n \t\t   N_(\"deepen history of shallow clone\")),\n+\tOPT_BOOL(0, \"deepen\", &deepen, N_(\"deepen\")),\n \t{ OPTION_SET_INT, 0, \"unshallow\", &unshallow, NULL,\n \t\t   N_(\"convert to a complete repository\"),\n \t\t   PARSE_OPT_NONEG | PARSE_OPT_NOARG, NULL, 1 },\n@@ -756,7 +757,7 @@ static int fetch_refs(struct transport *transport, struct ref *ref_map)\n \tint ret = quickfetch(ref_map);\n \tif (ret)\n \t\tret = transport_fetch_refs(transport, ref_map);\n-\tif (!ret)\n+\tif (!ret && !deepen)\n \t\tret |= store_updated_refs(transport->url,\n \t\t\t\ttransport->remote->name,\n \t\t\t\tref_map);\n@@ -853,6 +854,8 @@ static struct transport *prepare_transport(struct remote *remote)\n \t\tset_option(transport, TRANS_OPT_KEEP, \"yes\");\n \tif (depth)\n \t\tset_option(transport, TRANS_OPT_DEPTH, depth);\n+\tif (deepen)\n+\t\tset_option(transport, TRANS_OPT_DEEPEN, \"yes\");\n \tif (update_shallow)\n \t\tset_option(transport, TRANS_OPT_UPDATE_SHALLOW, \"yes\");\n \treturn transport;\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex 655ee64..3c6f17a 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -295,6 +295,7 @@ static int find_common(struct fetch_pack_args *args,\n \t\t\tif (no_done)            strbuf_addstr(&c, \" no-done\");\n \t\t\tif (use_sideband == 2)  strbuf_addstr(&c, \" side-band-64k\");\n \t\t\tif (use_sideband == 1)  strbuf_addstr(&c, \" side-band\");\n+\t\t\tif (args->deepen_mode)  strbuf_addstr(&c, \" deepen-mode\");\n \t\t\tif (args->use_thin_pack) strbuf_addstr(&c, \" thin-pack\");\n \t\t\tif (args->no_progress)   strbuf_addstr(&c, \" no-progress\");\n \t\t\tif (args->include_tag)   strbuf_addstr(&c, \" include-tag\");\ndiff --git a/fetch-pack.h b/fetch-pack.h\nindex bb7fd76..f575c8b 100644\n--- a/fetch-pack.h\n+++ b/fetch-pack.h\n@@ -10,6 +10,7 @@ struct fetch_pack_args {\n \tconst char *uploadpack;\n \tint unpacklimit;\n \tint depth;\n+\tunsigned deepen_mode:1;\n \tunsigned quiet:1;\n \tunsigned keep_pack:1;\n \tunsigned lock_pack:1;\ndiff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\nindex d78f320..3b960c8 100755\n--- a/t/t5510-fetch.sh\n+++ b/t/t5510-fetch.sh\n@@ -708,4 +708,16 @@ test_expect_success 'fetching a one-level ref works' '\n \t)\n '\n\n+test_expect_success 'fetching deepen' '\n+\tgit clone . deepen --depth=1 && (\n+\t\tcd deepen &&\n+\t\tgit fetch .. foo --depth=1\n+\t\tgit show foo\n+\t\ttest_must_fail git show foo~\n+\t\tgit fetch .. foo --depth=1 --deepen\n+\t\tgit show foo~\n+\t\ttest_must_fail git show foo~2\n+\t)\n+'\n+\n test_done\ndiff --git a/transport.c b/transport.c\nindex 0694a7c..0a43474 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -478,6 +478,9 @@ static int set_git_option(struct git_transport_options *opts,\n \t\t\t\tdie(\"transport: invalid depth option '%s'\", value);\n \t\t}\n \t\treturn 0;\n+\t} else if (!strcmp(name, TRANS_OPT_DEEPEN)) {\n+\t\topts->deepen_mode = !!value;\n+\t\treturn 0;\n \t} else if (!strcmp(name, TRANS_OPT_PUSH_CERT)) {\n \t\topts->push_cert = !!value;\n \t\treturn 0;\n@@ -534,6 +537,7 @@ static int fetch_refs_via_pack(struct transport *transport,\n \targs.quiet = (transport->verbose < 0);\n \targs.no_progress = !transport->progress;\n \targs.depth = data->options.depth;\n+\targs.deepen_mode = data->options.deepen_mode;\n \targs.check_self_contained_and_connected =\n \t\tdata->options.check_self_contained_and_connected;\n \targs.cloning = transport->cloning;\ndiff --git a/transport.h b/transport.h\nindex 18d2cf8..357a57b 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -13,6 +13,7 @@ struct git_transport_options {\n \tunsigned self_contained_and_connected : 1;\n \tunsigned update_shallow : 1;\n \tunsigned push_cert : 1;\n+\tunsigned deepen_mode : 1;\n \tint depth;\n \tconst char *uploadpack;\n \tconst char *receivepack;\n@@ -153,6 +154,9 @@ struct transport *transport_get(struct remote *, const char *);\n /* Limit the depth of the fetch if not null */\n #define TRANS_OPT_DEPTH \"depth\"\n\n+/* Limit the deepen of the fetch if not null */\n+#define TRANS_OPT_DEEPEN \"deepen\"\n+\n /* Aggressively fetch annotated tags if possible */\n #define TRANS_OPT_FOLLOWTAGS \"followtags\"\n\ndiff --git a/upload-pack.c b/upload-pack.c\nindex b531a32..fbe095a 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -31,6 +31,7 @@ static const char upload_pack_usage[] = \"git upload-pack [--strict] [--timeout=<\n\n static unsigned long oldest_have;\n\n+static int deepen_mode;\n static int multi_ack;\n static int no_done;\n static int use_thin_pack, use_ofs_delta, use_include_tag;\n@@ -577,6 +578,8 @@ static void receive_needs(void)\n\n \t\tfeatures = line + 45;\n\n+\t\tif (parse_feature_request(features, \"deepen-mode\"))\n+\t\t\tdeepen_mode = 1;\n \t\tif (parse_feature_request(features, \"multi_ack_detailed\"))\n \t\t\tmulti_ack = 2;\n \t\telse if (parse_feature_request(features, \"multi_ack\"))\n@@ -596,15 +599,17 @@ static void receive_needs(void)\n \t\tif (parse_feature_request(features, \"include-tag\"))\n \t\t\tuse_include_tag = 1;\n\n-\t\to = parse_object(sha1_buf);\n-\t\tif (!o)\n-\t\t\tdie(\"git upload-pack: not our ref %s\",\n-\t\t\t    sha1_to_hex(sha1_buf));\n-\t\tif (!(o->flags & WANTED)) {\n-\t\t\to->flags |= WANTED;\n-\t\t\tif (!is_our_ref(o))\n-\t\t\t\thas_non_tip = 1;\n-\t\t\tadd_object_array(o, NULL, &want_obj);\n+\t\tif (!deepen_mode) {\n+\t\t\to = parse_object(sha1_buf);\n+\t\t\tif (!o)\n+\t\t\t\tdie(\"git upload-pack: not our ref %s\",\n+\t\t\t\t\t\tsha1_to_hex(sha1_buf));\n+\t\t\tif (!(o->flags & WANTED)) {\n+\t\t\t\to->flags |= WANTED;\n+\t\t\t\tif (!is_our_ref(o))\n+\t\t\t\t\thas_non_tip = 1;\n+\t\t\t\tadd_object_array(o, NULL, &want_obj);\n+\t\t\t}\n \t\t}\n \t}\n\n@@ -631,6 +636,10 @@ static void receive_needs(void)\n \t\t\t\tstruct object *object = shallows.objects[i].item;\n \t\t\t\tobject->flags |= NOT_SHALLOW;\n \t\t\t}\n+\t\telse if (deepen_mode)\n+\t\t\tbackup = result =\n+\t\t\t\tget_shallow_commits(&shallows, depth + 1,\n+\t\t\t\t\t\t    SHALLOW, NOT_SHALLOW);\n \t\telse\n \t\t\tbackup = result =\n \t\t\t\tget_shallow_commits(&want_obj, depth,\n--\n2.3.3.221.g72ad08d.dirty\n"},{"id":"258306","messageId":"CAPig+cSSQaMyW4fbb0guBqJ---VAG8m4trt2a52qLYAANT01og@mail.gmail.com","threadId":"38795","inReplyTo":"39ef84113d77ee6fa371ac2f23bbb0ef321323ba.1427031746.git.dongcan.jiang@gmail.com","subject":"Re: [PATCH v2/GSoC/RFC] fetch: git fetch --deepen","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-03-22T19:15:20Z","receivedAt":"2015-03-22T19:15:20Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Mar 22, 2015 at 11:24 AM, Dongcan Jiang <dongcan.jiang@gmail.com> wrote:\n> This patch is just for discusstion. An option --deepen is added to\n> 'git fetch'. When it comes to '--deepen', git should fetch N more\n> commits ahead the local shallow commit, where N is indicated by\n> '--depth=N'. [1]\n> Signed-off-by: Dongcan Jiang <dongcan.jiang@gmail.com>\n> ---\n> diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\n> index d78f320..3b960c8 100755\n> --- a/t/t5510-fetch.sh\n> +++ b/t/t5510-fetch.sh\n> @@ -708,4 +708,16 @@ test_expect_success 'fetching a one-level ref works' '\n>         )\n>  '\n>\n> +test_expect_success 'fetching deepen' '\n> +       git clone . deepen --depth=1 && (\n\nQuoting Junio[1]: \"...make sure tests serve as good examples, tests\nshould stick to 'options first and then arguments'...\"\n\n> +               cd deepen &&\n> +               git fetch .. foo --depth=1\n\nSee [1].\n\n> +               git show foo\n> +               test_must_fail git show foo~\n> +               git fetch .. foo --depth=1 --deepen\n\nSee [1].\n\n> +               git show foo~\n> +               test_must_fail git show foo~2\n\nMentioned previously[2]: Broken &&-chain throughout this test.\n\n> +       )\n> +'\n> +\n>  test_done\n\n[1]: http://article.gmane.org/gmane.comp.version-control.git/265248\n[2]: http://article.gmane.org/gmane.comp.version-control.git/265419\n"},{"id":"258326","messageId":"CACsJy8A6ffMvySRUhfx8VCvTe2z+s7R-F6z+S+dex_xQ4kORKg@mail.gmail.com","threadId":"38795","inReplyTo":"39ef84113d77ee6fa371ac2f23bbb0ef321323ba.1427031746.git.dongcan.jiang@gmail.com","subject":"Re: [PATCH v2/GSoC/RFC] fetch: git fetch --deepen","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-03-22T23:23:28Z","receivedAt":"2015-03-22T23:23:28Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sun, Mar 22, 2015 at 10:24 PM, Dongcan Jiang <dongcan.jiang@gmail.com> wrote:\n> This patch is just for discusstion. An option --deepen is added to\n> 'git fetch'. When it comes to '--deepen', git should fetch N more\n> commits ahead the local shallow commit, where N is indicated by\n> '--depth=N'. [1]\n>\n> e.g.\n>\n>>  (upstream)\n>>   ---o---o---o---A---B\n>>\n>>  (you)\n>>                  A---B\n>\n> After excuting \"git fetch --depth=1 --deepen\", (you) get one more\n> tip and it becomes\n>\n>>  (you)\n>>              o---A---B\n>\n> '--deepen' is designed to be a boolean option in this patch, which\n> is a little different from [1]. It's designed in this way, because\n> it can reuse '--depth' in the program, and just costs one more bit\n> in some data structure, such as fetch_pack_args,\n> git_transport_options.\n>\n> Of course, as a patch for discussion, it remains a long way to go\n> before being complete.\n>\n>         1) Documents should be completed.\n>         2) More test cases, expecially corner cases, should be added.\n>         3) No need to get remote refs when it comes to '--deepen' option.\n>         4) Validity on options combination should be checked.\n>         5) smart-http protocol remains to be supported. [2]\n\nQuick notes before $DAYJOB starts. Cool pictures, perhaps they could\nbe part of the commit message too.\n\nPersonally i still don't think not moving the refs is worth the effort\n(and it's a waste if we have to send then drop objects for these\nupdated refs, but I didn't check carefully). So if you we don't needs\nref updates, we probably don't need to send \"want\" lines and sort of\nsimplify processing at upload-pack side.\n\nAnd it makes me realise, we're loosing security a bit here. We\nnormally don't send anything that's not reachable from the visible ref\nset. But we now would accept any shallow sha-1 and send some objects\nregardless if these sha-1 are connected to any refs. We may need some\nmore checking in place to avoid this. See check_non_sha1_tip() for a\nway to do it. Pack bitmaps may help as well, but I think that's behind\nthe scene (i.e. behind rev-list and we already can take advantage of\nit).\n-- \nDuy\n"}]}