Re: [PATCH v6 16/21] mingw: try to work around issues with the test cleanup
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Mar 24, 2021, 12:01 UTC
- Message-ID
- <nycvar.QRO.7.76.6.2103241257580.50@tvgsbejvaqbjf.bet>
- In-Reply-To
- <87a6qxzocs.fsf@evledraar.gmail.com>
Hi Ævar,
On Sat, 20 Mar 2021, Ævar Arnfjörð Bjarmason wrote:
Show 43 quoted lines
> 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