Re: [PATCH] diff: correct newline in summary for renamed files

From: Jeff King <peff@peff.net>
Date: Sep 27, 2017, 22:09 UTC
Message-ID: <20170927220925.u622metv2fcdtcfq@sigill.intra.peff.net>
In-Reply-To: <20170927215852.20163-1-sbeller@google.com>

On Wed, Sep 27, 2017 at 02:58:52PM -0700, Stefan Beller wrote:

Show 10 quoted lines

> From: Linus Torvalds <torvalds@linux-foundation.org>
> 
> In 146fdb0dfe (diff.c: emit_diff_symbol learns about DIFF_SYMBOL_SUMMARY,
> 2017-06-29), the conversion from direct printing to the symbol emission
> dropped the new line character for renamed, copied and rewritten files.
> 
> Add the emission of a newline, add a test for this case.
> 
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Stefan Beller <sbeller@google.com>

The overall substance looks good, but...

Show 12 quoted lines

> diff --git a/t/t4016-diff-quote.sh b/t/t4016-diff-quote.sh
> index 9c48e5c2c9..514056dd10 100755
> --- a/t/t4016-diff-quote.sh
> +++ b/t/t4016-diff-quote.sh
> @@ -30,6 +30,7 @@ test_expect_success setup '
>  	git add . &&
>  	git commit -m initial &&
>  	git mv "$P0.0" "R$P0.0" &&
> +	chmod a+x "R$P0.0" &&
>  	git mv "$P0.1" "R$P1.0" &&
>  	git mv "$P0.2" "R$P2.0" &&
>  	git mv "$P0.3" "R$P3.0" &&

Won't this chmod be a problem for platforms without an executable bit? I think you'd need to use "update-index --chmod=+x" here, or require the FILEMODE prereq.

The whole script is marked as !MINGW, so that makes it less of a problem, but it's still possible have !FILEMODE on a Linux system, if you're on a funny filesystem. That also seems like a good reason to make sure this is in a script which is run more widely, since Windows folks would want to run this test, too.

-Peff