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

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 12, 2008, 23:39 UTC
Message-ID: <7vd4i5fkny.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <48F25CB6.10702@gmail.com>

Mark Levedahl <mlevedahl@gmail.com> writes:

Show 12 quoted lines

> 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.