sparse-checkout: make 'clean' clear more files

The 'git sparse-checkout clean' command is designed to be a one-command
way to get the worktree in a state such that a sparse index would
operate efficiently. The previous change demonstrated that files outside
the sparse-checkout that were committed due to a merge conflict would
persist despite attempts to run 'git sparse-checkout clean' and instead
a 'git sparse-checkout reapply' would be required.

Instead of requiring users to run both commands, update 'clean' to be
more ruthless about tracked sparse directories. The key here is to make
sure that the SKIP_WORKTREE bit is removed from more paths in the index
using update_sparsity() before compressing the index to a sparse one
in-memory.

The tricky part here is that update_sparsity() was previously assuming
that it would be in 'update' mode and would change the worktree as it
made changes. However, we do not want to make these worktree changes at
this point, instead relying on our later logic (that integrates with
--dry-run and --verbose options) to perform those steps.

One side-effect here is that we also clear out staged files that exist
in the worktree, but they would also appear in the verbose output as
part of the dry run.

The final test in t1091 demonstrates that we no longer need the
'reapply' subcommand for merge resolutions. It also fixes an earlier
case where 'git add --sparse' clears the SKIP_WORKTREE bit and avoids a
directory deletion.

Signed-off-by: Derrick Stolee <[email protected]>

Author: derrickstolee

builtin/sparse-checkout.c

@@ -962,6 +962,7 @@ static int sparse_checkout_clean(int argc, const char **argv,
 	size_t worktree_len;
 	int force = 0, dry_run = 0, verbose = 0;
 	int require_force = 1;
+	struct unpack_trees_options o = { 0 };
 
 	struct option builtin_sparse_checkout_clean_options[] = {
 		OPT__DRY_RUN(&dry_run, N_("dry run")),
@@ -990,6 +991,13 @@ static int sparse_checkout_clean(int argc, const char **argv,
 	if (repo_read_index(repo) < 0)
 		die(_("failed to read index"));
 
+	o.verbose_update = verbose;
+	o.update = 0; /* skip modifying the worktree here. */
+	o.head_idx = -1;
+	o.src_index = o.dst_index = repo->index;
+	if (update_sparsity(&o, NULL))
+		warning(_("failed to reapply sparse-checkout patterns"));
+
 	if (convert_to_sparse(repo->index, SPARSE_INDEX_MEMORY_ONLY) ||
 	    repo->index->sparse_index == INDEX_EXPANDED)
 		die(_("failed to convert index to a sparse index; resolve merge conflicts and try again"));

t/t1091-sparse-checkout-builtin.sh

@@ -1123,35 +1123,25 @@ test_expect_success 'clean with sparse file states' '
 	# Now, stage the change to the tracked file.
 	git -C repo add --sparse folder2/a &&
 
-	# Clean will continue not doing anything.
-	git -C repo sparse-checkout clean >out &&
-	test_line_count = 0 out &&
-	test_path_exists repo/folder2/a &&
-	test_path_exists repo/folder2/file &&
-
-	# But we can reapply to remove the staged change.
-	git -C repo sparse-checkout reapply 2>err &&
-	test_grep folder2 err &&
-	test_path_is_missing repo/folder2/a &&
-	test_path_exists repo/folder2/file &&
-
-	# We can clean now.
+	# Clean will update the skip-worktree bit and remove this file.
 	cat >expect <<-\EOF &&
 	Removing folder2/
 	EOF
 	git -C repo sparse-checkout clean >out &&
 	test_cmp expect out &&
 	test_path_is_missing repo/folder2 &&
 
-	# At the moment, the file is staged.
+	# We are now in a strange state: the change is staged but it
+	# is also removed from the worktree.
 	cat >expect <<-\EOF &&
-	M  folder2/a
+	MD folder2/a
 	EOF
 
 	git -C repo status -s >out &&
 	test_cmp expect out &&
 
-	# Reapply persists the modified state.
+	# Reapply updates the skip-worktree bit to remove this
+	# "deleted" state.
 	git -C repo sparse-checkout reapply &&
 	cat >expect <<-\EOF &&
 	M  folder2/a
@@ -1168,15 +1158,21 @@ test_expect_success 'clean with sparse file states' '
 	mkdir repo/folder2/ &&
 	echo dirtier >repo/folder2/a &&
 	git -C repo add --sparse folder2/a &&
+	cat >expect <<-\EOF &&
+	Removing folder2/
+	EOF
 	git -C repo sparse-checkout clean >out &&
-	test_must_be_empty out &&
-	test_path_exists repo/folder2/a &&
+	test_cmp expect out &&
+	test_path_is_missing repo/folder2 &&
 
 	# Committing without reapplying makes it look like a deletion
 	# due to no skip-worktree bit.
 	git -C repo commit -m "dirtier" &&
+	cat >expect <<-\EOF &&
+	 D folder2/a
+	EOF
 	git -C repo status -s >out &&
-	test_must_be_empty out &&
+	test_cmp expect out &&
 
 	git -C repo sparse-checkout reapply &&
 	git -C repo status -s >out &&
@@ -1202,7 +1198,11 @@ test_expect_success 'sparse-checkout operations with merge conflicts' '
 		git commit -a -m "left" &&
 
 		git checkout -b merge &&
-		git sparse-checkout set deep/deeper1 &&
+
+		touch deep/deeper2/extra &&
+		git sparse-checkout set deep/deeper1 2>err &&
+		grep "contains untracked files" err &&
+		test_path_exists deep/deeper2/extra &&
 
 		test_must_fail git merge -m "will-conflict" right &&
 
@@ -1214,15 +1214,26 @@ test_expect_success 'sparse-checkout operations with merge conflicts' '
 		git merge --continue &&
 
 		test_path_exists folder1/even/more/dirs/file &&
+		test_path_exists deep/deeper2/extra &&
 
-		# clean does not remove the file, because the
-		# SKIP_WORKTREE bit was not cleared by the merge command.
+		cat >expect <<-\EOF &&
+		Removing deep/deeper2/
+		Removing folder1/
+		EOF
 		git sparse-checkout clean -f >out &&
-		test_line_count = 0 out &&
-		test_path_exists folder1/even/more/dirs/file &&
+		test_cmp expect out &&
+		test_path_is_missing folder1 &&
+		test_path_is_missing deep/deeper2 &&
+
+		cat >expect <<-\EOF &&
+		 D folder1/even/more/dirs/file
+		EOF
+		git status -s -uno >out &&
+		test_cmp expect out &&
 
 		git sparse-checkout reapply &&
-		test_path_is_missing folder1
+		git status -s -uno >out &&
+		test_must_be_empty out
 	)
 '
 

unpack-trees.c

@@ -2138,7 +2138,7 @@ enum update_sparsity_result update_sparsity(struct unpack_trees_options *o,
 	index_state_init(&o->internal.result, o->src_index->repo);
 
 	/* Sanity checks */
-	if (!o->update || o->index_only || o->skip_sparse_checkout)
+	if (o->index_only || o->skip_sparse_checkout)
 		BUG("update_sparsity() is for reflecting sparsity patterns in working directory");
 	if (o->src_index != o->dst_index || o->fn)
 		BUG("update_sparsity() called wrong");