Hi Ævar,

On Sat, 20 Mar 2021, Ævar Arnfjörð Bjarmason wrote:

> On Fri, Mar 19 2021, Johannes Schindelin wrote:
>
> >> On Tue, Jan 29 2019, Johannes Schindelin via GitGitGadget wrote:
> >>
> >> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >> >
> >> > It seems that every once in a while in the Git for Windows SDK, there
> >> > are some transient file locking issues preventing the test clean up to
> >> > delete the trash directory. Let's be gentle and try again five seconds
> >> > later, and only error out if it still fails the second time.
> >> >
> >> > This change helps Windows, and does not hurt any other platform
> >> > (normally, it is highly unlikely that said deletion fails, and if it
> >> > does, normally it will fail again even 5 seconds later).
> >> >
> >> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> >> > ---
> >> >  t/test-lib.sh | 6 +++++-
> >> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/t/test-lib.sh b/t/test-lib.sh
> >> > index f31a1c8f79..9c0ca5effb 100644
> >> > --- a/t/test-lib.sh
> >> > +++ b/t/test-lib.sh
> >> > @@ -1104,7 +1104,11 @@ test_done () {
> >> >  			error "Tests passed but trash directory already removed before test cleanup; aborting"
> >> >
> >> >  			cd "$TRASH_DIRECTORY/.." &&
> >> > -			rm -fr "$TRASH_DIRECTORY" ||
> >> > +			rm -fr "$TRASH_DIRECTORY" || {
> >> > +				# try again in a bit
> >> > +				sleep 5;
> >> > +				rm -fr "$TRASH_DIRECTORY"
> >> > +			} ||
> >> >  			error "Tests passed but test cleanup failed; aborting"
> >> >  		fi
> >> >  		test_at_end_hook_
> >>
> >> I saw this sleep while reading some test-lib.sh code, doesn't this break
> >> df4c0d1a79 (test-lib: abort when can't remove trash directory,
> >> 2017-04-20) for non-Windows platforms?
> >
> > It does not really break it, it just delays the inevitable failure.

I still think this is the best answer to this (implicit) question:

> In any case, your patch clearly undoes whatever canary for gc issues
> df4c0d1a792 was trying to put into the test-lib, but didn't say so in
> its commit message.

I was not _really_ paying attention to that commit when I implemented the
work-around you mentioned above. At the same time I think it does _not_
undo the canary. If the trash directory cannot be removed via `rm -fr`,
and if that is an indicator for something fishy going on, chances are that
the second `rm -fr` a couple seconds later will _also_ fail, and we still
get that error message.

The only reason why the second `rm` should succeed, at least that I can
think of, is that something on Windows blocked those files from being
deleted, and it is no longer blocking after a couple seconds, and that
usually means that an anti-malware scanned those files.

Ciao,
Dscho