From: Siddh Raman Pant <siddh.raman.pant@oracle•com>
To: "j6t@kdbg•org" <j6t@kdbg•org>
Cc: "git@vger•kernel.org" <git@vger•kernel.org>,
"gitster@pobox•com" <gitster@pobox•com>,
"newren@gmail•com" <newren@gmail•com>, "ps@pks•im" <ps@pks•im>,
"oswald.buddenhagen@gmx•de" <oswald.buddenhagen@gmx•de>,
"code@khaugsbakk•name" <code@khaugsbakk•name>
Subject: Re: [PATCH 4/9] run-command: add support for timeout in command finisher
Date: Thu, 21 May 2026 09:59:09 +0000 [thread overview]
Message-ID: <2f7eea03273ffaacc50a9ae186673da88fc3345f.camel@oracle.com> (raw)
In-Reply-To: <b69605a6-e841-47b9-a899-a57e184d3c8b@kdbg.org>
[-- Attachment #1: Type: text/plain, Size: 1730 bytes --]
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
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2026-05-21 9:59 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-19 16:30 [PATCH 0/9] Add support for an external command for fetching notes Siddh Raman Pant
2026-05-19 16:30 ` [PATCH 1/9] Documentation/git-range-diff: add missing notes options in synopsis Siddh Raman Pant
2026-05-19 23:47 ` Junio C Hamano
2026-05-20 7:00 ` Siddh Raman Pant
2026-05-21 0:28 ` Junio C Hamano
2026-05-21 4:13 ` Siddh Raman Pant
2026-05-19 16:30 ` [PATCH 2/9] notes: convert raw arg in format_display_notes() to bool Siddh Raman Pant
2026-05-19 16:30 ` [PATCH 3/9] wrapper: add sleep_nanosec Siddh Raman Pant
2026-05-19 23:50 ` Junio C Hamano
2026-05-20 7:07 ` Siddh Raman Pant
2026-05-19 16:30 ` [PATCH 4/9] run-command: add support for timeout in command finisher Siddh Raman Pant
2026-05-21 7:21 ` Johannes Sixt
2026-05-21 8:39 ` Oswald Buddenhagen
2026-05-21 9:59 ` Siddh Raman Pant [this message]
2026-05-21 14:36 ` Johannes Sixt
2026-05-22 0:10 ` Junio C Hamano
2026-05-22 5:46 ` Siddh Raman Pant
2026-05-22 5:10 ` Jeff King
2026-05-22 5:59 ` Siddh Raman Pant
2026-05-19 16:30 ` [PATCH 5/9] wrapper: add support for timeout and deadline in read helpers Siddh Raman Pant
2026-05-19 16:30 ` [PATCH 6/9] t3301: cover generic displayed notes behavior Siddh Raman Pant
2026-05-19 16:30 ` [PATCH 7/9] notes: support an external command to display notes Siddh Raman Pant
2026-05-20 0:03 ` Junio C Hamano
2026-05-20 6:59 ` Siddh Raman Pant
2026-05-21 1:12 ` brian m. carlson
2026-05-21 4:12 ` Siddh Raman Pant
2026-05-21 21:18 ` brian m. carlson
2026-05-19 16:30 ` [PATCH 8/9] Documentation: document external notes command options Siddh Raman Pant
2026-05-19 16:30 ` [PATCH 9/9] t: add tests for external notes command Siddh Raman Pant
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2f7eea03273ffaacc50a9ae186673da88fc3345f.camel@oracle.com \
--to=siddh.raman.pant@oracle$(echo .)com \
--cc=code@khaugsbakk$(echo .)name \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--cc=j6t@kdbg$(echo .)org \
--cc=newren@gmail$(echo .)com \
--cc=oswald.buddenhagen@gmx$(echo .)de \
--cc=ps@pks$(echo .)im \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox