Re: [PATCH v3 2/2] transport-helper: check if remote helper is alive

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 8, 2013, 18:46 UTC
Message-ID: <7vwqscx3l5.fsf@alter.siamese.dyndns.org>
In-Reply-To: <CAMP44s2PJGJQBM86CGsA5BOn1RLi4mqNsq+d0oyY0jA=XHQGDA@mail.gmail.com>

Felipe Contreras <felipe.contreras@gmail.com> writes:

Show 5 quoted lines

> I'm not talking about the time it took to come up with the criticism
> below, I'm talking about the comment regarding the commit message. If
> the commit message did indeed provide *zero* explanation, that's
> certainly something that should be immediately visible, no? It could
> have been mentioned six months ago.

Given that all of your log messages tend to be missing or terser than necessary, I wouldn't be surprised Peff stopped bothering about it.

But now we know this needs to be better explained, and your v4 has better explanation that will help you avoid having to repeat yourself in the discussion, so let's not dwell on it too much.

Show 9 quoted lines

>> ...  But if we keep
>> helper running, who will be communicating with it via these open
>> pipes?  The process that is calling finish_command() on fast-import
>> and disconnecting from the helper won't be, as read/write to the
>> pipe, even if we do not disconnect from here, will result in errors
>> if the helper has already exited at this point.
>
> Nobody will send any further input, but in theory we could redirect
> the pipe and send more commands. That's how it was designed.

Who does the redirection to whom? How would the process tree and piping constructed around the current system?

I am not trying to say it is just theoretical mental exercise (which I have seen you do not do at all on this list). I am trying to find out what the practical use case is that you have in mind, because disconnecting will prove to be not an improvement but a regression for that use case. But you do not have to answer this question directly, because...

> And in fact, I'm thinking doing exactly that, so we can send another
> command to fetch the foreign commit ids and append notes with them.

...we will see the answer in that change anyway.