diff mbox series

[v2,10/18] sparse-checkout: use new update_sparsity() function

Message ID 27ed4a3f7a28c66d8185cb4c484e8b0af81b15c6.1584813609.git.gitgitgadget@gmail.com (mailing list archive)
State New, archived
Headers show
Series Sparse checkout improvements -- improved sparsity updating | expand

Commit Message

Johannes Schindelin via GitGitGadget March 21, 2020, 6 p.m. UTC 
From: Elijah Newren <newren@gmail.com>

Remove the equivalent of 'git read-tree -mu HEAD' in the sparse-checkout
codepaths for setting the SKIP_WORKTREE bits and instead use the new
update_sparsity() function.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/sparse-checkout.c          | 40 ++++++++----------------------
 t/t1091-sparse-checkout-builtin.sh | 35 +++++++++++++++++++-------
 2 files changed, 36 insertions(+), 39 deletions(-)

Comments

Derrick Stolee March 23, 2020, 6:07 p.m. UTC | #1 
On 3/21/2020 2:00 PM, Elijah Newren via GitGitGadget wrote:
> From: Elijah Newren <newren@gmail.com>
> 
> Remove the equivalent of 'git read-tree -mu HEAD' in the sparse-checkout
> codepaths for setting the SKIP_WORKTREE bits and instead use the new
> update_sparsity() function.
> 
[snip]
> diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
> index 8607a8e6d1a..86ae422ff5c 100755
> --- a/t/t1091-sparse-checkout-builtin.sh
> +++ b/t/t1091-sparse-checkout-builtin.sh
> @@ -277,16 +277,23 @@ test_expect_success 'cone mode: add parent path' '
>  	check_files repo a deep folder1
>  '
>  
> -test_expect_success 'revert to old sparse-checkout on bad update' '
> +test_expect_success 'not-up-to-date does not block rest of sparsification' '
>  	test_when_finished git -C repo sparse-checkout disable &&
>  	test_when_finished git -C repo reset --hard &&
>  	git -C repo sparse-checkout set deep &&
> +
>  	echo update >repo/deep/deeper2/a &&
>  	cp repo/.git/info/sparse-checkout expect &&
> -	test_must_fail git -C repo sparse-checkout set deep/deeper1 2>err &&
> -	test_i18ngrep "cannot set sparse-checkout patterns" err &&
> -	test_cmp repo/.git/info/sparse-checkout expect &&
> -	check_files repo/deep a deeper1 deeper2
> +	test_write_lines "!/deep/*/" "/deep/deeper1/" >>expect &&
> +
> +	git -C repo sparse-checkout set deep/deeper1 2>err &&
> +
> +	test_i18ngrep "Cannot update sparse checkout" err &&
> +	test_cmp expect repo/.git/info/sparse-checkout &&
> +	check_files repo/deep a deeper1 deeper2 &&
> +	check_files repo/deep/deeper1 a deepest &&
> +	check_files repo/deep/deeper1/deepest a &&
> +	check_files repo/deep/deeper2 a
>  '

This demonstrates some of the value of using update_sparsity.
I expect some more specific tests are coming later in the
series.

