Re: [PATCH] init - Honour the global core.filemode setting

From: Torsten Bögershausen <tboegi@web.de>
Date: Sep 28, 2014, 11:52 UTC
Message-ID: <5427F68E.5030003@web.de>
In-Reply-To: <CAE1pOi0zhnUNNdHsrq+4H_6LiFnr-qoY-owrcJquy6dyG+Mk4g@mail.gmail.com>

On 2014-09-28 02.37, Hilco Wijbenga wrote:

Show 8 quoted lines

> If "~/.gitconfig" contains a "core.filemode" entry then "git init"
> should honour that setting.
> 
> Signed-off-by: Hilco Wijbenga <hilco.wijbenga@gmail.com>
> ---
> This bit me at work where I have to work with Windows. Git on Cygwin
> and the Eclipse Git plugin do not agree on file attributes so I had
> set "filemode = false" in ~/.gitconfig.

This feels strange. Each and every repo has a core.filemode setting. Or should have.

Did you manage to create a repo without core.filemode in repo/.git/config ? And if yes, how?

> 
> A few weeks later, I did a "git init" and, some time later yet, I
> noticed the strange behaviour of Cygwin/Eclipse again.

I do not fully understand which "strange behaviour" you experied, so I need to guess.

 This was very

> surprising because things had been working well until then. It took
> quite a bit of research before I realized that "git init" always sets
> "filemode". I think "filemode" should only be set if not set already
> in the global config (similar to log_all_ref_updates).

That is part of the whole story: In general, "git init" probes the file system, if the executable bit is working as expected. So if you create a Git repository under VFAT, the executable bit is not supported.

Git will notice that, and set core.filemode = false.

NTFS is a different story: Cygwin has support for the executable bit under NTFS, but Msysit does not. So if you "share" a Git repository between Msysgit and cygwin, it may be better to set core.filemode to false.

There is however a problem with your patch, or 2:

When you set core.filemode = false in your ~/.gitconfig, another developer may have core.filemode = true in his config. If you manage to share the repo using a network, git will behave different for the 2 users. Solution: Set core.filemode for this repo alwways in the repo. (as we do today in git.git)

When you run "git init" with ~/.gitconfig = true, you should anyway probe the file system, as it may not support file mode, and core.filemode may be false.

So the solution that I can see is: (Some pseudo-code:)

if (git config (global config ) == false) ||
   (git config (~/.config ) == false) then
  git_config_set("core.filemode", "false");
else
  probe the file system and set core.filemode as we do today
fi

Show 8 quoted lines

> 
> The usual caveat applies: this is my first patch. Having said that,
> please feel free to be pedantic and strict. It's a small patch so I
> would imagine that fixing any problems should not take long (assuming
> it is acceptable at all, of course). I'd like to know I did it right.
> :-)
> 
> AFAICT, all tests passed. Should a separate test be added for this change?

I think yes.

Under which system did you test ?

Windows? CYWGIN ? MingWW/Msysgit ? Linux ?

Show 18 quoted lines

> - /* Check filemode trustability */
> - filemode = TEST_FILEMODE;
> - if (TEST_FILEMODE && !lstat(path, &st1)) {
> - struct stat st2;
> - filemode = (!chmod(path, st1.st_mode ^ S_IXUSR) &&
> - !lstat(path, &st2) &&
> - st1.st_mode != st2.st_mode);
> + /* Do not override the global filemode setting. */
> + if (trust_executable_bit == -1) {
> + /* Check filemode trustability */
> + filemode = TEST_FILEMODE;
> + if (TEST_FILEMODE && !lstat(path, &st1)) {
> + struct stat st2;
> + filemode = (!chmod(path, st1.st_mode ^ S_IXUSR) &&
> + !lstat(path, &st2) &&
> + st1.st_mode != st2.st_mode);
> + }
> + git_config_set("core.filemode", filemode ? "true" : "false");

The indentation seems to be broken ? (We use one TAB, for better info please see Documentation/CodingGuidelines) [snip]