diff mbox series

sparse-checkout: stop blocking empty workdirs

Message ID pull.624.git.1588616864222.gitgitgadget@gmail.com (mailing list archive)
State New, archived
Headers show
Series sparse-checkout: stop blocking empty workdirs | expand

Commit Message

Johannes Schindelin via GitGitGadget May 4, 2020, 6:27 p.m. UTC
From: Derrick Stolee <dstolee@microsoft.com>

Remove the error condition when updating the sparse-checkout leaves
an empty working directory.

This behavior was added in 9e1afb167 (sparse checkout: inhibit empty
worktree, 2009-08-20). The comment was added in a7bc906f2 (Add
explanation why we do not allow to sparse checkout to empty working
tree, 2011-09-22) in response to a "dubious" comment in 84563a624
(unpack-trees.c: cosmetic fix, 2010-12-22).

With the recent "cone mode" and "git sparse-checkout init [--cone]"
command, it is common to set a reasonable sparse-checkout pattern
set of

	/*
	!/*/

which matches only files at root. If the repository has no such files,
then their "git sparse-checkout init" command will fail.

Now that we expect this to be a common pattern, we should not have the
commands fail on an empty working directory. If it is a confusing
result, then the user can recover with "git sparse-checkout disable"
or "git sparse-checkout set". This is especially simple when using cone
mode.

Reported-by: Lars Schneider <larsxschneider@gmail.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
    sparse-checkout: stop blocking empty workdirs
    
    This is based on en/sparse-checkout.
    
    This is something that Lars Schneider discovered working with a repo
    that had no files at root.
    
    Thanks, -Stolee

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-624%2Fderrickstolee%2Fsparse-checkout-allow-empty-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-624/derrickstolee/sparse-checkout-allow-empty-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/624

 t/t1011-read-tree-sparse-checkout.sh | 12 +++++++---
 t/t1091-sparse-checkout-builtin.sh   |  8 +++----
 unpack-trees.c                       | 34 +---------------------------
 3 files changed, 13 insertions(+), 41 deletions(-)


base-commit: 5644ca28cded68972c74614fc06d6e0e1db1a7de

Comments

Elijah Newren May 4, 2020, 6:41 p.m. UTC | #1
On Mon, May 4, 2020 at 11:27 AM Derrick Stolee via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Derrick Stolee <dstolee@microsoft.com>
>
> Remove the error condition when updating the sparse-checkout leaves
> an empty working directory.
>
> This behavior was added in 9e1afb167 (sparse checkout: inhibit empty
> worktree, 2009-08-20). The comment was added in a7bc906f2 (Add
> explanation why we do not allow to sparse checkout to empty working
> tree, 2011-09-22) in response to a "dubious" comment in 84563a624
> (unpack-trees.c: cosmetic fix, 2010-12-22).
>
> With the recent "cone mode" and "git sparse-checkout init [--cone]"
> command, it is common to set a reasonable sparse-checkout pattern
> set of
>
>         /*
>         !/*/
>
> which matches only files at root. If the repository has no such files,
> then their "git sparse-checkout init" command will fail.
>
> Now that we expect this to be a common pattern, we should not have the
> commands fail on an empty working directory. If it is a confusing
> result, then the user can recover with "git sparse-checkout disable"
> or "git sparse-checkout set". This is especially simple when using cone
> mode.

Yeah, given that setting up a sparse-checkout is now easy (as opposed
to setting both extensions.worktreeConfig and core.sparseCheckout
settings, editing a .git/info/sparse-checkout file that is documented
in some obscure section of the manual, and discovering the
undocumented `git read-tree -mu HEAD` command that you need to run)
and drastically less error-prone, and that recovery is now easy (also
in contrast to before), I think this makes a lot of sense.

I actually hit this error a couple times while testing with the old
style and thought it was annoying (though understandable when the
route for usage was so arcane and easy to mess up), but in my case I
wasn't on a real world repository.  If we've got an example of people
hitting it on real world repos, then by all means let's get rid of
this annoying check.

