Re: [PATCH v8 00/11] Git filter protocol

2016-10-06 Thread Junio C Hamano
Jeff King writes: > I am not sure if I have followed all of this discussion, but I am of the > opinion that Git should not be doing any timeouts at all. > ... > If this is debugging output of the form "now I am calling wait() on all > of the filters", without respect to any

Re: [PATCH v8 00/11] Git filter protocol

2016-10-06 Thread Jeff King
On Thu, Oct 06, 2016 at 03:13:19PM +0200, Lars Schneider wrote: > >>> Well, it would be good to tell users _why_ Git is hanging, see below. > >> > >> Agreed. Do you think it is OK to write the message to stderr? > > > > On the other hand, this is why GIT_TRACE (and GIT_TRACE_PERFORMANCE) > >

Re: [PATCH v8 00/11] Git filter protocol

2016-10-06 Thread Lars Schneider
> On 04 Oct 2016, at 21:04, Jakub Narębski wrote: > > W dniu 03.10.2016 o 19:13, Lars Schneider pisze: >>> On 01 Oct 2016, at 22:48, Jakub Narębski wrote: >>> W dniu 01.10.2016 o 20:59, Lars Schneider pisze: On 29 Sep 2016, at 23:27, Junio C Hamano

Re: [PATCH v8 00/11] Git filter protocol

2016-10-04 Thread Jakub Narębski
W dniu 03.10.2016 o 19:13, Lars Schneider pisze: >> On 01 Oct 2016, at 22:48, Jakub Narębski wrote: >> W dniu 01.10.2016 o 20:59, Lars Schneider pisze: >>> On 29 Sep 2016, at 23:27, Junio C Hamano wrote: Lars Schneider

Re: [PATCH v8 00/11] Git filter protocol

2016-10-04 Thread Junio C Hamano
Jeff King writes: > On Mon, Oct 03, 2016 at 10:02:14AM -0700, Junio C Hamano wrote: > >> The timeout would be good for you to give a message "filter process >> running the script '%s' is not exiting; I am waiting for it". The >> user is still left with a hung Git, and can then

Re: [PATCH v8 00/11] Git filter protocol

2016-10-04 Thread Jeff King
On Mon, Oct 03, 2016 at 10:02:14AM -0700, Junio C Hamano wrote: > The timeout would be good for you to give a message "filter process > running the script '%s' is not exiting; I am waiting for it". The > user is still left with a hung Git, and can then see if that process > is hanging around.

Re: [PATCH v8 00/11] Git filter protocol

2016-10-03 Thread Lars Schneider
> On 03 Oct 2016, at 19:02, Junio C Hamano wrote: > > Lars Schneider writes: > >>> If the filter process refuses to die forever when Git told it to >>> shutdown (by closing the pipe to it, for example), that filter >>> process is simply buggy. I

Re: [PATCH v8 00/11] Git filter protocol

2016-10-03 Thread Lars Schneider
> On 01 Oct 2016, at 22:48, Jakub Narębski wrote: > > W dniu 01.10.2016 o 20:59, Lars Schneider pisze: >> On 29 Sep 2016, at 23:27, Junio C Hamano wrote: >>> Lars Schneider writes: >>> >>> If the filter process refuses to die

Re: [PATCH v8 00/11] Git filter protocol

2016-10-03 Thread Junio C Hamano
Lars Schneider writes: >> If the filter process refuses to die forever when Git told it to >> shutdown (by closing the pipe to it, for example), that filter >> process is simply buggy. I think we want users to become aware of >> that, instead of Git leaving it behind,

Re: [PATCH v8 00/11] Git filter protocol

2016-10-01 Thread Jakub Narębski
W dniu 01.10.2016 o 20:59, Lars Schneider pisze: > On 29 Sep 2016, at 23:27, Junio C Hamano wrote: >> Lars Schneider writes: >> >>> We discussed that issue in v4 and v6: >>>

Re: [PATCH v8 00/11] Git filter protocol

2016-10-01 Thread Lars Schneider
> On 29 Sep 2016, at 23:27, Junio C Hamano wrote: > > Lars Schneider writes: > >> We discussed that issue in v4 and v6: >> http://public-inbox.org/git/20160803225313.pk3tfe5ovz4y3...@sigill.intra.peff.net/ >>

Re: [PATCH v8 00/11] Git filter protocol

2016-09-29 Thread Junio C Hamano
Lars Schneider writes: > We discussed that issue in v4 and v6: > http://public-inbox.org/git/20160803225313.pk3tfe5ovz4y3...@sigill.intra.peff.net/ > http://public-inbox.org/git/xmqqbn0a3wy3@gitster.mtv.corp.google.com/ > > My impression was that you don't want Git

Re: [PATCH v8 00/11] Git filter protocol

2016-09-29 Thread Junio C Hamano
Jeff King writes: > I don't necessarily agree, though, that the timing of filter-process > cleanup needs to be part of the public interface. So in your list: > >> 3) Git waits until the filter process finishes. > > That seems simple and elegant, but I can think of reasons we

