Re: git.git as of tonight

From: Stefan Beller <sbeller@google.com>
Date: Nov 3, 2015, 23:00 UTC
Message-ID: <CAGZ79kZM-Q2oxVkMO9=v=tAdJkpWWOTVkaSMRDKZSZia2MY5Ng@mail.gmail.com>
In-Reply-To: <56392106.1010401@kdbg.org>

On Tue, Nov 3, 2015 at 1:03 PM, Johannes Sixt <j6t@kdbg.org> wrote:

Show 36 quoted lines

> Am 03.11.2015 um 19:18 schrieb Stefan Beller:
>>
>> ... ReadFileEx ... "overlapped" operation.
>
>
> Let's not go there just yet.
>
>>>   1. Make this an optional feature so that platforms can compile it
>>>      out, if it is not already done.  My preference, even if we go
>>>      that route, would be to see if we can find a way to preserve the
>>>      overall code structure (e.g. instead of spawning multiple
>>>      workers, which is why the code needs NONBLOCK to avoid getting
>>>      stuck on reading from one while others are working, perhaps we
>>>      can spawn only one and not do a nonblock read?).
>>
>>
>> Yeah that would be my understanding as well. If we don't come up with
>> a good solution for parallelism in Windows now, we'd need to make it at
>> least working in the jobs=1 case as well as it worked before.
>
>
> That should be possible. I discovered today that we have this function:
>
> 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");
> }
>
> Notice that it is not a fatal condition if O_NONBLOCK cannot be established.
> (BTW, did you ever test this condition?)

No, as I viewed it more like a severe problem, not to be happen in the near future. But if it were to happen we would still need to finish the command instead of giving up because of degraded output. (I would not know how to test this system call to fail, so maybe I am just making up excuses)

I added an #ifdef just as you proposed below and the output itself doesn't look too bad except for the warning message themselves. If we'd just remove them it would look better to me.

So maybe we could just go with

    static void set_nonblocking(int fd)
    {
    #ifndef GIT_WINDOWS_NATIVE
        int flags = fcntl(fd, F_GETFL);
        if (!(flags < 0))
            fcntl(fd, F_SETFL, flags | O_NONBLOCK)
    #endif
    }

and see how people react to the output then?

Show 22 quoted lines

> If we add two lines (which remove
> the stuff that does not work on Windows) like this:
>
> static void set_nonblocking(int fd)
> {
> #ifndef GIT_WINDOWS_NATIVE
>         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))
> #endif
>                 warning("Could not set file status flags, "
>                         "output will be degraded");
> }
>
> we should get something that works, theoretically. We still need a more
> complete waitpid emulation, but that does not look like rocket science. I'll
> investigate further in this direction.
>
> -- Hannes
>