Re: [PATCH] clean: new option --exclude-from

6 messages, 3 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH] clean: new option --exclude-from

From: Eric Sunshine <hidden>
Date: 2016-06-15 23:07:21

On Tue, Dec 1, 2015 at 4:36 PM, James Rouzier [off-list ref] wrote:

Eric thank you for the feedback.

[re-adding git@vger.kernel.org to recipient list since this response
was likely intended to be public]

On Sun, Nov 29, 2015 at 9:24 PM, Eric Sunshine [off-list ref]
wrote:

quoted

On Thu, Nov 26, 2015 at 9:44 AM, James [off-list ref] wrote:

quoted

From: James Rouzier <redacted>

Specify a file to read for exclude patterns.
---

@@ -61,6 +61,9 @@ OPTIONS
        $GIT_DIR/info/exclude, also consider these patterns to be in the
        set of the ignore rules in effect.

+--exclude-from=<file>::
+       Read exclude patterns from <file>; 1 per line.

s/;/,/ maybe?

I copied this from Documentation/git-ls-files.txt to try and keep the
documentation style consistent.
However if it is believed to be better I will change it here and also in a
separate patch for Documentation/git-ls-files.txt

I don't feel strongly about it. Existing precedence may be a good
argument in its favor.

quoted

Also, why move the memset() all the way up here as opposed, say, to
moving it just before the parse_options() invocation? Is it just to
make it easier for the next person who comes along wanting to
manipulate 'dir' early on (before git_config(), for instance)?

Yes I want to make sure that the 'dir' is initialized before any usage.

quoted

+               git clean -f --exclude-from=.git/clean-exclude &&
+               test -e 1 &&
+               test -e 2 &&
+               ! (test -e 3) &&

I see that you copied this from the "git clean -e" test, but it's not
obvious why parentheses are needed or wanted, and none of the other
tests use parentheses when negating the return of 'test', thus they
probably ought to be dropped.

Ok will do

quoted

+               test -e known

Modern scripts would normally use test_path_is_file() and
test_path_is_missing() instead of 'test -e', however, you are again
matching existing style in this script, so 'test -e' may be
reasonable.

Since it is the standard I could just take the time to upgrade 'test -e' in
this test file to use newer standard.

This test script is probably relatively quiescent right now, so such
cleanup may be reasonable. Since it is conceptually distinct from the
purpose of the current patch, you would want to do the cleanup as a
preparatory patch, thus making this a 2-patch series.

quoted

+test_expect_success 'git clean -e --exclude-from' '
+       rm -fr repo &&
+       mkdir repo &&
+       (
+               cd repo &&
+               git init &&
+               touch known 1 2 3 &&
+               git add known &&
+               echo 1 >> .git/clean-exclude &&
+               git clean -f -e 2 --exclude-from=.git/clean-exclude &&
+               test -e 1 &&
+               test -e 2 &&
+               ! (test -e 3) &&
+               test -e known
+       )
+'

Should a test be added which uses --exclude-from multiple times in the
same git-clean invocation?

That does make sense will do.

quoted

Would it make sense add a test checking the behavior when the file
named by --exclude-from doesn't exist or is otherwise unusable as an
exclusion file?

At the moment the add_excludes_from_file function will exit the program if
there is a problem loading the exclude file.
I could add a test for that behavior. In case in the future this behavior is
changed.

[PATCH v2 2/2] clean: new option --exclude-from

From: James <hidden>
Date: 2016-06-15 23:07:23

From: James Rouzier <redacted>

Specify a file to read for exclude patterns.
---
 Documentation/git-clean.txt |  5 +++-
 builtin/clean.c             | 15 ++++++++++--
 t/t7300-clean.sh            | 60 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 77 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt
index 641681f..ef5dc41 100644
--- a/Documentation/git-clean.txt
+++ b/Documentation/git-clean.txt

@@ -8,7 +8,7 @@ git-clean - Remove untracked files from the working tree
 SYNOPSIS
 --------
 [verse]
-'git clean' [-d] [-f] [-i] [-n] [-q] [-e <pattern>] [-x | -X] [--] <path>...
+'git clean' [-d] [-f] [-i] [-n] [-q] [-e <pattern>] [--exclude-from=<file>] [-x | -X] [--] <path>...
 
 DESCRIPTION
 -----------

@@ -61,6 +61,9 @@ OPTIONS
 	$GIT_DIR/info/exclude, also consider these patterns to be in the
 	set of the ignore rules in effect.
 
+--exclude-from=<file>::
+	Read exclude patterns from <file>; 1 per line.
+
 -x::
 	Don't use the standard ignore rules read from .gitignore (per
 	directory) and $GIT_DIR/info/exclude, but do still use the ignore

