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

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 11, 2008, 21:34 UTC
Message-ID: <7v1vymke85.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <1223751859-3540-1-git-send-email-mlevedahl@gmail.com>

Mark Levedahl <mlevedahl@gmail.com> writes:

Show 5 quoted lines

> Cygwin's POSIX emulation allows use of core.filemode true, unlike native
> Window's implementation of stat / lstat, and Cygwin/git users who have
> configured core.filemode true in various repositories will be very
> unpleasantly surprised to find that git is no longer honoring that option.
> So, this patch fores use of Cygwin's stat functions if core.filemode is

s/fores/forces/;

>  static int native_stat = 1;
> +static int core_filemode = 0;

Makes me wonder why "trust_executable_bit" is unavailable here. Perhaps git_cygwin_config() does not fall back to git_default_config() for a reason?

Show 12 quoted lines

>  static int git_cygwin_config(const char *var, const char *value, void *cb)
>  {
> +	if (!strcmp(var, "core.filemode")) {
> +			core_filemode = git_config_bool(var, value);
> +			native_stat &= !core_filemode;
> +	}
>  	if (!strcmp(var, "core.ignorecygwinfstricks"))
> -		native_stat = git_config_bool(var, value);
> +		native_stat = git_config_bool(var, value) &&
> +			!core_filemode;
>  	return 0;
>  }

If you can safely determine if you would want to use cygwin_stat or not only after you have read both core.filemode and core.ignorecygwinfstricks, perhaps keeping the config reader as is (this includes not falling back to git_default_config()) and instead doing:

	static int init_stat(void)
	{
		...
		git_config(git_sygwin_config, NULL);
		if (!core_filemode && native_stat) {
			cygwin_stat_fn = cygwin_stat;
			cygwin_lstat_fn = cygwin_lstat;
		} else {
			cygwin_stat_fn = stat;
			cygwin_lstat_fn = lstat;
		}
		...

is less yucky?