Re: [PATCH] compat/cygwin.c - Use cygwin's stat if core.filemode == true

[Junio C Hamano <gitster@pobox.com>]

Mark Levedahl <mlevedahl@gmail.com> writes:

> Junio C Hamano wrote:
>
>> Before you do that, can you explain why git_cygwin_config() should
>> duplicate the setting of that variable instead of using the existing
>> parser for that variable by calling git_default_config()?
>
> I don't like traversing the same list of configuration variables /
> files twice, each time looking for a different variable just so that
> one of those traversals can use a standard function, rather than just
> traversing once and getting exactly what this routine needs; this
> seems wasteful to me. However, I am (obviously) not the maintainer of
> this code so I will do this however it is desired.

I am not sure what you meant by "traversing twice"; when the first call to
stat or lstat is made, you call init_stat() and read the config file,
feeding each and every variable definition to the callback functions.  And
then whenever the command implementations call git_config() to getting the
configuration necessary for their own use, they read the config, again
feeding each and every variable definition to the callback functions.
This is "traversing twice" which is unfortunate, but that is not something
you can control from this codepath.

But in any case, I was worried about something different.

If you call git_default_config() as a fallback from git_cygwin_config(),
which is what all the other standard configuration callback functions do,
and/or set trust_executable_bit, command implementations that do not call
git_default_config() on their own would see different trust_executable_bit
value when running on non-Cygwin platform and on Cygwin.

I was wondering if git_cygwin_config() was originally written not to call
git_default_config() because some command implementations do not to want
to call git_default_core_config() (and/or read trust_executable_bit
variable from the configuration) for some reason (which would be just
hiding bugs in other parts of the system, I suspect).

If that is the case, we would have to fix such broken parts of the system,
but until that happens your original patch to use a separate variable and
keeping trust_executable_bit untouched would be much safer than this
latest patch.  Hence the question.