diff --git a/builtin/clean.c b/builtin/clean.c
index d7acb94..8e652f8 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c

@@ -22,7 +22,7 @@ static struct string_list del_list = STRING_LIST_INIT_DUP;
 static unsigned int colopts;
 
 static const char *const builtin_clean_usage[] = {
-	N_("git clean [-d] [-f] [-i] [-n] [-q] [-e <pattern>] [-x | -X] [--] <paths>..."),
+	N_("git clean [-d] [-f] [-i] [-n] [-q] [-e <pattern>] [--exclude-from=<file>] [-x | -X] [--] <paths>..."),
 	NULL
 };

@@ -875,6 +875,14 @@ static void interactive_main_loop(void)
 	}
 }
 
+static int exclude_from_cb(const struct option *opt,
+				     const char *arg, int unset)
+{
+	struct dir_struct *dir = opt->value;
+	add_excludes_from_file(dir, arg);
+	return 0;
+}
+
 int cmd_clean(int argc, const char **argv, const char *prefix)
 {
 	int i, res;

@@ -898,12 +906,16 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 				N_("remove whole directories")),
 		{ OPTION_CALLBACK, 'e', "exclude", &exclude_list, N_("pattern"),
 		  N_("add <pattern> to ignore rules"), PARSE_OPT_NONEG, exclude_cb },
+		{ OPTION_CALLBACK, 0, "exclude-from", &dir, N_("file"),
+			N_("read exclude patterns from <file>"),
+			0, exclude_from_cb },
 		OPT_BOOL('x', NULL, &ignored, N_("remove ignored files, too")),
 		OPT_BOOL('X', NULL, &ignored_only,
 				N_("remove only ignored files")),
 		OPT_END()
 	};
 
+	memset(&dir, 0, sizeof(dir));
 	git_config(git_clean_config, NULL);
 	if (force < 0)
 		force = 0;

@@ -913,7 +925,6 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, prefix, options, builtin_clean_usage,
 			     0);
 
-	memset(&dir, 0, sizeof(dir));
 	if (ignored_only)
 		dir.flags |= DIR_SHOW_IGNORED;

diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh
index d555bb6..c6bfdda 100755
--- a/t/t7300-clean.sh
+++ b/t/t7300-clean.sh

@@ -628,6 +628,66 @@ test_expect_success 'git clean -e' '
 	test_path_is_file known
 '
 
+test_expect_success 'git clean --exclude-from' '
+	rm -fr repo &&
+	mkdir repo &&
+	cd repo &&
+	git init &&
+	touch known 1 2 3 &&
+	git add known &&
+	cat >.git/clean-exclude <<-\EOF &&
+	1
+	2
+	EOF
+	git clean -f --exclude-from=.git/clean-exclude &&
+	test_path_is_file 1 &&
+	test_path_is_file 2 &&
+	test_path_is_missing 3 &&
+	test_path_is_file known
+'
+
+test_expect_success 'git clean -e --exclude-from' '
+	rm -fr repo &&
+	mkdir repo &&
+	cd repo &&
+	git init &&
+	touch known 1 2 3 &&
+	git add known &&
+	echo 1 >> .git/clean-exclude &&
+	git clean -f -e 2 --exclude-from=.git/clean-exclude &&
+	test_path_is_file 1 &&
+	test_path_is_file 2 &&
+	test_path_is_missing 3 &&
+	test_path_is_file known
+'
+
+test_expect_success 'git clean --exclude-from --exclude-from' '
+	rm -fr repo &&
+	mkdir repo &&
+	git init &&
+	touch known 1 2 3 &&
+	git add known &&
+	cat >.git/clean-exclude1 <<-\EOF &&
+	1
+	EOF
+	cat >.git/clean-exclude2 <<-\EOF &&
+	2
+	EOF
+	git clean -f --exclude-from=.git/clean-exclude1 --exclude-from=.git/clean-exclude2 &&
+	test_path_is_file 1 &&
+	test_path_is_file 2 &&
+	test_path_is_missing 3 &&
+	test_path_is_file known
+'
+
+test_expect_success 'git clean --exclude-from=BADFILE' '
+	rm -fr repo &&
+	mkdir repo &&
+	cd repo &&
+	git init &&
+	test_expect_code 128 git clean -f --exclude-from=.git/clean-exclude-not-there
+'
+
 test_expect_success SANITY 'git clean -d with an unreadable empty directory' '
 	mkdir foo &&
 	chmod a= foo &&

-- 
2.3.6

