From: Siddh Raman Pant <siddh.raman.pant@oracle.com>
Date: Fri, 22 May 2026 05:59:20 GMT
Subject: Re: [PATCH 4/9] run-command: add support for timeout in command finisher
Message-ID: <a0916ed8965d2024ee0f6005c942f77a16f28dbb.camel@oracle.com>
In-Reply-To: <20260522051048.GA862219@coredump.intra.peff.net>

On Fri, May 22 2026 at 10:40:48 +0530, Jeff King wrote:
> On Thu, May 21, 2026 at 04:36:05PM +0200, Johannes Sixt wrote:
> 
> > Am 21.05.26 um 11:59 schrieb Siddh Raman Pant:
> > > The timeout is for the failure path, where the external helper has
> > > already stopped following that protocol or is blocked on something
> > > outside git's control. Since git starts the helper and puts it on the
> > > log/grep path, git also needs a bounded way to recover when that helper
> > > does not make progress. Otherwise an optional note source can prevent
> > > the main git command from completing.
> > 
> > That Git communicates with a process that looks like it stopped is the
> > normal case, for example:
> > 
> > - Output is sent to the pager. The user can take their time to study the
> > output. All the while, git waits patiently for the user to advance the
> > pager.
> > 
> > - Git fetch transfers large amounts of data across the network. Most of
> > the time it waits for data to arrive and does nothing. The peer process
> > looks like it hangs. Git does not decide to kill the connection at any
> > time. It is the user's decision to do so.
> > 
> > If the notes provider hangs, then it is not on Git to decide when it has
> > waited long enough.
> 
> Yeah, I agree with your point of view. If I understand this patch series
> correctly, it is about adding an external process to map commit ids to
> note data. So I can think of some existing features that are quite close
> to that in nature, none of which use timeouts:
> 
>   - textconv filters and external diffs which process data in the middle
>     of a git-log invocation
> 
>   - long-lived clean/smudge filters map blobs to arbitrarily large text
> 
>   - cat-file's batch mode maps object ids to user-specified data about
>     that object
> 
> As you note, it's up to the command to be well-behaved. Git should
> notice and respond appropriately if the command closes the pipe, of
> course. Sometimes a timeout can help with a poorly behaved command, but
> IMHO it is not worth the cost of non-determinism that it brings.
> 
> Moreover, the bits touching run-command here make me nervous, especially
> after the challenges we saw in the child-cleanup topic that was reverted
> just after v2.54. There is often a shell interposed between Git and the
> sub-command, and we don't always know how the shell will react to
> signals. Using SIGKILL will eventually get us _something_ to wait() on,
> but it might not even be the process we care about!
> 
> I don't really care much about this external-notes feature one way or
> the other, but if we are going to do it, I don't see any reason why it
> would not behave like all of the other similar parts of Git.
> 
> -Peff

Okay, since the consensus here is pretty clear, I will remove this
commit and send a v2.

Thanks,
Siddh