Re: [PATCH 2/3] t1301-*.sh: Fix the 'forced modes' test on cygwin

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 17, 2011, 21:27 UTC
Message-ID: <7vsjr8f8sl.fsf@alter.siamese.dyndns.org>
In-Reply-To: <4DFB0C66.5080904@kdbg.org>

Johannes Sixt <j6t@kdbg.org> writes:

Show 25 quoted lines

>>  static int init_stat(void)
>>  {
>> -	if (have_git_dir()) {
>> -		git_config(git_cygwin_config, NULL);
>> +	if (have_git_dir() && git_config(git_cygwin_config,NULL)) {
>>  		if (!core_filemode && native_stat) {
>>  			cygwin_stat_fn = cygwin_stat;
>>  			cygwin_lstat_fn = cygwin_lstat;
>
> So, this means that if neither core.filemode nor
> core.ignorecygwinfstricks is assigned a value, then regular (Cygwin's)
> l/stat is used. Ok, that's what we need: the default value of
> core.filemode is true, which means we need Cygwin's l/stat; it trumps
> the default value of core.ignorecygwinfstricks, which is also
> true. Good!
>
> BTW, it seems the patch fixes a bug when the two config parameters are
> not assigned a value: the initialization looks like this[*]:
>
> static int native_stat = 1;
> static int core_filemode;
>
> i.e., the default value of core.filemode seen by compat/cygwin.c is
> actually false, and the fast native l/stat would be used, contrary to
> the documentation. Am I missing something?

Probably you are not missing anything. It is a regression introduced by 7974843 (compat/cygwin.c: make runtime detection of lstat/stat lessor impact, 2008-10-23).

What the added "&& git_config()" is doing is that until we actually open and read .git/config and the like, we do not take that if (), meaning we leave the cygwin_stat_fn pointing at the cygwin_stat_stub (same for lstat), using the "unoptimized" cygwin version. Once we do read from config we are likely to have core.filemode defined (prepared by git init), so the "default" value here would probably not matter in practice.