[PATCH v2 1/2] modernize t7300

From: James <hidden>
Date: 2016-06-15 23:07:23

From: James Rouzier <redacted>

---
 t/t7300-clean.sh | 382 +++++++++++++++++++++++++++----------------------------
 1 file changed, 190 insertions(+), 192 deletions(-)

diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh
index 86ceb38..d555bb6 100755
--- a/t/t7300-clean.sh
+++ b/t/t7300-clean.sh

@@ -28,15 +28,15 @@ test_expect_success 'git clean with skip-worktree .gitignore' '
 	mkdir -p build docs &&
 	touch a.out src/part3.c docs/manual.txt obj.o build/lib.so &&
 	git clean &&
-	test -f Makefile &&
-	test -f README &&
-	test -f src/part1.c &&
-	test -f src/part2.c &&
-	test ! -f a.out &&
-	test ! -f src/part3.c &&
-	test -f docs/manual.txt &&
-	test -f obj.o &&
-	test -f build/lib.so &&
+	test_path_is_file Makefile &&
+	test_path_is_file README &&
+	test_path_is_file src/part1.c &&
+	test_path_is_file src/part2.c &&
+	test_path_is_missing a.out &&
+	test_path_is_missing src/part3.c &&
+	test_path_is_file docs/manual.txt &&
+	test_path_is_file obj.o &&
+	test_path_is_file build/lib.so &&
 	git update-index --no-skip-worktree .gitignore &&
 	git checkout .gitignore
 '

@@ -46,15 +46,15 @@ test_expect_success 'git clean' '
 	mkdir -p build docs &&
 	touch a.out src/part3.c docs/manual.txt obj.o build/lib.so &&
 	git clean &&
-	test -f Makefile &&
-	test -f README &&
-	test -f src/part1.c &&
-	test -f src/part2.c &&
-	test ! -f a.out &&
-	test ! -f src/part3.c &&
-	test -f docs/manual.txt &&
-	test -f obj.o &&
-	test -f build/lib.so
+	test_path_is_file Makefile &&
+	test_path_is_file README &&
+	test_path_is_file src/part1.c &&
+	test_path_is_file src/part2.c &&
+	test_path_is_missing a.out &&
+	test_path_is_missing src/part3.c &&
+	test_path_is_file docs/manual.txt &&
+	test_path_is_file obj.o &&
+	test_path_is_file build/lib.so
 
 '

@@ -63,15 +63,15 @@ test_expect_success 'git clean src/' '
 	mkdir -p build docs &&
 	touch a.out src/part3.c docs/manual.txt obj.o build/lib.so &&
 	git clean src/ &&
-	test -f Makefile &&
-	test -f README &&
-	test -f src/part1.c &&
-	test -f src/part2.c &&
-	test -f a.out &&
-	test ! -f src/part3.c &&
-	test -f docs/manual.txt &&
-	test -f obj.o &&
-	test -f build/lib.so
+	test_path_is_file Makefile &&
+	test_path_is_file README &&
+	test_path_is_file src/part1.c &&
+	test_path_is_file src/part2.c &&
+	test_path_is_file a.out &&
+	test_path_is_missing src/part3.c &&
+	test_path_is_file docs/manual.txt &&
+	test_path_is_file obj.o &&
+	test_path_is_file build/lib.so
 
 '

@@ -80,15 +80,15 @@ test_expect_success 'git clean src/ src/' '
 	mkdir -p build docs &&
 	touch a.out src/part3.c docs/manual.txt obj.o build/lib.so &&
 	git clean src/ src/ &&
-	test -f Makefile &&
-	test -f README &&
-	test -f src/part1.c &&
-	test -f src/part2.c &&
-	test -f a.out &&
-	test ! -f src/part3.c &&
-	test -f docs/manual.txt &&
-	test -f obj.o &&
-	test -f build/lib.so
+	test_path_is_file Makefile &&
+	test_path_is_file README &&
+	test_path_is_file src/part1.c &&
+	test_path_is_file src/part2.c &&
+	test_path_is_file a.out &&
+	test_path_is_missing src/part3.c &&
+	test_path_is_file docs/manual.txt &&
+	test_path_is_file obj.o &&
+	test_path_is_file build/lib.so
 
 '

@@ -97,16 +97,16 @@ test_expect_success 'git clean with prefix' '
 	mkdir -p build docs src/test &&
 	touch a.out src/part3.c docs/manual.txt obj.o build/lib.so src/test/1.c &&
 	(cd src/ && git clean) &&
