git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[RFC PATCH 7/7] rm: honor sparse checkout patterns

From
Matheus Tavares <matheus.bernardino@usp.br>
Date
Feb 17, 2021, 21:02 UTC
Message-ID
<61a77cd5f45ba02c7dff4b7932abdebb17c1667f.1613593946.git.matheus.bernardino@usp.br>
In-Reply-To
<cover.1613593946.git.matheus.bernardino@usp.br>

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

Suggested-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
---
 Documentation/config/advice.txt  |  5 +--
 Documentation/git-rm.txt         |  4 ++-
 builtin/rm.c                     | 35 ++++++++++++++-------
 t/t3600-rm.sh                    | 54 ++++++++++++++++++++++++++++++++
 t/t7011-skip-worktree-reading.sh |  5 ---
 5 files changed, 84 insertions(+), 19 deletions(-)
diff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt
index d53eafa00b..bdd423ade4 100644
--- a/Documentation/config/advice.txt
+++ b/Documentation/config/advice.txt
@@ -120,6 +120,7 @@ advice.*::
 		Advice shown if a user runs the add command without providing
 		the pathspec parameter.
 	updateSparsePath::
-		Advice shown if the pathspec given to linkgit:git-add[1] only
-		matches index entries outside the current sparse-checkout.
+		Advice shown if the pathspec given to linkgit:git-add[1] or
+		linkgit:git-rm[1] only matches index entries outside the
+		current sparse-checkout.
 --
diff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt
index ab750367fd..26e9b28470 100644
--- a/Documentation/git-rm.txt
+++ b/Documentation/git-rm.txt
@@ -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
diff --git a/builtin/rm.c b/builtin/rm.c
index 4858631e0f..d23a3b2164 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -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;
 }
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index 7547f11a5c..0e5cf945d9 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -907,4 +907,58 @@ test_expect_success 'rm empty string should fail' '
 	test_must_fail git rm -rf ""
 '
 
+test_expect_success 'setup repo for tests with sparse-checkout' '
+	git init sparse &&
+	(
+		cd sparse &&
+		mkdir -p sub/dir &&
+		touch a b c sub/d sub/dir/e &&
+		git add -A &&
+		git commit -m files
+	) &&
+
+	cat >sparse_entry_b_error <<-EOF &&
+	The following pathspecs only matched index entries outside the current
+	sparse checkout:
+	b
+	EOF
+
+	cat >b_error_and_hint sparse_entry_b_error - <<-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
+'
+
+test_expect_success 'rm should respect sparse-checkout' '
+	git -C sparse sparse-checkout set "/a" &&
+	test_must_fail git -C sparse rm b 2>stderr &&
+	test_i18ncmp b_error_and_hint stderr
+'
+
+test_expect_success 'rm obeys advice.updateSparsePath' '
+	git -C sparse reset --hard &&
+	git -C sparse sparse-checkout set "/a" &&
+	test_must_fail git -C sparse -c advice.updateSparsePath=false rm b 2>stderr &&
+	test_i18ncmp sparse_entry_b_error stderr
+
+'
+
+test_expect_success 'recursive rm should respect sparse-checkout' '
+	(
+		cd sparse &&
+		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 'do not advice about sparse entries when they do not match the pathspec' '
+	test_must_fail git -C sparse rm nonexistent 2>stderr &&
+	test_i18ngrep "fatal: pathspec .nonexistent. did not match any files" stderr &&
+	test_i18ngrep ! "The following pathspecs only matched index entries" stderr
+'
+
 test_done
diff --git a/t/t7011-skip-worktree-reading.sh b/t/t7011-skip-worktree-reading.sh
index 37525cae3a..f87749951f 100755
--- a/t/t7011-skip-worktree-reading.sh
+++ b/t/t7011-skip-worktree-reading.sh
@@ -141,11 +141,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 &&

-- 
2.29.2
Previous: Matheus Tavares BernardinoNext: Elijah Newren
Message 33 of 42 in “rm: honor sparse checkout patterns”
  1. rm: honor sparse checkout patternsMatheus Tavares, Nov 12, 2020
  2. Elijah NewrenNov 12, 2020
  3. Derrick StoleeNov 13, 2020
  4. Matheus Tavares BernardinoNov 15, 2020
  5. Johannes SixtNov 15, 2020
  6. Matheus Tavares BernardinoNov 16, 2020
  7. Johannes SchindelinNov 23, 2020
  8. Matheus Tavares BernardinoNov 24, 2020
  9. Jeff HostetlerNov 16, 2020
  10. Elijah NewrenNov 17, 2020
  11. rm: honor sparse checkout patternsMatheus Tavares, Nov 16, 2020
  12. 0/7 add/rm: honor sparse checkout and warn on sparse pathsMatheus Tavares, Feb 17, 2021
  13. 1/7 add --chmod: don't update index when --dry-run is usedMatheus Tavares, Feb 17, 2021
  14. Junio C HamanoFeb 17, 2021
  15. Matheus TavaresFeb 18, 2021
  16. 2/7 add: include magic part of pathspec on --refresh errorMatheus Tavares, Feb 17, 2021
  17. Junio C HamanoFeb 17, 2021
  18. 3/7 t3705: add tests for `git add` in sparse checkoutsMatheus Tavares, Feb 17, 2021
  19. Junio C HamanoFeb 17, 2021
  20. Eric SunshineFeb 17, 2021
  21. Junio C HamanoFeb 17, 2021
  22. Matheus Tavares BernardinoFeb 18, 2021
  23. Matheus Tavares BernardinoFeb 18, 2021
  24. Matheus TavaresFeb 18, 2021
  25. Junio C HamanoFeb 18, 2021
  26. Junio C HamanoFeb 18, 2021
  27. Elijah NewrenFeb 22, 2021
  28. 4/7 add: make --chmod and --renormalize honor sparse checkoutsMatheus Tavares, Feb 17, 2021
  29. 5/7 pathspec: allow to ignore SKIP_WORKTREE entries on index matchingMatheus Tavares, Feb 17, 2021
  30. 6/7 add: warn when pathspec only matches SKIP_WORKTREE entriesMatheus Tavares, Feb 17, 2021
  31. Junio C HamanoFeb 19, 2021
  32. Matheus Tavares BernardinoFeb 19, 2021
  33. 7/7 rm: honor sparse checkout patternsMatheus Tavares, Feb 17, 2021
  34. Elijah NewrenFeb 22, 2021
  35. Junio C HamanoNov 16, 2020
  36. Elijah NewrenNov 17, 2020
  37. Elijah NewrenNov 20, 2020
  38. sparse-checkout questions and proposals [Was: Re: [PATCH] rm: honor sparse checkout patterns]Elijah Newren, Dec 31, 2020
  39. Derrick StoleeJan 4, 2021
  40. Elijah NewrenJan 6, 2021
  41. Derrick StoleeJan 7, 2021
  42. Elijah NewrenJan 7, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.