Re: [PATCH v2 1/6] compat: add function to enable nonblocking pipes
From: Jeff King <hidden>
Date: 2022-08-18 05:41:45
On Wed, Aug 17, 2022 at 01:23:25PM -0700, Junio C Hamano wrote:
The only small "huh?" factor was that the name of the helper is not enable_nonblock(), but singles out pipe as valid thing to work on. I think it is perfectly fine, given that the current plan we have is to use this on the pipe with the command we spawn with pipe_command(), and it probably is even preferrable to explicitly declare that this is designed to only work with pipes and future developers who try to abuse it on other kinds of file descriptors are on their own. And "pipe" in the name of this helper may be such a declaration that is strong enough.
My goal was to explain that "huh" with this bit in the commit message:
quoted
The interface is as narrow as possible to let platforms do what's natural there (rather than having to implement fcntl() and a fake O_NONBLOCK for example, or having to handle other types of descriptors).
without going into too many details. I do think it would be easier to explain if the Windows implementation was added in the same commit (since it is explicitly just about pipes), but I wanted to credit René as the author there. I'm not sure if it's worth folding them together and adding a co-author credit instead. Or maybe I should state outright in this commit message that we're doing it for Windows. ;) -Peff