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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Nov 4, 2015 at 1:19 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Stefan Beller <sbeller@xxxxxxxxxx> writes:
>
>> So more like:
>>
>>     if (platform_capable_non_blocking_IO())
>>         set_nonblocking_or_die(&pp->children[i].process.err);
>>     else
>>         pp->children[i].process.err = 2; /* ugly intermixed output is possible*/
>
> When I mentioned #if..#endif, I didn't mean it as a dogmatic
> "conditional compilation is wrong" sense.  It was more about "the
> high-level flow of logic should not have to know too much about
> platform peculiarities".  As platform_capable_non_blocking_IO()
> function would be a constant function after you compile it for a
> single platform, if you add 10 instances of such if/else in a patch
> that adds 250 lines, unless the change is to add a set of lowest
> level helpers to be called from the higher-level flow of logic so
> that the callers do not have to know about the platform details,
> that's just as bad as adding 10 instances of #if..#endif.

Right. Yeah I was just outlining the program flow as I understood it.

Specially:
 * It's fine if the platform doesn't support non blocking IO
 * but if the platform claims to support it and fails to, this is a hard error.
    How is this worse than the platform claiming to not support it at all?

I mean another solution there would be to try to set the non blocking IO
and if that fails we put that process-to-be-started into a special queue,
which will start the process once the stderr channel is not occupied any more
(i.e. the process which is the current output owner has ended or there is
no such process)

That way we would fall back to no parallelism in Windows and could
even gracefully fallback in other systems, which suddenly have problems
setting non blocking IO. (Though I suspect that isn't required here).

>
>>> On the other hand, on a platform that is known to be incapable
>>> (e.g. lacks SETFL or NONBLOCK), we have two options.
>>>
>>> 1. If we can arrange to omit the intermediary buffer processing
>>>    without butchering the flow of the main logic with many
>>>    #ifdef..#endif, then that would make a lot of sense to do so, and
>>>    running the processes in parallel with mixed output might be OK.
>>>    It may not be very nice, but should be an acceptable compromise.
>>
>> From what I hear this kind of output is very annoying. (One of the
>> main complaints of repo users beside missing atomic fetch transactions)
>
> When (1) "parallelism with sequential output" is the desired
> outcome, (2) on some platforms we haven't found a way to achieve
> both, and (3) a non-sequential output is unacceptable, then
> parallelism has to give :-(.

Ok, then I will go that route.

>
> I was getting an impression from your "not buffer" suggestion that
> "sequential output" would be the one that can be sacrificed, but
> that is OK.  Until we find a way to achieve both at the same time,
> achieving only either one or the other is better than achieving
> nothing.

I was not sure what is better to sacrifice. Scarifying parallelism sounds
safer to me (no apparent change compared to before).

>
>>> Either way, bringing "parallelism with sequential output" to
>>> platforms without nonblock IO can be left for a later day, when we
>>> find either (1) a good approach that does not require nonblock IO to
>>> do this, or (2) a good approach to do a nonblock IO on these
>>> platforms (we know about Windows, but there may be others; I dunno).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]