Re: [PATCH v8 00/11] Git filter protocol

2016-09-29 Thread Junio C Hamano
Jakub Narębski writes: > Or even better: make filter driver write its pid to pidfile, and then > "wait $(cat rot13-filter.pid)". That's what we do in lib-git-daemon.sh > (I think). I am not sure if "wait"ing on a random process that is not a direct child is a reasonable thing

Re: [PATCH v8 00/11] Git filter protocol

2016-09-29 Thread Junio C Hamano
Lars Schneider writes: > A pragmatic approach: > > I could drop the "STOP" message that the filter writes to the log > on exit and everything would work as is. We could argue that this > is OK because Git doesn't care anyways if the filter process has > stopped or

Re: [PATCH v8 00/11] Git filter protocol

2016-09-29 Thread Jakub Narębski
W dniu 29.09.2016 o 13:57, Torsten Bögershausen pisze: > On 29/09/16 12:28, Lars Schneider wrote: >> This is what happens: >> >> 1) Git exits >> 2) The filter process receives EOF and prints "STOP" to the log >> 3) t0021 checks the content of the log >> >> Sometimes 3 happened before 2 which

Re: [PATCH v8 00/11] Git filter protocol

2016-09-29 Thread Lars Schneider
> On 29 Sep 2016, at 18:57, Junio C Hamano wrote: > > Torsten Bögershausen writes: > >>> 1) Git exits >>> 2) The filter process receives EOF and prints "STOP" to the log >>> 3) t0021 checks the content of the log >>> >>> Sometimes 3 happened before 2 which

Re: [PATCH v8 00/11] Git filter protocol

2016-09-29 Thread Johannes Sixt
Am 29.09.2016 um 20:18 schrieb Torsten Bögershausen: I would agree that Git should not wait for the filter. But does the test suite need to wait for the filter ? We have fixed a test case on Windows recently where a process hung around too long (5babb5bd). So, yes, the test suite has to wait

Re: [PATCH v8 00/11] Git filter protocol

2016-09-29 Thread Torsten Bögershausen
On 29/09/16 19:57, Lars Schneider wrote: On 29 Sep 2016, at 18:57, Junio C Hamano wrote: Torsten Bögershausen writes: 1) Git exits 2) The filter process receives EOF and prints "STOP" to the log 3) t0021 checks the content of the log Sometimes 3 happened

Re: [PATCH v8 00/11] Git filter protocol

2016-09-29 Thread Jeff King
On Thu, Sep 29, 2016 at 09:57:57AM -0700, Junio C Hamano wrote: > > +wait_for_filter_termination () { > > + while ! grep "STOP" LOGFILENAME >/dev/null > > + do > > + echo "Waiting for /t0021/rot13-filter.pl to finish..." > > + sleep 1 > > + done > > +} > > Running "ps"

Re: [PATCH v8 00/11] Git filter protocol

2016-09-29 Thread Lars Schneider
> On 29 Sep 2016, at 18:57, Junio C Hamano wrote: > > Torsten Bögershausen writes: > >>> 1) Git exits >>> 2) The filter process receives EOF and prints "STOP" to the log >>> 3) t0021 checks the content of the log >>> >>> Sometimes 3 happened before 2 which

Re: [PATCH v8 00/11] Git filter protocol

2016-09-29 Thread Junio C Hamano
Torsten Bögershausen writes: >> 1) Git exits >> 2) The filter process receives EOF and prints "STOP" to the log >> 3) t0021 checks the content of the log >> >> Sometimes 3 happened before 2 which makes the test fail. >> (Example: https://travis-ci.org/git/git/jobs/162660563 ) >>

Re: [PATCH v8 00/11] Git filter protocol

2016-09-29 Thread Torsten Bögershausen
On 29/09/16 12:28, Lars Schneider wrote: On 28 Sep 2016, at 23:49, Junio C Hamano wrote: I suspect that you are preparing a reroll already, but the one that is sitting in 'pu' seems to be flaky in t/t0021 and I seem to see occasional failures from it. I didn't trace where

Re: [PATCH v8 00/11] Git filter protocol

2016-09-29 Thread Lars Schneider
> On 28 Sep 2016, at 23:49, Junio C Hamano wrote: > > I suspect that you are preparing a reroll already, but the one that > is sitting in 'pu' seems to be flaky in t/t0021 and I seem to see > occasional failures from it. > > I didn't trace where the test goes wrong, but one

Re: [PATCH v8 00/11] Git filter protocol

2016-09-28 Thread Junio C Hamano
I suspect that you are preparing a reroll already, but the one that is sitting in 'pu' seems to be flaky in t/t0021 and I seem to see occasional failures from it. I didn't trace where the test goes wrong, but one easy mistake you could make (I am not saying that is the reason of the failure) is