Re: [PATCH] Fix failing test t3700-add.sh

[Jeff King <peff@peff.net>]

[+cc Ed, who wrote 4e55ed3 (add: add --chmod=+x / --chmod=-x options,
2016-05-31)]

On Fri, Jul 29, 2016 at 02:31:28PM +0200, Ingo Brückl wrote:

> At the time of the test xfoo1 already exists and is a link.
> As a result, the check for file mode 100644 fails.
> 
> Create not yet existing file xfoo instead.

Hrm. So in the original code:

>  test_expect_success 'git add --chmod=-x stages an executable file with -x' '
> -	echo foo >xfoo1 &&
> -	chmod 755 xfoo1 &&
> -	git add --chmod=-x xfoo1 &&
> -	case "$(git ls-files --stage xfoo1)" in
> -	100644" "*xfoo1) echo pass;;
> -	*) echo fail; git ls-files --stage xfoo1; (exit 1);;

I would have expected "git add --chmod" to drop the "-x" bit in addition
to actually overwriting the file contents (and switching a symlink to a
file). And it does. The culprit is actually the "echo foo >xfoo1" line.
If "xfoo1" is a symlink, then it silently writes to the symlink
destination, and xfoo1 remains a symlink (and thus tweaking its execute
bit is a noop).

I was also puzzled why the test fails for you; it does not for me.
Running the test script as root does make it fail. There are some
earlier tests which are skipped in this case, which run "git reset
--hard" with xfoo1 in the index, which cleans it up.

> +	echo foo >xfoo &&
> +	chmod 755 xfoo &&
> +	git add --chmod=-x xfoo &&
> +	case "$(git ls-files --stage xfoo)" in
> +	100644" "*xfoo) echo pass;;
> +	*) echo fail; git ls-files --stage xfoo; (exit 1);;

Here you just pick another name, "xfoo", which does happen to work. But
it seems like that has the same potential for flakiness if earlier tests
get adjusted or skipped, since they also use that name.

How about just:

  rm -f xfoo1

at the top of the test, which explicitly documents the state we are
looking for?

I also wondered if this test, which calls "chmod 755 xfoo1", should be
marked with the POSIXPERM prerequisite. But I guess since its goal is to
strip the executable bit, it "works" even on systems where that chmod is
a noop (the "git add --chmod" doesn't do anything, but one way or the
other we end up at the end state we expect).

-Peff