Re: [PATCH] compat: Allow static initializer for pthreads on Windows

From: Johannes Sixt <j6t@kdbg.org>
Date: Oct 27, 2016, 20:05 UTC
Message-ID: <a2e5acd6-485d-0387-7a85-6042dee702f7@kdbg.org>
In-Reply-To: <CAGZ79kY_fZ_pDtVnwJoDkR6PjTNoqDMN5OC70Z8SH_J0Wvkq-w@mail.gmail.com>

Am 27.10.2016 um 21:08 schrieb Stefan Beller:

Show 20 quoted lines

> On Thu, Oct 27, 2016 at 11:49 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Johannes Sixt <j6t@kdbg.org> writes:
>>
>>> Am 27.10.2016 um 19:01 schrieb Stefan Beller:
>>> ...
>>> It is not possible to mark a mutex uninitialized on Windows without an
>>> extra piece of data. A solution would become quite complicated quite
>>> quickly, and at the cost of additional operations that are in the same
>>> ballpark as an uncontended mutex. I'm not enthused.
>>>
>>>> The positive aspect of this way the patch proposes would be that any
>>>> future contributor not knowing the details of how to do mutexes right
>>>> on Windows, would not totally break the whole system, i.e. this seems
>>>> to be more maintainable in the future as it reduces the friction between
>>>> pthreads mutexes and the way we can do things in Git in a platform
>>>> independent way
>>>
>>> This is a non-argument. Coders have to know their tools.
>
> Windows is not my tool.

The tool I meant is pthreads. For example, you can't have a pthread_mutex_t variable and not initialize it with either PTHREAD_MUTEX_INITIALIZER or pthread_mutex_init.

Show 7 quoted lines

>> The codebase should strive to give coders a coherent abstraction
>> that can be implemented efficiently on platforms, so that coders do
>> not have to care too deeply about quirks that exist on individual
>> platforms.
>
> Currently working as a coder I care about "submodules, that work on
> linux." I do not care about Windows in the big picture.

I don't know what you meant to say with this sentence, but taken literally, are you on the right ship here, I have to ask?

> I am however
> willing to go an extra step to not break Windows.

"Not break Windows" is equivalent to "make it work on Windows", mind you. We can't have a new feature only on Linux when there is no reason not to have it on Windows as well. Sorry, "Git is for Unix only" is long over.

Show 7 quoted lines

> However that requires
> a little bit of effort from both me and you:
> * I need to be aware of what I cannot do with "not-my-tools". (So please
>   somebody tell me, also in the future when I break obscure platforms. Mind
>   that I don't equate obscure with not widely used or in any other way negative.
>   It's just that people working on linux find some quirks on Windows
> not "obvious")

That goes without saying.

> * the workaround should not be too time consuming in the bigger picture,

That, however, is wishful thinking. If you want to have a feature dearly, you have to make it work, and that may take its time. I'm not saying that *you* have to make it work (there are platform experts around to lend a hand), but just an extra step to "not break" an important platform is not enough.

>   which is why I propose to make the API a bit clearer by emulating posix
>   mutexes harder. (From a Windows POV this might sound like making it
>   more obscure because posix mutexes itself are obscure.)

So, when you think that POSIX mutexes are obscure, why don't we settle on the simpler Windows critical sections? Emulating them with pthreads should be child's play.

> The implementation under discussion (well we did not discuss the
> implementation a
> whole lot yet) ...

There's not a whole lot to discuss: it must be rewritten from scratch (it's not just the memory barriers, it is everything else, too). But time is much better spent on an attr_start() solution.

-- Hannes