-	test -f Makefile &&
-	test -f README &&
-	test -f src/part1.c &&
-	test -f src/part2.c &&
-	test -f a.out &&
-	test ! -f src/part3.c &&
-	test -f src/test/1.c &&
-	test -f docs/manual.txt &&
-	test -f obj.o &&
-	test -f build/lib.so
+	test_path_is_file Makefile &&
+	test_path_is_file README &&
+	test_path_is_file src/part1.c &&
+	test_path_is_file src/part2.c &&
+	test_path_is_file a.out &&
+	test_path_is_missing src/part3.c &&
+	test_path_is_file src/test/1.c &&
+	test_path_is_file docs/manual.txt &&
+	test_path_is_file obj.o &&
+	test_path_is_file build/lib.so
 
 '

@@ -160,16 +160,16 @@ test_expect_success 'git clean -d with prefix and path' '
 	mkdir -p build docs src/feature &&
 	touch a.out src/part3.c src/feature/file.c docs/manual.txt obj.o build/lib.so &&
 	(cd src/ && git clean -d feature/) &&
-	test -f Makefile &&
-	test -f README &&
-	test -f src/part1.c &&
-	test -f src/part2.c &&
-	test -f a.out &&
-	test -f src/part3.c &&
-	test ! -f src/feature/file.c &&
-	test -f docs/manual.txt &&
-	test -f obj.o &&
-	test -f build/lib.so
+	test_path_is_file Makefile &&
+	test_path_is_file README &&
+	test_path_is_file src/part1.c &&
+	test_path_is_file src/part2.c &&
+	test_path_is_file a.out &&
+	test_path_is_file src/part3.c &&
+	test_path_is_missing src/feature/file.c &&
+	test_path_is_file docs/manual.txt &&
+	test_path_is_file obj.o &&
+	test_path_is_file build/lib.so
 
 '

@@ -179,16 +179,16 @@ test_expect_success SYMLINKS 'git clean symbolic link' '
 	touch a.out src/part3.c docs/manual.txt obj.o build/lib.so &&
 	ln -s docs/manual.txt src/part4.c &&
 	git clean &&
-	test -f Makefile &&
-	test -f README &&
-	test -f src/part1.c &&
-	test -f src/part2.c &&
-	test ! -f a.out &&
-	test ! -f src/part3.c &&
-	test ! -f src/part4.c &&
-	test -f docs/manual.txt &&
-	test -f obj.o &&
-	test -f build/lib.so
+	test_path_is_file Makefile &&
+	test_path_is_file README &&
+	test_path_is_file src/part1.c &&
+	test_path_is_file src/part2.c &&
+	test_path_is_missing a.out &&
+	test_path_is_missing src/part3.c &&
+	test_path_is_missing src/part4.c &&
+	test_path_is_file docs/manual.txt &&
+	test_path_is_file obj.o &&
+	test_path_is_file build/lib.so
 
 '

@@ -196,13 +196,13 @@ test_expect_success 'git clean with wildcard' '
 
 	touch a.clean b.clean other.c &&
 	git clean "*.clean" &&
-	test -f Makefile &&
-	test -f README &&
-	test -f src/part1.c &&
-	test -f src/part2.c &&
-	test ! -f a.clean &&
-	test ! -f b.clean &&
-	test -f other.c
+	test_path_is_file Makefile &&
+	test_path_is_file README &&
+	test_path_is_file src/part1.c &&
+	test_path_is_file src/part2.c &&
+	test_path_is_missing a.clean &&
+	test_path_is_missing b.clean &&
+	test_path_is_file other.c
 
 '

@@ -211,15 +211,15 @@ test_expect_success 'git clean -n' '
 	mkdir -p build docs &&
 	touch a.out src/part3.c docs/manual.txt obj.o build/lib.so &&
 	git clean -n &&
-	test -f Makefile &&
-	test -f README &&
-	test -f src/part1.c &&
-	test -f src/part2.c &&
-	test -f a.out &&
-	test -f src/part3.c &&
-	test -f docs/manual.txt &&
-	test -f obj.o &&
-	test -f build/lib.so
+	test_path_is_file Makefile &&
+	test_path_is_file README &&
+	test_path_is_file src/part1.c &&
+	test_path_is_file src/part2.c &&
+	test_path_is_file a.out &&
+	test_path_is_file src/part3.c &&
+	test_path_is_file docs/manual.txt &&
+	test_path_is_file obj.o &&
+	test_path_is_file build/lib.so
 
 '

@@ -228,15 +228,15 @@ test_expect_success 'git clean -d' '
 	mkdir -p build docs &&
 	touch a.out src/part3.c docs/manual.txt obj.o build/lib.so &&
 	git clean -d &&