> Reported-by: Lars Schneider <larsxschneider@gmail.com>
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
>     sparse-checkout: stop blocking empty workdirs
>
>     This is based on en/sparse-checkout.
>
>     This is something that Lars Schneider discovered working with a repo
>     that had no files at root.
>
>     Thanks, -Stolee
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-624%2Fderrickstolee%2Fsparse-checkout-allow-empty-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-624/derrickstolee/sparse-checkout-allow-empty-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/624
>
>  t/t1011-read-tree-sparse-checkout.sh | 12 +++++++---
>  t/t1091-sparse-checkout-builtin.sh   |  8 +++----
>  unpack-trees.c                       | 34 +---------------------------
>  3 files changed, 13 insertions(+), 41 deletions(-)
>
> diff --git a/t/t1011-read-tree-sparse-checkout.sh b/t/t1011-read-tree-sparse-checkout.sh
> index 63223e13bd1..140f4599773 100755
> --- a/t/t1011-read-tree-sparse-checkout.sh
> +++ b/t/t1011-read-tree-sparse-checkout.sh
> @@ -74,13 +74,19 @@ test_expect_success 'read-tree --no-sparse-checkout with empty .git/info/sparse-
>  test_expect_success 'read-tree with empty .git/info/sparse-checkout' '
>         git config core.sparsecheckout true &&
>         echo >.git/info/sparse-checkout &&
> -       read_tree_u_must_fail -m -u HEAD &&
> +       read_tree_u_must_succeed -m -u HEAD &&
>         git ls-files --stage >result &&
>         test_cmp expected result &&
>         git ls-files -t >result &&
> +       cat >expected.swt <<-\EOF &&
> +       S init.t
> +       S sub/added
> +       S sub/addedtoo
> +       S subsub/added
> +       EOF
>         test_cmp expected.swt result &&
> -       test -f init.t &&
> -       test -f sub/added
> +       ! test -f init.t &&
> +       ! test -f sub/added
>  '
>
>  test_expect_success 'match directories with trailing slash' '
> diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
> index dee99eeec30..88cdde255cd 100755
> --- a/t/t1091-sparse-checkout-builtin.sh
> +++ b/t/t1091-sparse-checkout-builtin.sh
> @@ -106,10 +106,8 @@ test_expect_success 'set enables config' '
>                 cd empty-config &&
>                 test_commit test file &&
>                 test_path_is_missing .git/config.worktree &&
> -               test_must_fail git sparse-checkout set nothing &&
> +               git sparse-checkout set nothing &&
>                 test_path_is_file .git/config.worktree &&
> -               test_must_fail git config core.sparseCheckout &&
> -               git sparse-checkout set "/*" &&
>                 test_cmp_config true core.sparseCheckout
>         )
>  '
> @@ -302,8 +300,8 @@ test_expect_success 'revert to old sparse-checkout on empty update' '
>                 echo >file &&
>                 git add file &&
>                 git commit -m "test" &&
> -               test_must_fail git sparse-checkout set nothing 2>err &&
> -               test_i18ngrep "Sparse checkout leaves no entry on working directory" err &&
> +               git sparse-checkout set nothing 2>err &&
> +               test_i18ngrep ! "Sparse checkout leaves no entry on working directory" err &&
>                 test_i18ngrep ! ".git/index.lock" err &&
>                 git sparse-checkout set file
>         )
> diff --git a/unpack-trees.c b/unpack-trees.c
> index b43f3e775ad..9a3ccd9d083 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -1677,8 +1677,6 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
>         }
>
>         if (!o->skip_sparse_checkout) {
> -               int empty_worktree = 1;
> -
>                 /*
>                  * Sparse checkout loop #2: set NEW_SKIP_WORKTREE on entries not in loop #1
>                  * If they will have NEW_SKIP_WORKTREE, also set CE_SKIP_WORKTREE
> @@ -1706,19 +1704,6 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
>
>                         if (apply_sparse_checkout(&o->result, ce, o))
>                                 ret = 1;
> -
> -                       if (!ce_skip_worktree(ce))
> -                               empty_worktree = 0;
> -               }
> -               /*
> -                * Sparse checkout is meant to narrow down checkout area
> -                * but it does not make sense to narrow down to empty working
> -                * tree. This is usually a mistake in sparse checkout rules.
> -                * Do not allow users to do that.
> -                */
> -               if (o->result.cache_nr && empty_worktree) {
> -                       ret = unpack_failed(o, "Sparse checkout leaves no entry on working directory");
> -                       goto done;
>                 }
>                 if (ret == 1) {
>                         /*
> @@ -1779,7 +1764,7 @@ enum update_sparsity_result update_sparsity(struct unpack_trees_options *o)
>  {
>         enum update_sparsity_result ret = UPDATE_SPARSITY_SUCCESS;
>         struct pattern_list pl;
> -       int i, empty_worktree;
> +       int i;
>         unsigned old_show_all_errors;
>         int free_pattern_list = 0;
>
> @@ -1810,7 +1795,6 @@ enum update_sparsity_result update_sparsity(struct unpack_trees_options *o)
>
>         /* Then loop over entries and update/remove as needed */
>         ret = UPDATE_SPARSITY_SUCCESS;
> -       empty_worktree = 1;
>         for (i = 0; i < o->src_index->cache_nr; i++) {
>                 struct cache_entry *ce = o->src_index->cache[i];
>
> @@ -1824,28 +1808,12 @@ enum update_sparsity_result update_sparsity(struct unpack_trees_options *o)
>
>                 if (apply_sparse_checkout(o->src_index, ce, o))
>                         ret = UPDATE_SPARSITY_WARNINGS;
> -
> -               if (!ce_skip_worktree(ce))
> -                       empty_worktree = 0;
> -       }
> -
> -       /*
> -        * Sparse checkout is meant to narrow down checkout area
> -        * but it does not make sense to narrow down to empty working
> -        * tree. This is usually a mistake in sparse checkout rules.
> -        * Do not allow users to do that.
> -        */
> -       if (o->src_index->cache_nr && empty_worktree) {
> -               unpack_failed(o, "Sparse checkout leaves no entry on working directory");
> -               ret = UPDATE_SPARSITY_INDEX_UPDATE_FAILURES;
> -               goto done;
>         }
>
>  skip_sparse_checkout:
>         if (check_updates(o, o->src_index))
>                 ret = UPDATE_SPARSITY_WORKTREE_UPDATE_FAILURES;
>
> -done:
>         display_warning_msgs(o);
>         o->show_all_errors = old_show_all_errors;
>         if (free_pattern_list)
>
> base-commit: 5644ca28cded68972c74614fc06d6e0e1db1a7de

Patch looks good to me.
Junio C Hamano May 4, 2020, 7:53 p.m. UTC | #2
Elijah Newren <newren@gmail.com> writes:

>> Remove the error condition when updating the sparse-checkout leaves
>> an empty working directory.
>>
>> This behavior was added in 9e1afb167 (sparse checkout: inhibit empty
>> worktree, 2009-08-20). The comment was added in a7bc906f2 (Add
>> explanation why we do not allow to sparse checkout to empty working
>> tree, 2011-09-22) in response to a "dubious" comment in 84563a624
>> (unpack-trees.c: cosmetic fix, 2010-12-22).
> ...
>
> Patch looks good to me.

Thanks, both.
Taylor Blau May 4, 2020, 10 p.m. UTC | #3
On Mon, May 04, 2020 at 11:41:38AM -0700, Elijah Newren wrote:
> On Mon, May 4, 2020 at 11:27 AM Derrick Stolee via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
> >
> > From: Derrick Stolee <dstolee@microsoft.com>
> >
> > Remove the error condition when updating the sparse-checkout leaves
> > an empty working directory.
> >
> > This behavior was added in 9e1afb167 (sparse checkout: inhibit empty
> > worktree, 2009-08-20). The comment was added in a7bc906f2 (Add
> > explanation why we do not allow to sparse checkout to empty working
> > tree, 2011-09-22) in response to a "dubious" comment in 84563a624
> > (unpack-trees.c: cosmetic fix, 2010-12-22).
> >
> > With the recent "cone mode" and "git sparse-checkout init [--cone]"
> > command, it is common to set a reasonable sparse-checkout pattern
> > set of
> >
> >         /*
> >         !/*/
> >
> > which matches only files at root. If the repository has no such files,
> > then their "git sparse-checkout init" command will fail.
> >
> > Now that we expect this to be a common pattern, we should not have the
> > commands fail on an empty working directory. If it is a confusing
> > result, then the user can recover with "git sparse-checkout disable"
> > or "git sparse-checkout set". This is especially simple when using cone
> > mode.
>
> Yeah, given that setting up a sparse-checkout is now easy (as opposed
> to setting both extensions.worktreeConfig and core.sparseCheckout
> settings, editing a .git/info/sparse-checkout file that is documented
> in some obscure section of the manual, and discovering the
> undocumented `git read-tree -mu HEAD` command that you need to run)
> and drastically less error-prone, and that recovery is now easy (also
> in contrast to before), I think this makes a lot of sense.
>
> I actually hit this error a couple times while testing with the old
> style and thought it was annoying (though understandable when the
> route for usage was so arcane and easy to mess up), but in my case I
> wasn't on a real world repository.  If we've got an example of people
> hitting it on real world repos, then by all means let's get rid of
> this annoying check.
>
> > Reported-by: Lars Schneider <larsxschneider@gmail.com>
> > Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> > ---
> >     sparse-checkout: stop blocking empty workdirs
> >
> >     This is based on en/sparse-checkout.
> >
> >     This is something that Lars Schneider discovered working with a repo
> >     that had no files at root.
> >
> >     Thanks, -Stolee
> >
> > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-624%2Fderrickstolee%2Fsparse-checkout-allow-empty-v1
> > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-624/derrickstolee/sparse-checkout-allow-empty-v1
> > Pull-Request: https://github.com/gitgitgadget/git/pull/624
> >
> >  t/t1011-read-tree-sparse-checkout.sh | 12 +++++++---
> >  t/t1091-sparse-checkout-builtin.sh   |  8 +++----
> >  unpack-trees.c                       | 34 +---------------------------
> >  3 files changed, 13 insertions(+), 41 deletions(-)
> >
> > diff --git a/t/t1011-read-tree-sparse-checkout.sh b/t/t1011-read-tree-sparse-checkout.sh
> > index 63223e13bd1..140f4599773 100755
> > --- a/t/t1011-read-tree-sparse-checkout.sh
> > +++ b/t/t1011-read-tree-sparse-checkout.sh
> > @@ -74,13 +74,19 @@ test_expect_success 'read-tree --no-sparse-checkout with empty .git/info/sparse-
> >  test_expect_success 'read-tree with empty .git/info/sparse-checkout' '
> >         git config core.sparsecheckout true &&
> >         echo >.git/info/sparse-checkout &&
> > -       read_tree_u_must_fail -m -u HEAD &&
> > +       read_tree_u_must_succeed -m -u HEAD &&
> >         git ls-files --stage >result &&
> >         test_cmp expected result &&
> >         git ls-files -t >result &&
> > +       cat >expected.swt <<-\EOF &&
> > +       S init.t
> > +       S sub/added
> > +       S sub/addedtoo
> > +       S subsub/added
> > +       EOF
> >         test_cmp expected.swt result &&
> > -       test -f init.t &&
> > -       test -f sub/added
> > +       ! test -f init.t &&
> > +       ! test -f sub/added
> >  '
> >
> >  test_expect_success 'match directories with trailing slash' '
> > diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
> > index dee99eeec30..88cdde255cd 100755
> > --- a/t/t1091-sparse-checkout-builtin.sh
> > +++ b/t/t1091-sparse-checkout-builtin.sh
> > @@ -106,10 +106,8 @@ test_expect_success 'set enables config' '
> >                 cd empty-config &&
> >                 test_commit test file &&
> >                 test_path_is_missing .git/config.worktree &&
> > -               test_must_fail git sparse-checkout set nothing &&
> > +               git sparse-checkout set nothing &&
> >                 test_path_is_file .git/config.worktree &&
> > -               test_must_fail git config core.sparseCheckout &&
> > -               git sparse-checkout set "/*" &&
> >                 test_cmp_config true core.sparseCheckout
> >         )
> >  '
> > @@ -302,8 +300,8 @@ test_expect_success 'revert to old sparse-checkout on empty update' '
> >                 echo >file &&
> >                 git add file &&
> >                 git commit -m "test" &&
> > -               test_must_fail git sparse-checkout set nothing 2>err &&
> > -               test_i18ngrep "Sparse checkout leaves no entry on working directory" err &&
> > +               git sparse-checkout set nothing 2>err &&
> > +               test_i18ngrep ! "Sparse checkout leaves no entry on working directory" err &&
> >                 test_i18ngrep ! ".git/index.lock" err &&
> >                 git sparse-checkout set file
> >         )
> > diff --git a/unpack-trees.c b/unpack-trees.c
> > index b43f3e775ad..9a3ccd9d083 100644
> > --- a/unpack-trees.c
> > +++ b/unpack-trees.c
> > @@ -1677,8 +1677,6 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
> >         }
> >
> >         if (!o->skip_sparse_checkout) {
> > -               int empty_worktree = 1;
> > -
> >                 /*
> >                  * Sparse checkout loop #2: set NEW_SKIP_WORKTREE on entries not in loop #1
> >                  * If they will have NEW_SKIP_WORKTREE, also set CE_SKIP_WORKTREE
> > @@ -1706,19 +1704,6 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
> >
> >                         if (apply_sparse_checkout(&o->result, ce, o))
> >                                 ret = 1;
> > -
> > -                       if (!ce_skip_worktree(ce))
> > -                               empty_worktree = 0;
> > -               }
> > -               /*
> > -                * Sparse checkout is meant to narrow down checkout area
> > -                * but it does not make sense to narrow down to empty working
> > -                * tree. This is usually a mistake in sparse checkout rules.
> > -                * Do not allow users to do that.
> > -                */
> > -               if (o->result.cache_nr && empty_worktree) {
> > -                       ret = unpack_failed(o, "Sparse checkout leaves no entry on working directory");
> > -                       goto done;
> >                 }
> >                 if (ret == 1) {
> >                         /*
> > @@ -1779,7 +1764,7 @@ enum update_sparsity_result update_sparsity(struct unpack_trees_options *o)
> >  {
> >         enum update_sparsity_result ret = UPDATE_SPARSITY_SUCCESS;
> >         struct pattern_list pl;
> > -       int i, empty_worktree;
> > +       int i;
> >         unsigned old_show_all_errors;
> >         int free_pattern_list = 0;
> >
> > @@ -1810,7 +1795,6 @@ enum update_sparsity_result update_sparsity(struct unpack_trees_options *o)
> >
> >         /* Then loop over entries and update/remove as needed */
> >         ret = UPDATE_SPARSITY_SUCCESS;
> > -       empty_worktree = 1;
> >         for (i = 0; i < o->src_index->cache_nr; i++) {
> >                 struct cache_entry *ce = o->src_index->cache[i];
> >
> > @@ -1824,28 +1808,12 @@ enum update_sparsity_result update_sparsity(struct unpack_trees_options *o)
> >
> >                 if (apply_sparse_checkout(o->src_index, ce, o))
> >                         ret = UPDATE_SPARSITY_WARNINGS;
> > -
> > -               if (!ce_skip_worktree(ce))
> > -                       empty_worktree = 0;
> > -       }
> > -
> > -       /*
> > -        * Sparse checkout is meant to narrow down checkout area
> > -        * but it does not make sense to narrow down to empty working
> > -        * tree. This is usually a mistake in sparse checkout rules.
> > -        * Do not allow users to do that.
> > -        */
> > -       if (o->src_index->cache_nr && empty_worktree) {
> > -               unpack_failed(o, "Sparse checkout leaves no entry on working directory");
> > -               ret = UPDATE_SPARSITY_INDEX_UPDATE_FAILURES;
> > -               goto done;
> >         }
> >
> >  skip_sparse_checkout:
> >         if (check_updates(o, o->src_index))
> >                 ret = UPDATE_SPARSITY_WORKTREE_UPDATE_FAILURES;
> >
> > -done:
> >         display_warning_msgs(o);
> >         o->show_all_errors = old_show_all_errors;
> >         if (free_pattern_list)
> >
> > base-commit: 5644ca28cded68972c74614fc06d6e0e1db1a7de
>
> Patch looks good to me.

To me as well. Thanks for sending something so quickly after Lars'
initial report.

  Reviewed-by: Taylor Blau <me@ttaylorr.com>

Thanks,
Taylor
Christian Couder April 13, 2021, 4:55 a.m. UTC | #4
On Mon, May 4, 2020 at 8:30 PM Derrick Stolee via GitGitGadget
<gitgitgadget@gmail.com> wrote:

> @@ -302,8 +300,8 @@ test_expect_success 'revert to old sparse-checkout on empty update' '
>                 echo >file &&
>                 git add file &&
>                 git commit -m "test" &&
> -               test_must_fail git sparse-checkout set nothing 2>err &&
> -               test_i18ngrep "Sparse checkout leaves no entry on working directory" err &&
> +               git sparse-checkout set nothing 2>err &&
> +               test_i18ngrep ! "Sparse checkout leaves no entry on working directory" err &&

It looks like this check is obsolete as the "Sparse checkout leaves no
entry on working directory" error has been removed by this patch
below...

>                 test_i18ngrep ! ".git/index.lock" err &&
>                 git sparse-checkout set file
>         )

[...]

> @@ -1706,19 +1704,6 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
>
>                         if (apply_sparse_checkout(&o->result, ce, o))
>                                 ret = 1;
> -
> -                       if (!ce_skip_worktree(ce))
> -                               empty_worktree = 0;
> -               }
> -               /*
> -                * Sparse checkout is meant to narrow down checkout area
> -                * but it does not make sense to narrow down to empty working
> -                * tree. This is usually a mistake in sparse checkout rules.
> -                * Do not allow users to do that.
> -                */
> -               if (o->result.cache_nr && empty_worktree) {
> -                       ret = unpack_failed(o, "Sparse checkout leaves no entry on working directory");

...here...

> -                       goto done;
>                 }
>                 if (ret == 1) {
>                         /*

[...]

> @@ -1824,28 +1808,12 @@ enum update_sparsity_result update_sparsity(struct unpack_trees_options *o)
>
>                 if (apply_sparse_checkout(o->src_index, ce, o))
>                         ret = UPDATE_SPARSITY_WARNINGS;
> -
> -               if (!ce_skip_worktree(ce))
> -                       empty_worktree = 0;
> -       }
> -
> -       /*
> -        * Sparse checkout is meant to narrow down checkout area
> -        * but it does not make sense to narrow down to empty working
> -        * tree. This is usually a mistake in sparse checkout rules.
> -        * Do not allow users to do that.
> -        */
> -       if (o->src_index->cache_nr && empty_worktree) {
> -               unpack_failed(o, "Sparse checkout leaves no entry on working directory");

...and here.

> -               ret = UPDATE_SPARSITY_INDEX_UPDATE_FAILURES;
> -               goto done;
>         }

So maybe instead of the 3 lines below:

> +               git sparse-checkout set nothing 2>err &&
> +               test_i18ngrep ! "Sparse checkout leaves no entry on working directory" err &&
>                 test_i18ngrep ! ".git/index.lock" err &&

we should just have:

               git sparse-checkout set nothing &&

?
diff mbox series

Patch

diff --git a/t/t1011-read-tree-sparse-checkout.sh b/t/t1011-read-tree-sparse-checkout.sh
index 63223e13bd1..140f4599773 100755
--- a/t/t1011-read-tree-sparse-checkout.sh
+++ b/t/t1011-read-tree-sparse-checkout.sh
@@ -74,13 +74,19 @@  test_expect_success 'read-tree --no-sparse-checkout with empty .git/info/sparse-
 test_expect_success 'read-tree with empty .git/info/sparse-checkout' '
 	git config core.sparsecheckout true &&
 	echo >.git/info/sparse-checkout &&
-	read_tree_u_must_fail -m -u HEAD &&
+	read_tree_u_must_succeed -m -u HEAD &&
 	git ls-files --stage >result &&
 	test_cmp expected result &&
 	git ls-files -t >result &&
+	cat >expected.swt <<-\EOF &&
+	S init.t
+	S sub/added
+	S sub/addedtoo
+	S subsub/added
+	EOF
 	test_cmp expected.swt result &&
-	test -f init.t &&
-	test -f sub/added
+	! test -f init.t &&
+	! test -f sub/added
 '
 
 test_expect_success 'match directories with trailing slash' '
diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
index dee99eeec30..88cdde255cd 100755
--- a/t/t1091-sparse-checkout-builtin.sh
+++ b/t/t1091-sparse-checkout-builtin.sh
@@ -106,10 +106,8 @@  test_expect_success 'set enables config' '
 		cd empty-config &&
 		test_commit test file &&
 		test_path_is_missing .git/config.worktree &&
-		test_must_fail git sparse-checkout set nothing &&
+		git sparse-checkout set nothing &&
 		test_path_is_file .git/config.worktree &&
-		test_must_fail git config core.sparseCheckout &&
-		git sparse-checkout set "/*" &&
 		test_cmp_config true core.sparseCheckout
 	)
 '
