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

2017-04-24 Thread Junio C Hamano
Jeff King writes: > Sort of. It still has the bug that it dies with error() when "--debug" > is used. Ah, I forgot about that one. Let me squash it in without the remove_trash change then, perhaps, but not today.

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

2017-04-24 Thread Jeff King
On Mon, Apr 24, 2017 at 11:05:15PM -0700, Junio C Hamano wrote: > Jeff King writes: > > > Good point. There's only one caller, but it does care about being in > > that directory. > > > >> Second try that hopefully is much less damaging > > I've been carrying it as a SQUASH??? patch, but I think

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

2017-04-24 Thread Junio C Hamano
Jeff King writes: > Good point. There's only one caller, but it does care about being in > that directory. > >> Second try that hopefully is much less damaging I've been carrying it as a SQUASH??? patch, but I think it is better to split it as a separate pach, as removal of $remove_trash is an o

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

2017-04-24 Thread Junio C Hamano
Torsten Bögershausen writes: > [] >>> >>> - cd "$(dirname "$remove_trash")" && >>> - rm -rf "$(basename "$remove_trash")" || >>> - error "Tests passed but test cleanup failed; aborting" >>> + cd "$(dirname "$TRASH_DIRECTORY")" && >>> +

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

2017-04-24 Thread Jeff King
On Mon, Apr 24, 2017 at 11:39:26AM +0200, Torsten Bögershausen wrote: > [] > > > > > > - cd "$(dirname "$remove_trash")" && > > > - rm -rf "$(basename "$remove_trash")" || > > > - error "Tests passed but test cleanup failed; aborting" > > > + cd "$(dirname

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

2017-04-24 Thread Torsten Bögershausen
[] - cd "$(dirname "$remove_trash")" && - rm -rf "$(basename "$remove_trash")" || - error "Tests passed but test cleanup failed; aborting" + cd "$(dirname "$TRASH_DIRECTORY")" && + rm -fr "$TRASH_DIRECTORY" ||

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

2017-04-24 Thread Jeff King
On Sun, Apr 23, 2017 at 09:02:41PM -0700, Junio C Hamano wrote: > >> 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 i

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

2017-04-23 Thread Junio C Hamano
Junio C Hamano writes: >> 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... In addition to "some platforms

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

2017-04-23 Thread Junio C Hamano
Jeff King 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 th

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

2017-04-23 Thread Jeff King
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

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

2017-04-23 Thread Junio C Hamano
Jeff King writes: >> -test -d "$remove_trash" && >> +test -d "$remove_trash" || >> +error "Tests passed but trash directory already removed before >> test cleanup; aborting" > > I think I found out why this "test -d" was here in the first place: > > $ ./t000

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

2017-04-21 Thread Jeff King
On Thu, Apr 20, 2017 at 06:52:30PM +0200, SZEDER Gábor wrote: > diff --git a/t/test-lib.sh b/t/test-lib.sh > index 13b569682..e9e6f677d 100644 > --- a/t/test-lib.sh > +++ b/t/test-lib.sh > @@ -761,9 +761,12 @@ test_done () { > say "1..$test_count$skip_all" > fi

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

2017-04-21 Thread SZEDER Gábor
On Fri, Apr 21, 2017 at 2:48 AM, Junio C Hamano wrote: > SZEDER Gábor writes: > >> We had two similar bugs in the tests sporadically triggering error >> messages during the removal of the trash directory, see commits >> bb05510e5 (t5510: run auto-gc in the foreground, 2016-05-01) and >> ef09036cf

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

2017-04-20 Thread Junio C Hamano
SZEDER Gábor writes: > We had two similar bugs in the tests sporadically triggering error > messages during the removal of the trash directory, see commits > bb05510e5 (t5510: run auto-gc in the foreground, 2016-05-01) and > ef09036cf (t6500: wait for detached auto gc at the end of the test > scr

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

2017-04-20 Thread Jeff King
On Thu, Apr 20, 2017 at 06:52:30PM +0200, SZEDER Gábor wrote: > We had two similar bugs in the tests sporadically triggering error > messages during the removal of the trash directory, see commits > bb05510e5 (t5510: run auto-gc in the foreground, 2016-05-01) and > ef09036cf (t6500: wait for detac

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

2017-04-20 Thread SZEDER Gábor
We had two similar bugs in the tests sporadically triggering error messages during the removal of the trash directory, see commits bb05510e5 (t5510: run auto-gc in the foreground, 2016-05-01) and ef09036cf (t6500: wait for detached auto gc at the end of the test script, 2017-04-13). The test scrip