{"thread":{"id":"53034","subject":"[PATCH 0/2] Slightly simplify partial clone user experience","startedAt":"2020-03-19T17:28:12Z","lastAt":"2020-03-24T05:26:09Z","messageCount":15,"participants":["Derrick Stolee via GitGitGadget","Derrick Stolee","Junio C Hamano","Jeff King","Christian Couder","Taylor Blau","Jonathan Nieder"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"393479","messageId":"pull.586.git.1584638887.gitgitgadget@gmail.com","threadId":"53034","inReplyTo":null,"subject":"[PATCH 0/2] Slightly simplify partial clone user experience","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-03-19T17:28:05Z","receivedAt":"2020-03-19T17:28:12Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"This was something discussed briefly at the contributor summit: users will\nhave a hard time remembering git clone --filter=blob:none . This series\nsimply adds a --partial option that is equivalent to --filter=blob:none,\nwith the ability to specify a size using --partial= that is equivalent\nto --filter=blob:limit=.\n\nWhile going to the git clone documentation to add this option, I noticed\nthat there is no reference to the --filter option there, nor any discussion\nof partial clone. I resolved the former.\n\nREQUEST FOR HELP: If anyone out there is looking for an opportunity to\ncontribute documentation, I feel there is a need to include a section in the \ngit clone documentation devoted to partial clone. Hopefully such a section\nwould include answers to these questions:\n\n 1. What is a partial clone?\n 2. Why would I want a partial clone?\n 3. How do I need to be careful when using a partial clone?\n\nI don't have time to write such a document right now, which is why I'm\nasking for help if anyone is looking for a non-code way to contribute to the\nproject.\n\nThanks, -Stolee\n\nDerrick Stolee (2):\n partial-clone: set default filter with --partial\n clone: document --partial and --filter options\n\n Documentation/git-clone.txt | 15 ++++++++++++-\n list-objects-filter-options.c | 18 +++++++++++++++\n list-objects-filter-options.h | 8 ++++++-\n t/t5616-partial-clone.sh | 42 ++++++++++++++++++++++++++---------\n 4 files changed, 71 insertions(+), 12 deletions(-)\n\n\nbase-commit: 6c85aac65fb455af85745130ce35ddae4678db84\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-586%2Fderrickstolee%2Fpartial-clone-ux-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-586/derrickstolee/partial-clone-ux-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/586\n-- \ngitgitgadget\n"},{"id":"393480","messageId":"6f340d9aadf71d394ad320ad162f1d140b632f2c.1584638887.git.gitgitgadget@gmail.com","threadId":"53034","inReplyTo":"pull.586.git.1584638887.gitgitgadget@gmail.com","subject":"[PATCH 1/2] partial-clone: set default filter with --partial","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-03-19T17:28:06Z","receivedAt":"2020-03-19T17:28:13Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nPartial clone means a lot of different things, including filtering\nout all blobs, large blobs, or objects along a certain pathspec.\nThe pathspec option has limited uses, in particular due to the\nperformance challenges in serving with such filters. The blob size\noption can be helpful for repositories with a small number of large\nbinaries, but otherwise it is difficult to find a meaningful split\nbetween \"small\" and \"large\" blobs.\n\nWhen I think of or recommend partial clone, I specifically mention\nthe case of filtering out all blobs, and downloading those blobs\nonly as needed.\n\nThis case is extremely useful, since it takes the best part of\nshallow clone (a very small initial download) without any of the\ndownsides of restricted history.\n\nHowever, the command-line interface can be confusing:\n\n\tgit clone --filter=blob:none \n\nAdd a simpler \"--partial\" option that defaults to this case:\n\n\tgit clone --partial \n\nThis should make the feature more discoverable. However, there is\na significant interest in the size-limited filters as that behaves\nvery similarly to Git LFS. For those cases, the following is\navailable:\n\n\tgit clone --partial= \n\nThere are quite a few commands using OPT_PARSE_LIST_OBJECTS_FILTER,\nincludeing clone, fetch, and pack-objects. Augment this macro to\ninclude the \"--partial[=]\" mode for free.\n\nModify the first partial clone test that checks --filter=blob:none\nto also check --partial with the same expected conditions after\nclone. The diff is much simpler to see when ignoring whitespace,\nsince several lines added a leading tab. This test is essentially\ncopied to include the two ways to specify a blob size limit of\none byte.\n\nSigned-off-by: Derrick Stolee \n---\n list-objects-filter-options.c | 18 +++++++++++++++\n list-objects-filter-options.h | 8 ++++++-\n t/t5616-partial-clone.sh | 42 ++++++++++++++++++++++++++---------\n 3 files changed, 57 insertions(+), 11 deletions(-)\n\ndiff --git a/list-objects-filter-options.c b/list-objects-filter-options.c\nindex 256bcfbdfe6..a71716ef75e 100644\n--- a/list-objects-filter-options.c\n+++ b/list-objects-filter-options.c\n@@ -270,6 +270,24 @@ int opt_parse_list_objects_filter(const struct option *opt,\n \treturn 0;\n }\n \n+int opt_set_blob_none_filter(const struct option *opt,\n+\t\t\t const char *arg, int unset)\n+{\n+\tstruct strbuf filter_arg = STRBUF_INIT;\n+\tstruct list_objects_filter_options *filter_options = opt->value;\n+\t\n+\tif (unset || !arg || !strcmp(arg, \"0\")) {\n+\t\tparse_list_objects_filter(filter_options, \"blob:none\");\n+\t\treturn 0;\n+\t}\n+\t\n+\tstrbuf_addf(&filter_arg, \"blob:limit=%s\", arg);\n+\tparse_list_objects_filter(filter_options, filter_arg.buf);\n+\tstrbuf_release(&filter_arg);\n+\n+\treturn 0;\n+}\n+\n const char *list_objects_filter_spec(struct list_objects_filter_options *filter)\n {\n \tif (!filter->filter_spec.nr)\ndiff --git a/list-objects-filter-options.h b/list-objects-filter-options.h\nindex 2ffb39222c4..ac38ffcbe86 100644\n--- a/list-objects-filter-options.h\n+++ b/list-objects-filter-options.h\n@@ -62,6 +62,7 @@ struct list_objects_filter_options {\n \n /* Normalized command line arguments */\n #define CL_ARG__FILTER \"filter\"\n+#define CL_ARG__PARTIAL \"partial\"\n \n void list_objects_filter_die_if_populated(\n \tstruct list_objects_filter_options *filter_options);\n@@ -80,11 +81,16 @@ void parse_list_objects_filter(\n \n int opt_parse_list_objects_filter(const struct option *opt,\n \t\t\t\t const char *arg, int unset);\n+int opt_set_blob_none_filter(const struct option *opt,\n+\t\t\t const char *arg, int unset);\n \n #define OPT_PARSE_LIST_OBJECTS_FILTER(fo) \\\n \t{ OPTION_CALLBACK, 0, CL_ARG__FILTER, fo, N_(\"args\"), \\\n \t N_(\"object filtering\"), 0, \\\n-\t opt_parse_list_objects_filter }\n+\t opt_parse_list_objects_filter }, \\\n+\t{ OPTION_CALLBACK, 0, CL_ARG__PARTIAL, fo, N_(\"size\"), \\\n+\t N_(\"partial clone with blob filter\"), \\\n+\t PARSE_OPT_OPTARG | PARSE_OPT_NONEG , opt_set_blob_none_filter }\n \n /*\n * Translates abbreviated numbers in the filter's filter_spec into their\ndiff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh\nindex 77bb91e9769..c42cef61296 100755\n--- a/t/t5616-partial-clone.sh\n+++ b/t/t5616-partial-clone.sh\n@@ -33,17 +33,39 @@ test_expect_success 'setup bare clone for server' '\n # confirm we are missing all of the known blobs.\n # confirm partial clone was registered in the local config.\n test_expect_success 'do partial clone 1' '\n-\tgit clone --no-checkout --filter=blob:none \"file://$(pwd)/srv.bare\" pc1 &&\n-\n-\tgit -C pc1 rev-list --quiet --objects --missing=print HEAD >revs &&\n-\tawk -f print_1.awk revs |\n-\tsed \"s/?//\" |\n-\tsort >observed.oids &&\n+\tfor option in \"--filter=blob:none\" \"--partial\"\n+\tdo\n+\t\trm -rf pc1 &&\n+\t\tgit clone --no-checkout \"$option\" \"file://$(pwd)/srv.bare\" pc1 &&\n+\n+\t\tgit -C pc1 rev-list --quiet --objects --missing=print HEAD >revs &&\n+\t\tawk -f print_1.awk revs |\n+\t\tsed \"s/?//\" |\n+\t\tsort >observed.oids &&\n+\n+\t\ttest_cmp expect_1.oids observed.oids &&\n+\t\ttest \"$(git -C pc1 config --local core.repositoryformatversion)\" = \"1\" &&\n+\t\ttest \"$(git -C pc1 config --local remote.origin.promisor)\" = \"true\" &&\n+\t\ttest \"$(git -C pc1 config --local remote.origin.partialclonefilter)\" = \"blob:none\"\n+\tdone\n+'\n \n-\ttest_cmp expect_1.oids observed.oids &&\n-\ttest \"$(git -C pc1 config --local core.repositoryformatversion)\" = \"1\" &&\n-\ttest \"$(git -C pc1 config --local remote.origin.promisor)\" = \"true\" &&\n-\ttest \"$(git -C pc1 config --local remote.origin.partialclonefilter)\" = \"blob:none\"\n+test_expect_success 'do partial clone with size limit' '\n+\tfor option in \"--filter=blob:limit=1\" \"--partial=1\"\n+\tdo\n+\t\trm -rf pc-limit &&\n+\t\tgit clone --no-checkout \"$option\" \"file://$(pwd)/srv.bare\" pc-limit &&\n+\n+\t\tgit -C pc-limit rev-list --quiet --objects --missing=print HEAD >revs &&\n+\t\tawk -f print_1.awk revs |\n+\t\tsed \"s/?//\" |\n+\t\tsort >observed.oids &&\n+\n+\t\ttest_cmp expect_1.oids observed.oids &&\n+\t\ttest \"$(git -C pc-limit config --local core.repositoryformatversion)\" = \"1\" &&\n+\t\ttest \"$(git -C pc-limit config --local remote.origin.promisor)\" = \"true\" &&\n+\t\ttest \"$(git -C pc-limit config --local remote.origin.partialclonefilter)\" = \"blob:limit=1\"\n+\tdone\n '\n \n test_expect_success 'verify that .promisor file contains refs fetched' '\n-- \ngitgitgadget\n\n"},{"id":"393481","messageId":"9baf4c8ba383414e41d3d7726cf40c39cbf197e7.1584638887.git.gitgitgadget@gmail.com","threadId":"53034","inReplyTo":"pull.586.git.1584638887.gitgitgadget@gmail.com","subject":"[PATCH 2/2] clone: document --partial and --filter options","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-03-19T17:28:07Z","receivedAt":"2020-03-19T17:28:13Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe previous change added the \"--partial[=]\" option to\n\"git clone\" equivalent to \"--filter=blob:none\" or\n\"--filter=blob:limit=\" but did not document that addition.\nIt turns out that the \"--filter=\" option was not\ndocumented anywhere in the \"git clone\" page, and instead is\ndetailed carefully in \"git rev-list\" where it serves a\ndifferent purpose.\n\nAdd a small bit about these options in the documentation. It\nwould be worth some time to create a subsection in the \"git clone\"\ndocumentation about partial clone as a concept and how it can be\na surprising experience. For example, \"git checkout\" will likely\ntrigger a pack download.\n\nSigned-off-by: Derrick Stolee \n---\n Documentation/git-clone.txt | 15 ++++++++++++++-\n 1 file changed, 14 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\nindex bf24f1813ad..eafa1c39927 100644\n--- a/Documentation/git-clone.txt\n+++ b/Documentation/git-clone.txt\n@@ -15,7 +15,8 @@ SYNOPSIS\n \t [--dissociate] [--separate-git-dir ]\n \t [--depth ] [--[no-]single-branch] [--no-tags]\n \t [--recurse-submodules[=]] [--[no-]shallow-submodules]\n-\t [--[no-]remote-submodules] [--jobs ] [--sparse] [--] \n+\t [--[no-]remote-submodules] [--jobs ] [--sparse]\n+\t [--partial[=]|--filter=] [--] \n \t []\n \n DESCRIPTION\n@@ -162,6 +163,18 @@ objects from the source repository into a pack in the cloned repository.\n \tof the repository. The sparse-checkout file can be\n \tmodified to grow the working directory as needed.\n \n+--partial[=]::\n+--filter=::\n+\tUse the partial clone feature and request that the server sends\n+\ta subset of reachable objects according to a given object filter.\n+\tWhen using `--filter`, the supplied `` is used for\n+\tthe partial clone filter. When using `--partial` with no ``,\n+\tthe `blob:none` filter is applied to filter all blobs. When using\n+\t`--partial=` the `blob:limit=` filter is applied to\n+\tfilter all blobs with size larger than ``. For more details\n+\ton filter specifications, see the `--filter` option in\n+\tlinkgit:git-rev-list[1].\n+\n --mirror::\n \tSet up a mirror of the source repository. This implies `--bare`.\n \tCompared to `--bare`, `--mirror` not only maps local branches of the\n-- \ngitgitgadget\n"},{"id":"393522","messageId":"0b84c2db-6639-6985-ae4e-6ba3b74006df@gmail.com","threadId":"53034","inReplyTo":"pull.586.git.1584638887.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/2] Slightly simplify partial clone user experience","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-03-20T12:27:58Z","receivedAt":"2020-03-20T12:28:04Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 3/19/2020 1:28 PM, Derrick Stolee via GitGitGadget wrote:\n> REQUEST FOR HELP: If anyone out there is looking for an opportunity to\n> contribute documentation, I feel there is a need to include a section in the \n> git clone documentation devoted to partial clone. Hopefully such a section\n> would include answers to these questions:\n> \n> 1. What is a partial clone?\n> 2. Why would I want a partial clone?\n> 3. How do I need to be careful when using a partial clone?\n> \n> I don't have time to write such a document right now, which is why I'm\n> asking for help if anyone is looking for a non-code way to contribute to the\n> project.\n\nI reached out on Twitter to see if anyone was interested, and I got a\nresponse. I'm working with them to get started on the mailing list. I'll\nsend a proper introduction when I know which email address they plan to\nuse on the list.\n\nThanks,\n-Stolee\n"},{"id":"393545","messageId":"xmqqr1xmbwn1.fsf@gitster.c.googlers.com","threadId":"53034","inReplyTo":"6f340d9aadf71d394ad320ad162f1d140b632f2c.1584638887.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/2] partial-clone: set default filter with --partial","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-03-20T20:26:42Z","receivedAt":"2020-03-20T20:26:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Derrick Stolee via GitGitGadget\" writes:\n\n> diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c\n> index 256bcfbdfe6..a71716ef75e 100644\n> --- a/list-objects-filter-options.c\n> +++ b/list-objects-filter-options.c\n> @@ -270,6 +270,24 @@ int opt_parse_list_objects_filter(const struct option *opt,\n> \treturn 0;\n> }\n> \n> +int opt_set_blob_none_filter(const struct option *opt,\n> +\t\t\t const char *arg, int unset)\n\nIsn't the convention to use \"opt_parse_\" for canned parse-options\ncallbacks?\n\n> +{\n> +\tstruct strbuf filter_arg = STRBUF_INIT;\n> +\tstruct list_objects_filter_options *filter_options = opt->value;\n> +\t\n> +\tif (unset || !arg || !strcmp(arg, \"0\")) {\n> +\t\tparse_list_objects_filter(filter_options, \"blob:none\");\n> +\t\treturn 0;\n\nIf \"--no-partial\" were allowed, it should be usable to countermand\n\"--partial\" earlier on the command line or perhaps configured\ndefault. But the above (especially the \"unset ||\" part) makes\n\"--no-partial\" a synonym to \"--filter=blob:none\", no?\n\n> +\t}\n> +\t\n> +\tstrbuf_addf(&filter_arg, \"blob:limit=%s\", arg);\n> +\tparse_list_objects_filter(filter_options, filter_arg.buf);\n> +\tstrbuf_release(&filter_arg);\n\nI would have expected the body of the function to read more like\nthis:\n\n\tif (unset) {\n \t... clear filter_options stored in opt->value ...\n\t} else {\n\t\tstruct strbuf filter_arg = STRBUF_INIT;\n\t\tif (!arg)\n\t\t\tstrbuf_addstr(&filter_arg, \"blob:none\");\n\t\telse\n\t\t\tstrbuf_addf(&filter_arg, \"blob:limit=%s\", arg);\n\t\tparse_list_objects_filter(filter_options, filter_arg.buf);\n\t\tstrbuf_release(&filter_arg);\n\t}\n\nSpecifically, I find it unsatisifying to see the \"0\" optimization at\nthis level. Shouldn't it be done in parse_list_objects_filter() to\nparse \"blob:limit=\" and after realizing is zero, pretend\nas if it got \"blob:none\" to optimize? That way, people can even say\n\"--partial=0k\" and get it interpreted as \"--filter=blob:none\", right?\n\n> #define OPT_PARSE_LIST_OBJECTS_FILTER(fo) \\\n> \t{ OPTION_CALLBACK, 0, CL_ARG__FILTER, fo, N_(\"args\"), \\\n> \t N_(\"object filtering\"), 0, \\\n> -\t opt_parse_list_objects_filter }\n> +\t opt_parse_list_objects_filter }, \\\n> +\t{ OPTION_CALLBACK, 0, CL_ARG__PARTIAL, fo, N_(\"size\"), \\\n> +\t N_(\"partial clone with blob filter\"), \\\n> +\t PARSE_OPT_OPTARG | PARSE_OPT_NONEG , opt_set_blob_none_filter }\n\nPARSE_OPT_NONEG means \"--no-partial\" is forbidden and the callback\nwon't see unset==1 at all, right?\n\nThanks.\n"},{"id":"393546","messageId":"2a75325b-ea50-a6f6-87dc-12184e706ac2@gmail.com","threadId":"53034","inReplyTo":"xmqqr1xmbwn1.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 1/2] partial-clone: set default filter with --partial","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-03-20T20:38:27Z","receivedAt":"2020-03-20T20:38:31Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"\n\nOn 3/20/2020 4:26 PM, Junio C Hamano wrote:\n> \"Derrick Stolee via GitGitGadget\" writes:\n> \n>> diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c\n>> index 256bcfbdfe6..a71716ef75e 100644\n>> --- a/list-objects-filter-options.c\n>> +++ b/list-objects-filter-options.c\n>> @@ -270,6 +270,24 @@ int opt_parse_list_objects_filter(const struct option *opt,\n>> \treturn 0;\n>> }\n>> \n>> +int opt_set_blob_none_filter(const struct option *opt,\n>> +\t\t\t const char *arg, int unset)\n> \n> Isn't the convention to use \"opt_parse_\" for canned parse-options\n> callbacks?\n\nI can do that.\n\n>> +{\n>> +\tstruct strbuf filter_arg = STRBUF_INIT;\n>> +\tstruct list_objects_filter_options *filter_options = opt->value;\n>> +\t\n>> +\tif (unset || !arg || !strcmp(arg, \"0\")) {\n>> +\t\tparse_list_objects_filter(filter_options, \"blob:none\");\n>> +\t\treturn 0;\n> \n> If \"--no-partial\" were allowed, it should be usable to countermand\n> \"--partial\" earlier on the command line or perhaps configured\n> default. But the above (especially the \"unset ||\" part) makes\n> \"--no-partial\" a synonym to \"--filter=blob:none\", no?\n\nI should have been more careful about the use of \"unset\" (which\nwould never be true with the current option parsing).\n\n>> +\t}\n>> +\t\n>> +\tstrbuf_addf(&filter_arg, \"blob:limit=%s\", arg);\n>> +\tparse_list_objects_filter(filter_options, filter_arg.buf);\n>> +\tstrbuf_release(&filter_arg);\n> \n> I would have expected the body of the function to read more like\n> this:\n> \n> \tif (unset) {\n> \t... clear filter_options stored in opt->value ...\n> \t} else {\n> \t\tstruct strbuf filter_arg = STRBUF_INIT;\n> \t\tif (!arg)\n> \t\t\tstrbuf_addstr(&filter_arg, \"blob:none\");\n> \t\telse\n> \t\t\tstrbuf_addf(&filter_arg, \"blob:limit=%s\", arg);\n> \t\tparse_list_objects_filter(filter_options, filter_arg.buf);\n> \t\tstrbuf_release(&filter_arg);\n> \t}\n\nThis is a better organization and I will use it.\n\n> Specifically, I find it unsatisifying to see the \"0\" optimization at\n> this level. Shouldn't it be done in parse_list_objects_filter() to\n> parse \"blob:limit=\" and after realizing is zero, pretend\n> as if it got \"blob:none\" to optimize? That way, people can even say\n> \"--partial=0k\" and get it interpreted as \"--filter=blob:none\", right?\n\nI suppose it would be worth checking the recent server-side improvements\nto see if they translate a limit=0k to a \"size 0\" and then ignore the\nsize check and simply remove all blobs.\n\n>> #define OPT_PARSE_LIST_OBJECTS_FILTER(fo) \\\n>> \t{ OPTION_CALLBACK, 0, CL_ARG__FILTER, fo, N_(\"args\"), \\\n>> \t N_(\"object filtering\"), 0, \\\n>> -\t opt_parse_list_objects_filter }\n>> +\t opt_parse_list_objects_filter }, \\\n>> +\t{ OPTION_CALLBACK, 0, CL_ARG__PARTIAL, fo, N_(\"size\"), \\\n>> +\t N_(\"partial clone with blob filter\"), \\\n>> +\t PARSE_OPT_OPTARG | PARSE_OPT_NONEG , opt_set_blob_none_filter }\n> \n> PARSE_OPT_NONEG means \"--no-partial\" is forbidden and the callback\n> won't see unset==1 at all, right?\n\nYou're right. I'm being inconsistent. Your reasons above point to a\ngood reason to have --no-partial available.\n\nThanks,\n-Stolee\n"},{"id":"393697","messageId":"20200322094659.GA635598@coredump.intra.peff.net","threadId":"53034","inReplyTo":"2a75325b-ea50-a6f6-87dc-12184e706ac2@gmail.com","subject":"Re: [PATCH 1/2] partial-clone: set default filter with --partial","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-03-22T09:46:59Z","receivedAt":"2020-03-22T09:47:02Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Mar 20, 2020 at 04:38:27PM -0400, Derrick Stolee wrote:\n\n> > Specifically, I find it unsatisifying to see the \"0\" optimization at\n> > this level. Shouldn't it be done in parse_list_objects_filter() to\n> > parse \"blob:limit=\" and after realizing is zero, pretend\n> > as if it got \"blob:none\" to optimize? That way, people can even say\n> > \"--partial=0k\" and get it interpreted as \"--filter=blob:none\", right?\n> \n> I suppose it would be worth checking the recent server-side improvements\n> to see if they translate a limit=0k to a \"size 0\" and then ignore the\n> size check and simply remove all blobs.\n\nThe new bitmap code doesn't do anything special there. It does rely on\nthe normal filter parsing, though. If we rewrote it at that level,\nperhaps like this (completely untested):\n\ndiff --git a/list-objects-filter-options.c b/list-objects-filter-options.c\nindex 256bcfbdfe..0225b61912 100644\n--- a/list-objects-filter-options.c\n+++ b/list-objects-filter-options.c\n@@ -49,7 +49,10 @@ static int gently_parse_list_objects_filter(\n \n \t} else if (skip_prefix(arg, \"blob:limit=\", &v0)) {\n \t\tif (git_parse_ulong(v0, &filter_options->blob_limit_value)) {\n-\t\t\tfilter_options->choice = LOFC_BLOB_LIMIT;\n+\t\t\tif (filter_options->blob_limit_value)\n+\t\t\t\tfilter_options->choice = LOFC_BLOB_LIMIT;\n+\t\t\telse\n+\t\t\t\tfilter_options->choice = LOFC_BLOB_NONE;\n \t\t\treturn 0;\n \t\t}\n \n\nthen it would just work for regular and bitmapped filters.\n\nOne interesting user-visible effect would be in the patches we've been\ndiscussing to limit which filters are allowed. If you allowed, say,\nblob:none but not blob:limit, this would quietly allow blob:limit=0\n(because it really _is_ blob:none under the hood). \n\nI'm not sure if that would be confusing or convenient. I doubt anybody\ncares much for the blob filters (you'd either enable neither or both,\nbecause they cost about the same). But in another thread, I mentioned\nthat doing \"tree:depth=0\" _would_ be cheap to do with bitmaps, but\ntree:depth=1\" wouldn't. If we quietly rewrote the former to tree:none\n(which doesn't yet exist, but could), that would let you distinguish\nbetween the two (of course if tree:none existed, perhaps nobody would\nhave any reason to write tree:depth=0 in the first place).\n\n-Peff\n"},{"id":"393698","messageId":"20200322095137.GB635598@coredump.intra.peff.net","threadId":"53034","inReplyTo":"pull.586.git.1584638887.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/2] Slightly simplify partial clone user experience","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-03-22T09:51:37Z","receivedAt":"2020-03-22T09:51:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 19, 2020 at 05:28:05PM +0000, Derrick Stolee via GitGitGadget wrote:\n\n> This was something discussed briefly at the contributor summit: users will\n> have a hard time remembering git clone --filter=blob:none . This series\n> simply adds a --partial option that is equivalent to --filter=blob:none,\n> with the ability to specify a size using --partial= that is equivalent\n> to --filter=blob:limit=.\n\nI have mixed feelings on this. I do like making things less arcane for\nusers. But are we locking in a behavior for --partial that we might not\nwant to live with forever? I.e., the current thinking for partial clones\nis to fetch no blobs at all, get all commits and trees, apply sparse\nfilters, and then fault in the blobs we need. But imagine we later grow\nthe ability to easily avoid fetching all of the trees. Would we regret\nhaving the simple name \"--partial\" taken?\n\n-Peff\n"},{"id":"393700","messageId":"CAP8UFD1mCXvcfUQxY4uMEPEjZp8+yPwu6SgTmZBEH6GR=8ixSA@mail.gmail.com","threadId":"53034","inReplyTo":"20200322095137.GB635598@coredump.intra.peff.net","subject":"Re: [PATCH 0/2] Slightly simplify partial clone user experience","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2020-03-22T10:58:58Z","receivedAt":"2020-03-22T10:59:16Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Sun, Mar 22, 2020 at 10:51 AM Jeff King wrote:\n>\n> On Thu, Mar 19, 2020 at 05:28:05PM +0000, Derrick Stolee via GitGitGadget wrote:\n>\n> > This was something discussed briefly at the contributor summit: users will\n> > have a hard time remembering git clone --filter=blob:none . This series\n> > simply adds a --partial option that is equivalent to --filter=blob:none,\n> > with the ability to specify a size using --partial= that is equivalent\n> > to --filter=blob:limit=.\n>\n> I have mixed feelings on this. I do like making things less arcane for\n> users. But are we locking in a behavior for --partial that we might not\n> want to live with forever? I.e., the current thinking for partial clones\n> is to fetch no blobs at all, get all commits and trees, apply sparse\n> filters, and then fault in the blobs we need. But imagine we later grow\n> the ability to easily avoid fetching all of the trees. Would we regret\n> having the simple name \"--partial\" taken?\n\nI agree with that. Something like \"--filter-blobs\" for\n\"--filter=blob:none\" and perhaps \"--filter-blobs=\" for\n\"--filter=blob:limit=\" might be worth it though.\n"},{"id":"393721","messageId":"20200322160324.GA53845@syl.local","threadId":"53034","inReplyTo":"pull.586.git.1584638887.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/2] Slightly simplify partial clone user experience","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2020-03-22T16:03:24Z","receivedAt":"2020-03-22T16:03:29Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Thu, Mar 19, 2020 at 05:28:05PM +0000, Derrick Stolee via GitGitGadget wrote:\n> This was something discussed briefly at the contributor summit: users will\n> have a hard time remembering git clone --filter=blob:none .\n\nI think that this is probably my fault :).\n\nI was saying at the contributor summit that I think I may have created\nmore confusion than clarity when I was describing how to use partial\nclones in my last release blog post.\n\nIn that post, I introduced partial clones by asking and answering the\nfollowing series of four questions.\n\n 1. What do you do when your repository is too large? Partial clones.\n 2. How do you only fetch down some of the objects? Object filters.\n 3. How do you restrict which parts of your working copy are populated?\n Write to '.git/info/sparse-checkout'.\n 4. Simplify things even further by using 'git sparse-checkout' instead.\n\nI think that some of the criticism was along the lines of \"you said that\npartial clones would be easy, but nothing in the steps 2-4 is\nstraightforward\".\n\nI think that things would have been much clearer if I said:\n\n If your repository is too large to clone all at once, or has a\n checkout footprint that is too big, you can issue a partial clone\n against it by writing the following:\n\n $ git clone --filter=blob:none --sparse /path/to/your/repo\n\n> This series simply adds a --partial option that is equivalent to\n> --filter=blob:none, with the ability to specify a size using\n> --partial= that is equivalent to --filter=blob:limit=.\n\nI think that I have mixed feelings about this, too. On the one hand, it\nirrefutably makes the above incantation a lot easier to write out and\nremember. On the other hand, I worry about it locking in options that we\ndon't want to keep around forever, like adding more filters, changing\nsome rules about sparse checkout and so on.\n\nMaybe these concerns aren't well-founded, and that we're not likely to\nchange either of those anytime soon. But, I'd rather be wrong and not be\nsquatting '--partial[=]' than not.\n\n> While going to the git clone documentation to add this option, I noticed\n> that there is no reference to the --filter option there, nor any discussion\n> of partial clone. I resolved the former.\n>\n> REQUEST FOR HELP: If anyone out there is looking for an opportunity to\n> contribute documentation, I feel there is a need to include a section in the\n> git clone documentation devoted to partial clone. Hopefully such a section\n> would include answers to these questions:\n>\n> 1. What is a partial clone?\n> 2. Why would I want a partial clone?\n> 3. How do I need to be careful when using a partial clone?\n>\n> I don't have time to write such a document right now, which is why I'm\n> asking for help if anyone is looking for a non-code way to contribute to the\n> project.\n\nThis seems like a worthy goal outside of this series, and I would\ncertainly love to have some more documentation in this area.\n\n> Thanks, -Stolee\n>\n> Derrick Stolee (2):\n> partial-clone: set default filter with --partial\n> clone: document --partial and --filter options\n>\n> Documentation/git-clone.txt | 15 ++++++++++++-\n> list-objects-filter-options.c | 18 +++++++++++++++\n> list-objects-filter-options.h | 8 ++++++-\n> t/t5616-partial-clone.sh | 42 ++++++++++++++++++++++++++---------\n> 4 files changed, 71 insertions(+), 12 deletions(-)\n>\n>\n> base-commit: 6c85aac65fb455af85745130ce35ddae4678db84\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-586%2Fderrickstolee%2Fpartial-clone-ux-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-586/derrickstolee/partial-clone-ux-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/586\n> --\n> gitgitgadget\n\nThanks,\nTaylor\n"},{"id":"393723","messageId":"5baf9f4c-9ce0-0384-1113-431038c2a646@gmail.com","threadId":"53034","inReplyTo":"CAP8UFD1mCXvcfUQxY4uMEPEjZp8+yPwu6SgTmZBEH6GR=8ixSA@mail.gmail.com","subject":"Re: [PATCH 0/2] Slightly simplify partial clone user experience","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-03-22T16:45:29Z","receivedAt":"2020-03-22T16:45:33Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 3/22/2020 6:58 AM, Christian Couder wrote:\n> On Sun, Mar 22, 2020 at 10:51 AM Jeff King wrote:\n>>\n>> On Thu, Mar 19, 2020 at 05:28:05PM +0000, Derrick Stolee via GitGitGadget wrote:\n>>\n>>> This was something discussed briefly at the contributor summit: users will\n>>> have a hard time remembering git clone --filter=blob:none . This series\n>>> simply adds a --partial option that is equivalent to --filter=blob:none,\n>>> with the ability to specify a size using --partial= that is equivalent\n>>> to --filter=blob:limit=.\n>>\n>> I have mixed feelings on this. I do like making things less arcane for\n>> users. But are we locking in a behavior for --partial that we might not\n>> want to live with forever? I.e., the current thinking for partial clones\n>> is to fetch no blobs at all, get all commits and trees, apply sparse\n>> filters, and then fault in the blobs we need. But imagine we later grow\n>> the ability to easily avoid fetching all of the trees. Would we regret\n>> having the simple name \"--partial\" taken?\n> \n> I agree with that. Something like \"--filter-blobs\" for\n> \"--filter=blob:none\" and perhaps \"--filter-blobs=\" for\n> \"--filter=blob:limit=\" might be worth it though.\n\nThanks for the perspective on this. The --filter-blobs[=] should\nbe less likely to collide with an alternative definition of \"partial\".\n\nWhile we are thinking in this space, what if we had a \"partial-clone\"\nbuiltin? It could be a light wrapper around \"git clone\" where\n\n git partial-clone [--limit=] [options] []\n\nwould do the same thing as\n\n git clone --filter=blob:[none|limit=] [options] []\n\nJust spit-balling here.\n\nIn the meantime, I'll work to adjust my patches to only be the\ndocumentation of the --filter option.\n\nThanks,\n-Stolee\n"},{"id":"393737","messageId":"20200322192234.GC671552@coredump.intra.peff.net","threadId":"53034","inReplyTo":"5baf9f4c-9ce0-0384-1113-431038c2a646@gmail.com","subject":"Re: [PATCH 0/2] Slightly simplify partial clone user experience","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-03-22T19:22:34Z","receivedAt":"2020-03-22T19:22:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Mar 22, 2020 at 12:45:29PM -0400, Derrick Stolee wrote:\n\n> >> I have mixed feelings on this. I do like making things less arcane for\n> >> users. But are we locking in a behavior for --partial that we might not\n> >> want to live with forever? I.e., the current thinking for partial clones\n> >> is to fetch no blobs at all, get all commits and trees, apply sparse\n> >> filters, and then fault in the blobs we need. But imagine we later grow\n> >> the ability to easily avoid fetching all of the trees. Would we regret\n> >> having the simple name \"--partial\" taken?\n> > \n> > I agree with that. Something like \"--filter-blobs\" for\n> > \"--filter=blob:none\" and perhaps \"--filter-blobs=\" for\n> > \"--filter=blob:limit=\" might be worth it though.\n> \n> Thanks for the perspective on this. The --filter-blobs[=] should\n> be less likely to collide with an alternative definition of \"partial\".\n\nYes, though I wonder if it's really that big an improvement over\n\"--filter=blob:none\".\n\n> While we are thinking in this space, what if we had a \"partial-clone\"\n> builtin? It could be a light wrapper around \"git clone\" where\n> \n> git partial-clone [--limit=] [options] []\n> \n> would do the same thing as\n> \n> git clone --filter=blob:[none|limit=] [options] []\n> \n> Just spit-balling here.\n\nI think that just shifts my concern out to the other command; i.e.,\nwould we later want the partial-clone command to behave differently.\nAnd introducing a wrapper creates a weird non-orthogonality where it's\nnot necessary. I _do_ think a wrapper could make sense when it's doing\nsubstantially different things, or multiple steps that don't belong in\nthe original program (e.g., the way Scalar works).\n\n> In the meantime, I'll work to adjust my patches to only be the\n> documentation of the --filter option.\n\nThanks, that seems like an obvious and easy improvement.\n\n-Peff\n"},{"id":"393738","messageId":"pull.586.v2.git.1584906606469.gitgitgadget@gmail.com","threadId":"53034","inReplyTo":"pull.586.git.1584638887.gitgitgadget@gmail.com","subject":"[PATCH v2] clone: document --filter options","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-03-22T19:50:06Z","receivedAt":"2020-03-22T19:51:03Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nIt turns out that the \"--filter=\" option is not\ndocumented anywhere in the \"git clone\" page, and instead is\ndetailed carefully in \"git rev-list\" where it serves a\ndifferent purpose.\n\nAdd a small bit about this option in the documentation. It\nwould be worth some time to create a subsection in the \"git clone\"\ndocumentation about partial clone as a concept and how it can be\na surprising experience. For example, \"git checkout\" will likely\ntrigger a pack download.\n\nSigned-off-by: Derrick Stolee \n---\n Slightly simplify partial clone user experience\n \n V2: Only update the documentation of --filter.\n \n Thanks, -Stolee\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-586%2Fderrickstolee%2Fpartial-clone-ux-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-586/derrickstolee/partial-clone-ux-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/586\n\nRange-diff vs v1:\n\n 1: 6f340d9aadf < -: ----------- partial-clone: set default filter with --partial\n 2: 9baf4c8ba38 ! 1: a55c2d975ab clone: document --partial and --filter options\n @@ -1,16 +1,13 @@\n Author: Derrick Stolee \n \n - clone: document --partial and --filter options\n + clone: document --filter options\n \n - The previous change added the \"--partial[=]\" option to\n - \"git clone\" equivalent to \"--filter=blob:none\" or\n - \"--filter=blob:limit=\" but did not document that addition.\n - It turns out that the \"--filter=\" option was not\n + It turns out that the \"--filter=\" option is not\n documented anywhere in the \"git clone\" page, and instead is\n detailed carefully in \"git rev-list\" where it serves a\n different purpose.\n \n - Add a small bit about these options in the documentation. It\n + Add a small bit about this option in the documentation. It\n would be worth some time to create a subsection in the \"git clone\"\n documentation about partial clone as a concept and how it can be\n a surprising experience. For example, \"git checkout\" will likely\n @@ -27,7 +24,7 @@\n \t [--recurse-submodules[=]] [--[no-]shallow-submodules]\n -\t [--[no-]remote-submodules] [--jobs ] [--sparse] [--] \n +\t [--[no-]remote-submodules] [--jobs ] [--sparse]\n -+\t [--partial[=]|--filter=] [--] \n ++\t [--filter=] [--] \n \t []\n \n DESCRIPTION\n @@ -35,17 +32,15 @@\n \tof the repository. The sparse-checkout file can be\n \tmodified to grow the working directory as needed.\n \n -+--partial[=]::\n +--filter=::\n +\tUse the partial clone feature and request that the server sends\n +\ta subset of reachable objects according to a given object filter.\n +\tWhen using `--filter`, the supplied `` is used for\n -+\tthe partial clone filter. When using `--partial` with no ``,\n -+\tthe `blob:none` filter is applied to filter all blobs. When using\n -+\t`--partial=` the `blob:limit=` filter is applied to\n -+\tfilter all blobs with size larger than ``. For more details\n -+\ton filter specifications, see the `--filter` option in\n -+\tlinkgit:git-rev-list[1].\n ++\tthe partial clone filter. For example, `--filter=blob:none` will\n ++\tfilter out all blobs (file contents) until needed by Git. Also,\n ++\t`--filter=blob:limit=` will filter out all blobs of size\n ++\tat least ``. For more details on filter specifications, see\n ++\tthe `--filter` option in linkgit:git-rev-list[1].\n +\n --mirror::\n \tSet up a mirror of the source repository. This implies `--bare`.\n\n\n Documentation/git-clone.txt | 13 ++++++++++++-\n 1 file changed, 12 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\nindex bf24f1813ad..08d6045c4a8 100644\n--- a/Documentation/git-clone.txt\n+++ b/Documentation/git-clone.txt\n@@ -15,7 +15,8 @@ SYNOPSIS\n \t [--dissociate] [--separate-git-dir ]\n \t [--depth ] [--[no-]single-branch] [--no-tags]\n \t [--recurse-submodules[=]] [--[no-]shallow-submodules]\n-\t [--[no-]remote-submodules] [--jobs ] [--sparse] [--] \n+\t [--[no-]remote-submodules] [--jobs ] [--sparse]\n+\t [--filter=] [--] \n \t []\n \n DESCRIPTION\n@@ -162,6 +163,16 @@ objects from the source repository into a pack in the cloned repository.\n \tof the repository. The sparse-checkout file can be\n \tmodified to grow the working directory as needed.\n \n+--filter=::\n+\tUse the partial clone feature and request that the server sends\n+\ta subset of reachable objects according to a given object filter.\n+\tWhen using `--filter`, the supplied `` is used for\n+\tthe partial clone filter. For example, `--filter=blob:none` will\n+\tfilter out all blobs (file contents) until needed by Git. Also,\n+\t`--filter=blob:limit=` will filter out all blobs of size\n+\tat least ``. For more details on filter specifications, see\n+\tthe `--filter` option in linkgit:git-rev-list[1].\n+\n --mirror::\n \tSet up a mirror of the source repository. This implies `--bare`.\n \tCompared to `--bare`, `--mirror` not only maps local branches of the\n\nbase-commit: 6c85aac65fb455af85745130ce35ddae4678db84\n-- \ngitgitgadget\n"},{"id":"393849","messageId":"20200324034015.GA51305@coredump.intra.peff.net","threadId":"53034","inReplyTo":"pull.586.v2.git.1584906606469.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] clone: document --filter options","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-03-24T03:40:15Z","receivedAt":"2020-03-24T03:40:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Mar 22, 2020 at 07:50:06PM +0000, Derrick Stolee via GitGitGadget wrote:\n\n> Slightly simplify partial clone user experience\n> \n> V2: Only update the documentation of --filter.\n\nThanks, this part is definitely an improvement (and I read over the\nproposed text again, and it looks very good).\n\n-Peff\n"},{"id":"393853","messageId":"20200324051754.GA13508@google.com","threadId":"53034","inReplyTo":"pull.586.v2.git.1584906606469.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] clone: document --filter options","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2020-03-24T05:17:54Z","receivedAt":"2020-03-24T05:26:09Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Derrick Stolee wrote:\n\n> It turns out that the \"--filter=\" option is not\n> documented anywhere in the \"git clone\" page, and instead is\n> detailed carefully in \"git rev-list\" where it serves a\n> different purpose.\n>\n> Add a small bit about this option in the documentation.\n[...]\n> Documentation/git-clone.txt | 13 ++++++++++++-\n> 1 file changed, 12 insertions(+), 1 deletion(-)\n\nReviewed-by: Jonathan Nieder \n\nThanks.\n\n> It\n> would be worth some time to create a subsection in the \"git clone\"\n> documentation about partial clone as a concept and how it can be\n> a surprising experience. For example, \"git checkout\" will likely\n> trigger a pack download.\n\nI think that belongs in its own git-partial-clone(7) page.\n\nThanks,\nJonathan\n"}]}