>  test_expect_success 'revert to old sparse-checkout on empty update' '
> @@ -316,12 +323,22 @@ test_expect_success '.gitignore should not warn about cone mode' '
>  	test_i18ngrep ! "disabling cone patterns" err
>  '
>  
> -test_expect_success 'sparse-checkout (init|set|disable) fails with dirty status' '
> +test_expect_success 'sparse-checkout (init|set|disable) warns with dirty status' '
>  	git clone repo dirty &&
>  	echo dirty >dirty/folder1/a &&
> -	test_must_fail git -C dirty sparse-checkout init &&
> -	test_must_fail git -C dirty sparse-checkout set /folder2/* /deep/deeper1/* &&
> -	test_must_fail git -C dirty sparse-checkout disable &&
> +
> +	git -C dirty sparse-checkout init 2>err &&
> +	test_i18ngrep "error" err &&
> +	test_i18ngrep "Cannot update sparse checkout" err &&
> +
> +	git -C dirty sparse-checkout set /folder2/* /deep/deeper1/* &&
> +	test_i18ngrep "error" err &&
> +	test_i18ngrep "Cannot update sparse checkout" err &&
> +
> +	git -C dirty sparse-checkout disable &&
> +	test_i18ngrep "error" err &&
> +	test_i18ngrep "Cannot update sparse checkout" err &&
> +

Should the "error" messages we grep for be more specific? Are they
the same as the "Cannot update sparse checkout"?
Elijah Newren March 23, 2020, 6:14 p.m. UTC | #2 
On Mon, Mar 23, 2020 at 11:07 AM Derrick Stolee <stolee@gmail.com> wrote:
>
> On 3/21/2020 2:00 PM, Elijah Newren via GitGitGadget wrote:
> > From: Elijah Newren <newren@gmail.com>
> >
> > Remove the equivalent of 'git read-tree -mu HEAD' in the sparse-checkout
> > codepaths for setting the SKIP_WORKTREE bits and instead use the new
> > update_sparsity() function.
> >
> [snip]
> > diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
> > index 8607a8e6d1a..86ae422ff5c 100755
> > --- a/t/t1091-sparse-checkout-builtin.sh
> > +++ b/t/t1091-sparse-checkout-builtin.sh
> > @@ -277,16 +277,23 @@ test_expect_success 'cone mode: add parent path' '
> >       check_files repo a deep folder1
> >  '
> >
> > -test_expect_success 'revert to old sparse-checkout on bad update' '
> > +test_expect_success 'not-up-to-date does not block rest of sparsification' '
> >       test_when_finished git -C repo sparse-checkout disable &&
> >       test_when_finished git -C repo reset --hard &&
> >       git -C repo sparse-checkout set deep &&
> > +
> >       echo update >repo/deep/deeper2/a &&
> >       cp repo/.git/info/sparse-checkout expect &&
> > -     test_must_fail git -C repo sparse-checkout set deep/deeper1 2>err &&
> > -     test_i18ngrep "cannot set sparse-checkout patterns" err &&
> > -     test_cmp repo/.git/info/sparse-checkout expect &&
> > -     check_files repo/deep a deeper1 deeper2
> > +     test_write_lines "!/deep/*/" "/deep/deeper1/" >>expect &&
> > +
> > +     git -C repo sparse-checkout set deep/deeper1 2>err &&
> > +
> > +     test_i18ngrep "Cannot update sparse checkout" err &&
> > +     test_cmp expect repo/.git/info/sparse-checkout &&
> > +     check_files repo/deep a deeper1 deeper2 &&
> > +     check_files repo/deep/deeper1 a deepest &&
> > +     check_files repo/deep/deeper1/deepest a &&
> > +     check_files repo/deep/deeper2 a
> >  '
>
> This demonstrates some of the value of using update_sparsity.
> I expect some more specific tests are coming later in the
> series.
>
> >  test_expect_success 'revert to old sparse-checkout on empty update' '
> > @@ -316,12 +323,22 @@ test_expect_success '.gitignore should not warn about cone mode' '
> >       test_i18ngrep ! "disabling cone patterns" err
> >  '
> >
> > -test_expect_success 'sparse-checkout (init|set|disable) fails with dirty status' '
> > +test_expect_success 'sparse-checkout (init|set|disable) warns with dirty status' '
> >       git clone repo dirty &&
> >       echo dirty >dirty/folder1/a &&
> > -     test_must_fail git -C dirty sparse-checkout init &&
> > -     test_must_fail git -C dirty sparse-checkout set /folder2/* /deep/deeper1/* &&
> > -     test_must_fail git -C dirty sparse-checkout disable &&
> > +
> > +     git -C dirty sparse-checkout init 2>err &&
> > +     test_i18ngrep "error" err &&
> > +     test_i18ngrep "Cannot update sparse checkout" err &&
> > +
> > +     git -C dirty sparse-checkout set /folder2/* /deep/deeper1/* &&
> > +     test_i18ngrep "error" err &&
> > +     test_i18ngrep "Cannot update sparse checkout" err &&
> > +
> > +     git -C dirty sparse-checkout disable &&
> > +     test_i18ngrep "error" err &&
> > +     test_i18ngrep "Cannot update sparse checkout" err &&
> > +
>
> Should the "error" messages we grep for be more specific? Are they
> the same as the "Cannot update sparse checkout"?

The two lines are checking for the same message, it's just that the
error message is split across multiple lines and thus requires two
greps.  The error messages are actually a bit of a mess, particularly
when they are triggered for multiple paths.  That's something that
subsequent commits will clean up.

I can add a note about this to the commit message.
Derrick Stolee March 23, 2020, 6:22 p.m. UTC | #3 
On 3/23/2020 2:14 PM, Elijah Newren wrote:
> On Mon, Mar 23, 2020 at 11:07 AM Derrick Stolee <stolee@gmail.com> wrote:
>>
>> Should the "error" messages we grep for be more specific? Are they
>> the same as the "Cannot update sparse checkout"?
> 
> The two lines are checking for the same message, it's just that the
> error message is split across multiple lines and thus requires two
> greps.  The error messages are actually a bit of a mess, particularly
> when they are triggered for multiple paths.  That's something that
> subsequent commits will clean up.

I see that now! The very next commit cleans this up nicely.

> I can add a note about this to the commit message.

Thanks!
diff mbox series

Patch

diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index d102a9697fd..a55c60d7594 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -94,49 +94,35 @@  static int sparse_checkout_list(int argc, const char **argv)
 
 static int update_working_directory(struct pattern_list *pl)
 {
-	int result = 0;
+	enum update_sparsity_result result;
 	struct unpack_trees_options o;
 	struct lock_file lock_file = LOCK_INIT;
-	struct object_id oid;
-	struct tree *tree;
-	struct tree_desc t;
 	struct repository *r = the_repository;
 
-	if (repo_read_index_unmerged(r))
-		die(_("you need to resolve your current index first"));
-
-	if (get_oid("HEAD", &oid))
-		return 0;
-
-	tree = parse_tree_indirect(&oid);
-	parse_tree(tree);
-	init_tree_desc(&t, tree->buffer, tree->size);
-
 	memset(&o, 0, sizeof(o));
 	o.verbose_update = isatty(2);
-	o.merge = 1;
 	o.update = 1;
-	o.fn = oneway_merge;
 	o.head_idx = -1;
 	o.src_index = r->index;
 	o.dst_index = r->index;
 	o.skip_sparse_checkout = 0;
 	o.pl = pl;
 
-	resolve_undo_clear_index(r->index);
 	setup_work_tree();
 
-	cache_tree_free(&r->index->cache_tree);
-
 	repo_hold_locked_index(r, &lock_file, LOCK_DIE_ON_ERROR);
 
-	core_apply_sparse_checkout = 1;
-	result = unpack_trees(1, &t, &o);
+	result = update_sparsity(&o);
 
-	if (!result) {
-		prime_cache_tree(r, r->index, tree);
+	if (result == UPDATE_SPARSITY_WARNINGS)
+		/*
+		 * We don't do any special handling of warnings from untracked
+		 * files in the way or dirty entries that can't be removed.
+		 */
+		result = UPDATE_SPARSITY_SUCCESS;
+	if (result == UPDATE_SPARSITY_SUCCESS)
 		write_locked_index(r->index, &lock_file, COMMIT_LOCK);
