{"thread":{"id":"10126","subject":"[PATCH] git-init: don't base core.filemode on the ability to chmod.","startedAt":"2007-10-03T10:55:01Z","lastAt":"2007-10-10T09:47:04Z","messageCount":14,"participants":["Martin Waitz","Johannes Sixt","Johannes Schindelin","Andreas Ericsson","Junio C Hamano","Jan Hudec"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"54698","messageId":"20071003105501.GD7085@admingilde.org","threadId":"10126","inReplyTo":null,"subject":"[PATCH] git-init: don't base core.filemode on the ability to chmod.","fromName":"Martin Waitz","fromEmail":"tali@admingilde.org","sentAt":"2007-10-03T10:55:01Z","receivedAt":"2007-10-03T10:55:01Z","isPatch":true,"sender":{"key":"tali@admingilde.org","avatar":"https://gravatar.com/avatar/3f89b03eee362187effabe257898735b475673a12265c398ea9161259ae91553?d=mp&s=160"},"body":"At least on Linux the vfat file system honors chmod calls but does not\nstore them permanently (as there is no on-disk format for it).\nSo the filemode test which tries to chmod a file thinks that the file\nsystem does support file modes.  This will result in problems when the\nfile system gets mounted for the next time and all the executable bits\nare back.\n\nA more reliable test for file systems without filemode support is to\nsimply check if new files are created with the executable bit set.\n\nSigned-off-by: Martin Waitz <tali@admingilde.org>\n---\n builtin-init-db.c |    5 +----\n 1 files changed, 1 insertions(+), 4 deletions(-)\n\n\nNOTE: this is only tested on Linux with ext3 and vfat file systems.\nI do not know enough about the behaviour of other systems so there\nmay be regressions.\n\n\ndiff --git a/builtin-init-db.c b/builtin-init-db.c\nindex 763fa55..fbccacb 100644\n--- a/builtin-init-db.c\n+++ b/builtin-init-db.c\n@@ -246,10 +246,7 @@ static int create_default_files(const char *git_dir, const char *template_path)\n \t/* Check filemode trustability */\n \tfilemode = TEST_FILEMODE;\n \tif (TEST_FILEMODE && !lstat(path, &st1)) {\n-\t\tstruct stat st2;\n-\t\tfilemode = (!chmod(path, st1.st_mode ^ S_IXUSR) &&\n-\t\t\t\t!lstat(path, &st2) &&\n-\t\t\t\tst1.st_mode != st2.st_mode);\n+\t\tfilemode = !(st1.st_mode & S_IXUSR);\n \t}\n \tgit_config_set(\"core.filemode\", filemode ? \"true\" : \"false\");\n \n-- \n1.5.3.3.8.g367dc7\n\n\n-- \nMartin Waitz\n"},{"id":"54702","messageId":"470388DC.4040504@viscovery.net","threadId":"10126","inReplyTo":"20071003105501.GD7085@admingilde.org","subject":"Re: [PATCH] git-init: don't base core.filemode on the ability to chmod.","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2007-10-03T12:19:40Z","receivedAt":"2007-10-03T12:19:40Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Martin Waitz schrieb:\n> At least on Linux the vfat file system honors chmod calls but does not\n> store them permanently (as there is no on-disk format for it).\n> So the filemode test which tries to chmod a file thinks that the file\n> system does support file modes.  This will result in problems when the\n> file system gets mounted for the next time and all the executable bits\n> are back.\n> \n> A more reliable test for file systems without filemode support is to\n> simply check if new files are created with the executable bit set.\n\nOn Windows, we don't get an executable bit at all. Better use both \nheuristics, i.e. set core.filemode false if either one diagnoses an \nunreliable x-bit.\n\n-- Hannes\n"},{"id":"54778","messageId":"20071003231941.GA20800@admingilde.org","threadId":"10126","inReplyTo":"470388DC.4040504@viscovery.net","subject":"[PATCH] git-init: don't base core.filemode on the ability to chmod.","fromName":"Martin Waitz","fromEmail":"tali@admingilde.org","sentAt":"2007-10-03T23:19:41Z","receivedAt":"2007-10-03T23:19:41Z","isPatch":true,"sender":{"key":"tali@admingilde.org","avatar":"https://gravatar.com/avatar/3f89b03eee362187effabe257898735b475673a12265c398ea9161259ae91553?d=mp&s=160"},"body":"At least on Linux the vfat file system honors chmod calls but does not\nstore them permanently (as there is no on-disk format for it).\nSo the filemode test which tries to chmod a file thinks that the file system\ndoes support file modes which will result in problems later after the\nfile system got remounted.\n\nNow we check both that new files are created without the executable bit\nand that we can actually modify it with chmod.\n\nSigned-off-by: Martin Waitz <tali@admingilde.org>\n---  8<  ---\n builtin-init-db.c |    5 ++++-\n 1 files changed, 4 insertions(+), 1 deletions(-)\n\nOn Wed, Oct 03, 2007 at 02:19:40PM +0200, Johannes Sixt wrote:\n> On Windows, we don't get an executable bit at all. Better use both \n> heuristics, i.e. set core.filemode false if either one diagnoses an \n> unreliable x-bit.\n\nthis should work better for Windows.\nPreviously I sent it only to Johannes and forgot to Cc the list.\n\n\ndiff --git a/builtin-init-db.c b/builtin-init-db.c\nindex 763fa55..1d92916 100644\n--- a/builtin-init-db.c\n+++ b/builtin-init-db.c\n@@ -247,7 +247,10 @@ static int create_default_files(const char *git_dir, const char *template_path)\n \tfilemode = TEST_FILEMODE;\n \tif (TEST_FILEMODE && !lstat(path, &st1)) {\n \t\tstruct stat st2;\n-\t\tfilemode = (!chmod(path, st1.st_mode ^ S_IXUSR) &&\n+\t\t/* test that new files are not created with X bit */\n+\t\tfilemode = !(st1.st_mode & S_IXUSR);\n+\t\t/* test that we can modify the X bit */\n+\t\tfilemode &= (!chmod(path, st1.st_mode ^ S_IXUSR) &&\n \t\t\t\t!lstat(path, &st2) &&\n \t\t\t\tst1.st_mode != st2.st_mode);\n \t}\n-- \n1.5.3.3.8.g367dc7\n\n-- \nMartin Waitz\n"},{"id":"54781","messageId":"Pine.LNX.4.64.0710040053380.28395@racer.site","threadId":"10126","inReplyTo":"20071003231941.GA20800@admingilde.org","subject":"Re: [PATCH] git-init: don't base core.filemode on the ability to chmod.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-10-03T23:54:06Z","receivedAt":"2007-10-03T23:54:06Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 4 Oct 2007, Martin Waitz wrote:\n\n> -\t\tfilemode = (!chmod(path, st1.st_mode ^ S_IXUSR) &&\n> +\t\t/* test that new files are not created with X bit */\n> +\t\tfilemode = !(st1.st_mode & S_IXUSR);\n> +\t\t/* test that we can modify the X bit */\n> +\t\tfilemode &= (!chmod(path, st1.st_mode ^ S_IXUSR) &&\n\nShould that not be &&=?\n\nCiao,\nDscho\n"},{"id":"54790","messageId":"470482A2.3080907@op5.se","threadId":"10126","inReplyTo":"Pine.LNX.4.64.0710040053380.28395@racer.site","subject":"Re: [PATCH] git-init: don't base core.filemode on the ability to chmod.","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2007-10-04T06:05:22Z","receivedAt":"2007-10-04T06:05:22Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Johannes Schindelin wrote:\n> Hi,\n> \n> On Thu, 4 Oct 2007, Martin Waitz wrote:\n> \n>> -\t\tfilemode = (!chmod(path, st1.st_mode ^ S_IXUSR) &&\n>> +\t\t/* test that new files are not created with X bit */\n>> +\t\tfilemode = !(st1.st_mode & S_IXUSR);\n>> +\t\t/* test that we can modify the X bit */\n>> +\t\tfilemode &= (!chmod(path, st1.st_mode ^ S_IXUSR) &&\n> \n> Should that not be &&=?\n> \n\nI should think |=\n\n-- \nAndreas Ericsson                   andreas.ericsson@op5.se\nOP5 AB                             www.op5.se\nTel: +46 8-230225                  Fax: +46 8-230231\n"},{"id":"54791","messageId":"7vr6kbbdph.fsf@gitster.siamese.dyndns.org","threadId":"10126","inReplyTo":"470482A2.3080907@op5.se","subject":"Re: [PATCH] git-init: don't base core.filemode on the ability to chmod.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-10-04T06:23:22Z","receivedAt":"2007-10-04T06:23:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andreas Ericsson <ae@op5.se> writes:\n\n> Johannes Schindelin wrote:\n>> Hi,\n>>\n>> On Thu, 4 Oct 2007, Martin Waitz wrote:\n>>\n>>> -\t\tfilemode = (!chmod(path, st1.st_mode ^ S_IXUSR) &&\n>>> +\t\t/* test that new files are not created with X bit */\n>>> +\t\tfilemode = !(st1.st_mode & S_IXUSR);\n>>> +\t\t/* test that we can modify the X bit */\n>>> +\t\tfilemode &= (!chmod(path, st1.st_mode ^ S_IXUSR) &&\n>>\n>> Should that not be &&=?\n>>\n>\n> I should think |=\n\nIs it?\n\nThe issue that started the thread was that chmod + stat check we\noriginally had would say executable bit \"seems to be\" kept,\nwhile that is only true until the information is cached at VFS\nlayer.\n\nWe create config file without asking for executable bit, so if\nwe read it back as executable then that is a sure sign that the\nfilesystem does not know what it is talking about, and we set\nfilemode to zero in such a case.  Similarly, if the chmod + stat\ncheck says we cannot set executable bit and read it back, then\nwe also know the filesystem does not know about filemode.\n\nSo I think we can write it like this (indentation aside)...\n\nfilemode = !( (st1.st_mode & S_IXUSR)\n        \t/* we did not ask for x-bit -- bogus FS */\n\t    || chmod(path, st1.st_mode & S_IXUSR)\n        \t/* it does not let us flip x-bit -- bogus FS */\n\t    || lstat(path, &st2)\n        \t/* it does not let us read back -- bogus FS */\n\t    || (st1.st_mode == st2.st_mode)\n\t        /* it forgets we flipped -- bogus FS */\n\t    );\n"},{"id":"54794","messageId":"20071004071517.GC20800@admingilde.org","threadId":"10126","inReplyTo":"Pine.LNX.4.64.0710040053380.28395@racer.site","subject":"Re: [PATCH] git-init: don't base core.filemode on the ability to chmod.","fromName":"Martin Waitz","fromEmail":"tali@admingilde.org","sentAt":"2007-10-04T07:15:17Z","receivedAt":"2007-10-04T07:15:17Z","isPatch":true,"sender":{"key":"tali@admingilde.org","avatar":"https://gravatar.com/avatar/3f89b03eee362187effabe257898735b475673a12265c398ea9161259ae91553?d=mp&s=160"},"body":"hoi :)\n\nOn Thu, Oct 04, 2007 at 12:54:06AM +0100, Johannes Schindelin wrote:\n> > -\t\tfilemode = (!chmod(path, st1.st_mode ^ S_IXUSR) &&\n> > +\t\t/* test that new files are not created with X bit */\n> > +\t\tfilemode = !(st1.st_mode & S_IXUSR);\n> > +\t\t/* test that we can modify the X bit */\n> > +\t\tfilemode &= (!chmod(path, st1.st_mode ^ S_IXUSR) &&\n> \n> Should that not be &&=?\n\noriginally I wrote it that way but the compiler complaint.\nI guess we are too used to perl ;-)\n\n-- \nMartin Waitz\n"},{"id":"54796","messageId":"20071004071751.GD20800@admingilde.org","threadId":"10126","inReplyTo":"7vr6kbbdph.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] git-init: don't base core.filemode on the ability to chmod.","fromName":"Martin Waitz","fromEmail":"tali@admingilde.org","sentAt":"2007-10-04T07:17:51Z","receivedAt":"2007-10-04T07:17:51Z","isPatch":true,"sender":{"key":"tali@admingilde.org","avatar":"https://gravatar.com/avatar/3f89b03eee362187effabe257898735b475673a12265c398ea9161259ae91553?d=mp&s=160"},"body":"hoi :)\n\nOn Wed, Oct 03, 2007 at 11:23:22PM -0700, Junio C Hamano wrote:\n> filemode = !( (st1.st_mode & S_IXUSR)\n>         \t/* we did not ask for x-bit -- bogus FS */\n> \t    || chmod(path, st1.st_mode & S_IXUSR)\n>         \t/* it does not let us flip x-bit -- bogus FS */\n> \t    || lstat(path, &st2)\n>         \t/* it does not let us read back -- bogus FS */\n> \t    || (st1.st_mode == st2.st_mode)\n> \t        /* it forgets we flipped -- bogus FS */\n> \t    );\n\nthat looks good.\n\n-- \nMartin Waitz\n"},{"id":"54798","messageId":"7vir5nb89d.fsf@gitster.siamese.dyndns.org","threadId":"10126","inReplyTo":"20071004071751.GD20800@admingilde.org","subject":"Re: [PATCH] git-init: don't base core.filemode on the ability to chmod.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-10-04T08:21:02Z","receivedAt":"2007-10-04T08:21:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin Waitz <tali@admingilde.org> writes:\n\n> hoi :)\n>\n> On Wed, Oct 03, 2007 at 11:23:22PM -0700, Junio C Hamano wrote:\n>> filemode = !( (st1.st_mode & S_IXUSR)\n>>         \t/* we did not ask for x-bit -- bogus FS */\n>> \t    || chmod(path, st1.st_mode & S_IXUSR)\n>>         \t/* it does not let us flip x-bit -- bogus FS */\n>> \t    || lstat(path, &st2)\n>>         \t/* it does not let us read back -- bogus FS */\n>> \t    || (st1.st_mode == st2.st_mode)\n>> \t        /* it forgets we flipped -- bogus FS */\n>> \t    );\n>\n> that looks good.\n\nFWIW, I did not mean it to be an example for preferred\nindentation nor code layout, but as a better way to explain what\nthe logic is computing.\n\nI do not think git on Cygwin nor WinGit creates $GIT_DIR/config\nwith executable bit set.  Is this pretty much a workaround only\nfor vfat-on-Linux ?\n"},{"id":"54801","messageId":"4704A5FF.6070105@viscovery.net","threadId":"10126","inReplyTo":"7vir5nb89d.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] git-init: don't base core.filemode on the ability to chmod.","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2007-10-04T08:36:15Z","receivedAt":"2007-10-04T08:36:15Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Junio C Hamano schrieb:\n> Martin Waitz <tali@admingilde.org> writes:\n>> On Wed, Oct 03, 2007 at 11:23:22PM -0700, Junio C Hamano wrote:\n>>> filemode = !( (st1.st_mode & S_IXUSR)\n>>>         \t/* we did not ask for x-bit -- bogus FS */\n>>> \t    || chmod(path, st1.st_mode & S_IXUSR)\n>>>         \t/* it does not let us flip x-bit -- bogus FS */\n>>> \t    || lstat(path, &st2)\n>>>         \t/* it does not let us read back -- bogus FS */\n>>> \t    || (st1.st_mode == st2.st_mode)\n>>> \t        /* it forgets we flipped -- bogus FS */\n>>> \t    );\n>> that looks good.\n> \n> I do not think git on Cygwin nor WinGit creates $GIT_DIR/config\n> with executable bit set.  Is this pretty much a workaround only\n> for vfat-on-Linux ?\n\nI think so. Here on Windows, 'ls -l' after 'git init' tells that .git/config \nis not executable (both FAT and NTFS). But, anyway, as far as the MinGW port \nis concerned, at least the last condition in the sequence above triggers and \ncauses filemode=false, which is good.\n\n-- Hannes\n"},{"id":"54803","messageId":"20071004084237.GE20800@admingilde.org","threadId":"10126","inReplyTo":"7vir5nb89d.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] git-init: don't base core.filemode on the ability to chmod.","fromName":"Martin Waitz","fromEmail":"tali@admingilde.org","sentAt":"2007-10-04T08:42:37Z","receivedAt":"2007-10-04T08:42:37Z","isPatch":true,"sender":{"key":"tali@admingilde.org","avatar":"https://gravatar.com/avatar/3f89b03eee362187effabe257898735b475673a12265c398ea9161259ae91553?d=mp&s=160"},"body":"hoi :)\n\nOn Thu, Oct 04, 2007 at 01:21:02AM -0700, Junio C Hamano wrote:\n> FWIW, I did not mean it to be an example for preferred\n> indentation nor code layout, but as a better way to explain what\n> the logic is computing.\n\nsure, and I think it makes sense the way you wrote it.\n\n> I do not think git on Cygwin nor WinGit creates $GIT_DIR/config\n> with executable bit set.  Is this pretty much a workaround only\n> for vfat-on-Linux ?\n\nI just checked Cygwin, it creates files without executable bit and\ndisregards a chmod +x.  So yes, this seems to be a Linux-only problem.\n\n-- \nMartin Waitz\n"},{"id":"54807","messageId":"4704C161.3000006@op5.se","threadId":"10126","inReplyTo":"7vr6kbbdph.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] git-init: don't base core.filemode on the ability to chmod.","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2007-10-04T10:33:05Z","receivedAt":"2007-10-04T10:33:05Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Junio C Hamano wrote:\n> Andreas Ericsson <ae@op5.se> writes:\n> \n>> Johannes Schindelin wrote:\n>>> Hi,\n>>>\n>>> On Thu, 4 Oct 2007, Martin Waitz wrote:\n>>>\n>>>> -\t\tfilemode = (!chmod(path, st1.st_mode ^ S_IXUSR) &&\n>>>> +\t\t/* test that new files are not created with X bit */\n>>>> +\t\tfilemode = !(st1.st_mode & S_IXUSR);\n>>>> +\t\t/* test that we can modify the X bit */\n>>>> +\t\tfilemode &= (!chmod(path, st1.st_mode ^ S_IXUSR) &&\n>>> Should that not be &&=?\n>>>\n>> I should think |=\n> \n> Is it?\n> \n\nNopes. I misread the first expression and simply assumed that \"filemode\"\nshould be != 0 for FS not supporting the x bit. I'd rename the variable\nto bogus_fs and flip the logic, but I have no strong opinion either way.\n\n> \n> So I think we can write it like this (indentation aside)...\n> \n> filemode = !( (st1.st_mode & S_IXUSR)\n>         \t/* we did not ask for x-bit -- bogus FS */\n> \t    || chmod(path, st1.st_mode & S_IXUSR)\n>         \t/* it does not let us flip x-bit -- bogus FS */\n> \t    || lstat(path, &st2)\n>         \t/* it does not let us read back -- bogus FS */\n> \t    || (st1.st_mode == st2.st_mode)\n> \t        /* it forgets we flipped -- bogus FS */\n> \t    );\n\nFor \"filemode=0 means FS doesn't support x-bit\" it looks about right,\nbut kinda cumbersome to read.\n\n-- \nAndreas Ericsson                   andreas.ericsson@op5.se\nOP5 AB                             www.op5.se\nTel: +46 8-230225                  Fax: +46 8-230231\n"},{"id":"54813","messageId":"Pine.LNX.4.64.0710041345280.4174@racer.site","threadId":"10126","inReplyTo":"7vir5nb89d.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] git-init: don't base core.filemode on the ability to chmod.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-10-04T12:49:43Z","receivedAt":"2007-10-04T12:49:43Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 4 Oct 2007, Junio C Hamano wrote:\n\n> I do not think git on Cygwin nor WinGit creates $GIT_DIR/config with \n> executable bit set.  Is this pretty much a workaround only for \n> vfat-on-Linux ?\n\nI do not know precisely about Cygwin (a quick test on my USB stick shows \nthat .git/config is created without the executable bit set), but in MSys \nplain files which start with a she-bang are automatically +x, while all \nother plain files are automatically -x (therefore this applies to \n.git/config).\n\nGiven these findings, I fail to see what the patch should achieve, as \nthe first test (for -x) always succeeds...\n\nCiao,\nDscho\n"},{"id":"55357","messageId":"20071010094704.GA7865@efreet.light.src","threadId":"10126","inReplyTo":"20071004084237.GE20800@admingilde.org","subject":"Re: [PATCH] git-init: don't base core.filemode on the ability to chmod.","fromName":"Jan Hudec","fromEmail":"bulb@ucw.cz","sentAt":"2007-10-10T09:47:04Z","receivedAt":"2007-10-10T09:47:04Z","isPatch":true,"sender":{"key":"bulb@ucw.cz","avatar":null},"body":"On Thu, Oct 04, 2007 at 10:42:37 +0200, Martin Waitz wrote:\n> hoi :)\n> \n> On Thu, Oct 04, 2007 at 01:21:02AM -0700, Junio C Hamano wrote:\n> > FWIW, I did not mean it to be an example for preferred\n> > indentation nor code layout, but as a better way to explain what\n> > the logic is computing.\n> \n> sure, and I think it makes sense the way you wrote it.\n> \n> > I do not think git on Cygwin nor WinGit creates $GIT_DIR/config\n> > with executable bit set.  Is this pretty much a workaround only\n> > for vfat-on-Linux ?\n> \n> I just checked Cygwin, it creates files without executable bit and\n> disregards a chmod +x.  So yes, this seems to be a Linux-only problem.\n\nAFAIR in cygwin it depends on both configuration of cygwin and whether you\nare on NTFS or FAT. On NTFS with proper setting (CYGWIN=ntsec IIRC), cygwin\nactually implements the x bit (uses the NT ACL to somewhat represent it). On\nother filesystem there is an option somewhere to tell whether it should show\nthe x bit always set or always cleared (for me it seems to be always set).\n\nOn the other hand MSYS shows the x bit as set whenever the file has\nexecutable extension. I don't think cygwin has this mode.\n\n-- \n\t\t\t\t\t\t Jan 'Bulb' Hudec <bulb@ucw.cz>\n"}]}