rm: honor sparse checkout patterns

`git add` refrains from adding or updating index entries that are
outside the current sparse checkout, but `git rm` doesn't follow the
same restriction. This is somewhat counter-intuitive and inconsistent.
So make `rm` honor the sparsity rules and advise on how to remove
SKIP_WORKTREE entries just like `add` does. Also add some tests for the
new behavior.

Suggested-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
Matheus Tavares 2021-04-08 17:41:28 -03:00 committed by Junio C Hamano
parent a20f70478f
commit d5f4b8260f
5 changed files with 108 additions and 19 deletions

View File

@ -120,6 +120,7 @@ advice.*::
Advice shown if a user runs the add command without providing
the pathspec parameter.
updateSparsePath::
Advice shown when linkgit:git-add[1] is asked to update index
entries outside the current sparse checkout.
Advice shown when either linkgit:git-add[1] or linkgit:git-rm[1]
is asked to update index entries outside the current sparse
checkout.
--

View File

@ -23,7 +23,9 @@ branch, and no updates to their contents can be staged in the index,
though that default behavior can be overridden with the `-f` option.
When `--cached` is given, the staged content has to
match either the tip of the branch or the file on disk,
allowing the file to be removed from just the index.
allowing the file to be removed from just the index. When
sparse-checkouts are in use (see linkgit:git-sparse-checkout[1]),
`git rm` will only remove paths within the sparse-checkout patterns.
OPTIONS

View File

@ -5,6 +5,7 @@
*/
#define USE_THE_INDEX_COMPATIBILITY_MACROS
#include "builtin.h"
#include "advice.h"
#include "config.h"
#include "lockfile.h"
#include "dir.h"
@ -254,7 +255,7 @@ static struct option builtin_rm_options[] = {
int cmd_rm(int argc, const char **argv, const char *prefix)
{
struct lock_file lock_file = LOCK_INIT;
int i;
int i, ret = 0;
struct pathspec pathspec;
char *seen;
@ -295,6 +296,8 @@ 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 (ce_skip_worktree(ce))
continue;
if (!ce_path_match(&the_index, ce, &pathspec, seen))
continue;
ALLOC_GROW(list.entry, list.nr + 1, list.alloc);
@ -308,24 +311,34 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
if (pathspec.nr) {
const char *original;
int seen_any = 0;
char *skip_worktree_seen = NULL;
struct string_list only_match_skip_worktree = STRING_LIST_INIT_NODUP;
for (i = 0; i < pathspec.nr; i++) {
original = pathspec.items[i].original;
if (!seen[i]) {
if (!ignore_unmatch) {
die(_("pathspec '%s' did not match any files"),
original);
}
}
else {
if (seen[i])
seen_any = 1;
}
else if (ignore_unmatch)
continue;
else if (matches_skip_worktree(&pathspec, i, &skip_worktree_seen))
string_list_append(&only_match_skip_worktree, original);
else
die(_("pathspec '%s' did not match any files"), original);
if (!recursive && seen[i] == MATCHED_RECURSIVELY)
die(_("not removing '%s' recursively without -r"),
*original ? original : ".");
}
if (only_match_skip_worktree.nr) {
advise_on_updating_sparse_paths(&only_match_skip_worktree);
ret = 1;
}
free(skip_worktree_seen);
string_list_clear(&only_match_skip_worktree, 0);
if (!seen_any)
exit(0);
exit(ret);
}
if (!index_only)
@ -405,5 +418,5 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
COMMIT_LOCK | SKIP_IF_UNCHANGED))
die(_("Unable to write new index file"));
return 0;
return ret;
}

78
t/t3602-rm-sparse-checkout.sh Executable file
View File

@ -0,0 +1,78 @@
#!/bin/sh
test_description='git rm in sparse checked out working trees'
. ./test-lib.sh
test_expect_success 'setup' "
mkdir -p sub/dir &&
touch a b c sub/d sub/dir/e &&
git add -A &&
git commit -m files &&
cat >sparse_error_header <<-EOF &&
The following pathspecs didn't match any eligible path, but they do match index
entries outside the current sparse checkout:
EOF
cat >sparse_hint <<-EOF &&
hint: Disable or modify the sparsity rules if you intend to update such entries.
hint: Disable this message with \"git config advice.updateSparsePath false\"
EOF
echo b | cat sparse_error_header - >sparse_entry_b_error &&
cat sparse_entry_b_error sparse_hint >b_error_and_hint
"
for opt in "" -f --dry-run
do
test_expect_success "rm${opt:+ $opt} does not remove sparse entries" '
git sparse-checkout set a &&
test_must_fail git rm $opt b 2>stderr &&
test_cmp b_error_and_hint stderr &&
git ls-files --error-unmatch b
'
done
test_expect_success 'recursive rm does not remove sparse entries' '
git reset --hard &&
git sparse-checkout set sub/dir &&
git rm -r sub &&
git status --porcelain -uno >actual &&
echo "D sub/dir/e" >expected &&
test_cmp expected actual
'
test_expect_success 'rm obeys advice.updateSparsePath' '
git reset --hard &&
git sparse-checkout set a &&
test_must_fail git -c advice.updateSparsePath=false rm b 2>stderr &&
test_cmp sparse_entry_b_error stderr
'
test_expect_success 'do not advice about sparse entries when they do not match the pathspec' '
git reset --hard &&
git sparse-checkout set a &&
test_must_fail git rm nonexistent 2>stderr &&
grep "fatal: pathspec .nonexistent. did not match any files" stderr &&
! grep -F -f sparse_error_header stderr
'
test_expect_success 'do not warn about sparse entries when pathspec matches dense entries' '
git reset --hard &&
git sparse-checkout set a &&
git rm "[ba]" 2>stderr &&
test_must_be_empty stderr &&
git ls-files --error-unmatch b &&
test_must_fail git ls-files --error-unmatch a
'
test_expect_success 'do not warn about sparse entries with --ignore-unmatch' '
git reset --hard &&
git sparse-checkout set a &&
git rm --ignore-unmatch b 2>stderr &&
test_must_be_empty stderr &&
git ls-files --error-unmatch b
'
test_done

View File

@ -132,11 +132,6 @@ test_expect_success 'diff-files does not examine skip-worktree dirty entries' '
test -z "$(git diff-files -- one)"
'
test_expect_success 'git-rm succeeds on skip-worktree absent entries' '
setup_absent &&
git rm 1
'
test_expect_success 'commit on skip-worktree absent entries' '
git reset &&
setup_absent &&