-	test -f Makefile &&
-	test -f README &&
-	test -f src/part1.c &&
-	test -f src/part2.c &&
-	test ! -f a.out &&
-	test ! -f src/part3.c &&
-	test ! -d docs &&
-	test -f obj.o &&
-	test -f build/lib.so
+	test_path_is_file Makefile &&
+	test_path_is_file README &&
+	test_path_is_file src/part1.c &&
+	test_path_is_file src/part2.c &&
+	test_path_is_missing a.out &&
+	test_path_is_missing src/part3.c &&
+	test_path_is_missing docs &&
+	test_path_is_file obj.o &&
+	test_path_is_file build/lib.so
 
 '

@@ -245,16 +245,16 @@ test_expect_success 'git clean -d src/ examples/' '
 	mkdir -p build docs examples &&
 	touch a.out src/part3.c docs/manual.txt obj.o build/lib.so examples/1.c &&
 	git clean -d src/ examples/ &&
-	test -f Makefile &&
-	test -f README &&
-	test -f src/part1.c &&
-	test -f src/part2.c &&
-	test -f a.out &&
-	test ! -f src/part3.c &&
-	test ! -f examples/1.c &&
-	test -f docs/manual.txt &&
-	test -f obj.o &&
-	test -f build/lib.so
+	test_path_is_file Makefile &&
+	test_path_is_file README &&
+	test_path_is_file src/part1.c &&
+	test_path_is_file src/part2.c &&
+	test_path_is_file a.out &&
+	test_path_is_missing src/part3.c &&
+	test_path_is_missing examples/1.c &&
+	test_path_is_file docs/manual.txt &&
+	test_path_is_file obj.o &&
+	test_path_is_file build/lib.so
 
 '

@@ -263,15 +263,15 @@ test_expect_success 'git clean -x' '
 	mkdir -p build docs &&
 	touch a.out src/part3.c docs/manual.txt obj.o build/lib.so &&
 	git clean -x &&
-	test -f Makefile &&
-	test -f README &&
-	test -f src/part1.c &&
-	test -f src/part2.c &&
-	test ! -f a.out &&
-	test ! -f src/part3.c &&
-	test -f docs/manual.txt &&
-	test ! -f obj.o &&
-	test -f build/lib.so
+	test_path_is_file Makefile &&
+	test_path_is_file README &&
+	test_path_is_file src/part1.c &&
+	test_path_is_file src/part2.c &&
+	test_path_is_missing a.out &&
+	test_path_is_missing src/part3.c &&
+	test_path_is_file docs/manual.txt &&
+	test_path_is_missing obj.o &&
+	test_path_is_file build/lib.so
 
 '

@@ -280,15 +280,15 @@ test_expect_success 'git clean -d -x' '
 	mkdir -p build docs &&
 	touch a.out src/part3.c docs/manual.txt obj.o build/lib.so &&
 	git clean -d -x &&
-	test -f Makefile &&
-	test -f README &&
-	test -f src/part1.c &&
-	test -f src/part2.c &&
-	test ! -f a.out &&
-	test ! -f src/part3.c &&
-	test ! -d docs &&
-	test ! -f obj.o &&
-	test ! -d build
+	test_path_is_file Makefile &&
+	test_path_is_file README &&
+	test_path_is_file src/part1.c &&
+	test_path_is_file src/part2.c &&
+	test_path_is_missing a.out &&
+	test_path_is_missing src/part3.c &&
+	test_path_is_missing docs &&
+	test_path_is_missing obj.o &&
+	test_path_is_missing build
 
 '

@@ -297,15 +297,15 @@ test_expect_success 'git clean -d -x with ignored tracked directory' '
 	mkdir -p build docs &&
 	touch a.out src/part3.c docs/manual.txt obj.o build/lib.so &&
 	git clean -d -x -e src &&
-	test -f Makefile &&
-	test -f README &&
-	test -f src/part1.c &&
-	test -f src/part2.c &&
-	test ! -f a.out &&
-	test -f src/part3.c &&
-	test ! -d docs &&
-	test ! -f obj.o &&
-	test ! -d build
+	test_path_is_file Makefile &&
+	test_path_is_file README &&
+	test_path_is_file src/part1.c &&
+	test_path_is_file src/part2.c &&
+	test_path_is_missing a.out &&
+	test_path_is_file src/part3.c &&
+	test_path_is_missing docs &&
+	test_path_is_missing obj.o &&
+	test_path_is_missing build
 
 '

@@ -314,15 +314,15 @@ test_expect_success 'git clean -X' '
 	mkdir -p build docs &&
 	touch a.out src/part3.c docs/manual.txt obj.o build/lib.so &&
 	git clean -X &&
