On 11/17/2014 02:40 AM, Eric Sunshine wrote:
On Sun, Nov 16, 2014 at 2:21 AM, Michael Haggerty <mhagger@xxxxxxxxxxxx> wrote:
Since time immemorial, the test of whether to set "core.filemode" has
been done by trying to toggle the u+x bit on $GIT_DIR/config and then
testing whether the change "took". It is somewhat odd to use the
config file for this test, but whatever.
The test code didn't set the u+x bit back to its original state
itself, instead relying on the subsequent call to git_config_set() to
re-write the config file with correct permissions.
But ever since
daa22c6f8d config: preserve config file permissions on edits (2014-05-06)
git_config_set() copies the permissions from the old config file to
the new one. This is a good change in and of itself, but it interacts
badly with create_default_files()'s sloppiness, causing "git init" to
leave the executable bit set on $GIT_DIR/config.
So change create_default_files() to reset the permissions on
$GIT_DIR/config after its test.
Signed-off-by: Michael Haggerty <mhagger@xxxxxxxxxxxx>
---
Should this patch include a test in t1300 to ensure that this bug does
not resurface (and to prove that this patch indeed fixes it)?
builtin/init-db.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 56f85e2..4c8021d 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -255,6 +255,7 @@ static int create_default_files(const char *template_path)
filemode = (!chmod(path, st1.st_mode ^ S_IXUSR) &&
!lstat(path, &st2) &&
st1.st_mode != st2.st_mode);
+ filemode &= !chmod(path, st1.st_mode);
}
git_config_set("core.filemode", filemode ? "true" : "false");
--
Sorry for the late reply, I actually had prepared a complete different patch
for a different problem, but it touches the very same lines of code.
(And we may want to add a
!chmod(path, st1.st_mode & ~S_IXUSR)
at the end of the operation)
There are systems when a file created with 666 have 766, and we
do not handle this situation yet.
Michael, if there is a chance that you integrate my small code changes
in your patch ?
-------------
commit 3228bedef6d45bfaf8986b6367f9388738476345
Author: Torsten Bögershausen <tboegi@xxxxxx>
Date: Sun Oct 19 00:15:00 2014 +0200
Improve the filemode trustability check
Some file systems do not fully support the executable bit:
a) The user executable bit is always 0
b) The user executable bit is always 1
c) Is similar to b):
When a file is created with mode 0666 the file mode on disc is 766
and the user executable bit is 1 even if it should be 0 like b)
There are smbfs implementations where the file mode can be
maintained
locally and chmod -x changes the file mode from 0766 to 0666.
When the file system is unmounted and remounted,
the file mode is 0766 and executable bit is 1 again.
A typical example for a) is a VFAT drive mounted with -onoexec,
or cifs with -ofile_mode=0644.
b) is VFAT mounted with -oexec or cifs is mounted with -ofile_mode=0755
The behaviour described in c) has been observed when a Windows
machine with
NTFS exports a share via smb (or afp) to Mac OS X.
(CIFS is an enhanced version of SMB
The command to mount on the command line may differ,
e.g mount.cifs under Linux or mount_smbfs under Mac OS X)
Case a) and b) are detected by the current code.
Case c) qualifies as "non trustable executable bit",
and core.filemode should be false by default.
Solution:
Detect when ".git/config" has the user executable bit set after
creat(".git/config", 0666) and set core.filemode to false.
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 587a505..d3e4fb3 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -249,13 +249,11 @@ static int create_default_files(const char
*template_path)
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);
- }
+ filemode = TEST_FILEMODE &&
+ !lstat(path, &st1) && !(st1.st_mode & S_IXUSR) &&
+ !chmod(path, st1.st_mode | S_IXUSR) &&
+ !lstat(path, &st1) && (st1.st_mode & S_IXUSR);
+
git_config_set("core.filemode", filemode ? "true" : "false");
if (is_bare_repository())
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html