Re: What's cooking in git.git (topics)
From: Jeff King <hidden>
Date: 2016-06-15 22:43:56
Subsystem:
the rest · Maintainer:
Linus Torvalds
Possibly related (same subject, not in this thread)
- 2016-08-11 · Re: What's cooking in git.git (topics) · Junio C Hamano <hidden>
- 2016-08-11 · Re: What's cooking in git.git (topics) · Alexandre Julliard <hidden>
- 2016-08-11 · Re: What's cooking in git.git (topics) · Andy Parkins <hidden>
- 2016-08-11 · What's cooking in git.git (topics) · Junio C Hamano <hidden>
- 2016-08-11 · Re: What's cooking in git.git (topics) · Johannes Schindelin <hidden>
On Wed, Dec 05, 2007 at 02:59:16AM -0800, Junio C Hamano wrote:
* jc/clean-fix (Tue Dec 4 23:55:41 2007 -0800) 1 commit - git-clean: Honor pathspec. This does fix limited test cases I tried, but I didn't check the directory related options at all. Sanity checking appreciated. We need a regression fix before v1.5.4
Hrm, I took a look at this and I'm a bit stumped. I think the logic in builtin-clean is a bit suspect, and I have a patch below that fixes it. However, I still can't get something as simple as: mkdir dir.clean && touch dir.clean/file && git clean -d "*.clean/" to work, and I think the pathspec matching is to blame. If I use "*.clean/", then read_directory assumes that "*.clean" is a directory to be opened, without considering that it might be a wildcard, which is just wrong. If I use "*.clean", then I get the correct directory listing, but match_pathspec fails because read_directory returns "dir.clean/". We could fix this by passing match_pathspec ent->len - 1, but that actually ends up getting ignored! It ends up handing the string to fnmatch, which treats it like a C string. Am I crazy, or do we need to fix the wildcard semantics for directories with both read_directory and with match_pathspec? Below is my partial patch for reference. -Peff ---
diff --git a/builtin-clean.c b/builtin-clean.c
index 61ae851..f4cf39f 100644
--- a/builtin-clean.c
+++ b/builtin-clean.c@@ -117,7 +117,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix) } if (!lstat(ent->name, &st) && (S_ISDIR(st.st_mode))) { - int matched_path = 0; + int matched_path = !pathspec; strbuf_addstr(&directory, ent->name); if (pathspec) { /*
@@ -128,11 +128,11 @@ int cmd_clean(int argc, const char **argv, const char *prefix) baselen, seen)) matched_path = 1; } - if (show_only && (remove_directories || matched_path)) { + if (show_only && (remove_directories && matched_path)) { printf("Would remove %s\n", directory.buf); - } else if (quiet && (remove_directories || matched_path)) { + } else if (quiet && (remove_directories && matched_path)) { remove_dir_recursively(&directory, 0); - } else if (remove_directories || matched_path) { + } else if (remove_directories && matched_path) { printf("Removing %s\n", directory.buf); remove_dir_recursively(&directory, 0); } else if (show_only) {
diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh
index dfd1188..f204a50 100755
--- a/t/t7300-clean.sh
+++ b/t/t7300-clean.sh@@ -192,6 +192,34 @@ test_expect_success 'git-clean -d src/ examples/' ' ' +test_expect_success 'git-clean with directory wildcards' ' + + mkdir -p dir.clean dir.stay && + touch dir.clean/file dir.stay/file && + git clean "*.clean" && + test -f Makefile && + test -f README && + test -f src/part1.c && + test -f src/part2.c && + test -f dir.stay/file && + test -f dir.clean/file + +' + +test_expect_success 'git-clean -d with directory wildcards' ' + + mkdir -p dir.clean dir.stay && + touch dir.clean/file dir.stay/file && + git clean -d "*.clean" && + test -f Makefile && + test -f README && + test -f src/part1.c && + test -f src/part2.c && + test -f dir.stay/file && + test ! -f dir.clean/file + +' + test_expect_success 'git-clean -x' ' mkdir -p build docs &&