rm: skip sparse paths with missing SKIP_WORKTREE

If a path does not match the sparse-checkout cone but is somehow missing
the SKIP_WORKTREE bit, then 'git rm' currently succeeds in removing the
file. One reason a user might be in this situation is a merge conflict
outside of the sparse-checkout cone. Removing such a file might be
problematic for users who are not sure what they are doing.

Add a check to path_in_sparse_checkout() when 'git rm' is checking if a
path should be considered for deletion. Of course, this check is ignored
if the '--sparse' option is specified, allowing users who accept the
risks to continue with the removal.

This also removes a confusing behavior where a user asks for a directory
to be removed, but only the entries that are within the sparse-checkout
definition are removed. Now, 'git rm <dir>' will fail without '--sparse'
and will succeed in removing all contained paths with '--sparse'.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/rm.c b/builtin/rm.c
index 4208f3f9a5fb2695171f70554186cc49651ec0f5..a6da03da2be43bc4c97eb593446a6d0b8acc1fc6 100644 (file)
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -301,7 +301,9 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
        for (i = 0; i < active_nr; i++) {
                const struct cache_entry *ce = active_cache[i];
 
-               if (!include_sparse && ce_skip_worktree(ce))
+               if (!include_sparse &&
+                   (ce_skip_worktree(ce) ||
+                    !path_in_sparse_checkout(ce->name, &the_index)))
                        continue;
                if (!ce_path_match(&the_index, ce, &pathspec, seen))
                        continue;

diff --git a/t/t3602-rm-sparse-checkout.sh b/t/t3602-rm-sparse-checkout.sh
index 493c8f636b80f2dfe4d916c8720accfb05e9dce7..5f92b60a56a9aa4a591772d854c18c42b19b0726 100755 (executable)
--- a/t/t3602-rm-sparse-checkout.sh
+++ b/t/t3602-rm-sparse-checkout.sh
@@ -37,9 +37,13 @@ done
 test_expect_success 'recursive rm does not remove sparse entries' '
        git reset --hard &&
        git sparse-checkout set sub/dir &&
-       git rm -r sub &&
+       test_must_fail git rm -r sub &&
+       git rm --sparse -r sub &&
        git status --porcelain -uno >actual &&
-       echo "D  sub/dir/e" >expected &&
+       cat >expected <<-\EOF &&
+       D  sub/d
+       D  sub/dir/e
+       EOF
        test_cmp expected actual
 '
 
@@ -87,4 +91,15 @@ test_expect_success 'do not warn about sparse entries with --ignore-unmatch' '
        git ls-files --error-unmatch b
 '
 
+test_expect_success 'refuse to rm a non-skip-worktree path outside sparse cone' '
+       git reset --hard &&
+       git sparse-checkout set a &&
+       git update-index --no-skip-worktree b &&
+       test_must_fail git rm b 2>stderr &&
+       test_cmp b_error_and_hint stderr &&
+       git rm --sparse b 2>stderr &&
+       test_must_be_empty stderr &&
+       test_path_is_missing b
+'
+
 test_done