{"thread":{"id":"53391","subject":"[PATCH] sparse-checkout: stop blocking empty workdirs","startedAt":"2020-05-04T18:27:49Z","lastAt":"2021-04-13T04:55:45Z","messageCount":5,"participants":["Derrick Stolee via GitGitGadget","Elijah Newren","Junio C Hamano","Taylor Blau","Christian Couder"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"396950","messageId":"pull.624.git.1588616864222.gitgitgadget@gmail.com","threadId":"53391","inReplyTo":null,"subject":"[PATCH] sparse-checkout: stop blocking empty workdirs","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-05-04T18:27:43Z","receivedAt":"2020-05-04T18:27:49Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee <dstolee@microsoft.com>\n\nRemove the error condition when updating the sparse-checkout leaves\nan empty working directory.\n\nThis behavior was added in 9e1afb167 (sparse checkout: inhibit empty\nworktree, 2009-08-20). The comment was added in a7bc906f2 (Add\nexplanation why we do not allow to sparse checkout to empty working\ntree, 2011-09-22) in response to a \"dubious\" comment in 84563a624\n(unpack-trees.c: cosmetic fix, 2010-12-22).\n\nWith the recent \"cone mode\" and \"git sparse-checkout init [--cone]\"\ncommand, it is common to set a reasonable sparse-checkout pattern\nset of\n\n\t/*\n\t!/*/\n\nwhich matches only files at root. If the repository has no such files,\nthen their \"git sparse-checkout init\" command will fail.\n\nNow that we expect this to be a common pattern, we should not have the\ncommands fail on an empty working directory. If it is a confusing\nresult, then the user can recover with \"git sparse-checkout disable\"\nor \"git sparse-checkout set\". This is especially simple when using cone\nmode.\n\nReported-by: Lars Schneider <larsxschneider@gmail.com>\nSigned-off-by: Derrick Stolee <dstolee@microsoft.com>\n---\n    sparse-checkout: stop blocking empty workdirs\n    \n    This is based on en/sparse-checkout.\n    \n    This is something that Lars Schneider discovered working with a repo\n    that had no files at root.\n    \n    Thanks, -Stolee\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-624%2Fderrickstolee%2Fsparse-checkout-allow-empty-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-624/derrickstolee/sparse-checkout-allow-empty-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/624\n\n t/t1011-read-tree-sparse-checkout.sh | 12 +++++++---\n t/t1091-sparse-checkout-builtin.sh   |  8 +++----\n unpack-trees.c                       | 34 +---------------------------\n 3 files changed, 13 insertions(+), 41 deletions(-)\n\ndiff --git a/t/t1011-read-tree-sparse-checkout.sh b/t/t1011-read-tree-sparse-checkout.sh\nindex 63223e13bd1..140f4599773 100755\n--- a/t/t1011-read-tree-sparse-checkout.sh\n+++ b/t/t1011-read-tree-sparse-checkout.sh\n@@ -74,13 +74,19 @@ test_expect_success 'read-tree --no-sparse-checkout with empty .git/info/sparse-\n test_expect_success 'read-tree with empty .git/info/sparse-checkout' '\n \tgit config core.sparsecheckout true &&\n \techo >.git/info/sparse-checkout &&\n-\tread_tree_u_must_fail -m -u HEAD &&\n+\tread_tree_u_must_succeed -m -u HEAD &&\n \tgit ls-files --stage >result &&\n \ttest_cmp expected result &&\n \tgit ls-files -t >result &&\n+\tcat >expected.swt <<-\\EOF &&\n+\tS init.t\n+\tS sub/added\n+\tS sub/addedtoo\n+\tS subsub/added\n+\tEOF\n \ttest_cmp expected.swt result &&\n-\ttest -f init.t &&\n-\ttest -f sub/added\n+\t! test -f init.t &&\n+\t! test -f sub/added\n '\n \n test_expect_success 'match directories with trailing slash' '\ndiff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh\nindex dee99eeec30..88cdde255cd 100755\n--- a/t/t1091-sparse-checkout-builtin.sh\n+++ b/t/t1091-sparse-checkout-builtin.sh\n@@ -106,10 +106,8 @@ test_expect_success 'set enables config' '\n \t\tcd empty-config &&\n \t\ttest_commit test file &&\n \t\ttest_path_is_missing .git/config.worktree &&\n-\t\ttest_must_fail git sparse-checkout set nothing &&\n+\t\tgit sparse-checkout set nothing &&\n \t\ttest_path_is_file .git/config.worktree &&\n-\t\ttest_must_fail git config core.sparseCheckout &&\n-\t\tgit sparse-checkout set \"/*\" &&\n \t\ttest_cmp_config true core.sparseCheckout\n \t)\n '\n@@ -302,8 +300,8 @@ test_expect_success 'revert to old sparse-checkout on empty update' '\n \t\techo >file &&\n \t\tgit add file &&\n \t\tgit commit -m \"test\" &&\n-\t\ttest_must_fail git sparse-checkout set nothing 2>err &&\n-\t\ttest_i18ngrep \"Sparse checkout leaves no entry on working directory\" err &&\n+\t\tgit sparse-checkout set nothing 2>err &&\n+\t\ttest_i18ngrep ! \"Sparse checkout leaves no entry on working directory\" err &&\n \t\ttest_i18ngrep ! \".git/index.lock\" err &&\n \t\tgit sparse-checkout set file\n \t)\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex b43f3e775ad..9a3ccd9d083 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -1677,8 +1677,6 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n \t}\n \n \tif (!o->skip_sparse_checkout) {\n-\t\tint empty_worktree = 1;\n-\n \t\t/*\n \t\t * Sparse checkout loop #2: set NEW_SKIP_WORKTREE on entries not in loop #1\n \t\t * If they will have NEW_SKIP_WORKTREE, also set CE_SKIP_WORKTREE\n@@ -1706,19 +1704,6 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n \n \t\t\tif (apply_sparse_checkout(&o->result, ce, o))\n \t\t\t\tret = 1;\n-\n-\t\t\tif (!ce_skip_worktree(ce))\n-\t\t\t\tempty_worktree = 0;\n-\t\t}\n-\t\t/*\n-\t\t * Sparse checkout is meant to narrow down checkout area\n-\t\t * but it does not make sense to narrow down to empty working\n-\t\t * tree. This is usually a mistake in sparse checkout rules.\n-\t\t * Do not allow users to do that.\n-\t\t */\n-\t\tif (o->result.cache_nr && empty_worktree) {\n-\t\t\tret = unpack_failed(o, \"Sparse checkout leaves no entry on working directory\");\n-\t\t\tgoto done;\n \t\t}\n \t\tif (ret == 1) {\n \t\t\t/*\n@@ -1779,7 +1764,7 @@ enum update_sparsity_result update_sparsity(struct unpack_trees_options *o)\n {\n \tenum update_sparsity_result ret = UPDATE_SPARSITY_SUCCESS;\n \tstruct pattern_list pl;\n-\tint i, empty_worktree;\n+\tint i;\n \tunsigned old_show_all_errors;\n \tint free_pattern_list = 0;\n \n@@ -1810,7 +1795,6 @@ enum update_sparsity_result update_sparsity(struct unpack_trees_options *o)\n \n \t/* Then loop over entries and update/remove as needed */\n \tret = UPDATE_SPARSITY_SUCCESS;\n-\tempty_worktree = 1;\n \tfor (i = 0; i < o->src_index->cache_nr; i++) {\n \t\tstruct cache_entry *ce = o->src_index->cache[i];\n \n@@ -1824,28 +1808,12 @@ enum update_sparsity_result update_sparsity(struct unpack_trees_options *o)\n \n \t\tif (apply_sparse_checkout(o->src_index, ce, o))\n \t\t\tret = UPDATE_SPARSITY_WARNINGS;\n-\n-\t\tif (!ce_skip_worktree(ce))\n-\t\t\tempty_worktree = 0;\n-\t}\n-\n-\t/*\n-\t * Sparse checkout is meant to narrow down checkout area\n-\t * but it does not make sense to narrow down to empty working\n-\t * tree. This is usually a mistake in sparse checkout rules.\n-\t * Do not allow users to do that.\n-\t */\n-\tif (o->src_index->cache_nr && empty_worktree) {\n-\t\tunpack_failed(o, \"Sparse checkout leaves no entry on working directory\");\n-\t\tret = UPDATE_SPARSITY_INDEX_UPDATE_FAILURES;\n-\t\tgoto done;\n \t}\n \n skip_sparse_checkout:\n \tif (check_updates(o, o->src_index))\n \t\tret = UPDATE_SPARSITY_WORKTREE_UPDATE_FAILURES;\n \n-done:\n \tdisplay_warning_msgs(o);\n \to->show_all_errors = old_show_all_errors;\n \tif (free_pattern_list)\n\nbase-commit: 5644ca28cded68972c74614fc06d6e0e1db1a7de\n-- \ngitgitgadget\n"},{"id":"396953","messageId":"CABPp-BGO1AYsW-V1a_bSToihWg8yBYyUe3GFUqfaADFpcfEDoQ@mail.gmail.com","threadId":"53391","inReplyTo":"pull.624.git.1588616864222.gitgitgadget@gmail.com","subject":"Re: [PATCH] sparse-checkout: stop blocking empty workdirs","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2020-05-04T18:41:38Z","receivedAt":"2020-05-04T18:41:52Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Mon, May 4, 2020 at 11:27 AM Derrick Stolee via GitGitGadget\n<gitgitgadget@gmail.com> wrote:\n>\n> From: Derrick Stolee <dstolee@microsoft.com>\n>\n> Remove the error condition when updating the sparse-checkout leaves\n> an empty working directory.\n>\n> This behavior was added in 9e1afb167 (sparse checkout: inhibit empty\n> worktree, 2009-08-20). The comment was added in a7bc906f2 (Add\n> explanation why we do not allow to sparse checkout to empty working\n> tree, 2011-09-22) in response to a \"dubious\" comment in 84563a624\n> (unpack-trees.c: cosmetic fix, 2010-12-22).\n>\n> With the recent \"cone mode\" and \"git sparse-checkout init [--cone]\"\n> command, it is common to set a reasonable sparse-checkout pattern\n> set of\n>\n>         /*\n>         !/*/\n>\n> which matches only files at root. If the repository has no such files,\n> then their \"git sparse-checkout init\" command will fail.\n>\n> Now that we expect this to be a common pattern, we should not have the\n> commands fail on an empty working directory. If it is a confusing\n> result, then the user can recover with \"git sparse-checkout disable\"\n> or \"git sparse-checkout set\". This is especially simple when using cone\n> mode.\n\nYeah, given that setting up a sparse-checkout is now easy (as opposed\nto setting both extensions.worktreeConfig and core.sparseCheckout\nsettings, editing a .git/info/sparse-checkout file that is documented\nin some obscure section of the manual, and discovering the\nundocumented `git read-tree -mu HEAD` command that you need to run)\nand drastically less error-prone, and that recovery is now easy (also\nin contrast to before), I think this makes a lot of sense.\n\nI actually hit this error a couple times while testing with the old\nstyle and thought it was annoying (though understandable when the\nroute for usage was so arcane and easy to mess up), but in my case I\nwasn't on a real world repository.  If we've got an example of people\nhitting it on real world repos, then by all means let's get rid of\nthis annoying check.\n\n> Reported-by: Lars Schneider <larsxschneider@gmail.com>\n> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>\n> ---\n>     sparse-checkout: stop blocking empty workdirs\n>\n>     This is based on en/sparse-checkout.\n>\n>     This is something that Lars Schneider discovered working with a repo\n>     that had no files at root.\n>\n>     Thanks, -Stolee\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-624%2Fderrickstolee%2Fsparse-checkout-allow-empty-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-624/derrickstolee/sparse-checkout-allow-empty-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/624\n>\n>  t/t1011-read-tree-sparse-checkout.sh | 12 +++++++---\n>  t/t1091-sparse-checkout-builtin.sh   |  8 +++----\n>  unpack-trees.c                       | 34 +---------------------------\n>  3 files changed, 13 insertions(+), 41 deletions(-)\n>\n> diff --git a/t/t1011-read-tree-sparse-checkout.sh b/t/t1011-read-tree-sparse-checkout.sh\n> index 63223e13bd1..140f4599773 100755\n> --- a/t/t1011-read-tree-sparse-checkout.sh\n> +++ b/t/t1011-read-tree-sparse-checkout.sh\n> @@ -74,13 +74,19 @@ test_expect_success 'read-tree --no-sparse-checkout with empty .git/info/sparse-\n>  test_expect_success 'read-tree with empty .git/info/sparse-checkout' '\n>         git config core.sparsecheckout true &&\n>         echo >.git/info/sparse-checkout &&\n> -       read_tree_u_must_fail -m -u HEAD &&\n> +       read_tree_u_must_succeed -m -u HEAD &&\n>         git ls-files --stage >result &&\n>         test_cmp expected result &&\n>         git ls-files -t >result &&\n> +       cat >expected.swt <<-\\EOF &&\n> +       S init.t\n> +       S sub/added\n> +       S sub/addedtoo\n> +       S subsub/added\n> +       EOF\n>         test_cmp expected.swt result &&\n> -       test -f init.t &&\n> -       test -f sub/added\n> +       ! test -f init.t &&\n> +       ! test -f sub/added\n>  '\n>\n>  test_expect_success 'match directories with trailing slash' '\n> diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh\n> index dee99eeec30..88cdde255cd 100755\n> --- a/t/t1091-sparse-checkout-builtin.sh\n> +++ b/t/t1091-sparse-checkout-builtin.sh\n> @@ -106,10 +106,8 @@ test_expect_success 'set enables config' '\n>                 cd empty-config &&\n>                 test_commit test file &&\n>                 test_path_is_missing .git/config.worktree &&\n> -               test_must_fail git sparse-checkout set nothing &&\n> +               git sparse-checkout set nothing &&\n>                 test_path_is_file .git/config.worktree &&\n> -               test_must_fail git config core.sparseCheckout &&\n> -               git sparse-checkout set \"/*\" &&\n>                 test_cmp_config true core.sparseCheckout\n>         )\n>  '\n> @@ -302,8 +300,8 @@ test_expect_success 'revert to old sparse-checkout on empty update' '\n>                 echo >file &&\n>                 git add file &&\n>                 git commit -m \"test\" &&\n> -               test_must_fail git sparse-checkout set nothing 2>err &&\n> -               test_i18ngrep \"Sparse checkout leaves no entry on working directory\" err &&\n> +               git sparse-checkout set nothing 2>err &&\n> +               test_i18ngrep ! \"Sparse checkout leaves no entry on working directory\" err &&\n>                 test_i18ngrep ! \".git/index.lock\" err &&\n>                 git sparse-checkout set file\n>         )\n> diff --git a/unpack-trees.c b/unpack-trees.c\n> index b43f3e775ad..9a3ccd9d083 100644\n> --- a/unpack-trees.c\n> +++ b/unpack-trees.c\n> @@ -1677,8 +1677,6 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n>         }\n>\n>         if (!o->skip_sparse_checkout) {\n> -               int empty_worktree = 1;\n> -\n>                 /*\n>                  * Sparse checkout loop #2: set NEW_SKIP_WORKTREE on entries not in loop #1\n>                  * If they will have NEW_SKIP_WORKTREE, also set CE_SKIP_WORKTREE\n> @@ -1706,19 +1704,6 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n>\n>                         if (apply_sparse_checkout(&o->result, ce, o))\n>                                 ret = 1;\n> -\n> -                       if (!ce_skip_worktree(ce))\n> -                               empty_worktree = 0;\n> -               }\n> -               /*\n> -                * Sparse checkout is meant to narrow down checkout area\n> -                * but it does not make sense to narrow down to empty working\n> -                * tree. This is usually a mistake in sparse checkout rules.\n> -                * Do not allow users to do that.\n> -                */\n> -               if (o->result.cache_nr && empty_worktree) {\n> -                       ret = unpack_failed(o, \"Sparse checkout leaves no entry on working directory\");\n> -                       goto done;\n>                 }\n>                 if (ret == 1) {\n>                         /*\n> @@ -1779,7 +1764,7 @@ enum update_sparsity_result update_sparsity(struct unpack_trees_options *o)\n>  {\n>         enum update_sparsity_result ret = UPDATE_SPARSITY_SUCCESS;\n>         struct pattern_list pl;\n> -       int i, empty_worktree;\n> +       int i;\n>         unsigned old_show_all_errors;\n>         int free_pattern_list = 0;\n>\n> @@ -1810,7 +1795,6 @@ enum update_sparsity_result update_sparsity(struct unpack_trees_options *o)\n>\n>         /* Then loop over entries and update/remove as needed */\n>         ret = UPDATE_SPARSITY_SUCCESS;\n> -       empty_worktree = 1;\n>         for (i = 0; i < o->src_index->cache_nr; i++) {\n>                 struct cache_entry *ce = o->src_index->cache[i];\n>\n> @@ -1824,28 +1808,12 @@ enum update_sparsity_result update_sparsity(struct unpack_trees_options *o)\n>\n>                 if (apply_sparse_checkout(o->src_index, ce, o))\n>                         ret = UPDATE_SPARSITY_WARNINGS;\n> -\n> -               if (!ce_skip_worktree(ce))\n> -                       empty_worktree = 0;\n> -       }\n> -\n> -       /*\n> -        * Sparse checkout is meant to narrow down checkout area\n> -        * but it does not make sense to narrow down to empty working\n> -        * tree. This is usually a mistake in sparse checkout rules.\n> -        * Do not allow users to do that.\n> -        */\n> -       if (o->src_index->cache_nr && empty_worktree) {\n> -               unpack_failed(o, \"Sparse checkout leaves no entry on working directory\");\n> -               ret = UPDATE_SPARSITY_INDEX_UPDATE_FAILURES;\n> -               goto done;\n>         }\n>\n>  skip_sparse_checkout:\n>         if (check_updates(o, o->src_index))\n>                 ret = UPDATE_SPARSITY_WORKTREE_UPDATE_FAILURES;\n>\n> -done:\n>         display_warning_msgs(o);\n>         o->show_all_errors = old_show_all_errors;\n>         if (free_pattern_list)\n>\n> base-commit: 5644ca28cded68972c74614fc06d6e0e1db1a7de\n\nPatch looks good to me.\n"},{"id":"396977","messageId":"xmqqtv0vfpde.fsf@gitster.c.googlers.com","threadId":"53391","inReplyTo":"CABPp-BGO1AYsW-V1a_bSToihWg8yBYyUe3GFUqfaADFpcfEDoQ@mail.gmail.com","subject":"Re: [PATCH] sparse-checkout: stop blocking empty workdirs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-05-04T19:53:49Z","receivedAt":"2020-05-04T19:53:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren <newren@gmail.com> writes:\n\n>> Remove the error condition when updating the sparse-checkout leaves\n>> an empty working directory.\n>>\n>> This behavior was added in 9e1afb167 (sparse checkout: inhibit empty\n>> worktree, 2009-08-20). The comment was added in a7bc906f2 (Add\n>> explanation why we do not allow to sparse checkout to empty working\n>> tree, 2011-09-22) in response to a \"dubious\" comment in 84563a624\n>> (unpack-trees.c: cosmetic fix, 2010-12-22).\n> ...\n>\n> Patch looks good to me.\n\nThanks, both.\n\n"},{"id":"396999","messageId":"20200504220042.GD45250@syl.local","threadId":"53391","inReplyTo":"CABPp-BGO1AYsW-V1a_bSToihWg8yBYyUe3GFUqfaADFpcfEDoQ@mail.gmail.com","subject":"Re: [PATCH] sparse-checkout: stop blocking empty workdirs","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2020-05-04T22:00:42Z","receivedAt":"2020-05-04T22:00:47Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, May 04, 2020 at 11:41:38AM -0700, Elijah Newren wrote:\n> On Mon, May 4, 2020 at 11:27 AM Derrick Stolee via GitGitGadget\n> <gitgitgadget@gmail.com> wrote:\n> >\n> > From: Derrick Stolee <dstolee@microsoft.com>\n> >\n> > Remove the error condition when updating the sparse-checkout leaves\n> > an empty working directory.\n> >\n> > This behavior was added in 9e1afb167 (sparse checkout: inhibit empty\n> > worktree, 2009-08-20). The comment was added in a7bc906f2 (Add\n> > explanation why we do not allow to sparse checkout to empty working\n> > tree, 2011-09-22) in response to a \"dubious\" comment in 84563a624\n> > (unpack-trees.c: cosmetic fix, 2010-12-22).\n> >\n> > With the recent \"cone mode\" and \"git sparse-checkout init [--cone]\"\n> > command, it is common to set a reasonable sparse-checkout pattern\n> > set of\n> >\n> >         /*\n> >         !/*/\n> >\n> > which matches only files at root. If the repository has no such files,\n> > then their \"git sparse-checkout init\" command will fail.\n> >\n> > Now that we expect this to be a common pattern, we should not have the\n> > commands fail on an empty working directory. If it is a confusing\n> > result, then the user can recover with \"git sparse-checkout disable\"\n> > or \"git sparse-checkout set\". This is especially simple when using cone\n> > mode.\n>\n> Yeah, given that setting up a sparse-checkout is now easy (as opposed\n> to setting both extensions.worktreeConfig and core.sparseCheckout\n> settings, editing a .git/info/sparse-checkout file that is documented\n> in some obscure section of the manual, and discovering the\n> undocumented `git read-tree -mu HEAD` command that you need to run)\n> and drastically less error-prone, and that recovery is now easy (also\n> in contrast to before), I think this makes a lot of sense.\n>\n> I actually hit this error a couple times while testing with the old\n> style and thought it was annoying (though understandable when the\n> route for usage was so arcane and easy to mess up), but in my case I\n> wasn't on a real world repository.  If we've got an example of people\n> hitting it on real world repos, then by all means let's get rid of\n> this annoying check.\n>\n> > Reported-by: Lars Schneider <larsxschneider@gmail.com>\n> > Signed-off-by: Derrick Stolee <dstolee@microsoft.com>\n> > ---\n> >     sparse-checkout: stop blocking empty workdirs\n> >\n> >     This is based on en/sparse-checkout.\n> >\n> >     This is something that Lars Schneider discovered working with a repo\n> >     that had no files at root.\n> >\n> >     Thanks, -Stolee\n> >\n> > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-624%2Fderrickstolee%2Fsparse-checkout-allow-empty-v1\n> > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-624/derrickstolee/sparse-checkout-allow-empty-v1\n> > Pull-Request: https://github.com/gitgitgadget/git/pull/624\n> >\n> >  t/t1011-read-tree-sparse-checkout.sh | 12 +++++++---\n> >  t/t1091-sparse-checkout-builtin.sh   |  8 +++----\n> >  unpack-trees.c                       | 34 +---------------------------\n> >  3 files changed, 13 insertions(+), 41 deletions(-)\n> >\n> > diff --git a/t/t1011-read-tree-sparse-checkout.sh b/t/t1011-read-tree-sparse-checkout.sh\n> > index 63223e13bd1..140f4599773 100755\n> > --- a/t/t1011-read-tree-sparse-checkout.sh\n> > +++ b/t/t1011-read-tree-sparse-checkout.sh\n> > @@ -74,13 +74,19 @@ test_expect_success 'read-tree --no-sparse-checkout with empty .git/info/sparse-\n> >  test_expect_success 'read-tree with empty .git/info/sparse-checkout' '\n> >         git config core.sparsecheckout true &&\n> >         echo >.git/info/sparse-checkout &&\n> > -       read_tree_u_must_fail -m -u HEAD &&\n> > +       read_tree_u_must_succeed -m -u HEAD &&\n> >         git ls-files --stage >result &&\n> >         test_cmp expected result &&\n> >         git ls-files -t >result &&\n> > +       cat >expected.swt <<-\\EOF &&\n> > +       S init.t\n> > +       S sub/added\n> > +       S sub/addedtoo\n> > +       S subsub/added\n> > +       EOF\n> >         test_cmp expected.swt result &&\n> > -       test -f init.t &&\n> > -       test -f sub/added\n> > +       ! test -f init.t &&\n> > +       ! test -f sub/added\n> >  '\n> >\n> >  test_expect_success 'match directories with trailing slash' '\n> > diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh\n> > index dee99eeec30..88cdde255cd 100755\n> > --- a/t/t1091-sparse-checkout-builtin.sh\n> > +++ b/t/t1091-sparse-checkout-builtin.sh\n> > @@ -106,10 +106,8 @@ test_expect_success 'set enables config' '\n> >                 cd empty-config &&\n> >                 test_commit test file &&\n> >                 test_path_is_missing .git/config.worktree &&\n> > -               test_must_fail git sparse-checkout set nothing &&\n> > +               git sparse-checkout set nothing &&\n> >                 test_path_is_file .git/config.worktree &&\n> > -               test_must_fail git config core.sparseCheckout &&\n> > -               git sparse-checkout set \"/*\" &&\n> >                 test_cmp_config true core.sparseCheckout\n> >         )\n> >  '\n> > @@ -302,8 +300,8 @@ test_expect_success 'revert to old sparse-checkout on empty update' '\n> >                 echo >file &&\n> >                 git add file &&\n> >                 git commit -m \"test\" &&\n> > -               test_must_fail git sparse-checkout set nothing 2>err &&\n> > -               test_i18ngrep \"Sparse checkout leaves no entry on working directory\" err &&\n> > +               git sparse-checkout set nothing 2>err &&\n> > +               test_i18ngrep ! \"Sparse checkout leaves no entry on working directory\" err &&\n> >                 test_i18ngrep ! \".git/index.lock\" err &&\n> >                 git sparse-checkout set file\n> >         )\n> > diff --git a/unpack-trees.c b/unpack-trees.c\n> > index b43f3e775ad..9a3ccd9d083 100644\n> > --- a/unpack-trees.c\n> > +++ b/unpack-trees.c\n> > @@ -1677,8 +1677,6 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n> >         }\n> >\n> >         if (!o->skip_sparse_checkout) {\n> > -               int empty_worktree = 1;\n> > -\n> >                 /*\n> >                  * Sparse checkout loop #2: set NEW_SKIP_WORKTREE on entries not in loop #1\n> >                  * If they will have NEW_SKIP_WORKTREE, also set CE_SKIP_WORKTREE\n> > @@ -1706,19 +1704,6 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n> >\n> >                         if (apply_sparse_checkout(&o->result, ce, o))\n> >                                 ret = 1;\n> > -\n> > -                       if (!ce_skip_worktree(ce))\n> > -                               empty_worktree = 0;\n> > -               }\n> > -               /*\n> > -                * Sparse checkout is meant to narrow down checkout area\n> > -                * but it does not make sense to narrow down to empty working\n> > -                * tree. This is usually a mistake in sparse checkout rules.\n> > -                * Do not allow users to do that.\n> > -                */\n> > -               if (o->result.cache_nr && empty_worktree) {\n> > -                       ret = unpack_failed(o, \"Sparse checkout leaves no entry on working directory\");\n> > -                       goto done;\n> >                 }\n> >                 if (ret == 1) {\n> >                         /*\n> > @@ -1779,7 +1764,7 @@ enum update_sparsity_result update_sparsity(struct unpack_trees_options *o)\n> >  {\n> >         enum update_sparsity_result ret = UPDATE_SPARSITY_SUCCESS;\n> >         struct pattern_list pl;\n> > -       int i, empty_worktree;\n> > +       int i;\n> >         unsigned old_show_all_errors;\n> >         int free_pattern_list = 0;\n> >\n> > @@ -1810,7 +1795,6 @@ enum update_sparsity_result update_sparsity(struct unpack_trees_options *o)\n> >\n> >         /* Then loop over entries and update/remove as needed */\n> >         ret = UPDATE_SPARSITY_SUCCESS;\n> > -       empty_worktree = 1;\n> >         for (i = 0; i < o->src_index->cache_nr; i++) {\n> >                 struct cache_entry *ce = o->src_index->cache[i];\n> >\n> > @@ -1824,28 +1808,12 @@ enum update_sparsity_result update_sparsity(struct unpack_trees_options *o)\n> >\n> >                 if (apply_sparse_checkout(o->src_index, ce, o))\n> >                         ret = UPDATE_SPARSITY_WARNINGS;\n> > -\n> > -               if (!ce_skip_worktree(ce))\n> > -                       empty_worktree = 0;\n> > -       }\n> > -\n> > -       /*\n> > -        * Sparse checkout is meant to narrow down checkout area\n> > -        * but it does not make sense to narrow down to empty working\n> > -        * tree. This is usually a mistake in sparse checkout rules.\n> > -        * Do not allow users to do that.\n> > -        */\n> > -       if (o->src_index->cache_nr && empty_worktree) {\n> > -               unpack_failed(o, \"Sparse checkout leaves no entry on working directory\");\n> > -               ret = UPDATE_SPARSITY_INDEX_UPDATE_FAILURES;\n> > -               goto done;\n> >         }\n> >\n> >  skip_sparse_checkout:\n> >         if (check_updates(o, o->src_index))\n> >                 ret = UPDATE_SPARSITY_WORKTREE_UPDATE_FAILURES;\n> >\n> > -done:\n> >         display_warning_msgs(o);\n> >         o->show_all_errors = old_show_all_errors;\n> >         if (free_pattern_list)\n> >\n> > base-commit: 5644ca28cded68972c74614fc06d6e0e1db1a7de\n>\n> Patch looks good to me.\n\nTo me as well. Thanks for sending something so quickly after Lars'\ninitial report.\n\n  Reviewed-by: Taylor Blau <me@ttaylorr.com>\n\nThanks,\nTaylor\n"},{"id":"421828","messageId":"CAP8UFD13-osOHU7Ug_2iToQZyx2T2neOXzZB=ssmDZLEQoF4eg@mail.gmail.com","threadId":"53391","inReplyTo":"pull.624.git.1588616864222.gitgitgadget@gmail.com","subject":"Re: [PATCH] sparse-checkout: stop blocking empty workdirs","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2021-04-13T04:55:31Z","receivedAt":"2021-04-13T04:55:45Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Mon, May 4, 2020 at 8:30 PM Derrick Stolee via GitGitGadget\n<gitgitgadget@gmail.com> wrote:\n\n> @@ -302,8 +300,8 @@ test_expect_success 'revert to old sparse-checkout on empty update' '\n>                 echo >file &&\n>                 git add file &&\n>                 git commit -m \"test\" &&\n> -               test_must_fail git sparse-checkout set nothing 2>err &&\n> -               test_i18ngrep \"Sparse checkout leaves no entry on working directory\" err &&\n> +               git sparse-checkout set nothing 2>err &&\n> +               test_i18ngrep ! \"Sparse checkout leaves no entry on working directory\" err &&\n\nIt looks like this check is obsolete as the \"Sparse checkout leaves no\nentry on working directory\" error has been removed by this patch\nbelow...\n\n>                 test_i18ngrep ! \".git/index.lock\" err &&\n>                 git sparse-checkout set file\n>         )\n\n[...]\n\n> @@ -1706,19 +1704,6 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n>\n>                         if (apply_sparse_checkout(&o->result, ce, o))\n>                                 ret = 1;\n> -\n> -                       if (!ce_skip_worktree(ce))\n> -                               empty_worktree = 0;\n> -               }\n> -               /*\n> -                * Sparse checkout is meant to narrow down checkout area\n> -                * but it does not make sense to narrow down to empty working\n> -                * tree. This is usually a mistake in sparse checkout rules.\n> -                * Do not allow users to do that.\n> -                */\n> -               if (o->result.cache_nr && empty_worktree) {\n> -                       ret = unpack_failed(o, \"Sparse checkout leaves no entry on working directory\");\n\n...here...\n\n> -                       goto done;\n>                 }\n>                 if (ret == 1) {\n>                         /*\n\n[...]\n\n> @@ -1824,28 +1808,12 @@ enum update_sparsity_result update_sparsity(struct unpack_trees_options *o)\n>\n>                 if (apply_sparse_checkout(o->src_index, ce, o))\n>                         ret = UPDATE_SPARSITY_WARNINGS;\n> -\n> -               if (!ce_skip_worktree(ce))\n> -                       empty_worktree = 0;\n> -       }\n> -\n> -       /*\n> -        * Sparse checkout is meant to narrow down checkout area\n> -        * but it does not make sense to narrow down to empty working\n> -        * tree. This is usually a mistake in sparse checkout rules.\n> -        * Do not allow users to do that.\n> -        */\n> -       if (o->src_index->cache_nr && empty_worktree) {\n> -               unpack_failed(o, \"Sparse checkout leaves no entry on working directory\");\n\n...and here.\n\n> -               ret = UPDATE_SPARSITY_INDEX_UPDATE_FAILURES;\n> -               goto done;\n>         }\n\nSo maybe instead of the 3 lines below:\n\n> +               git sparse-checkout set nothing 2>err &&\n> +               test_i18ngrep ! \"Sparse checkout leaves no entry on working directory\" err &&\n>                 test_i18ngrep ! \".git/index.lock\" err &&\n\nwe should just have:\n\n               git sparse-checkout set nothing &&\n\n?\n"}]}