git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Johannes Sixt <j6t@kdbg.org>
Date
Jun 17, 2011, 08:12 UTC
Message-ID
<4DFB0C66.5080904@kdbg.org>
In-Reply-To
<4DFA6632.40607@ramsay1.demon.co.uk>
Am 16.06.2011 22:23, schrieb Ramsay Jones:
Show 38 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.
> 
> 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.
> 
> 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;

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?

[*] Note to bystanders: compat/cygwin.c keeps its own copy of core.filemode; see the comments near these variables.

-- Hannes
Previous: Ramsay JonesNext: Junio C Hamano
Message 4 of 7 in “t1301-*.sh: Fix the 'forced modes' test on cygwin”
  1. 2/3 t1301-*.sh: Fix the 'forced modes' test on cygwinRamsay Jones, Jun 16, 2011
  2. Junio C HamanoJun 16, 2011
  3. Ramsay JonesJun 17, 2011
  4. Johannes SixtJun 17, 2011
  5. Junio C HamanoJun 17, 2011
  6. Ramsay JonesJun 18, 2011
  7. Re* [PATCH 2/3] t1301-*.sh: Fix the 'forced modes' test on cygwinJunio C Hamano, Jun 20, 2011

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.