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

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 16, 2011, 22:10 UTC
Message-ID: <7v8vt1h1g5.fsf@alter.siamese.dyndns.org>
In-Reply-To: <4DFA6632.40607@ramsay1.demon.co.uk>

Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:

Show 5 quoted lines

> The 'forced modes' test fails on cygwin because the post-update
> hook loses it's executable bit when copied from the templates
> directory by git-init. The template loses it's executable bit
> because the lstat() function resolves to the "native Win32 API"
> implementation.

Show 7 quoted lines

> This call to lstat() happens after git-init has set the "git_dir"
> (so has_git_dir() returns true), but before the configuration has
> been fully initialised. At this point git_config() does not find
> any config files to parse and returns 0. Unfortunately, the code
> used to determine the cygwin l/stat() function bindings did not
> check the return from git_config() and assumed that the config
> was complete and accessible once "git_dir" was set.

Ok, so this is not really about "a test fails so we will sweep the issue under rag", but "we try to optimize too early, before we have enough information, so let the code take slow path before we know what is in the configuration file".

Show 23 quoted lines

> In order to fix the test, we simply change the binding code to
> test the return value from git_config(), to ensure that it actually
> had config values to read, before determining the requested binding.
>
> Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
> ---
>  compat/cygwin.c |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git a/compat/cygwin.c b/compat/cygwin.c
> index b4a51b9..b38dbd7 100644
> --- a/compat/cygwin.c
> +++ b/compat/cygwin.c
> @@ -114,8 +114,7 @@ static int git_cygwin_config(const char *var, const char *value, void *cb)
>  
>  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;