Re: [PATCH] test-lib.sh: try to re-chmod & retry on failed trash removal

Ævar Arnfjörð Bjarmason  [off-list ref] writes:

This can be demonstrated as e.g. (output snipped for less verbosity):

    $ ./t0004-unwritable.sh --run=3 --immediate
    ok 1 # skip setup (--run)
    ok 2 # skip write-tree should notice unwritable repository (--run)
    not ok 3 - commit should notice unwritable repository
    [...]
    $ ./t0004-unwritable.sh --run=3 --immediate
    rm: cannot remove '[...]/trash directory.t0004-unwritable/.git/objects/info': Permission denied
    FATAL: Cannot prepare test area
    [...]

I.e. if any of its tests failed, and the tests were being run under
"--debug"[2] or "--immediate"[3] (which was introduced after [1]) we
wouldn't re-chmod the object directory. We'd then fail on the next run
since the test setup couldn't remove the trash files.

OK, the first thing these scripts do is to create $TRASH_DIRECTORY
for their own use, and that is what fails (hence "Cannot prepare").

And making sure we try harder to prepare would be a good thing.
Given that understanding of the motivation... 

...it is puzzling why we are touching the code that happens _after_
a test is run.  If our philosophy is to leave the mess after a test
fails so that the debugging person can take a look without
contaminating the forensics data, but blindly and forcibly clean up
before starting a new round, reusing the "do more than usual to
remove" logic meant to be used for the latter here feels counter
productive and also a bit counter intuitive.

 			error "Tests passed but trash directory already removed before test cleanup; aborting"
 
 			cd "$TRASH_DIRECTORY/.." &&
-			rm -fr "$TRASH_DIRECTORY" || {
+			remove_trash_directory "$TRASH_DIRECTORY" || {
 				# try again in a bit
 				sleep 5;
-				rm -fr "$TRASH_DIRECTORY"
+				remove_trash_directory "$TRASH_DIRECTORY"
 			} ||
 			error "Tests passed but test cleanup failed; aborting"
 		fi

+# Try really hard to clean up our mess
+remove_trash_directory() {
+	dir="$1"
+	if ! rm -rf "$dir"
+	then
+		say_color info >&3 "Failed to remove trash directory, trying to re-chmod it first..."
+		chmod -R u+w "$dir" 2>/dev/null

If a test lost searchable bit from directories, "u+wx" may be
necessary to clean fully, no?

+		rm -rf "$dir"
+	fi
+	! test -d "$dir"
+}
+

On the other hand...

... this one does make sense and matches what the proposed log
message sold us the change as.

 	GIT_EXIT_OK=t
 	echo >&5 "FATAL: Cannot prepare test area"
 	exit 1

Thanks.