Re: [PATCHv3 02/11] run-command: report failure for degraded output just once

On Wed, Nov 4, 2015 at 11:32 PM, Junio C Hamano [off-list ref] wrote:

Jeff King [off-list ref] writes:

quoted

POSIX implies it is the case in the definition of read[2] in two ways:

  1. The O_NONBLOCK behavior for pipes is mentioned only when dealing
     with empty pipes.

  2. Later, it says:

       The value returned may be less than nbyte if the number of bytes
       left in the file is less than nbyte, if the read() request was
       interrupted by a signal, or if the file is a pipe or FIFO or
       special file and has fewer than nbyte bytes immediately available
       for reading.

     That is not explicit, but the "immediately" there seems to imply
     it.

We were reading the same book, but I was more worried about that
"may" there; it merely tells the caller of read(2) not to be alarmed
when the call returned without filling the entire buffer, without
mandating the implementation of read(2) never to block.

Having said that,...

quoted

quoted

So perhaps the original reasoning of doing nonblock was faulty, you
are saying?

I agree that the original reasoning was faulty. It happened in the first place,
because of how I approached the problem. (strbuf_read should return immediately
after reading and to communicate that we had non blocking read and checked for
EAGAIN).

Having read the man pages again, I agree with you that the non blocking is
bogus to begin with.

quoted

Exactly. And therefore a convenient way to deal with the portability
issue is to get rid of it. :)

... I do like the simplification you alluded to in the other
message.  Not having to worry about the nonblock (at least until it
is found problematic in the real world) is a very good first step,
especially because the approach allows us to collectively make
progress by letting all of us in various platforms build and
experiment with "something that works".

I'll send a patch to just remove set_nonblocking which should fix the compile
problems on Windows and make it work regardless on all platforms.

After that I continue with the update series.