Subject: Re: [PATCH] test-lib: abort when can't remove trash directory

From: Junio C Hamano <[email-masked]>
Date: Sun, 23 Apr 2017 19:58:44 -0700

Jeff King <[email-masked]> writes:

> On Sun, Apr 23, 2017 at 05:14:54PM -0700, Junio C Hamano wrote:
>
>> OK.  I am wondering why we do not do 
>> 
>> 	rm -fr "$TRASH_DIRECTORY"
>> 
>> and do this instead:
>> 
>> 	cd "$(dirname "$remove_trash")" &&
>> 	rm -rf "$(basename "$remove_trash")"
>> 
>> in the original.  It feels somewhat unnatural.
>
> I assumed the "cd" was there because some systems may be unhappy
> removing a directory which is our current working directory. That might
> just be superstition, though.

Ahh, OK, that makes sense.  I forgot about that.

> The use of "basename" in the second does seem superfluous, since the
> trash directory should be the full path (I suspect it wasn't in the
> early days, though).
>
>> So perhaps we can simplify and make it more robust by doing this?
>> [...]
>> +		if test -z "$debug"
>> +		then
>> +			test -d "$TRASH_DIRECTORY" ||
>> +			error "Tests passed but trash directory already removed before test cleanup; aborting"
>> +
>> +			rm -fr "$TRASH_DIRECTORY" ||
>> +			error "Tests passed but test cleanup failed; aborting"
>> +		fi
>
> That looks fine, assuming the answer to the "is the cwd important"
> question is "no".

And I do think the answer would be "yes", unfortunately.  There are
systems that do not even allow a file to be removed while it is
open, so...