git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 4/9] run-command: add support for timeout in command finisher

From
Siddh Raman Pant <siddh.raman.pant@oracle.com>
Date
May 21, 2026, 09:59 UTC
Message-ID
<2f7eea03273ffaacc50a9ae186673da88fc3345f.camel@oracle.com>
In-Reply-To
<b69605a6-e841-47b9-a899-a57e184d3c8b@kdbg.org>
On Thu, May 21 2026 at 12:51:51 +0530, Johannes Sixt wrote:
> This is extremely suspicious. A communication protocl with a child
> program that requires to kill the child looks like a design error. A
> band-aid like this timeout should not be necessary for a well-behaved
> child process.

I do not think this is a protocol design error. The normal protocol does not require killing the helper: git sends one object id, the helper sends one bounded response, and the helper exits when git closes its pipes.

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.

> If the (your?) problem is that the child process is actually not
> well-behaved, then I suggest to use a middle-man as child process that
> behaves well from the point of view of the git process, but can punish
> the ill-behaved downstream process when needed.

A middle-man would need the same timeout/termination/reaping logic, and git would still need to handle the middle-man itself hanging / failing. So I don't think it removes the problem, it just makes each user or deployment carry that process-supervision logic outside git.

External notes are additive. If the helper misbehaves, the intended behavior is to warn once, disable that source for the rest of the process, and let git continue without those notes. That seems preferable to leaving git stuck in finish_command().

Thanks, Siddh

Previous: Oswald BuddenhagenNext: Johannes Sixt
Message 10 of 29 in “Add support for an external command for fetching notes”
  1. 0/9 Add support for an external command for fetching notesSiddh Raman Pant, May 19, 2026
  2. 1/9 Documentation/git-range-diff: add missing notes options in synopsisSiddh Raman Pant, May 19, 2026
  3. Junio C HamanoMay 19, 2026
  4. Siddh Raman PantMay 20, 2026
  5. Junio C HamanoMay 21, 2026
  6. Siddh Raman PantMay 21, 2026
  7. 4/9 run-command: add support for timeout in command finisherSiddh Raman Pant, May 19, 2026
  8. Johannes SixtMay 21, 2026
  9. Oswald BuddenhagenMay 21, 2026
  10. Siddh Raman PantMay 21, 2026
  11. Johannes SixtMay 21, 2026
  12. Junio C HamanoMay 22, 2026
  13. Siddh Raman PantMay 22, 2026
  14. Jeff KingMay 22, 2026
  15. Siddh Raman PantMay 22, 2026
  16. 5/9 wrapper: add support for timeout and deadline in read helpersSiddh Raman Pant, May 19, 2026
  17. 6/9 t3301: cover generic displayed notes behaviorSiddh Raman Pant, May 19, 2026
  18. 7/9 notes: support an external command to display notesSiddh Raman Pant, May 19, 2026
  19. Junio C HamanoMay 20, 2026
  20. Siddh Raman PantMay 20, 2026
  21. brian m. carlsonMay 21, 2026
  22. Siddh Raman PantMay 21, 2026
  23. brian m. carlsonMay 21, 2026
  24. 2/9 notes: convert raw arg in format_display_notes() to boolSiddh Raman Pant, May 19, 2026
  25. 3/9 wrapper: add sleep_nanosecSiddh Raman Pant, May 19, 2026
  26. Junio C HamanoMay 19, 2026
  27. Siddh Raman PantMay 20, 2026
  28. 8/9 Documentation: document external notes command optionsSiddh Raman Pant, May 19, 2026
  29. 9/9 t: add tests for external notes commandSiddh Raman Pant, May 19, 2026

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.