On Thu, Apr 13, 2017 at 09:03:26PM +0200, SZEDER Gábor wrote:
> > Yeah, I had a similar thought. I can't think of any reason why it would
> > trigger a false positive, as long as we account for test-failure and the
> > --debug flag properly. I don't think we can just drop the "-f" from the
> > final "rm", because it may be overriding funny permissions left by
> > tests.
>
> FWIW, I used the following rather simple change during stress-testing
> these patches (barring white-space damage):
>
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 13b569682..d7fa15a69 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -763,7 +763,7 @@ test_done () {
>
> test -d "$remove_trash" &&
> cd "$(dirname "$remove_trash")" &&
> - rm -rf "$(basename "$remove_trash")"
> + rm -rf "$(basename "$remove_trash")" || exit 1
Oh, right. I thought "-f" would cause it to exit 0 even if some items
failed to be removed, but that only applies to ENOENT. So I think that
is probably a good change. I agree it's not a true error, but just a
sign of something fishy, but I don't think it hurts to have extra lint
checks.
Replacing it the "exit 1" with a "die" that has a message is probably a
good idea, though.
-Peff