Re: [PATCH 08/14] help.c: Fix detection of custom merge strategy on cygwin

[Erik Faye-Lund <kusmabite@gmail.com>]

On Tue, Dec 14, 2010 at 7:29 PM, Ramsay Jones
<ramsay@ramsay1.demon.co.uk> wrote:
>
> Test t7606-merge-custom.sh fails on cygwin when git-merge fails
> with an "Could not find merge strategy 'theirs'" error, despite
> the test correctly preparing an (executable) git-merge-theirs
> script.
>
> The cause of the failure is the mis-detection of the executable
> status of the script, by the is_executable() function, while the
> load_command_list() function is searching the path for additional
> merge strategy programs.
>
> Note that the l/stat() "functions" on cygwin are somewhat
> schizophrenic (see commits adbc0b6, 7faee6b and 7974843), and
> their behaviour depends on the timing of various git setup and
> config function calls. In particular, until the "git_dir" has
> been set (have_git_dir() returns true), the real cygwin (POSIX
> emulating) l/stat() functions are called. Once "git_dir" has
> been set, the "native Win32 API" implementations of l/stat()
> may, or may not, be called depending on the setting of the
> core.filemode and core.ignorecygwinfstricks config variables.
>
> We also note that, since commit c869753, core.filemode is forced
> to false, even on NTFS, by git-init and git-clone. A user (or a
> test) can, of course, reset core.filemode to true explicitly if
> the filesystem supports it (and he doesn't use any problematic
> windows software). The test-suite currently runs all tests on
> cygwin with core.filemode set to false.
>
> Given the above, we see that the built-in merge strategies are
> correctly detected as executable, since they are checked for
> before "git_dir" is set, whereas all custom merge strategies are
> not, since they are checked for after "git_dir" is set.
>
> In order to fix the mis-detection problem, we change the code in
> is_executable() to re-use the conditional WIN32 code section,
> which actually looks at the content of the file to determine if
> the file is executable. On cygwin we also make the additional
> code conditional on the executable bit of the file mode returned
> by the initial stat() call. (only the real cygwin function would
> set the executable bit in the file mode.)
>
> Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
> ---
>  help.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/help.c b/help.c
> index 7f4928e..eabadc9 100644
> --- a/help.c
> +++ b/help.c
> @@ -126,7 +126,10 @@ static int is_executable(const char *name)
>            !S_ISREG(st.st_mode))
>                return 0;
>
> -#ifdef WIN32
> +#if defined(WIN32) || defined(__CYGWIN__)
> +#if defined(__CYGWIN__)
> +if ((st.st_mode & S_IXUSR) == 0)
> +#endif

Perhaps the first check should simply be changed to check for _WIN32
instead of WIN32? IIRC _WIN32 is set on Cygwin, but I could be
mistaken...