Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> 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...
> @@ -1210,10 +1210,10 @@ test_done () {
...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...
> # Are we running this test at all?
> remove_trash=
> this_test=${0##*/}
> @@ -1388,7 +1400,7 @@ GNUPGHOME="$HOME/gnupg-home-not-used"
> export HOME GNUPGHOME USER_HOME
>
> # Test repository
> -rm -fr "$TRASH_DIRECTORY" || {
> +remove_trash_directory "$TRASH_DIRECTORY" || {
... 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.
