{"thread":{"id":"38574","subject":"odb_mkstemp's 0444 permission broke write/delete access on AFP","startedAt":"2015-02-16T17:54:33Z","lastAt":"2015-02-20T10:40:08Z","messageCount":22,"participants":["Fairuzan Roslan","Matthieu Moy","Junio C Hamano","Torsten Bögershausen","brian m. carlson"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"256156","messageId":"A403BFCC-D66F-49BD-B54C-BB86B467F1A1@gmail.com","threadId":"38574","inReplyTo":null,"subject":"odb_mkstemp's 0444 permission broke write/delete access on AFP","fromName":"Fairuzan Roslan","fromEmail":"fairuzan.roslan@gmail.com","sentAt":"2015-02-16T17:54:33Z","receivedAt":"2015-02-16T17:54:33Z","isPatch":false,"sender":{"key":"fairuzan.roslan@gmail.com","avatar":null},"body":"Hi,\n\nSomehow the “int mode = 0444;” in odb_mkstemp (environment.c) are causing a lot of issues (unable to unlink/write/rename) to those people who use AFP shares.\n\nIn order to be able to write/unlink/delete/rename a file on AFP filesystem the owner of the file must have at least a u+w access to it.\n\nThe issue was first introduced in https://github.com/git/git/blob/f80c7ae8fe9c0f3ce93c96a2dccaba34e456e33a/wrapper.c line 284.\n\nTo fix these issues the permission need to be adjusted to “int mode = 0644;” in odb_mkstemp (environment.c)\n\nPlease let me know if you need further detail.\n\nRegards,\nFairuzan"},{"id":"256161","messageId":"vpqtwyl90mx.fsf@anie.imag.fr","threadId":"38574","inReplyTo":"A403BFCC-D66F-49BD-B54C-BB86B467F1A1@gmail.com","subject":"Re: odb_mkstemp's 0444 permission broke write/delete access on AFP","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-02-16T18:23:02Z","receivedAt":"2015-02-16T18:23:02Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Fairuzan Roslan writes:\n\n> Hi,\n>\n> Somehow the “int mode = 0444;” in odb_mkstemp (environment.c) are\n> causing a lot of issues (unable to unlink/write/rename) to those\n> people who use AFP shares.\n\nIs it a problem when using Git (like \"git gc\" failing to remove old\npacks), or when trying to remove files outside Git?\n\n> The issue was first introduced in\n> https://github.com/git/git/blob/f80c7ae8fe9c0f3ce93c96a2dccaba34e456e33a/wrapper.c\n> line 284.\n\nI don't think so. The code before this commit did essentially a chmod\n444 on the file, so object files were already read-only before.\n\nThe pack files have been read-only since d83c9af5c6a437ddaa9dd27 (Junio,\nApr 22 2007).\n\n> To fix these issues the permission need to be adjusted to “int mode =\n> 0644;” in odb_mkstemp (environment.c)\n\nThe issue is that having object and pack files read-only on the\nfilesystem is a safety feature to prevent accidental modifications (even\nthough it's actually not that effective, since brute-force \"sed -i\" or\n\"perl -i\" still accept to modify read-only files).\n\nSo, I'd be a bit reluctant to remove this safety feature for all users\nif it's only for the benefit of a minority of users. Not that I think\nthe problem shouldn't be fixed, but I'd rather investigate alternate\nsolutions before using this mode = 0644.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"256162","messageId":"340435D1-2FEB-4A4A-BBD2-E301096C72D8@gmail.com","threadId":"38574","inReplyTo":"vpqtwyl90mx.fsf@anie.imag.fr","subject":"Re: odb_mkstemp's 0444 permission broke write/delete access on AFP","fromName":"Fairuzan Roslan","fromEmail":"fairuzan.roslan@gmail.com","sentAt":"2015-02-16T18:41:24Z","receivedAt":"2015-02-16T18:41:24Z","isPatch":false,"sender":{"key":"fairuzan.roslan@gmail.com","avatar":null},"body":"I don’t see the issue for the owner of his/her own file to have write access.\nSetting tmp idx & pack files to read-only even for the file owner is not a safety feature.\n\nThe real issue here is that in AFP file system can’t even unlink or rename or delete the tmp idx and pack file with no write access after it done, while other file system like ext4,hfs+, etc can.\n\nYou should at least give the user the option to set the permission in the config file and not hardcoded the permission in the binary.\n\nRegards,\nFairuzan\n\n\n> On Feb 17, 2015, at 2:23 AM, Matthieu Moy wrote:\n> \n> Fairuzan Roslan writes:\n> \n>> Hi,\n>> \n>> Somehow the “int mode = 0444;” in odb_mkstemp (environment.c) are\n>> causing a lot of issues (unable to unlink/write/rename) to those\n>> people who use AFP shares.\n> \n> Is it a problem when using Git (like \"git gc\" failing to remove old\n> packs), or when trying to remove files outside Git?\n> \n>> The issue was first introduced in\n>> https://github.com/git/git/blob/f80c7ae8fe9c0f3ce93c96a2dccaba34e456e33a/wrapper.c\n>> line 284.\n> \n> I don't think so. The code before this commit did essentially a chmod\n> 444 on the file, so object files were already read-only before.\n> \n> The pack files have been read-only since d83c9af5c6a437ddaa9dd27 (Junio,\n> Apr 22 2007).\n> \n>> To fix these issues the permission need to be adjusted to “int mode =\n>> 0644;” in odb_mkstemp (environment.c)\n> \n> The issue is that having object and pack files read-only on the\n> filesystem is a safety feature to prevent accidental modifications (even\n> though it's actually not that effective, since brute-force \"sed -i\" or\n> \"perl -i\" still accept to modify read-only files).\n> \n> So, I'd be a bit reluctant to remove this safety feature for all users\n> if it's only for the benefit of a minority of users. Not that I think\n> the problem shouldn't be fixed, but I'd rather investigate alternate\n> solutions before using this mode = 0644.\n> \n> --\n> Matthieu Moy\n> http://www-verimag.imag.fr/~moy/\n\n"},{"id":"256164","messageId":"xmqqsie565i9.fsf@gitster.dls.corp.google.com","threadId":"38574","inReplyTo":"vpqtwyl90mx.fsf@anie.imag.fr","subject":"Re: odb_mkstemp's 0444 permission broke write/delete access on AFP","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-02-16T19:06:06Z","receivedAt":"2015-02-16T19:06:06Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> The issue is that having object and pack files read-only on the\n> filesystem is a safety feature to prevent accidental modifications (even\n> though it's actually not that effective, since brute-force \"sed -i\" or\n> \"perl -i\" still accept to modify read-only files).\n\nI did not see it as a \"safety feature\", and instead saw it as a\nreminder to me that I am not supposed to write into them when I\ncheck them with \"ls -l\".\n\n> So, I'd be a bit reluctant to remove this safety feature for all users\n> if it's only for the benefit of a minority of users. Not that I think\n> the problem shouldn't be fixed, but I'd rather investigate alternate\n> solutions before using this mode = 0644.\n\nI fully agree with you that this should not be unconditional.\nHowever, I am not sure if there is an effective workaround to a\nfilesystem that pays attention to the mode bits of the file when\ndoing an operation on the directory the file is sitting within. It\nmay be OK to introduce a new configuration variable, perhaps call it\ncore.brokenFileSystemNeedsWritableFile or something, and probe and\nenable it inside init_db().\n\nBut I suspect that the single \"mode = 0444\" under discussion may not\ncover all places we create files, as the assumption that the we get\na sane semantics from the filesystem permeates throughout the code.\n\nWhat other glitches does AFP have? Does it share Windows' \"you\ncannot rename a file you have an open file descriptor on?\" Anything\nelse?\n"},{"id":"256165","messageId":"vpqiof14qu8.fsf@anie.imag.fr","threadId":"38574","inReplyTo":"340435D1-2FEB-4A4A-BBD2-E301096C72D8@gmail.com","subject":"Re: odb_mkstemp's 0444 permission broke write/delete access on AFP","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-02-16T19:08:15Z","receivedAt":"2015-02-16T19:08:15Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"[ Please, don't top post on this list ]\n\nFairuzan Roslan writes:\n\n> I don’t see the issue for the owner of his/her own file to have write\n> access.\n\nObject and pack files are not meant to be modified. Hence, they are\nread-only so that an (accidental) attempt to modify them fails.\n\n> Setting tmp idx & pack files to read-only even for the file owner is\n> not a safety feature.\n\nYes it is. If you do not think so, then please give some arguments.\n\n> You should at least give the user the option to set the permission in\n> the config file and not hardcoded the permission in the binary.\n\nThis is the kind of thing I meant by \"investigate alternate solutions\".\nI have no AFP share to test, so it would help if you answered the\nquestion I asked in my previous message:\n\n>> On Feb 17, 2015, at 2:23 AM, Matthieu Moy wrote:\n>> \n>> Fairuzan Roslan writes:\n>> \n>>> Hi,\n>>> \n>>> Somehow the “int mode = 0444;” in odb_mkstemp (environment.c) are\n>>> causing a lot of issues (unable to unlink/write/rename) to those\n>>> people who use AFP shares.\n>> \n>> Is it a problem when using Git (like \"git gc\" failing to remove old\n>> packs), or when trying to remove files outside Git?\n\n(BTW, why did you try to write/rename pack files?)\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"256167","messageId":"54E24A13.20405@web.de","threadId":"38574","inReplyTo":"xmqqsie565i9.fsf@gitster.dls.corp.google.com","subject":"Re: odb_mkstemp's 0444 permission broke write/delete access on AFP","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2015-02-16T19:50:43Z","receivedAt":"2015-02-16T19:50:43Z","isPatch":false,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 16.02.15 20:06, Junio C Hamano wrote:\n> Matthieu Moy writes:\n> \n>> The issue is that having object and pack files read-only on the\n>> filesystem is a safety feature to prevent accidental modifications (even\n>> though it's actually not that effective, since brute-force \"sed -i\" or\n>> \"perl -i\" still accept to modify read-only files).\n> \n> I did not see it as a \"safety feature\", and instead saw it as a\n> reminder to me that I am not supposed to write into them when I\n> check them with \"ls -l\".\n> \n>> So, I'd be a bit reluctant to remove this safety feature for all users\n>> if it's only for the benefit of a minority of users. Not that I think\n>> the problem shouldn't be fixed, but I'd rather investigate alternate\n>> solutions before using this mode = 0644.\n> \n> I fully agree with you that this should not be unconditional.\n> However, I am not sure if there is an effective workaround to a\n> filesystem that pays attention to the mode bits of the file when\n> doing an operation on the directory the file is sitting within. It\n> may be OK to introduce a new configuration variable, perhaps call it\n> core.brokenFileSystemNeedsWritableFile or something, and probe and\n> enable it inside init_db().\n> \n> But I suspect that the single \"mode = 0444\" under discussion may not\n> cover all places we create files, as the assumption that the we get\n> a sane semantics from the filesystem permeates throughout the code.\n> \n> What other glitches does AFP have? Does it share Windows' \"you\n> cannot rename a file you have an open file descriptor on?\" Anything\n> else?\n\nMay I ask which OS you have on the server side, and which on the client side?\n\nI'm aware that Mac OS \"speaks\" AFP, but even Linux can do and there is\nSW which enables AFP on a Windows machine (all this is server side).\n\nAs a client we may have Mac OS, Linux (not sure if Windows can use APF)\nWhat do you use ?\n"},{"id":"256180","messageId":"13683B35-70A8-4D9E-80E1-440E4E0DC7F0@gmail.com","threadId":"38574","inReplyTo":"vpqiof14qu8.fsf@anie.imag.fr","subject":"Re: odb_mkstemp's 0444 permission broke write/delete access on AFP","fromName":"Fairuzan Roslan","fromEmail":"fairuzan.roslan@gmail.com","sentAt":"2015-02-17T03:22:28Z","receivedAt":"2015-02-17T03:22:28Z","isPatch":false,"sender":{"key":"fairuzan.roslan@gmail.com","avatar":null},"body":"\n> On Feb 17, 2015, at 3:08 AM, Matthieu Moy wrote:\n> \n> [ Please, don't top post on this list ]\n> \n> Fairuzan Roslan writes:\n> \n>> I don’t see the issue for the owner of his/her own file to have write\n>> access.\n> \n> Object and pack files are not meant to be modified. Hence, they are\n> read-only so that an (accidental) attempt to modify them fails.\n> \n>> Setting tmp idx & pack files to read-only even for the file owner is\n>> not a safety feature.\n> \n> Yes it is. If you do not think so, then please give some arguments.\n> \n>> You should at least give the user the option to set the permission in\n>> the config file and not hardcoded the permission in the binary.\n> \n> This is the kind of thing I meant by \"investigate alternate solutions\".\n> I have no AFP share to test, so it would help if you answered the\n> question I asked in my previous message:\n> \n>>> On Feb 17, 2015, at 2:23 AM, Matthieu Moy wrote:\n>>> \n>>> Fairuzan Roslan writes:\n>>> \n>>>> Hi,\n>>>> \n>>>> Somehow the “int mode = 0444;” in odb_mkstemp (environment.c) are\n>>>> causing a lot of issues (unable to unlink/write/rename) to those\n>>>> people who use AFP shares.\n>>> \n>>> Is it a problem when using Git (like \"git gc\" failing to remove old\n>>> packs), or when trying to remove files outside Git?\n> \n> (BTW, why did you try to write/rename pack files?)\n> \n> --\n> Matthieu Moy\n> http://www-verimag.imag.fr/~moy/\n\nI think its easier if I just show you…\n\nOS : OS X 10.10.0 - 10.10.2\nClient : git version 1.9.3 (Apple Git-50) and git version 2.2.1\nAFP share : //user@hostname._afpovertcp._tcp.local/installer on /Volumes/installer (afpfs, nodev, nosuid, mounted by user)\n\n1. git clone example\n\n$ git clone https://github.com/robbyrussell/oh-my-zsh.git\nCloning into 'oh-my-zsh'...\nremote: Counting objects: 11830, done.\nremote: Total 11830 (delta 0), reused 0 (delta 0)\nReceiving objects: 100% (11830/11830), 2.12 MiB | 481.00 KiB/s, done.\nResolving deltas: 100% (6510/6510), done.\nwarning: unable to unlink /Volumes/installer/oh-my-zsh/.git/objects/pack/tmp_pack_zjPxuc: Operation not permitted\nerror: unable to write sha1 filename /Volumes/installer/oh-my-zsh/.git/objects/pack/pack-cceafdc9ef02bc58844138ba543ec6cc38252bb1.pack: Operation not permitted\nfatal: cannot store pack file\nfatal: index-pack failed\n\n$ ls -l oh-my-zsh/.git/objects/pack\ntotal 5008\n-rw------- 1 user staff 32 Feb 17 09:59 pack-cceafdc9ef02bc58844138ba543ec6cc38252bb1.keep\n-r--r--r-- 1 user staff 332312 Feb 17 09:59 tmp_idx_oUN1sb\n-r--r--r-- 1 user staff 2223007 Feb 17 09:59 tmp_pack_zjPxuc\n\n$ rm -rf oh-my-zsh/.git/objects/pack/tmp_*\nrm: oh-my-zsh/.git/objects/pack/tmp_idx_oUN1sb: Operation not permitted\nrm: oh-my-zsh/.git/objects/pack/tmp_pack_zjPxuc: Operation not permitted\n\nDetail Errors:\n1. delete_ref_loose (refs.c) -> unlink_or_msg (wrapper.c) -> \"unable to unlink %s: %s\"\n2. move_temp_to_file (sha1_file.c ) -> “unable to write sha1 filename %s: %s”\n\n2. git pull example\n\nTextual git:master $ git pull\nremote: Counting objects: 435, done.\nremote: Compressing objects: 100% (398/398), done.\nremote: Total 435 (delta 219), reused 18 (delta 12)\nReceiving objects: 100% (435/435), 1.22 MiB | 756.00 KiB/s, done.\nResolving deltas: 100% (219/219), done.\nwarning: unable to unlink .git/objects/pack/tmp_pack_vDaIZa: Operation not permitted\nerror: unable to write sha1 filename .git/objects/pack/pack-977a2dc0f4be3996dc1186e565a30d55d14b5e87.pack: Operation not permitted\nfatal: cannot store pack file\nfatal: index-pack failed\n\nTextual git:master $ ls -l .git/objects/pack/tmp_*\n-r--r--r-- 1 user staff 13252 Feb 17 10:51 .git/objects/pack/tmp_idx_uhnicb\n-r--r--r-- 1 user staff 1275487 Feb 17 10:51 .git/objects/pack/tmp_pack_vDaIZa\n\n= Same explanation as git clone example\n\n3. git gc example\n\nTextual git:master $ git gc\nCounting objects: 49691, done.\nDelta compression using up to 4 threads.\nCompressing objects: 100% (11347/11347), done.\nfatal: unable to rename temporary pack file: Operation not permitted\nerror: failed to run repack\n\nTextual git:master $ ls -l .git/objects/pack/tmp_*\n-r--r--r-- 1 user staff 1392420 Feb 17 10:58 .git/objects/pack/tmp_idx_77nr1b\n-r--r--r-- 1 user staff 96260304 Feb 17 10:58 .git/objects/pack/tmp_pack_RlAZc9\n\nDetail Error:\n1. finish_tmp_packfile (pack-write.c) -> die_errno(“unable to rename temporary pack file”);\n\n\nIf you insist on setting the tmp idx & pack file permission to 0444 at least give it a u+w permission whenever you try to unlink and rename it so it won’t fail.\n\nRegards,\nFairuzan\n\n\n\n\n\n"},{"id":"256184","messageId":"54E2D2F3.5080800@web.de","threadId":"38574","inReplyTo":"13683B35-70A8-4D9E-80E1-440E4E0DC7F0@gmail.com","subject":"Re: odb_mkstemp's 0444 permission broke write/delete access on AFP","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2015-02-17T05:34:43Z","receivedAt":"2015-02-17T05:34:43Z","isPatch":false,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 02/17/2015 04:22 AM, Fairuzan Roslan wrote:\n>> On Feb 17, 2015, at 3:08 AM, Matthieu Moy wrote:\n>>\n>> [ Please, don't top post on this list ]\n>>\n>> Fairuzan Roslan writes:\n>>\n>>> I don’t see the issue for the owner of his/her own file to have write\n>>> access.\n>> Object and pack files are not meant to be modified. Hence, they are\n>> read-only so that an (accidental) attempt to modify them fails.\n>>\n>>> Setting tmp idx & pack files to read-only even for the file owner is\n>>> not a safety feature.\n>> Yes it is. If you do not think so, then please give some arguments.\n>>\n>>> You should at least give the user the option to set the permission in\n>>> the config file and not hardcoded the permission in the binary.\n>> This is the kind of thing I meant by \"investigate alternate solutions\".\n>> I have no AFP share to test, so it would help if you answered the\n>> question I asked in my previous message:\n>>\n>>>> On Feb 17, 2015, at 2:23 AM, Matthieu Moy wrote:\n>>>>\n>>>> Fairuzan Roslan writes:\n>>>>\n>>>>> Hi,\n>>>>>\n>>>>> Somehow the “int mode = 0444;” in odb_mkstemp (environment.c) are\n>>>>> causing a lot of issues (unable to unlink/write/rename) to those\n>>>>> people who use AFP shares.\n>>>> Is it a problem when using Git (like \"git gc\" failing to remove old\n>>>> packs), or when trying to remove files outside Git?\n>> (BTW, why did you try to write/rename pack files?)\n>>\n>> --\n>> Matthieu Moy\n>> http://www-verimag.imag.fr/~moy/\n> I think its easier if I just show you…\n>\n> OS : OS X 10.10.0 - 10.10.2\n> Client : git version 1.9.3 (Apple Git-50) and git version 2.2.1\n> AFP share : //user@hostname._afpovertcp._tcp.local/installer on /Volumes/installer (afpfs, nodev, nosuid, mounted by user)\n>\n> 1. git clone example\n>\n> $ git clone https://github.com/robbyrussell/oh-my-zsh.git\n> Cloning into 'oh-my-zsh'...\n> remote: Counting objects: 11830, done.\n> remote: Total 11830 (delta 0), reused 0 (delta 0)\n> Receiving objects: 100% (11830/11830), 2.12 MiB | 481.00 KiB/s, done.\n> Resolving deltas: 100% (6510/6510), done.\n> warning: unable to unlink /Volumes/installer/oh-my-zsh/.git/objects/pack/tmp_pack_zjPxuc: Operation not permitted\n> error: unable to write sha1 filename /Volumes/installer/oh-my-zsh/.git/objects/pack/pack-cceafdc9ef02bc58844138ba543ec6cc38252bb1.pack: Operation not permitted\n> fatal: cannot store pack file\n> fatal: index-pack failed\n>\n> $ ls -l oh-my-zsh/.git/objects/pack\n> total 5008\n> -rw------- 1 user staff 32 Feb 17 09:59 pack-cceafdc9ef02bc58844138ba543ec6cc38252bb1.keep\n> -r--r--r-- 1 user staff 332312 Feb 17 09:59 tmp_idx_oUN1sb\n> -r--r--r-- 1 user staff 2223007 Feb 17 09:59 tmp_pack_zjPxuc\n>\n> $ rm -rf oh-my-zsh/.git/objects/pack/tmp_*\n> rm: oh-my-zsh/.git/objects/pack/tmp_idx_oUN1sb: Operation not permitted\n> rm: oh-my-zsh/.git/objects/pack/tmp_pack_zjPxuc: Operation not permitted\n>\n> Detail Errors:\n> 1. delete_ref_loose (refs.c) -> unlink_or_msg (wrapper.c) -> \"unable to unlink %s: %s\"\n> 2. move_temp_to_file (sha1_file.c ) -> “unable to write sha1 filename %s: %s”\n>\n> 2. git pull example\n>\n> Textual git:master $ git pull\n> remote: Counting objects: 435, done.\n> remote: Compressing objects: 100% (398/398), done.\n> remote: Total 435 (delta 219), reused 18 (delta 12)\n> Receiving objects: 100% (435/435), 1.22 MiB | 756.00 KiB/s, done.\n> Resolving deltas: 100% (219/219), done.\n> warning: unable to unlink .git/objects/pack/tmp_pack_vDaIZa: Operation not permitted\n> error: unable to write sha1 filename .git/objects/pack/pack-977a2dc0f4be3996dc1186e565a30d55d14b5e87.pack: Operation not permitted\nI'm somewhat unsure how this is connected to 0444 ?\n\nIt seems as if you don't have write permissions for some reasons.\n(on the higher directory), what does\nls -ld .git/objects/pack/\nls -ld .git/objects/\ngive ?\n\ncan you run\nrm .git/objects/pack/pack-977a2dc0f4be3996dc1186e565a30d55d14b5e87.pack\n\non the command line ?\n"},{"id":"256185","messageId":"4051C354-C16E-49D3-8824-38882F68F61E@gmail.com","threadId":"38574","inReplyTo":"54E2D2F3.5080800@web.de","subject":"Re: odb_mkstemp's 0444 permission broke write/delete access on AFP","fromName":"Fairuzan Roslan","fromEmail":"fairuzan.roslan@gmail.com","sentAt":"2015-02-17T05:54:24Z","receivedAt":"2015-02-17T05:54:24Z","isPatch":false,"sender":{"key":"fairuzan.roslan@gmail.com","avatar":null},"body":"\n> On Feb 17, 2015, at 1:34 PM, Torsten Bögershausen wrote:\n> \n> On 02/17/2015 04:22 AM, Fairuzan Roslan wrote:\n>>> On Feb 17, 2015, at 3:08 AM, Matthieu Moy wrote:\n>>> \n>>> [ Please, don't top post on this list ]\n>>> \n>>> Fairuzan Roslan writes:\n>>> \n>>>> I don’t see the issue for the owner of his/her own file to have write\n>>>> access.\n>>> Object and pack files are not meant to be modified. Hence, they are\n>>> read-only so that an (accidental) attempt to modify them fails.\n>>> \n>>>> Setting tmp idx & pack files to read-only even for the file owner is\n>>>> not a safety feature.\n>>> Yes it is. If you do not think so, then please give some arguments.\n>>> \n>>>> You should at least give the user the option to set the permission in\n>>>> the config file and not hardcoded the permission in the binary.\n>>> This is the kind of thing I meant by \"investigate alternate solutions\".\n>>> I have no AFP share to test, so it would help if you answered the\n>>> question I asked in my previous message:\n>>> \n>>>>> On Feb 17, 2015, at 2:23 AM, Matthieu Moy wrote:\n>>>>> \n>>>>> Fairuzan Roslan writes:\n>>>>> \n>>>>>> Hi,\n>>>>>> \n>>>>>> Somehow the “int mode = 0444;” in odb_mkstemp (environment.c) are\n>>>>>> causing a lot of issues (unable to unlink/write/rename) to those\n>>>>>> people who use AFP shares.\n>>>>> Is it a problem when using Git (like \"git gc\" failing to remove old\n>>>>> packs), or when trying to remove files outside Git?\n>>> (BTW, why did you try to write/rename pack files?)\n>>> \n>>> --\n>>> Matthieu Moy\n>>> http://www-verimag.imag.fr/~moy/\n>> I think its easier if I just show you…\n>> \n>> OS : OS X 10.10.0 - 10.10.2\n>> Client : git version 1.9.3 (Apple Git-50) and git version 2.2.1\n>> AFP share : //user@hostname._afpovertcp._tcp.local/installer on /Volumes/installer (afpfs, nodev, nosuid, mounted by user)\n>> \n>> 1. git clone example\n>> \n>> $ git clone https://github.com/robbyrussell/oh-my-zsh.git\n>> Cloning into 'oh-my-zsh'...\n>> remote: Counting objects: 11830, done.\n>> remote: Total 11830 (delta 0), reused 0 (delta 0)\n>> Receiving objects: 100% (11830/11830), 2.12 MiB | 481.00 KiB/s, done.\n>> Resolving deltas: 100% (6510/6510), done.\n>> warning: unable to unlink /Volumes/installer/oh-my-zsh/.git/objects/pack/tmp_pack_zjPxuc: Operation not permitted\n>> error: unable to write sha1 filename /Volumes/installer/oh-my-zsh/.git/objects/pack/pack-cceafdc9ef02bc58844138ba543ec6cc38252bb1.pack: Operation not permitted\n>> fatal: cannot store pack file\n>> fatal: index-pack failed\n>> \n>> $ ls -l oh-my-zsh/.git/objects/pack\n>> total 5008\n>> -rw------- 1 user staff 32 Feb 17 09:59 pack-cceafdc9ef02bc58844138ba543ec6cc38252bb1.keep\n>> -r--r--r-- 1 user staff 332312 Feb 17 09:59 tmp_idx_oUN1sb\n>> -r--r--r-- 1 user staff 2223007 Feb 17 09:59 tmp_pack_zjPxuc\n>> \n>> $ rm -rf oh-my-zsh/.git/objects/pack/tmp_*\n>> rm: oh-my-zsh/.git/objects/pack/tmp_idx_oUN1sb: Operation not permitted\n>> rm: oh-my-zsh/.git/objects/pack/tmp_pack_zjPxuc: Operation not permitted\n>> \n>> Detail Errors:\n>> 1. delete_ref_loose (refs.c) -> unlink_or_msg (wrapper.c) -> \"unable to unlink %s: %s\"\n>> 2. move_temp_to_file (sha1_file.c ) -> “unable to write sha1 filename %s: %s”\n>> \n>> 2. git pull example\n>> \n>> Textual git:master $ git pull\n>> remote: Counting objects: 435, done.\n>> remote: Compressing objects: 100% (398/398), done.\n>> remote: Total 435 (delta 219), reused 18 (delta 12)\n>> Receiving objects: 100% (435/435), 1.22 MiB | 756.00 KiB/s, done.\n>> Resolving deltas: 100% (219/219), done.\n>> warning: unable to unlink .git/objects/pack/tmp_pack_vDaIZa: Operation not permitted\n>> error: unable to write sha1 filename .git/objects/pack/pack-977a2dc0f4be3996dc1186e565a30d55d14b5e87.pack: Operation not permitted\n> I'm somewhat unsure how this is connected to 0444 ?\n> \n> It seems as if you don't have write permissions for some reasons.\n> (on the higher directory), what does\n> ls -ld .git/objects/pack/\n> ls -ld .git/objects/\n> give ?\n> \n> can you run\n> rm .git/objects/pack/pack-977a2dc0f4be3996dc1186e565a30d55d14b5e87.pack\n> \n> on the command line ?\n\nNo. I have write permission on all of the folders.\ndrwxr-xr-x 1 user staff 264 Feb 17 11:05 .\ndrwxr-xr-x 1 user staff 264 Jan 30 12:52 ..\n\nIt has nothing to do with my folder permissions. Like I said earlier this only happened to people who use AFP shares.\n\nWhen odb_mkstemp being called and sets the tmp idx & pack files to 0444 and later functions like unlink_or_msg or finish_tmp_packfile tries to unlink or rename those files, it will fail\n\nIt would be much faster and easier if you can try it on a AFP shares or I can talk you through it over irc @freenode #git (riaf^)\n\nRegards,\nFairuzan\n\n"},{"id":"256188","messageId":"vpqr3tozzs5.fsf@anie.imag.fr","threadId":"38574","inReplyTo":"13683B35-70A8-4D9E-80E1-440E4E0DC7F0@gmail.com","subject":"Re: odb_mkstemp's 0444 permission broke write/delete access on AFP","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-02-17T08:51:38Z","receivedAt":"2015-02-17T08:51:38Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Fairuzan Roslan writes:\n\n> $ git clone https://github.com/robbyrussell/oh-my-zsh.git\n> Cloning into 'oh-my-zsh'...\n> remote: Counting objects: 11830, done.\n> remote: Total 11830 (delta 0), reused 0 (delta 0)\n> Receiving objects: 100% (11830/11830), 2.12 MiB | 481.00 KiB/s, done.\n> Resolving deltas: 100% (6510/6510), done.\n> warning: unable to unlink /Volumes/installer/oh-my-zsh/.git/objects/pack/tmp_pack_zjPxuc: Operation not permitted\n\nThis should be fixable from Git itself, by replacing the calls to\n\"unlink\" with something like\n\nint unlink_or_chmod(...) {\n\tif (unlink(...)) {\n\t\tchmod(...); // give user write permission\n\t\treturn unlink(...);\n\t}\n}\n\nThis does not add extra cost in the normal case, and would fix this\nparticular issue for afp shares. So, I think that would fix the biggest\nproblem for afp-share users without disturbing others. It seems\nreasonable to me to do that unconditionnally.\n\n> $ rm -rf oh-my-zsh/.git/objects/pack/tmp_*\n> rm: oh-my-zsh/.git/objects/pack/tmp_idx_oUN1sb: Operation not permitted\n> rm: oh-my-zsh/.git/objects/pack/tmp_pack_zjPxuc: Operation not permitted\n\nWhat happens if you do \"rm -fr oh-my-zsh/.git/objects/pack/\" (i.e.\nremove the directory, not the files)?\n\nIf you can still remove the directory, then I'd say the solution above\ncould be sufficient: the user isn't supposed to interfer with the\ncontent of .git/objects other than by using Git, and if he or she does,\nthen asking a chmod prior to an rm seems reasonable.\n\nIf you can't, then it's another problematic use-case (basically, you\ncan't just \"rm -fr\" a whole clone), and then it deserves at least an\nopt-in configuration to get writable pack files.\n\n(Unfortunately, I suspect we're in the later case)\n\n> If you insist on setting the tmp idx & pack file permission to 0444 at\n> least give it a u+w permission whenever you try to unlink and rename\n> it so it won’t fail.\n\nYes. In case you hadn't guessed, this is precisely what I had in mind\nwhen I asked \"Is it a problem when using Git [...] or when trying to\nremove files outside Git?\".\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"256205","messageId":"CA0F915F-74B1-4292-AFB8-D1A4C76C0137@gmail.com","threadId":"38574","inReplyTo":"vpqr3tozzs5.fsf@anie.imag.fr","subject":"Re: odb_mkstemp's 0444 permission broke write/delete access on AFP","fromName":"Fairuzan Roslan","fromEmail":"fairuzan.roslan@gmail.com","sentAt":"2015-02-17T16:58:25Z","receivedAt":"2015-02-17T16:58:25Z","isPatch":false,"sender":{"key":"fairuzan.roslan@gmail.com","avatar":null},"body":"\n> On Feb 17, 2015, at 4:51 PM, Matthieu Moy wrote:\n> \n> Fairuzan Roslan writes:\n> \n>> $ git clone https://github.com/robbyrussell/oh-my-zsh.git\n>> Cloning into 'oh-my-zsh'...\n>> remote: Counting objects: 11830, done.\n>> remote: Total 11830 (delta 0), reused 0 (delta 0)\n>> Receiving objects: 100% (11830/11830), 2.12 MiB | 481.00 KiB/s, done.\n>> Resolving deltas: 100% (6510/6510), done.\n>> warning: unable to unlink /Volumes/installer/oh-my-zsh/.git/objects/pack/tmp_pack_zjPxuc: Operation not permitted\n> \n> This should be fixable from Git itself, by replacing the calls to\n> \"unlink\" with something like\n> \n> int unlink_or_chmod(...) {\n> \tif (unlink(...)) {\n> \t\tchmod(...); // give user write permission\n> \t\treturn unlink(...);\n> \t}\n> }\n> \n> This does not add extra cost in the normal case, and would fix this\n> particular issue for afp shares. So, I think that would fix the biggest\n> problem for afp-share users without disturbing others. It seems\n> reasonable to me to do that unconditionnally.\n> \n>> $ rm -rf oh-my-zsh/.git/objects/pack/tmp_*\n>> rm: oh-my-zsh/.git/objects/pack/tmp_idx_oUN1sb: Operation not permitted\n>> rm: oh-my-zsh/.git/objects/pack/tmp_pack_zjPxuc: Operation not permitted\n> \n> What happens if you do \"rm -fr oh-my-zsh/.git/objects/pack/\" (i.e.\n> remove the directory, not the files)?\n> \n> If you can still remove the directory, then I'd say the solution above\n> could be sufficient: the user isn't supposed to interfer with the\n> content of .git/objects other than by using Git, and if he or she does,\n> then asking a chmod prior to an rm seems reasonable.\n> \n> If you can't, then it's another problematic use-case (basically, you\n> can't just \"rm -fr\" a whole clone), and then it deserves at least an\n> opt-in configuration to get writable pack files.\n> \n> (Unfortunately, I suspect we're in the later case)\n> \n>> If you insist on setting the tmp idx & pack file permission to 0444 at\n>> least give it a u+w permission whenever you try to unlink and rename\n>> it so it won’t fail.\n> \n> Yes. In case you hadn't guessed, this is precisely what I had in mind\n> when I asked \"Is it a problem when using Git [...] or when trying to\n> remove files outside Git?\".\n> \n> --\n> Matthieu Moy\n> http://www-verimag.imag.fr/~moy/\n\nYes. It’s a problem when using Git where it fails to unlink and rename the tmp idx and pack files.\nThe reason I tries to rm -rf the tmp_idx_XXXXXX and tmp_pack_XXXXXX is to proof a point why Git fails\n\nPerhaps my explanation wasn’t clear enough. Maybe it’s hard for you to understand without having to test it yourself on a AFP filesystem.\n\nLet me explain why AFP filesystem is more strict and different from your typical filesystem like ext4,hfs+,etc.\n\n$ mkdir testdir; chmod 0755 testdir; touch testdir/testfile; chmod 0444 testdir/testfile; ls -la testdir\ntotal 0\ndrwxr-xr-x 1 user staff 264 Feb 18 00:26 .\ndrwx------ 1 user staff 264 Feb 18 00:26 ..\n-r--r--r-- 1 user staff 0 Feb 18 00:26 testfile\n\n$ rm -rf testdir\nrm: testdir/testfile: Operation not permitted\nrm: testdir: Directory not empty\n\n$ chmod +w testdir/testfile; ls -la testdir\ntotal 0\ndrwxr-xr-x 1 riaf staff 264 Feb 18 00:26 .\ndrwx------ 1 riaf staff 264 Feb 18 00:26 ..\n-rw-r—r-- 1 riaf staff 0 Feb 18 00:26 testfile\n\n$ rm -rf testdir <—— No error message\n\nThis show that you cannot delete a directory or a file without a write permission in AFP filesystem.\n\nThe problem with Git failing is not because its inability to delete a directory but its inability to unlink and rename tmp_idx_XXXXXX and tmp_pack_XXXXXX because those files were set to 0444 by odb_mkstemp.\nTry google for “Git AFP” and you will see a lot people are facing with the same problem.\n\nRegarding your suggestion, yes I think it would work but you have to take care (chmod) every calls that rename or unlink or delete files with 0444 permission.\n\nRegards,\nFairuzan\n\n\n"},{"id":"256220","messageId":"xmqqwq3g31hj.fsf@gitster.dls.corp.google.com","threadId":"38574","inReplyTo":"vpqr3tozzs5.fsf@anie.imag.fr","subject":"Re: odb_mkstemp's 0444 permission broke write/delete access on AFP","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-02-17T17:13:28Z","receivedAt":"2015-02-17T17:13:28Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> This should be fixable from Git itself, by replacing the calls to\n> \"unlink\" with something like\n>\n> int unlink_or_chmod(...) {\n> \tif (unlink(...)) {\n> \t\tchmod(...); // give user write permission\n> \t\treturn unlink(...);\n> \t}\n> }\n\nI agree with the approach in principle, but I wonder if we want to\ncontaminate the generic codepath with unlink_or_chmod().\n\nDon't we want to have this\n\n\t#undef unlink\n\tint workaround_broken_unlink(...) {\n \t... the same ...\n\t}\n\nin compat/broken-unlink.c and something like this\n\n\t#ifdef BROKEN_UNLINK\n\t#define unlink(x) workaround_broken_unlink(x)\n #endif\n\nin git-compat-util.h instead? That way, people on well behaving\nsystems do not have to worry about clobbering errno and stuff,\nperhaps?\n"},{"id":"256222","messageId":"54E3804D.6020301@web.de","threadId":"38574","inReplyTo":"CA0F915F-74B1-4292-AFB8-D1A4C76C0137@gmail.com","subject":"Re: odb_mkstemp's 0444 permission broke write/delete access on AFP","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2015-02-17T17:54:21Z","receivedAt":"2015-02-17T17:54:21Z","isPatch":false,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 17/02/15 17:58, Fairuzan Roslan wrote:\n> \n>> On Feb 17, 2015, at 4:51 PM, Matthieu Moy wrote:\n>>\n>> Fairuzan Roslan writes:\n>>\n>>> $ git clone https://github.com/robbyrussell/oh-my-zsh.git\n>>> Cloning into 'oh-my-zsh'...\n>>> remote: Counting objects: 11830, done.\n>>> remote: Total 11830 (delta 0), reused 0 (delta 0)\n>>> Receiving objects: 100% (11830/11830), 2.12 MiB | 481.00 KiB/s, done.\n>>> Resolving deltas: 100% (6510/6510), done.\n>>> warning: unable to unlink /Volumes/installer/oh-my-zsh/.git/objects/pack/tmp_pack_zjPxuc: Operation not permitted\n>>\n>> This should be fixable from Git itself, by replacing the calls to\n>> \"unlink\" with something like\n>>\n>> int unlink_or_chmod(...) {\n>> \tif (unlink(...)) {\n>> \t\tchmod(...); // give user write permission\n>> \t\treturn unlink(...);\n>> \t}\n>> }\n>>\n>> This does not add extra cost in the normal case, and would fix this\n>> particular issue for afp shares. So, I think that would fix the biggest\n>> problem for afp-share users without disturbing others. It seems\n>> reasonable to me to do that unconditionnally.\n>>\n>>> $ rm -rf oh-my-zsh/.git/objects/pack/tmp_*\n>>> rm: oh-my-zsh/.git/objects/pack/tmp_idx_oUN1sb: Operation not permitted\n>>> rm: oh-my-zsh/.git/objects/pack/tmp_pack_zjPxuc: Operation not permitted\n>>\n>> What happens if you do \"rm -fr oh-my-zsh/.git/objects/pack/\" (i.e.\n>> remove the directory, not the files)?\n>>\n>> If you can still remove the directory, then I'd say the solution above\n>> could be sufficient: the user isn't supposed to interfer with the\n>> content of .git/objects other than by using Git, and if he or she does,\n>> then asking a chmod prior to an rm seems reasonable.\n>>\n>> If you can't, then it's another problematic use-case (basically, you\n>> can't just \"rm -fr\" a whole clone), and then it deserves at least an\n>> opt-in configuration to get writable pack files.\n>>\n>> (Unfortunately, I suspect we're in the later case)\n>>\n>>> If you insist on setting the tmp idx & pack file permission to 0444 at\n>>> least give it a u+w permission whenever you try to unlink and rename\n>>> it so it won’t fail.\n>>\n>> Yes. In case you hadn't guessed, this is precisely what I had in mind\n>> when I asked \"Is it a problem when using Git [...] or when trying to\n>> remove files outside Git?\".\n>>\n>> --\n>> Matthieu Moy\n>> http://www-verimag.imag.fr/~moy/\n> \n> Yes. It’s a problem when using Git where it fails to unlink and rename the tmp idx and pack files.\n> The reason I tries to rm -rf the tmp_idx_XXXXXX and tmp_pack_XXXXXX is to proof a point why Git fails\n> \n> Perhaps my explanation wasn’t clear enough. Maybe it’s hard for you to understand without having to test it yourself on a AFP filesystem.\n> \n> Let me explain why AFP filesystem is more strict and different from your typical filesystem like ext4,hfs+,etc.\n> \n> $ mkdir testdir; chmod 0755 testdir; touch testdir/testfile; chmod 0444 testdir/testfile; ls -la testdir\n> total 0\n> drwxr-xr-x 1 user staff 264 Feb 18 00:26 .\n> drwx------ 1 user staff 264 Feb 18 00:26 ..\n> -r--r--r-- 1 user staff 0 Feb 18 00:26 testfile\n> \n> $ rm -rf testdir\n> rm: testdir/testfile: Operation not permitted\n> rm: testdir: Directory not empty\n> \nThis works on my system (Mac OS 10.9 as server and client)\n\n> $ chmod +w testdir/testfile; ls -la testdir\n> total 0\n> drwxr-xr-x 1 riaf staff 264 Feb 18 00:26 .\n> drwx------ 1 riaf staff 264 Feb 18 00:26 ..\n> -rw-r—r-- 1 riaf staff 0 Feb 18 00:26 testfile\n> \n> $ rm -rf testdir <—— No error message\n> \n> This show that you cannot delete a directory or a file without a write permission in AFP filesystem.\n> \n> The problem with Git failing is not because its inability to delete a directory but its inability to unlink and rename tmp_idx_XXXXXX and tmp_pack_XXXXXX because those files were set to 0444 by odb_mkstemp.\n> Try google for “Git AFP” and you will see a lot people are facing with the same problem.\nYes, (at least to my knowledge) you seem to be one of the first to report it here, thanks for that.\n> \n> Regarding your suggestion, yes I think it would work but you have to take care (chmod) every calls that rename or unlink or delete files with 0444 permission.\n> \n> Regards,\n> Fairuzan\n> \nThe \"right\" solution is to make a wrapper function, and to re-define unlink() and rename() with help\nof the preprocessor.\n\ngit-compat-util.h has an example for fopen(), so that can be used for a patch.\n\nAnd no, as I can not reproduce it here, I can only help with reviews or so.\n\n\n\n\n\n\n\n \n"},{"id":"256257","messageId":"vpq61azab52.fsf@anie.imag.fr","threadId":"38574","inReplyTo":"54E3804D.6020301@web.de","subject":"Re: odb_mkstemp's 0444 permission broke write/delete access on AFP","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-02-18T08:15:21Z","receivedAt":"2015-02-18T08:15:21Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Torsten Bögershausen writes:\n\n> On 17/02/15 17:58, Fairuzan Roslan wrote:\n>\n>> $ rm -rf testdir\n>> rm: testdir/testfile: Operation not permitted\n>> rm: testdir: Directory not empty\n>> \n> This works on my system (Mac OS 10.9 as server and client)\n\nJust to be sure: by \"work\", you mean \"successfully removes the\ndirectory\", right?\n\n>> The problem with Git failing is not because its inability to delete a directory but its inability to unlink and rename tmp_idx_XXXXXX and tmp_pack_XXXXXX because those files were set to 0444 by odb_mkstemp.\n>> Try google for “Git AFP” and you will see a lot people are facing with the same problem.\n> Yes, (at least to my knowledge) you seem to be one of the first to report it here, thanks for that.\n\nAnd now I'm starting to wonder whether other people do have the same\nissue. Sure, googling \"Git AFP\" shows a lot of people having problems\nwith Git and AFP, but are they really the same problem?\n\nI googled 'git afp \"unable to unlink\"', and all results except one point\nto this thread:\n\nhttps://www.google.com/search?q=git+afp+%22warning%3A+unable+to+unlink%22\n\nThe only one which doesn't actually does not mention afp.\n\nFairuzan: are you sure you're not the only one having the issue? Can you\ngive more info on your system (OS version client and server side, ...)?\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"256267","messageId":"E28114B6-86FA-4F4A-9D5F-BFC03B9D79B8@gmail.com","threadId":"38574","inReplyTo":"vpq61azab52.fsf@anie.imag.fr","subject":"Re: odb_mkstemp's 0444 permission broke write/delete access on AFP","fromName":"Fairuzan Roslan","fromEmail":"fairuzan.roslan@gmail.com","sentAt":"2015-02-18T13:47:17Z","receivedAt":"2015-02-18T13:47:17Z","isPatch":false,"sender":{"key":"fairuzan.roslan@gmail.com","avatar":null},"body":"\n> On Feb 18, 2015, at 4:15 PM, Matthieu Moy wrote:\n> \n> And now I'm starting to wonder whether other people do have the same\n> issue. Sure, googling \"Git AFP\" shows a lot of people having problems\n> with Git and AFP, but are they really the same problem?\n> \n> I googled 'git afp \"unable to unlink\"', and all results except one point\n> to this thread:\n> \n> https://www.google.com/search?q=git+afp+%22warning%3A+unable+to+unlink%22\n> \n> The only one which doesn't actually does not mention afp.\n> \n> Fairuzan: are you sure you're not the only one having the issue? Can you\n> give more info on your system (OS version client and server side, …)?\n\nNo I don’t think I’m the only one with the permission issue here but I’m sure many people prefer using smb over afp\n\n\nClient: OS X 10.9 - 10.10.2\ngit client: git version 1.9.3 (Apple Git-50) and git version 2.2.1\n\nServer : Linux 3.2.40 (Synology DSM 5.1)\nAFP : Netatalk afpd 3.1.1\n\nFrom what I have tested whenever a file’s permission set to read-only on the AFP shares a user immutable flag were set as well hence why any action such as unlink or rename failed.\n\n$ ls -laO\ntotal 0\ndrwxr-xr-x 1 user staff - 264 Feb 18 21:31 .\ndrwx------ 1 user staff - 264 Feb 18 21:31 ..\n-rw-r--r-- 1 user staff - 0 Feb 18 21:31 testfile\n$ chmod 0444 testfile; ls -laO\ntotal 0\ndrwxr-xr-x 1 user staff - 264 Feb 18 21:31 .\ndrwx------ 1 user staff - 264 Feb 18 21:32 ..\n-r--r--r-- 1 user staff uchg 0 Feb 18 21:31 testfile\n\nI believe this to stop a read-only file to be modified by the user in Netatalk AFPD.\n\n$ chmod 0644 testfile; ls -laO\ntotal 0\ndrwxr-xr-x 1 user staff - 264 Feb 18 21:32 .\ndrwx------ 1 user staff - 264 Feb 18 21:43 ..\n-rw-r--r-- 1 user staff - 0 Feb 18 21:31 testfile\n\nChanging the permission to read-write will remove the user immutable flag.\n\nRegards,\nFairuzan\n\n"},{"id":"256269","messageId":"vpqr3tn2u3f.fsf@anie.imag.fr","threadId":"38574","inReplyTo":"E28114B6-86FA-4F4A-9D5F-BFC03B9D79B8@gmail.com","subject":"Re: odb_mkstemp's 0444 permission broke write/delete access on AFP","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-02-18T14:05:24Z","receivedAt":"2015-02-18T14:05:24Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Fairuzan Roslan writes:\n\n> Client: OS X 10.9 - 10.10.2\n> git client: git version 1.9.3 (Apple Git-50) and git version 2.2.1\n>\n> Server : Linux 3.2.40 (Synology DSM 5.1)\n> AFP : Netatalk afpd 3.1.1\n\nAny chance you can test this with a Mac OS server?\n\nPerhaps because the server is not a Mac OS, it doesn't have the uchg\nflag, and maps it to the w bit in the POSIX permission system (this is\npure speculation from me).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"256270","messageId":"293AD985-D133-4454-BBBA-B4106F16E91D@gmail.com","threadId":"38574","inReplyTo":"vpqr3tn2u3f.fsf@anie.imag.fr","subject":"Re: odb_mkstemp's 0444 permission broke write/delete access on AFP","fromName":"Fairuzan Roslan","fromEmail":"fairuzan.roslan@gmail.com","sentAt":"2015-02-18T14:23:21Z","receivedAt":"2015-02-18T14:23:21Z","isPatch":false,"sender":{"key":"fairuzan.roslan@gmail.com","avatar":null},"body":"\n> On Feb 18, 2015, at 10:05 PM, Matthieu Moy wrote:\n> \n> Fairuzan Roslan writes:\n> \n>> Client: OS X 10.9 - 10.10.2\n>> git client: git version 1.9.3 (Apple Git-50) and git version 2.2.1\n>> \n>> Server : Linux 3.2.40 (Synology DSM 5.1)\n>> AFP : Netatalk afpd 3.1.1\n> \n> Any chance you can test this with a Mac OS server?\n> \n> Perhaps because the server is not a Mac OS, it doesn't have the uchg\n> flag, and maps it to the w bit in the POSIX permission system (this is\n> pure speculation from me).\n> \n> --\n> Matthieu Moy\n> http://www-verimag.imag.fr/~moy/\n\nI don’t have any Mac OS X server in my disposal at this moment but if you want to test out with netatalk AFPD I believe it’s available in debian & ubuntu apt repo\n\nRegards,\nFairuzan\n"},{"id":"256280","messageId":"vpqwq3fup5g.fsf@anie.imag.fr","threadId":"38574","inReplyTo":"xmqqwq3g31hj.fsf@gitster.dls.corp.google.com","subject":"Re: odb_mkstemp's 0444 permission broke write/delete access on AFP","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-02-18T17:04:43Z","receivedAt":"2015-02-18T17:04:43Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano writes:\n\n> in compat/broken-unlink.c and something like this\n>\n> \t#ifdef BROKEN_UNLINK\n> \t#define unlink(x) workaround_broken_unlink(x)\n> #endif\n>\n> in git-compat-util.h instead? \n\nThat means we have to know BROKEN_UNLINK at compile-time. I had never\nheard about AFP before this thread, but they seem mountable on Linux and\nWindows. I don't know whether these platforms will have the same issue,\nbut I suspect they will (if the server rejects the unlink).\n\nSo, if my suspicion is right, we'd have to activate it on any platform\nable to mount AFP, i.e. essentially everywhere.\n\n> That way, people on well behaving systems do not have to worry about\n> clobbering errno and stuff, perhaps?\n\nWith my solution, unlink() is always the last call in the function, so\nit should behave correctly right? Or did I miss anything?\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"256282","messageId":"xmqq4mqj3zyx.fsf@gitster.dls.corp.google.com","threadId":"38574","inReplyTo":"vpqwq3fup5g.fsf@anie.imag.fr","subject":"Re: odb_mkstemp's 0444 permission broke write/delete access on AFP","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-02-18T17:13:10Z","receivedAt":"2015-02-18T17:13:10Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> Junio C Hamano writes:\n>\n>> in compat/broken-unlink.c and something like this\n>>\n>> \t#ifdef BROKEN_UNLINK\n>> \t#define unlink(x) workaround_broken_unlink(x)\n>> #endif\n>>\n>> in git-compat-util.h instead? \n>\n> That means we have to know BROKEN_UNLINK at compile-time. I had never\n> heard about AFP before this thread, but they seem mountable on Linux and\n> Windows. I don't know whether these platforms will have the same issue,\n> but I suspect they will (if the server rejects the unlink).\n>\n> So, if my suspicion is right, we'd have to activate it on any platform\n> able to mount AFP, i.e. essentially everywhere.\n\nSigh.\n\n>\n>> That way, people on well behaving systems do not have to worry about\n>> clobbering errno and stuff, perhaps?\n>\n> With my solution, unlink() is always the last call in the function, so\n> it should behave correctly right? Or did I miss anything?\n\nI am primarily worried about blindly attempting to run chmod() after\nseeing a failure from the first unlink() _without_ even making sure\nthat the failure is caused by this silly bug in a single filesystem.\n\nIf the first unlink() failed for some other reason, chmod()\nsucceeded, and then the second unlink() failed for the same reason\nas the first failure (because the mode bits of the file being\nunlinked did not have anything to do with it), that would leave a\nfile with wrong permission bits. And doing so when the user may\nknow that there is no AFP involved in her set-up would be doubly\nwrong, no?\n"},{"id":"256283","messageId":"xmqqy4nv2kjk.fsf@gitster.dls.corp.google.com","threadId":"38574","inReplyTo":"xmqq4mqj3zyx.fsf@gitster.dls.corp.google.com","subject":"Re: odb_mkstemp's 0444 permission broke write/delete access on AFP","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-02-18T17:31:43Z","receivedAt":"2015-02-18T17:31:43Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Matthieu Moy writes:\n>\n>> Junio C Hamano writes:\n>>\n>>> in compat/broken-unlink.c and something like this\n>>>\n>>> \t#ifdef BROKEN_UNLINK\n>>> \t#define unlink(x) workaround_broken_unlink(x)\n>>> #endif\n>>>\n>>> in git-compat-util.h instead? \n>>\n>> That means we have to know BROKEN_UNLINK at compile-time. I had never\n>> heard about AFP before this thread, but they seem mountable on Linux and\n>> Windows. I don't know whether these platforms will have the same issue,\n>> but I suspect they will (if the server rejects the unlink).\n>>\n>> So, if my suspicion is right, we'd have to activate it on any platform\n>> able to mount AFP, i.e. essentially everywhere.\n>\n> Sigh.\n\nThat is \"Sigh. It is unfortunate but you are correct.\".\n\nPerhaps we would need to do something ugly like this:\n\n * add \"core.brokenUnlink\" configuration (or whatever we end up\n calling this filesystem trait) and add \"int broken_unlink\" in\n environment.c (declare it in cache.h).\n\n * in init-db.c, autoprobe by doing something like this:\n\n create a test file with 0444 permission bits;\n if (unlink(that test file)) {\n\tchmod(that test file, 0644);\n if (!unlink(that test file)) {\n\t\tbroken_unlink = 1;\n\t\tgit_config_set(\"core.brokenunlink\", broken_unlink);\n\t} else {\n \tdie(\"aaargh\");\n\t}\n }\n\n * Do your unlink_or_chmod() thing in wrapper.c, but perhaps call it\n xunlink(), like this:\n\n int xunlink(...) {\n\t\tint ret = unlink(...);\n\t\tif (broken_unlink && ret) {\n chmod(..., 0644);\n ret = unlink(...);\n }\n return ret;\n }\n\nWe probably need something similar for xrename()?\n\t\n"},{"id":"256363","messageId":"20150219200833.GB5021@vauxhall.crustytoothpaste.net","threadId":"38574","inReplyTo":"vpqr3tozzs5.fsf@anie.imag.fr","subject":"Re: odb_mkstemp's 0444 permission broke write/delete access on AFP","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2015-02-19T20:08:33Z","receivedAt":"2015-02-19T20:08:33Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Tue, Feb 17, 2015 at 09:51:38AM +0100, Matthieu Moy wrote:\n> This should be fixable from Git itself, by replacing the calls to\n> \"unlink\" with something like\n> \n> int unlink_or_chmod(...) {\n> \tif (unlink(...)) {\n> \t\tchmod(...); // give user write permission\n> \t\treturn unlink(...);\n> \t}\n> }\n> \n> This does not add extra cost in the normal case, and would fix this\n> particular issue for afp shares. So, I think that would fix the biggest\n> problem for afp-share users without disturbing others. It seems\n> reasonable to me to do that unconditionnally.\n\nThis can have security issues if you're trying to unlink a symlink, as \nchmod will dereference the symlink but unlink will not. Giving the file \nowner write permission may not be sufficient, as the user may be a \nmember of a group with write access to the repo. A malicious user who \nalso has access to the repo could force the current user to chmod an \narbitrary file such that it had looser permissions.\n\nI've seen a case where Perl's ExtUtils::MakeMaker chmoded \n/etc/mime.types 0666 as a result of this.\n\nI don't think there's a secure way to implement this unless you're on an \nOS with lchmod or fchmodat that supports AT_SYMLINK_NOFOLLOW. Linux is \nnot one of those systems.\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\n+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187\n"},{"id":"256410","messageId":"vpqvbiwamt3.fsf@anie.imag.fr","threadId":"38574","inReplyTo":"20150219200833.GB5021@vauxhall.crustytoothpaste.net","subject":"Re: odb_mkstemp's 0444 permission broke write/delete access on AFP","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-02-20T10:40:08Z","receivedAt":"2015-02-20T10:40:08Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"\"brian m. carlson\" writes:\n\n> On Tue, Feb 17, 2015 at 09:51:38AM +0100, Matthieu Moy wrote:\n>> This should be fixable from Git itself, by replacing the calls to\n>> \"unlink\" with something like\n>> \n>> int unlink_or_chmod(...) {\n>> \tif (unlink(...)) {\n>> \t\tchmod(...); // give user write permission\n>> \t\treturn unlink(...);\n>> \t}\n>> }\n>> \n>> This does not add extra cost in the normal case, and would fix this\n>> particular issue for afp shares. So, I think that would fix the biggest\n>> problem for afp-share users without disturbing others. It seems\n>> reasonable to me to do that unconditionnally.\n>\n> This can have security issues if you're trying to unlink a symlink, as \n> chmod will dereference the symlink but unlink will not. Giving the file \n> owner write permission may not be sufficient, as the user may be a \n> member of a group with write access to the repo. A malicious user who \n> also has access to the repo could force the current user to chmod an \n> arbitrary file such that it had looser permissions.\n\nOuch, indeed. I don't think that would be so problematic in practice (if\nthe attacker has access to the repo, it's easier to write arbitrary code\nin hooks or config), but clearly we don't want to take the risk.\n\nSo, the right solution should be stg like Junio's\n\n * in init-db.c, autoprobe by doing something like this:\n\n create a test file with 0444 permission bits;\n if (unlink(that test file)) {\n\tchmod(that test file, 0644);\n if (!unlink(that test file)) {\n\t\tbroken_unlink = 1;\n\t\tgit_config_set(\"core.brokenunlink\", broken_unlink);\n\t} else {\n \tdie(\"aaargh\");\n\t}\n }\n\nBut when core.brokenunlink is set, just use 0666 instead of 0444 as\ndefault mode for object files.\n\nBut right now, I'm not sure we're actually to work around an issue with\nAFP shares or only one particular case of problematic configurtion for\none user.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"}]}