* [PATCH] sparse-checkout: stop blocking empty workdirs
@ 2020-05-04 18:27 Derrick Stolee via GitGitGadget
2020-05-04 18:41 ` Elijah Newren
2021-04-13 4:55 ` Christian Couder
0 siblings, 2 replies; 5+ messages in thread
From: Derrick Stolee via GitGitGadget @ 2020-05-04 18:27 UTC (permalink / raw)
To: git; +Cc: newren, larsxschneider, Derrick Stolee, Derrick Stolee
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(-)
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
--
gitgitgadget
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] sparse-checkout: stop blocking empty workdirs
2020-05-04 18:27 [PATCH] sparse-checkout: stop blocking empty workdirs Derrick Stolee via GitGitGadget
@ 2020-05-04 18:41 ` Elijah Newren
2020-05-04 19:53 ` Junio C Hamano
2020-05-04 22:00 ` Taylor Blau
2021-04-13 4:55 ` Christian Couder
1 sibling, 2 replies; 5+ messages in thread
From: Elijah Newren @ 2020-05-04 18:41 UTC (permalink / raw)
To: Derrick Stolee via GitGitGadget
Cc: Git Mailing List, Lars Schneider, Derrick Stolee
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.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] sparse-checkout: stop blocking empty workdirs
2020-05-04 18:41 ` Elijah Newren
@ 2020-05-04 19:53 ` Junio C Hamano
2020-05-04 22:00 ` Taylor Blau
1 sibling, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2020-05-04 19:53 UTC (permalink / raw)
To: Elijah Newren
Cc: Derrick Stolee via GitGitGadget, Git Mailing List, Lars Schneider,
Derrick Stolee
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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] sparse-checkout: stop blocking empty workdirs
2020-05-04 18:41 ` Elijah Newren
2020-05-04 19:53 ` Junio C Hamano
@ 2020-05-04 22:00 ` Taylor Blau
1 sibling, 0 replies; 5+ messages in thread
From: Taylor Blau @ 2020-05-04 22:00 UTC (permalink / raw)
To: Elijah Newren
Cc: Derrick Stolee via GitGitGadget, Git Mailing List, Lars Schneider,
Derrick Stolee
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] sparse-checkout: stop blocking empty workdirs
2020-05-04 18:27 [PATCH] sparse-checkout: stop blocking empty workdirs Derrick Stolee via GitGitGadget
2020-05-04 18:41 ` Elijah Newren
@ 2021-04-13 4:55 ` Christian Couder
1 sibling, 0 replies; 5+ messages in thread
From: Christian Couder @ 2021-04-13 4:55 UTC (permalink / raw)
To: Derrick Stolee via GitGitGadget
Cc: git, Elijah Newren, Lars Schneider, Derrick Stolee
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 &&
?
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-04-13 4:55 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-04 18:27 [PATCH] sparse-checkout: stop blocking empty workdirs Derrick Stolee via GitGitGadget
2020-05-04 18:41 ` Elijah Newren
2020-05-04 19:53 ` Junio C Hamano
2020-05-04 22:00 ` Taylor Blau
2021-04-13 4:55 ` Christian Couder
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).