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

[RFC PATCH 4/7] dir: Directories should be checked for matching pathspecs too

From
Elijah Newren <newren@gmail.com>
Date
Apr 5, 2018, 17:34 UTC
Message-ID
<20180405173446.32372-5-newren@gmail.com>
In-Reply-To
<20180405173446.32372-1-newren@gmail.com>

Even if a directory doesn't match a pathspec, it is possible, depending on the precise pathspecs, that some file underneath it might. So we special case and recurse into the directory for such situations. However, we previously always added any untracked directory that we recursed into to the list of untracked paths, regardless of whether the directory itself matched the pathspec.

For the case of git-clean and a set of pathspecs of "dir/file" and "more",
this caused a problem because we'd end up with dir entries for both of
  "dir"
  "dir/file"
Then correct_untracked_entries() would try to helpfully prune duplicates
for us by removing "dir/file" since it's under "dir", leaving us with
  "dir"
Since the original pathspec only had "dir/file", the only entry left
doesn't match and leaves nothing to be removed.  (Note that if only one
pathspec was specified, e.g. only "dir/file", then the common_prefix_len
optimizations in fill_directory would cause us to bypass this problem,
making it appear in simple tests that we could correctly remove manually
specified pathspecs.)

Fix this by actually checking whether the directory we are about to add to the list of dir entries actually matches the pathspec; only do this matching check after we have already returned from recursing into the directory.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 dir.c            | 5 +++++
 t/t7300-clean.sh | 4 ++--
 2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/dir.c b/dir.c
index c915a69385..e783431948 100644
--- a/dir.c
+++ b/dir.c
@@ -1973,6 +1973,11 @@ static enum path_treatment read_directory_recursive(struct dir_struct *dir,
 							 check_only, stop_at_first_file, pathspec);
 			if (subdir_state > dir_state)
 				dir_state = subdir_state;
+
+			if (!match_pathspec(pathspec, path.buf, path.len,
+					    0 /* prefix */, NULL,
+					    0 /* do NOT special case dirs */))
+				state = path_none;
 		}
 
 		if (check_only) {
diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh
index 3d260e21ea..31f2d0d8ba 100755
--- a/t/t7300-clean.sh
+++ b/t/t7300-clean.sh
@@ -677,7 +677,7 @@ test_expect_failure 'git clean handles being told what to clean' '
 	test_path_is_missing d2/ut
 '
 
-test_expect_failure 'git clean handles being told what to clean, with -d' '
+test_expect_success 'git clean handles being told what to clean, with -d' '
 	mkdir -p d1 d2 &&
 	touch d1/ut d2/ut &&
 	git clean -ffd */ut &&
@@ -693,7 +693,7 @@ test_expect_failure 'git clean handles being told a glob to clean' '
 	test_path_is_missing d2/ut
 '
 
-test_expect_failure 'git clean handles being told a glob to clean with -d' '
+test_expect_success 'git clean handles being told a glob to clean with -d' '
 	mkdir -p d1 d2 &&
 	touch d1/ut d2/ut &&
 	git clean -ffd "*ut" &&
-- 
2.17.0.7.g0b50f94d69
Previous: Elijah NewrenNext: Jeff King
Message 4 of 17 in “Fix `git clean` with pathspecs”
  1. 0/7 Fix `git clean` with pathspecsElijah Newren, Apr 5, 2018
  2. 7/7 If we do not want globs to recurse into subdirs without -d...Elijah Newren, Apr 5, 2018
  3. 6/7 dir: If our pathspec might match files under a dir, recurse into itElijah Newren, Apr 5, 2018
  4. 4/7 dir: Directories should be checked for matching pathspecs tooElijah Newren, Apr 5, 2018
  5. Jeff KingApr 5, 2018
  6. Elijah NewrenApr 5, 2018
  7. Jeff KingApr 5, 2018
  8. Junio C HamanoApr 9, 2018
  9. 3/7 t7300: Add some testcases showing failure to clean specified pathspecsElijah Newren, Apr 5, 2018
  10. 5/7 dir: Make the DO_MATCH_SUBMODULE code reusable for a non-submodule caseElijah Newren, Apr 5, 2018
  11. 1/7 dir.c: Fix typo in commentElijah Newren, Apr 5, 2018
  12. 2/7 dir.c: fix off-by-one error in match_pathspec_itemElijah Newren, Apr 5, 2018
  13. Jeff KingApr 5, 2018
  14. Elijah NewrenApr 5, 2018
  15. Jeff KingApr 5, 2018
  16. Elijah NewrenApr 5, 2018
  17. Jeff KingApr 6, 2018

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.