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

From: Jeff King <peff@peff.net>
Date: Dec 7, 2015, 20:56 UTC
Message-ID: <20151207205619.GE30203@sigill.intra.peff.net>
In-Reply-To: <1449413906-23256-2-git-send-email-rouzier@gmail.com>

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

Show 14 quoted lines

> +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.

Show 18 quoted lines

> +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).

Show 7 quoted lines

> +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