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

[PATCH v2] rm: honor sparse checkout patterns

From
Matheus Tavares <matheus.bernardino@usp.br>
Date
Nov 16, 2020, 13:58 UTC
Message-ID
<c17158e3b105d7c0679515446c5fd7b8d5fc8435.1605535068.git.matheus.bernardino@usp.br>
In-Reply-To
<9f2135f90ffea7f4ccb226f506bf554deab324cc.1605205427.git.matheus.bernardino@usp.br>

Make git-rm honor the 'sparse.restrictCmds' setting, by restricting its operation to the paths that match both the command line pathspecs and the repository's sparsity patterns. This better matches the expectations of users with sparse-checkout definitions, while still allowing them to optionally enable the old behavior with 'sparse.restrictCmds=false' or the global '--no-restrict-to-sparse-paths' option.

Suggested-by: Derrick Stolee <stolee@gmail.com>
Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
---
Based on mt/grep-sparse-checkout.
Changes since v1:
- Reworded git-rm docs to avoid confusion for those who never used
  sparse-checkout.
- Included an advice about disabling sparse.restrictCmds when the
  given pathspec doesn't match any files and sparse-checkout is enabled.
- Added test for `git rm -r` removing a dir that is only partially
  included in the sparse-checkout.
- Adjusted test in t7011 to use `-c sparse.restrictCmds=false`, instead
  of removing it.
 Documentation/config/sparse.txt  |  3 +-
 Documentation/git-rm.txt         | 10 +++++++
 builtin/rm.c                     | 24 ++++++++++------
 t/t3600-rm.sh                    | 47 ++++++++++++++++++++++++++++++++
 t/t7011-skip-worktree-reading.sh |  4 +--
 5 files changed, 77 insertions(+), 11 deletions(-)
diff --git a/Documentation/config/sparse.txt b/Documentation/config/sparse.txt
index 494761526e..79d7d173e9 100644
--- a/Documentation/config/sparse.txt
+++ b/Documentation/config/sparse.txt
@@ -12,7 +12,8 @@ When this option is true (default), some git commands may limit their behavior
 to the paths specified by the sparsity patterns, or to the intersection of
 those paths and any (like `*.c`) that the user might also specify on the
 command line. When false, the affected commands will work on full trees,
-ignoring the sparsity patterns. For now, only git-grep honors this setting.
+ignoring the sparsity patterns. For now, only git-grep and git-rm honor this
+setting.
 +
 Note: commands which export, integrity check, or create history will always
 operate on full trees (e.g. fast-export, format-patch, fsck, commit, etc.),
diff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt
index ab750367fd..33bec8c249 100644
--- a/Documentation/git-rm.txt
+++ b/Documentation/git-rm.txt
@@ -25,6 +25,16 @@ 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.
 
+CONFIGURATION
+-------------
+
+sparse.restrictCmds::
+	When sparse-checkouts are in use, by default git-rm will only
+	match and remove paths within the sparse-checkout patterns.
+	This behavior can be changed with the `sparse.restrictCmds`
+	setting or the global `--no-restrict-to-sparse-paths` option.
+	For more details, see the full `sparse.restrictCmds` definition
+	in linkgit:git-config[1].
 
 OPTIONS
 -------
diff --git a/builtin/rm.c b/builtin/rm.c
index 4858631e0f..90f6bb4cae 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -14,6 +14,7 @@
 #include "string-list.h"
 #include "submodule.h"
 #include "pathspec.h"
+#include "sparse-checkout.h"
 
 static const char * const builtin_rm_usage[] = {
 	N_("git rm [<options>] [--] <file>..."),
@@ -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, sparse_paths_only;
 	struct pathspec pathspec;
 	char *seen;
 
@@ -293,8 +294,13 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 
 	seen = xcalloc(pathspec.nr, 1);
 
+	sparse_paths_only = core_apply_sparse_checkout &&
+			    restrict_to_sparse_paths(the_repository);
+
 	for (i = 0; i < active_nr; i++) {
 		const struct cache_entry *ce = active_cache[i];
+		if (sparse_paths_only && ce_skip_worktree(ce))
+			continue;
 		if (!ce_path_match(&the_index, ce, &pathspec, seen))
 			continue;
 		ALLOC_GROW(list.entry, list.nr + 1, list.alloc);
@@ -310,14 +316,16 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 		int seen_any = 0;
 		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) {
+				const char *sparse_config_advice =
+					_("; disable sparse.restrictCmds if you intend to edit"
+					  " outside the current sparse-checkout definition");
+
+				die(_("pathspec '%s' did not match any files%s"),
+				    original,
+				    sparse_paths_only ? sparse_config_advice : "");
 			}
 			if (!recursive && seen[i] == MATCHED_RECURSIVELY)
 				die(_("not removing '%s' recursively without -r"),
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index efec8d13b6..25cd7187fa 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -892,4 +892,51 @@ test_expect_success 'rm empty string should fail' '
 	test_must_fail git rm -rf ""
 '
 
+test_expect_success 'rm should respect --[no]-restrict-to-sparse-paths' '
+	git init sparse-repo &&
+	(
+		cd sparse-repo &&
+		touch a b c &&
+		git add -A &&
+		git commit -m files &&
+		git sparse-checkout set "/a" &&
+
+		# By default, it should not rm paths outside the sparse-checkout
+		test_must_fail git rm b 2>stderr &&
+		test_i18ngrep "fatal: pathspec .b. did not match any files" stderr &&
+		test_i18ngrep "disable sparse.restrictCmds if you intend to edit outside" stderr &&
+
+		# But it should rm them with --no-restrict-to-sparse-paths
+		git --no-restrict-to-sparse-paths rm b &&
+
+		# And also with sparse.restrictCmds=false
+		git reset &&
+		git -c sparse.restrictCmds=false rm b
+	)
+'
+
+test_expect_success 'recursive rm should respect --[no]-restrict-to-sparse-paths' '
+	git init sparse-repo-2 &&
+	(
+		cd sparse-repo-2 &&
+		mkdir -p sub/dir &&
+		touch sub/f1 sub/dir/f2 &&
+		git add -A &&
+		git commit -m files &&
+		git sparse-checkout set "sub/dir" &&
+
+		git rm -r sub &&
+		echo "D  sub/dir/f2" >expected &&
+		git status --porcelain -uno >actual &&
+		test_cmp expected actual &&
+
+		git reset &&
+		git --no-restrict-to-sparse-paths rm -r sub &&
+		echo "D  sub/dir/f2" >expected-no-restrict &&
+		echo "D  sub/f1"     >>expected-no-restrict &&
+		git status --porcelain -uno >actual-no-restrict &&
+		test_cmp expected-no-restrict actual-no-restrict
+	)
+'
+
 test_done
diff --git a/t/t7011-skip-worktree-reading.sh b/t/t7011-skip-worktree-reading.sh
index 26852586ac..08ede90e14 100755
--- a/t/t7011-skip-worktree-reading.sh
+++ b/t/t7011-skip-worktree-reading.sh
@@ -132,9 +132,9 @@ 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' '
+test_expect_success 'git-rm succeeds on skip-worktree absent entries when sparse.restrictCmds=false' '
 	setup_absent &&
-	git rm 1
+	git -c sparse.restrictCmds=false rm 1
 '
 
 test_expect_success 'commit on skip-worktree absent entries' '

-- 
2.28.0
Previous: Elijah NewrenNext: Matheus Tavares
Message 11 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.