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

From: Jeff King <peff@peff.net>
Date: Sep 29, 2016, 18:02 UTC
Message-ID: <20160929180247.d4owhzwyawtanw3r@sigill.intra.peff.net>
In-Reply-To: <xmqqk2duhcdm.fsf@gitster.mtv.corp.google.com>

On Thu, Sep 29, 2016 at 09:57:57AM -0700, Junio C Hamano wrote:

Show 14 quoted lines

> > +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" and grepping for a command is not suitable for script
> to reliably tell things, so it is out of question.  Compared to
> that, your version looks slightly better, but what if the machinery
> that being tested, i.e. the part that drives the filter process, is
> buggy or becomes buggy and causes the filter process that writes
> "STOP" to die before it actually writes that string?

I'm of the opinion that any busy-waiting is a good sign that something is suboptimal. The right solution here seems like it should be signaling the test script via a descriptor.

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 might not want to wait (e.g., if the filter has to do some maintenance task and does not the user to have to wait).

OTOH, we already face this in git, and we solve it by explicitly backgrounding the maintenance task (i.e., auto-gc). So one could argue that it is the responsibility of the filter process to manage its own processes. It certainly makes the interaction with git simpler.

-Peff