@@ -302,8 +300,8 @@  test_expect_success 'revert to old sparse-checkout on empty update' '
 		echo >file &&
 		git add file &&
 		git commit -m "test" &&
-		test_must_fail git sparse-checkout set nothing 2>err &&
-		test_i18ngrep "Sparse checkout leaves no entry on working directory" err &&
+		git sparse-checkout set nothing 2>err &&
+		test_i18ngrep ! "Sparse checkout leaves no entry on working directory" err &&
 		test_i18ngrep ! ".git/index.lock" err &&
 		git sparse-checkout set file
 	)
diff --git a/unpack-trees.c b/unpack-trees.c
index b43f3e775ad..9a3ccd9d083 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1677,8 +1677,6 @@  int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
 	}
 
 	if (!o->skip_sparse_checkout) {
-		int empty_worktree = 1;
-
 		/*
 		 * Sparse checkout loop #2: set NEW_SKIP_WORKTREE on entries not in loop #1
 		 * If they will have NEW_SKIP_WORKTREE, also set CE_SKIP_WORKTREE
@@ -1706,19 +1704,6 @@  int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
 
 			if (apply_sparse_checkout(&o->result, ce, o))
 				ret = 1;
-
-			if (!ce_skip_worktree(ce))
-				empty_worktree = 0;
-		}
-		/*
-		 * Sparse checkout is meant to narrow down checkout area
-		 * but it does not make sense to narrow down to empty working
-		 * tree. This is usually a mistake in sparse checkout rules.
-		 * Do not allow users to do that.
-		 */
-		if (o->result.cache_nr && empty_worktree) {
-			ret = unpack_failed(o, "Sparse checkout leaves no entry on working directory");
-			goto done;
 		}
 		if (ret == 1) {
 			/*
@@ -1779,7 +1764,7 @@  enum update_sparsity_result update_sparsity(struct unpack_trees_options *o)
 {
 	enum update_sparsity_result ret = UPDATE_SPARSITY_SUCCESS;
 	struct pattern_list pl;
-	int i, empty_worktree;
+	int i;
 	unsigned old_show_all_errors;
 	int free_pattern_list = 0;
 
@@ -1810,7 +1795,6 @@  enum update_sparsity_result update_sparsity(struct unpack_trees_options *o)
 
 	/* Then loop over entries and update/remove as needed */
 	ret = UPDATE_SPARSITY_SUCCESS;
-	empty_worktree = 1;
 	for (i = 0; i < o->src_index->cache_nr; i++) {
 		struct cache_entry *ce = o->src_index->cache[i];
 
@@ -1824,28 +1808,12 @@  enum update_sparsity_result update_sparsity(struct unpack_trees_options *o)
 
 		if (apply_sparse_checkout(o->src_index, ce, o))
 			ret = UPDATE_SPARSITY_WARNINGS;
-
-		if (!ce_skip_worktree(ce))
-			empty_worktree = 0;
-	}
-
-	/*
-	 * Sparse checkout is meant to narrow down checkout area
-	 * but it does not make sense to narrow down to empty working
-	 * tree. This is usually a mistake in sparse checkout rules.
-	 * Do not allow users to do that.
-	 */
-	if (o->src_index->cache_nr && empty_worktree) {
-		unpack_failed(o, "Sparse checkout leaves no entry on working directory");
-		ret = UPDATE_SPARSITY_INDEX_UPDATE_FAILURES;
-		goto done;
 	}
 
 skip_sparse_checkout:
 	if (check_updates(o, o->src_index))
 		ret = UPDATE_SPARSITY_WORKTREE_UPDATE_FAILURES;
 
-done:
 	display_warning_msgs(o);
 	o->show_all_errors = old_show_all_errors;
 	if (free_pattern_list)