-	test -f Makefile &&
-	test -f README &&
-	test -f src/part1.c &&
-	test -f src/part2.c &&
-	test -f a.out &&
-	test -f src/part3.c &&
-	test -f docs/manual.txt &&
-	test ! -f obj.o &&
-	test -f build/lib.so
+	test_path_is_file Makefile &&
+	test_path_is_file README &&
+	test_path_is_file src/part1.c &&
+	test_path_is_file src/part2.c &&
+	test_path_is_file a.out &&
+	test_path_is_file src/part3.c &&
+	test_path_is_file docs/manual.txt &&
+	test_path_is_missing obj.o &&
+	test_path_is_file build/lib.so
 
 '

@@ -331,15 +331,15 @@ test_expect_success 'git clean -d -X' '
 	mkdir -p build docs &&
 	touch a.out src/part3.c docs/manual.txt obj.o build/lib.so &&
 	git clean -d -X &&
-	test -f Makefile &&
-	test -f README &&
-	test -f src/part1.c &&
-	test -f src/part2.c &&
-	test -f a.out &&
-	test -f src/part3.c &&
-	test -f docs/manual.txt &&
-	test ! -f obj.o &&
-	test ! -d build
+	test_path_is_file Makefile &&
+	test_path_is_file README &&
+	test_path_is_file src/part1.c &&
+	test_path_is_file src/part2.c &&
+	test_path_is_file a.out &&
+	test_path_is_file src/part3.c &&
+	test_path_is_file docs/manual.txt &&
+	test_path_is_missing obj.o &&
+	test_path_is_missing build
 
 '

@@ -348,15 +348,15 @@ test_expect_success 'git clean -d -X with ignored tracked directory' '
 	mkdir -p build docs &&
 	touch a.out src/part3.c docs/manual.txt obj.o build/lib.so &&
 	git clean -d -X -e src &&
-	test -f Makefile &&
-	test -f README &&
-	test -f src/part1.c &&
-	test -f src/part2.c &&
-	test -f a.out &&
-	test ! -f src/part3.c &&
-	test -f docs/manual.txt &&
-	test ! -f obj.o &&
-	test ! -d build
+	test_path_is_file Makefile &&
+	test_path_is_file README &&
+	test_path_is_file src/part1.c &&
+	test_path_is_file src/part2.c &&
+	test_path_is_file a.out &&
+	test_path_is_missing src/part3.c &&
+	test_path_is_file docs/manual.txt &&
+	test_path_is_missing obj.o &&
+	test_path_is_missing build
 
 '

@@ -379,29 +379,29 @@ test_expect_success 'clean.requireForce and -n' '
 	mkdir -p build docs &&
 	touch a.out src/part3.c docs/manual.txt obj.o build/lib.so &&
 	git clean -n &&
-	test -f Makefile &&
-	test -f README &&
-	test -f src/part1.c &&
-	test -f src/part2.c &&
-	test -f a.out &&
-	test -f src/part3.c &&
-	test -f docs/manual.txt &&
-	test -f obj.o &&
-	test -f build/lib.so
+	test_path_is_file Makefile &&
+	test_path_is_file README &&
+	test_path_is_file src/part1.c &&
+	test_path_is_file src/part2.c &&
+	test_path_is_file a.out &&
+	test_path_is_file src/part3.c &&
+	test_path_is_file docs/manual.txt &&
+	test_path_is_file obj.o &&
+	test_path_is_file build/lib.so
 
 '
 
 test_expect_success 'clean.requireForce and -f' '
 
 	git clean -f &&
-	test -f README &&
-	test -f src/part1.c &&
-	test -f src/part2.c &&
-	test ! -f a.out &&
-	test ! -f src/part3.c &&
-	test -f docs/manual.txt &&
-	test -f obj.o &&
-	test -f build/lib.so
+	test_path_is_file README &&
+	test_path_is_file src/part1.c &&
+	test_path_is_file src/part2.c &&
+	test_path_is_missing a.out &&
+	test_path_is_missing src/part3.c &&
+	test_path_is_file docs/manual.txt &&
+	test_path_is_file obj.o &&
+	test_path_is_file build/lib.so
 
 '

@@ -444,11 +444,11 @@ test_expect_success 'nested git work tree' '
 		test_commit deeply.nested deeper.world
 	) &&
 	git clean -f -d &&
