Re: [PATCH 1/2] run-command: Remove set_nonblocking
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Nov 5, 2015, 19:37 UTC
- Message-ID
- <xmqqio5g5jc3.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <CAGZ79kZgEtKoYqxa8+wj0PCJedW140CQAPX6XwEYib1W3fPhXw@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
Show 15 quoted lines
> On Thu, Nov 5, 2015 at 10:45 AM, Junio C Hamano <gitster@pobox.com> wrote: >> Stefan Beller <sbeller@google.com> writes: >> >>> strbuf_read_once can also operate on blocking file descriptors if we are >>> sure they are ready. The poll (2) command however makes sure this is the >>> case. >>> >>> Reading the manual for poll (2), there may be spurious returns indicating >>> readiness but that is for network sockets only. Pipes should be unaffected. >> >> Given the presence of "for example" in that bug section, I wouldn't >> say "only" or "should be unaffected". > > Reading the documentation we are in agreement, that we expect > no spurious returns, no?
Given the presence of "for example" in that bug section, I wouldn't say "only" or "should be unaffected". I cannot say "we expect no spurious returns".
>> Thanks. Let's apply these fixes on sb/submodule-parallel-fetch, >> merge the result to 'next' and have people play with it. > > Maybe the commit message was weakly crafted. Do you want me to resend?
I somehow feel that it is prudent to let this cook just above 'next' for a few days (not just for the log message but to verify the strategy and wait for others to come up with even better ideas), but then I'll be offline starting next week, so I expect that merging the final version to 'next' will be done by our interim maintainer, which means we still have time to polish ;-)
Here is what I queued for now.
-- >8 -- From: Stefan Beller <sbeller@google.com> Date: Thu, 5 Nov 2015 10:17:18 -0800 Subject: [PATCH] run-command: remove set_nonblocking()
strbuf_read_once can also operate on blocking file descriptors if we are sure they are ready. And the poll(2) we call before calling this ensures that this is the case.
Reading the manual for poll(2), there may be spurious returns indicating readiness but that is for network sockets only and pipes should be unaffected.
With this change, we rely on
- poll(2) returns only non-empty pipes; and - read(2) on a non-empty pipe does not block.
This should fix compilation on Windows.
Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- run-command.c | 13 ------------- 1 file changed, 13 deletions(-)
diff --git a/run-command.c b/run-command.c index 1fbd286..07424e9 100644 --- a/run-command.c +++ b/run-command.c @@ -996,17 +996,6 @@ static void pp_cleanup(struct parallel_processes *pp) sigchain_pop_common(); } -static void set_nonblocking(int fd) -{ - int flags = fcntl(fd, F_GETFL); - if (flags < 0) - warning("Could not get file status flags, " - "output will be degraded"); - else if (fcntl(fd, F_SETFL, flags | O_NONBLOCK)) - warning("Could not set file status flags, " - "output will be degraded"); -} - /* returns * 0 if a new task was started. * 1 if no new jobs was started (get_next_task ran out of work, non critical @@ -1042,8 +1031,6 @@ static int pp_start_one(struct parallel_processes *pp) return code ? -1 : 1; } - set_nonblocking(pp->children[i].process.err); - pp->nr_processes++; pp->children[i].in_use = 1; pp->pfd[i].fd = pp->children[i].process.err;
-- 2.6.2-539-g1c5cd50