-	} else
+	else
 		rollback_lock_file(&lock_file);
 
 	return result;
@@ -303,8 +289,6 @@  static int sparse_checkout_init(int argc, const char **argv)
 	};
 
 	repo_read_index(the_repository);
-	require_clean_work_tree(the_repository,
-				N_("initialize sparse-checkout"), NULL, 1, 0);
 
 	argc = parse_options(argc, argv, NULL,
 			     builtin_sparse_checkout_init_options,
@@ -559,8 +543,6 @@  static int sparse_checkout_set(int argc, const char **argv, const char *prefix,
 	};
 
 	repo_read_index(the_repository);
-	require_clean_work_tree(the_repository,
-				N_("set sparse-checkout patterns"), NULL, 1, 0);
 
 	argc = parse_options(argc, argv, prefix,
 			     builtin_sparse_checkout_set_options,
@@ -576,8 +558,6 @@  static int sparse_checkout_disable(int argc, const char **argv)
 	struct strbuf match_all = STRBUF_INIT;
 
 	repo_read_index(the_repository);
-	require_clean_work_tree(the_repository,
-				N_("disable sparse-checkout"), NULL, 1, 0);
 
 	memset(&pl, 0, sizeof(pl));
 	hashmap_init(&pl.recursive_hashmap, pl_hashmap_cmp, NULL, 0);
diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
index 8607a8e6d1a..86ae422ff5c 100755
--- a/t/t1091-sparse-checkout-builtin.sh
+++ b/t/t1091-sparse-checkout-builtin.sh
@@ -277,16 +277,23 @@  test_expect_success 'cone mode: add parent path' '
 	check_files repo a deep folder1
 '
 
-test_expect_success 'revert to old sparse-checkout on bad update' '
+test_expect_success 'not-up-to-date does not block rest of sparsification' '
 	test_when_finished git -C repo sparse-checkout disable &&
 	test_when_finished git -C repo reset --hard &&
 	git -C repo sparse-checkout set deep &&
+
 	echo update >repo/deep/deeper2/a &&
 	cp repo/.git/info/sparse-checkout expect &&
-	test_must_fail git -C repo sparse-checkout set deep/deeper1 2>err &&
-	test_i18ngrep "cannot set sparse-checkout patterns" err &&
-	test_cmp repo/.git/info/sparse-checkout expect &&
-	check_files repo/deep a deeper1 deeper2
+	test_write_lines "!/deep/*/" "/deep/deeper1/" >>expect &&
+
+	git -C repo sparse-checkout set deep/deeper1 2>err &&
+
+	test_i18ngrep "Cannot update sparse checkout" err &&
+	test_cmp expect repo/.git/info/sparse-checkout &&
+	check_files repo/deep a deeper1 deeper2 &&
+	check_files repo/deep/deeper1 a deepest &&
+	check_files repo/deep/deeper1/deepest a &&
+	check_files repo/deep/deeper2 a
 '
 
 test_expect_success 'revert to old sparse-checkout on empty update' '
@@ -316,12 +323,22 @@  test_expect_success '.gitignore should not warn about cone mode' '
 	test_i18ngrep ! "disabling cone patterns" err
 '
 
-test_expect_success 'sparse-checkout (init|set|disable) fails with dirty status' '
+test_expect_success 'sparse-checkout (init|set|disable) warns with dirty status' '
 	git clone repo dirty &&
 	echo dirty >dirty/folder1/a &&
-	test_must_fail git -C dirty sparse-checkout init &&
-	test_must_fail git -C dirty sparse-checkout set /folder2/* /deep/deeper1/* &&
-	test_must_fail git -C dirty sparse-checkout disable &&
+
+	git -C dirty sparse-checkout init 2>err &&
+	test_i18ngrep "error" err &&
+	test_i18ngrep "Cannot update sparse checkout" err &&
+
+	git -C dirty sparse-checkout set /folder2/* /deep/deeper1/* &&
+	test_i18ngrep "error" err &&
+	test_i18ngrep "Cannot update sparse checkout" err &&
+
+	git -C dirty sparse-checkout disable &&
+	test_i18ngrep "error" err &&
+	test_i18ngrep "Cannot update sparse checkout" err &&
+
 	git -C dirty reset --hard &&
 	git -C dirty sparse-checkout init &&
 	git -C dirty sparse-checkout set /folder2/* /deep/deeper1/* &&