[PATCH] init - Honour the global core.filemode setting
If "~/.gitconfig" contains a "core.filemode" entry then "git init"
should honour that setting.
Signed-off-by: Hilco Wijbenga
---
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.
A few weeks later, I did a "git init" and, some time later yet, I
noticed the strange behaviour of Cygwin/Eclipse again. 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).
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 used "git format-patch" and "git imap-send" to send this patch to
the ML but looking below I still do not see tabs? In fact, I do not
see any indentation.)
builtin/init-db.c | 19 +++
environment.c | 2 +-
2 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 56f85e2..19cdc58 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -248,15 +248,18 @@ static int create_default_files(const char *template_path)
path[len] = 0;
strcpy(path + len, "config");
- /* 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");
}
- git_config_set("core.filemode", filemode ? "true" : "false");
if (is_bare_repository())
git_config_set("core.bare", "true");
diff --git a/environment.c b/environment.c
index 565f652..875a498 100644
--- a/environment.c
+++ b/environment.c
@@ -12,7 +12,7 @@
#include "fmt-merge-msg.h"
#include "commit.h"
-int trust_executable_bit = 1;
+int trust_executable_bit = -1;
int trust_ctime = 1;
int check_stat = 1;
int has_symlinks = 1;
--
2.1.1.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] init - Honour the global core.filemode setting
On 2014-09-28 02.37, Hilco Wijbenga wrote:
> If "~/.gitconfig" contains a "core.filemode" entry then "git init"
> should honour that setting.
>
> Signed-off-by: Hilco Wijbenga
> ---
> 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
>
> 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 ?
> - /* 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]
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] init - Honour the global core.filemode setting
Hi Torsten,
Thank you for taking the time to review my patch.
On 28 September 2014 04:52, Torsten Bögershausen wrote:
> On 2014-09-28 02.37, Hilco Wijbenga wrote:
>> If "~/.gitconfig" contains a "core.filemode" entry then "git init"
>> should honour that setting.
>>
>> Signed-off-by: Hilco Wijbenga
>> ---
>> 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?
Perhaps I completely misunderstand the meaning of core.filemode but I
thought it determined whether Git cared about changes in file
properties? So this is client OS related and independent of the repo?
>> 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.
The problem is simply that Eclipse's Git sees changes that Cygwin's
Git does not. It's some sort of unfortunate consequence of trying to
pretend to be Linux on Windows, I guess. The only way to get them to
agree was to set core.filemode to false. Now you might rightly argue
that Eclipse and/or Windows and/or Cygwin should be fixed but that's a
much bigger undertaking than simply setting an existing Git property.
:-)
> 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.
Agreed. That is what I concluded from the code.
> So if you "share" a Git repository between Msysgit and cygwin, it may be
> better
> to set core.filemode to false.
Possibly. I would argue that that is up to the individual developer.
> 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.
Isn't that what everything in ~/gitconfig is for? So that I can set
attributes appropriate to my working environment? Besides, that is
already the case if developer A uses a VFAT system and developer B
uses NTFS or JFS or EXTn or ..., right? (As you also indicated above.)
> Solution:
> Set core.filemode for this repo alwways in the repo. (as we do today in
> git.git)
I suppose you could set it to false, yes. But then it affects
everybody, that seems like going for the lowest common denominator?
> 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
Yeah, I actually considered that (well, something less complete,
actually :-) ) but decided to go for the simpler approach that I
showed. My assumption is that the developer working with the repo
knows what he is doing. It seems wrong for Git to override that
decision. Then again, I don't really see any advantage in setting
core.filemode to true when working with, say, a VFAT filesystem, so
ignoring it in that case might be okay. Would such a setup do active
damage, though?
>> 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.
Okay, I'll have to figure out how to do that.
> Under which system did you test ?
>
> Windows?
> CYWGIN ?
> MingWW/Msysgit ?
> Linux ?
Only Linux. I don't really run Windows at home.
>> - /* 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
Re: [PATCH] init - Honour the global core.filemode setting
Hilco Wijbenga writes: > Perhaps I completely misunderstand the meaning of core.filemode but I > thought it determined whether Git cared about changes in file > properties? By setting it to "false", you tell Git that the filesystem you placed the repository does not correctly represent the filemode (especially the executable bit). "core.fileMode" in "git config --help" reads: core.fileMode If false, the executable bit differences between the index and the working tree are ignored; useful on broken filesystems like FAT. See git-update- index(1). The default is true, except git-clone(1) or git-init(1) will probe and set core.fileMode false if appropriate when the repository is created. Maybe our documentation is not clear enough. A contribution from somebody new to Git we would appreciate would be to point out which part of these sentences are unclear; that way, people can work on improving its phrasing. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] init - Honour the global core.filemode setting
On 2014-10-01 19.10, Junio C Hamano wrote: > Hilco Wijbenga writes: > >> Perhaps I completely misunderstand the meaning of core.filemode but I >> thought it determined whether Git cared about changes in file >> properties? > > By setting it to "false", you tell Git that the filesystem you > placed the repository does not correctly represent the filemode > (especially the executable bit). > > "core.fileMode" in "git config --help" reads: > >core.fileMode >If false, the executable bit differences between the >index and the working tree are ignored; useful on broken >filesystems like FAT. See git-update- index(1). Out of my head: Could the following be a starting point: core.fileMode If false, the executable bit differences between the index and the working tree are ignored. This may be usefull when visiting a cygwin repo with a non-cygwin Git client. (should we mention msysgit ? should we mention JGit/EGit ?) This may even be useful for a repo on a SAMBA network mount, which may show all file permissions as 0755. See git-update-index(1) for changing the executable bit in the index. The default is true, except git-clone(1) or git-init(1) will probe and set core.fileMode false if appropriate when the repository is created. > > Maybe our documentation is not clear enough. A contribution from > somebody new to Git we would appreciate would be to point out which > part of these sentences are unclear; that way, people can work on > improving its phrasing. > > Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] init - Honour the global core.filemode setting
Torsten Bögershausen writes: > On 2014-10-01 19.10, Junio C Hamano wrote: >> Hilco Wijbenga writes: >> >>> Perhaps I completely misunderstand the meaning of core.filemode but I >>> thought it determined whether Git cared about changes in file >>> properties? >> >> By setting it to "false", you tell Git that the filesystem you >> placed the repository does not correctly represent the filemode >> (especially the executable bit). >> >> "core.fileMode" in "git config --help" reads: >> >>core.fileMode >>If false, the executable bit differences between the >>index and the working tree are ignored; useful on broken >>filesystems like FAT. See git-update- index(1). > > Out of my head: Could the following be a starting point: > > core.fileMode > If false, the executable bit differences between the > index and the working tree are ignored. > This may be usefull when visiting a cygwin repo with a non-cygwin > Git client. (should we mention msysgit ? should we mention > JGit/EGit ?) Between these two sentences, there may still be the same cognitive gap that may have lead to the original confusion. The first sentence says what happens, as it should. But it is not directly clear what makes the executable bit differ and when it is a useful thing to ignore the differences, so the second sentence that says "This may be useful" does not give the reader very much. Here is my attempt. Tells Git if the executable bit of files in the working tree is to be honored. Some filesystems lose the executable bit when a file that is marked as executable is checked out, or checks out an non-executable file with executable bit on. "git init" and "git clone" probe the filesystem to see if it records executable bit correctly when they create a new repository and this variable is automatically set as necessary. A repository, however, may be on a filesystem that records the filemode correctly, and this variable is set to 'true' when created, but later may be made accessible from another environment that loses the filemode (e.g. exporting ext4 via CIFS mount, visiting a Cygwin managed repository with MsysGit). In such a case, it may be necessary to set this to 'false'. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] init - Honour the global core.filemode setting
On 2014-10-02 19.02, Junio C Hamano wrote: > Torsten Bögershausen writes: > >> On 2014-10-01 19.10, Junio C Hamano wrote: >>> Hilco Wijbenga writes: >>> Perhaps I completely misunderstand the meaning of core.filemode but I thought it determined whether Git cared about changes in file properties? >>> >>> By setting it to "false", you tell Git that the filesystem you >>> placed the repository does not correctly represent the filemode >>> (especially the executable bit). >>> >>> "core.fileMode" in "git config --help" reads: >>> >>>core.fileMode >>>If false, the executable bit differences between the >>>index and the working tree are ignored; useful on broken >>>filesystems like FAT. See git-update- index(1). >> >> Out of my head: Could the following be a starting point: >> >> core.fileMode >> If false, the executable bit differences between the >> index and the working tree are ignored. >> This may be usefull when visiting a cygwin repo with a non-cygwin >> Git client. (should we mention msysgit ? should we mention >> JGit/EGit ?) > > Between these two sentences, there may still be the same cognitive > gap that may have lead to the original confusion. > > The first sentence says what happens, as it should. > > But it is not directly clear what makes the executable bit differ > and when it is a useful thing to ignore the differences, so the > second sentence that says "This may be useful" does not give the > reader very much. > Clearly a major improvement. Does this (still) include the original line "See linkgit:git-update-index[1]" which helps the user to add *.sh files "executable" to the index, even if core.filemode is false ? One minor improvement below. > Here is my attempt. > > Tells Git if the executable bit of files in the working tree > is to be honored. > > Some filesystems lose the executable bit when a file that is > marked as executable is checked out, or checks out an > non-executable file with executable bit on. "git init" and > "git clone" probe the filesystem to see if it records > executable bit correctly when they create a new repository > and this variable is automatically set as necessary. > > A repository, however, may be on a filesystem that records > the filemode correctly, and this variable is set to 'true' > when created, but later may be made accessible from another > environment that loses the filemode (e.g. exporting ext4 via > CIFS mount, visiting a Cygwin managed repository with > MsysGit). In such a case, it may be necessary to set this > variable to 'false'. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] init - Honour the global core.filemode setting
Torsten Bögershausen writes: >> The first sentence says what happens, as it should. >> >> But it is not directly clear what makes the executable bit differ >> and when it is a useful thing to ignore the differences, so the >> second sentence that says "This may be useful" does not give the >> reader very much. >> > Clearly a major improvement. > > Does this (still) include the original line > "See linkgit:git-update-index[1]" > > which helps the user to add *.sh files "executable" to the index, even if > core.filemode is false ? Thanks for catching; I omitted the reference by accident. Perhaps end that sentence like this instead? ... may be necessary to set this variable to `false`, and maintain the executable bit with `git update-index --chmod` (see linkgit:git-update-index[1]). -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
