Thread (1 message) 1 message, 1 author, 2016-06-15

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:51:29

Johannes Sixt [off-list ref] writes:
quoted
 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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help