-	test -f foo/.git/index &&
-	test -f foo/hello.world &&
-	test -f baz/boo/.git/index &&
-	test -f baz/boo/deeper.world &&
-	! test -d bar
+	test_path_is_file foo/.git/index &&
+	test_path_is_file foo/hello.world &&
+	test_path_is_file baz/boo/.git/index &&
+	test_path_is_file baz/boo/deeper.world &&
+	test_path_is_missing bar
 '
 
 test_expect_success 'should clean things that almost look like git but are not' '

@@ -609,32 +609,30 @@ test_expect_success 'force removal of nested git work tree' '
 		test_commit deeply.nested deeper.world
 	) &&
 	git clean -f -f -d &&
-	! test -d foo &&
-	! test -d bar &&
-	! test -d baz
+	test_path_is_missing foo &&
+	test_path_is_missing bar &&
+	test_path_is_missing baz
 '
 
 test_expect_success 'git clean -e' '
 	rm -fr repo &&
 	mkdir repo &&
-	(
-		cd repo &&
-		git init &&
-		touch known 1 2 3 &&
-		git add known &&
-		git clean -f -e 1 -e 2 &&
-		test -e 1 &&
-		test -e 2 &&
-		! (test -e 3) &&
-		test -e known
-	)
+	cd repo &&
+	git init &&
+	touch known 1 2 3 &&
+	git add known &&
+	git clean -f -e 1 -e 2 &&
+	test_path_is_file 1 &&
+	test_path_is_file 2 &&
+	test_path_is_missing 3 &&
+	test_path_is_file known
 '
 
 test_expect_success SANITY 'git clean -d with an unreadable empty directory' '
 	mkdir foo &&
 	chmod a= foo &&
 	git clean -dfx foo &&
-	! test -d foo
+	test_path_is_missing foo
 '
 
 test_expect_success 'git clean -d respects pathspecs (dir is prefix of pathspec)' '

-- 
2.3.6

Re: [PATCH v2 2/2] clean: new option --exclude-from

From: Jeff King <hidden>
Date: 2016-06-15 23:07:24

On Sun, Dec 06, 2015 at 09:58:26AM -0500, James wrote:

+test_expect_success 'git clean -e --exclude-from' '
+	rm -fr repo &&
+	mkdir repo &&
+	cd repo &&
+	git init &&
+	touch known 1 2 3 &&
+	git add known &&
+	echo 1 >> .git/clean-exclude &&
+	git clean -f -e 2 --exclude-from=.git/clean-exclude &&
+	test_path_is_file 1 &&
+	test_path_is_file 2 &&
+	test_path_is_missing 3 &&
+	test_path_is_file known
+'

This checks that we exclude the union of the "-e" and "--exclude-from"
parameters. What happens if one excludes a path and the other negates
the exclusion? How is the precedence handled? Is it based on order on
the command-line, or something else?

I do not have much of a preference myself, but it probably makes sense
to document and test it.

+test_expect_success 'git clean --exclude-from --exclude-from' '
+	rm -fr repo &&
+	mkdir repo &&
+	git init &&
+	touch known 1 2 3 &&
+	git add known &&
+	cat >.git/clean-exclude1 <<-\EOF &&
+	1
+	EOF
+	cat >.git/clean-exclude2 <<-\EOF &&
+	2
+	EOF
+	git clean -f --exclude-from=.git/clean-exclude1 --exclude-from=.git/clean-exclude2 &&
+	test_path_is_file 1 &&
+	test_path_is_file 2 &&
+	test_path_is_missing 3 &&
+	test_path_is_file known
+'

Ditto here (as the same "type", the only precedence that would make any
sense is command-line order here).

+test_expect_success 'git clean --exclude-from=BADFILE' '
+	rm -fr repo &&
+	mkdir repo &&
+	cd repo &&
+	git init &&
+	test_expect_code 128 git clean -f --exclude-from=.git/clean-exclude-not-there
+'

Do you actually care about the 128 here, or is just "it should exit with
failure"? If the latter, we usually use test_must_fail.

-Peff

Re: [PATCH v2 1/2] modernize t7300

From: Eric Sunshine <hidden>
Date: 2016-06-15 23:07:24

On Sun, Dec 6, 2015 at 9:58 AM, James [off-list ref] wrote:

From: James Rouzier <redacted>

This would be a good place to explain how you are modernizing the test
script. Right now, you're just updating to take advantage of
test_path_is_foo(), but some other modernizations you could do
include:

* using '>' rather than 'touch' to create empty files when the
timestamp doesn't matter (which is all cases in this script)

* (optional) replace unnecessarily complex 'mkdir -p foo' with simpler
'mkdir foo' (but leave "mkdir -p foo/bar" as is)

* drop blank lines before and after test body; for instance:

    test_expect_success 'foo' '

        blah &&
        bloo

    '

becomes:

    test_expect_success 'foo' '
        blah &&
        bloo
    '

quoted hunk

---

diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh

@@ -609,32 +609,30 @@ test_expect_success 'force removal of nested git work tree' '
 test_expect_success 'git clean -e' '
        rm -fr repo &&
        mkdir repo &&
-       (
-               cd repo &&
-               git init &&
-               touch known 1 2 3 &&
-               git add known &&
-               git clean -f -e 1 -e 2 &&
-               test -e 1 &&
-               test -e 2 &&
-               ! (test -e 3) &&
-               test -e known
-       )
+       cd repo &&

The previous working directory is not automatically restored at the
end of the test, so this "cd" without corresponding "cd .." causes
following tests to execute at the incorrect location. Unfortunately,
you can't just place "cd .." at the end of a test since it will be
skipped if something before the "cd .." fails. The way the existing
code deals with this is by using a subshell:

    mkdir repo &&
    (
        cd repo &&
        ...
    )

The "cd repo" is inside the subshell, so it doesn't affect the parent
shell, and when the subshell exits, the parent shell remains at the
correct working directory (regardless of whether the test succeeded or
failed). Therefore, you don't want to drop the subshell.

+       git init &&
+       touch known 1 2 3 &&
+       git add known &&
+       git clean -f -e 1 -e 2 &&
+       test_path_is_file 1 &&
+       test_path_is_file 2 &&
+       test_path_is_missing 3 &&
+       test_path_is_file known
 '

Re: [PATCH v2 2/2] clean: new option --exclude-from

From: Eric Sunshine <hidden>
Date: 2016-06-15 23:07:24

In addition to Peff's and Junio's review comments...

On Sun, Dec 6, 2015 at 9:58 AM, James [off-list ref] wrote:

From: James Rouzier <redacted>

Specify a file to read for exclude patterns.

Missing Signed-off-by:.

quoted hunk

---

diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh

@@ -628,6 +628,66 @@ test_expect_success 'git clean -e' '
+test_expect_success 'git clean --exclude-from' '
+       rm -fr repo &&
+       mkdir repo &&
+       cd repo &&

See my review comments for patch 1/2 as to why you want to wrap 'cd'
and remaining statements in a subshell.

+       git init &&
+       touch known 1 2 3 &&

Likewise, use '>' rather than 'touch' to create empty files when the
timestamp isn't significant.

   >1 &&
   >2 &&
   >3 &&

+       git add known &&
+       cat >.git/clean-exclude <<-\EOF &&
+       1
+       2
+       EOF
+       git clean -f --exclude-from=.git/clean-exclude &&
+       test_path_is_file 1 &&
+       test_path_is_file 2 &&
+       test_path_is_missing 3 &&
+       test_path_is_file known
+'
+
+test_expect_success 'git clean -e --exclude-from' '
+       rm -fr repo &&
+       mkdir repo &&
+       cd repo &&
+       git init &&
+       touch known 1 2 3 &&
+       git add known &&
+       echo 1 >> .git/clean-exclude &&
+       git clean -f -e 2 --exclude-from=.git/clean-exclude &&
+       test_path_is_file 1 &&
+       test_path_is_file 2 &&
+       test_path_is_missing 3 &&
+       test_path_is_file known
+'
+
+test_expect_success 'git clean --exclude-from --exclude-from' '
+       rm -fr repo &&
+       mkdir repo &&
+       git init &&
+       touch known 1 2 3 &&
+       git add known &&
+       cat >.git/clean-exclude1 <<-\EOF &&
+       1
+       EOF
+       cat >.git/clean-exclude2 <<-\EOF &&
+       2
+       EOF

Creation of these single-line files probably would be more readable
using 'echo', as you do in the test just above (for
.git/clean-exclude):

    echo 1 >.git/clean-exclude1 &&
    echo 2 >.git/clean-exclude2 &&

+       git clean -f --exclude-from=.git/clean-exclude1 --exclude-from=.git/clean-exclude2 &&
+       test_path_is_file 1 &&
+       test_path_is_file 2 &&
+       test_path_is_missing 3 &&
+       test_path_is_file known
+'
+
+test_expect_success 'git clean --exclude-from=BADFILE' '
+       rm -fr repo &&
+       mkdir repo &&
+       cd repo &&
+       git init &&
+       test_expect_code 128 git clean -f --exclude-from=.git/clean-exclude-not-there
+'
+
 test_expect_success SANITY 'git clean -d with an unreadable empty directory' '
        mkdir foo &&
        chmod a= foo &&
--
2.3.6

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help