{"thread":{"id":"26630","subject":"Git changes permissions on directories when deleting files.","startedAt":"2011-03-01T01:42:40Z","lastAt":"2011-03-11T06:09:58Z","messageCount":19,"participants":["Chad Joan","Computer Druid","Jeff King","Matthieu Moy"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"162508","messageId":"AANLkTikJcOgBAZS=cCWULFYz4U_Mxx1gFMg51+r9qDo0@mail.gmail.com","threadId":"26630","inReplyTo":null,"subject":"Git changes permissions on directories when deleting files.","fromName":"Chad Joan","fromEmail":"chadjoan@gmail.com","sentAt":"2011-03-01T01:42:40Z","receivedAt":"2011-03-01T01:42:40Z","isPatch":false,"sender":{"key":"chadjoan@gmail.com","avatar":"https://gravatar.com/avatar/7ef94965e8ac203e77176e89aa990a0ab091b0ef08d275f835b308ff24267444?d=mp&s=160"},"body":"Hello,\n\nWhat I'm experiencing is this:\n\n$ cd ~/project\n$ ls -dl somedir\ndrwxrwx--- 1 cjoan cjoan 0 Feb 28 19:57 somedir\n$ echo \"some text\" > somedir/somefile.txt\n$ git add somedir/somefile.txt\n$ git rm -f somedir/somefile.txt\nrm 'somedir/somefile.txt'\n$ ls -dl somedir\ndrw------- 1 cjoan cjoan 0 Feb 28 19:57 somedir\n$ echo \"some text\" > somedir/somefile.txt\nbash: somedir/somefile.txt: Permission denied\n\n~/project is actually a CIFS mount, with the host being an OpenVMS machine.\n\nIf I use the normal rm command without using git then the permissions\nwill remain the same on 'somedir'.  This is why I suspect (and hope)\nthis isn't OpenVMS related.\n\nIt seems that execute bit is important for CIFS mounted files, because\nafter this happens I am no longer able to do /anything/ within the\n'somedir' directory.  This also affects branches via \"git checkout\nbranchname\": if the checkout happens to delete files then this will\nhappen, and it will salt the wound by failing to sync a bunch of files\nin 'somedir' (because I can't access them anymore) while still moving\nHEAD to the new branch.\n\nThe share on the CIFS host looks like this:\n\n[homes]\n        comment = Home Directories\n        read only = No\n        create mask = 0770\n        browseable = No\n        vfs objects = varvfc\n        vms path names = No\n        case sensitive = Yes\n\nThe fstab entry for the mount looks like this:\n\n//vms/homes  /home/cjoan/project  cifs\ncredentials=/home/cjoan/.cifs_credentials,_netdev,uid=cjoan,gid=cjoan\n0 0\n\nI'd really like my directories to keep their permissions.\nAny idea what might cause this?\n\n- Chad\n"},{"id":"162509","messageId":"AANLkTi=jCtR1NHs8ji0GmdXJXjZ+xPQ6-k1w-jRNtZX4@mail.gmail.com","threadId":"26630","inReplyTo":"AANLkTikJcOgBAZS=cCWULFYz4U_Mxx1gFMg51+r9qDo0@mail.gmail.com","subject":"Re: Git changes permissions on directories when deleting files.","fromName":"Chad Joan","fromEmail":"chadjoan@gmail.com","sentAt":"2011-03-01T01:45:17Z","receivedAt":"2011-03-01T01:45:17Z","isPatch":false,"sender":{"key":"chadjoan@gmail.com","avatar":"https://gravatar.com/avatar/7ef94965e8ac203e77176e89aa990a0ab091b0ef08d275f835b308ff24267444?d=mp&s=160"},"body":"I should probably also mention that my git version is 1.7.3.4-r1\n"},{"id":"162512","messageId":"AANLkTinCjaGMe3TnheqORe7Y_qWYTAr3p6UEsK3u4VyE@mail.gmail.com","threadId":"26630","inReplyTo":"AANLkTikJcOgBAZS=cCWULFYz4U_Mxx1gFMg51+r9qDo0@mail.gmail.com","subject":"Re: Git changes permissions on directories when deleting files.","fromName":"Computer Druid","fromEmail":"computerdruid@gmail.com","sentAt":"2011-03-01T02:19:58Z","receivedAt":"2011-03-01T02:19:58Z","isPatch":false,"sender":{"key":"computerdruid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/34696?v=4"},"body":"On Mon, Feb 28, 2011 at 8:42 PM, Chad Joan wrote:\n> Hello,\n>\n> What I'm experiencing is this:\n>\n> $ cd ~/project\n> $ ls -dl somedir\n> drwxrwx--- 1 cjoan cjoan 0 Feb 28 19:57 somedir\n> $ echo \"some text\" > somedir/somefile.txt\n> $ git add somedir/somefile.txt\n> $ git rm -f somedir/somefile.txt\n> rm 'somedir/somefile.txt'\n> $ ls -dl somedir\n> drw------- 1 cjoan cjoan 0 Feb 28 19:57 somedir\n> $ echo \"some text\" > somedir/somefile.txt\n> bash: somedir/somefile.txt: Permission denied\n\nAfter you remove the file, is \"somedir\" empty?\n\nGit doesn't track empty directories, and therefore git rm on the last\nfile in a directory deletes it:\n\n% git init\nInitialized empty Git repository in /home/cdruid/testrepo/.git/\n% mkdir dir\n% ls -l\ntotal 4\ndrwxr-xr-x 2 cdruid cdruid 4096 Feb 28 21:14 dir\n% touch dir/test.txt\n% git add dir/test.txt\n% git rm -f dir/test.txt\nrm 'dir/test.txt'\n% ls -l\ntotal 0\n\nMy guess is git is somehow failing to delete the directory, thus\ncausing your changed permissions issue.\n\n-Dan Johnson\n"},{"id":"162513","messageId":"AANLkTikFMg_yLWmanqyHveDMR==bw8kxjZgr4mSOmY-2@mail.gmail.com","threadId":"26630","inReplyTo":"AANLkTinCjaGMe3TnheqORe7Y_qWYTAr3p6UEsK3u4VyE@mail.gmail.com","subject":"Re: Git changes permissions on directories when deleting files.","fromName":"Chad Joan","fromEmail":"chadjoan@gmail.com","sentAt":"2011-03-01T04:00:26Z","receivedAt":"2011-03-01T04:00:26Z","isPatch":false,"sender":{"key":"chadjoan@gmail.com","avatar":"https://gravatar.com/avatar/7ef94965e8ac203e77176e89aa990a0ab091b0ef08d275f835b308ff24267444?d=mp&s=160"},"body":"On Mon, Feb 28, 2011 at 9:19 PM, Computer Druid wrote:\n> On Mon, Feb 28, 2011 at 8:42 PM, Chad Joan wrote:\n>> Hello,\n>>\n>> What I'm experiencing is this:\n>>\n>> $ cd ~/project\n>> $ ls -dl somedir\n>> drwxrwx--- 1 cjoan cjoan 0 Feb 28 19:57 somedir\n>> $ echo \"some text\" > somedir/somefile.txt\n>> $ git add somedir/somefile.txt\n>> $ git rm -f somedir/somefile.txt\n>> rm 'somedir/somefile.txt'\n>> $ ls -dl somedir\n>> drw------- 1 cjoan cjoan 0 Feb 28 19:57 somedir\n>> $ echo \"some text\" > somedir/somefile.txt\n>> bash: somedir/somefile.txt: Permission denied\n>\n> After you remove the file, is \"somedir\" empty?\n>\n\nNope.\n\n> Git doesn't track empty directories, and therefore git rm on the last\n> file in a directory deletes it:\n>\n> % git init\n> Initialized empty Git repository in /home/cdruid/testrepo/.git/\n> % mkdir dir\n> % ls -l\n> total 4\n> drwxr-xr-x 2 cdruid cdruid 4096 Feb 28 21:14 dir\n> % touch dir/test.txt\n> % git add dir/test.txt\n> % git rm -f dir/test.txt\n> rm 'dir/test.txt'\n> % ls -l\n> total 0\n>\n> My guess is git is somehow failing to delete the directory, thus\n> causing your changed permissions issue.\n>\n> -Dan Johnson\n>\n\n'somedir' still has plenty of files in it after the deletion, so I'm\nafraid this isn't the case.\n\n- Chad\n"},{"id":"162555","messageId":"AANLkTimw+TLYv3ANf_Gx6G3SaLwRnRf6PF1YUv86rC5J@mail.gmail.com","threadId":"26630","inReplyTo":"AANLkTikFMg_yLWmanqyHveDMR==bw8kxjZgr4mSOmY-2@mail.gmail.com","subject":"Re: Git changes permissions on directories when deleting files.","fromName":"Chad Joan","fromEmail":"chadjoan@gmail.com","sentAt":"2011-03-01T15:51:39Z","receivedAt":"2011-03-01T15:51:39Z","isPatch":false,"sender":{"key":"chadjoan@gmail.com","avatar":"https://gravatar.com/avatar/7ef94965e8ac203e77176e89aa990a0ab091b0ef08d275f835b308ff24267444?d=mp&s=160"},"body":"More info:\n\n$ mkdir foo\n$ mkdir foo/bar\n$ echo \"test\" > foo/bar/baz.txt\n$ echo \"somestuff\" > foo/bar/somefile.txt\n$ git add foo/bar/*\n$ ls -dl foo\ndrwxr-x--x 1 cjoan cjoan 0 Mar 1 10:46 foo\n$ ls -dl foo/bar\ndrwxr-x--x 1 cjoan cjoan 0 Mar 1 10:46 foo/bar\n$ git rm -f foo/bar/somefile.txt\nrm 'foo/bar/somefile.txt'\n$ ls -dl foo\ndrwxr-x--x 1 cjoan cjoan 0 Mar 1 10:46 foo\n$ ls -dl foo/bar\ndrw------- 1 cjoan cjoan 0 Mar 1 10:47 foo/bar\n$ git rm -f foo/bar/baz.txt\nrm 'foo/bar/baz.txt'\nfatal: git rm: 'foo/bar/baz.txt': Permission denied\n\n\n\nThis time I tried it with git 1.7.4.\n"},{"id":"162559","messageId":"AANLkTimx7s94wjPasgdY7O9eoyzXXmhWm6f+CB0_2sv3@mail.gmail.com","threadId":"26630","inReplyTo":"AANLkTimw+TLYv3ANf_Gx6G3SaLwRnRf6PF1YUv86rC5J@mail.gmail.com","subject":"Re: Git changes permissions on directories when deleting files.","fromName":"Computer Druid","fromEmail":"computerdruid@gmail.com","sentAt":"2011-03-01T17:11:26Z","receivedAt":"2011-03-01T17:11:26Z","isPatch":false,"sender":{"key":"computerdruid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/34696?v=4"},"body":"On Tue, Mar 1, 2011 at 10:51 AM, Chad Joan wrote:\n> More info:\n>\n> $ mkdir foo\n> $ mkdir foo/bar\n> $ echo \"test\" > foo/bar/baz.txt\n> $ echo \"somestuff\" > foo/bar/somefile.txt\nWhat happens if you \"rmdir foo/bar\" here? (while there are files still in it)\n\n-Dan Johnson\n"},{"id":"162570","messageId":"AANLkTimBrUo_O6sjhSEf2sPKrYhjMcr24hwRe0kH4CgO@mail.gmail.com","threadId":"26630","inReplyTo":"AANLkTimx7s94wjPasgdY7O9eoyzXXmhWm6f+CB0_2sv3@mail.gmail.com","subject":"Re: Git changes permissions on directories when deleting files.","fromName":"Chad Joan","fromEmail":"chadjoan@gmail.com","sentAt":"2011-03-01T19:35:41Z","receivedAt":"2011-03-01T19:35:41Z","isPatch":false,"sender":{"key":"chadjoan@gmail.com","avatar":"https://gravatar.com/avatar/7ef94965e8ac203e77176e89aa990a0ab091b0ef08d275f835b308ff24267444?d=mp&s=160"},"body":"On Tue, Mar 1, 2011 at 12:11 PM, Computer Druid wrote:\n> On Tue, Mar 1, 2011 at 10:51 AM, Chad Joan wrote:\n>> More info:\n>>\n>> $ mkdir foo\n>> $ mkdir foo/bar\n>> $ echo \"test\" > foo/bar/baz.txt\n>> $ echo \"somestuff\" > foo/bar/somefile.txt\n> What happens if you \"rmdir foo/bar\" here? (while there are files still in it)\n>\n> -Dan Johnson\n>\n\nSomething fairly interesting:\n\n$ mkdir foo\n$ mkdir foo/bar\n$ ls -dl foo/bar\ndrwxr-x--x 1 cjoan cjoan 0 Mar 1 14:31 foo/bar\n$ ls -dl foo\ndrwxr-x--x 1 cjoan cjoan 0 Mar 1 14:31 foo\n$ echo \"test\" > foo/bar/baz.txt\n$ echo \"somestuff\" > foo/bar/somefile.txt\n$ ls -dl foo/bar\ndrwxr-x--x 1 cjoan cjoan 0 Mar 1 14:31 foo/bar\n$ ls -dl foo\ndrwxr-x--x 1 cjoan cjoan 0 Mar 1 14:31 foo\n$ rmdir foo/bar\nrmdir: failed to remove `foo/bar': Directory not empty\n$ ls -dl foo/bar\ndrw------- 1 cjoan cjoan 0 Mar 1 14:32 foo/bar\n$ ls -dl foo\ndrwxr-x--x 1 cjoan cjoan 0 Mar 1 14:31 foo\n\n\nThe rmdir fails of course, but it also changes the permissions.\nSo I take it that git always runs an rmdir on the parent directory\nwhen it removes a file? Seems like it would be a sensible way to do\nit on a system without this behavior.\n\n- Chad\n"},{"id":"162574","messageId":"20110301194428.GD10082@sigill.intra.peff.net","threadId":"26630","inReplyTo":"AANLkTimBrUo_O6sjhSEf2sPKrYhjMcr24hwRe0kH4CgO@mail.gmail.com","subject":"Re: Git changes permissions on directories when deleting files.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-03-01T19:44:28Z","receivedAt":"2011-03-01T19:44:28Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 01, 2011 at 02:35:41PM -0500, Chad Joan wrote:\n\n> Something fairly interesting:\n> \n> $ mkdir foo\n> $ mkdir foo/bar\n> $ ls -dl foo/bar\n> drwxr-x--x 1 cjoan cjoan 0 Mar 1 14:31 foo/bar\n> $ ls -dl foo\n> drwxr-x--x 1 cjoan cjoan 0 Mar 1 14:31 foo\n> $ echo \"test\" > foo/bar/baz.txt\n> $ echo \"somestuff\" > foo/bar/somefile.txt\n> $ ls -dl foo/bar\n> drwxr-x--x 1 cjoan cjoan 0 Mar 1 14:31 foo/bar\n> $ ls -dl foo\n> drwxr-x--x 1 cjoan cjoan 0 Mar 1 14:31 foo\n> $ rmdir foo/bar\n> rmdir: failed to remove `foo/bar': Directory not empty\n> $ ls -dl foo/bar\n> drw------- 1 cjoan cjoan 0 Mar 1 14:32 foo/bar\n> $ ls -dl foo\n> drwxr-x--x 1 cjoan cjoan 0 Mar 1 14:31 foo\n> \n> \n> The rmdir fails of course, but it also changes the permissions.\n> So I take it that git always runs an rmdir on the parent directory\n> when it removes a file? Seems like it would be a sensible way to do\n> it on a system without this behavior.\n\nExactly. Rather than spend time figuring out if the directory is\nremovable (which would not be atomic, anyway), we just rmdir and ignore\nthe error condition.\n\nI would argue that your filesystem is broken. Even if we implemented a\nworkaround to opendir() and check for files, it would still have a race\ncondition that could cause this situation to occur.\n\n-Peff\n"},{"id":"162576","messageId":"AANLkTimCzBwsz4TV=jEGeSEScVtgwmGEiDWOomaeTgWD@mail.gmail.com","threadId":"26630","inReplyTo":"20110301194428.GD10082@sigill.intra.peff.net","subject":"Re: Git changes permissions on directories when deleting files.","fromName":"Chad Joan","fromEmail":"chadjoan@gmail.com","sentAt":"2011-03-01T19:57:19Z","receivedAt":"2011-03-01T19:57:19Z","isPatch":false,"sender":{"key":"chadjoan@gmail.com","avatar":"https://gravatar.com/avatar/7ef94965e8ac203e77176e89aa990a0ab091b0ef08d275f835b308ff24267444?d=mp&s=160"},"body":"On Tue, Mar 1, 2011 at 2:44 PM, Jeff King wrote:\n> On Tue, Mar 01, 2011 at 02:35:41PM -0500, Chad Joan wrote:\n>\n>> Something fairly interesting:\n>>\n>> $ mkdir foo\n>> $ mkdir foo/bar\n>> $ ls -dl foo/bar\n>> drwxr-x--x 1 cjoan cjoan 0 Mar  1 14:31 foo/bar\n>> $ ls -dl foo\n>> drwxr-x--x 1 cjoan cjoan 0 Mar  1 14:31 foo\n>> $ echo \"test\" > foo/bar/baz.txt\n>> $ echo \"somestuff\" > foo/bar/somefile.txt\n>> $ ls -dl foo/bar\n>> drwxr-x--x 1 cjoan cjoan 0 Mar  1 14:31 foo/bar\n>> $ ls -dl foo\n>> drwxr-x--x 1 cjoan cjoan 0 Mar  1 14:31 foo\n>> $ rmdir foo/bar\n>> rmdir: failed to remove `foo/bar': Directory not empty\n>> $ ls -dl foo/bar\n>> drw------- 1 cjoan cjoan 0 Mar  1 14:32 foo/bar\n>> $ ls -dl foo\n>> drwxr-x--x 1 cjoan cjoan 0 Mar  1 14:31 foo\n>>\n>>\n>> The rmdir fails of course, but it also changes the permissions.\n>> So I take it that git always runs an rmdir on the parent directory\n>> when it removes a file?  Seems like it would be a sensible way to do\n>> it on a system without this behavior.\n>\n> Exactly. Rather than spend time figuring out if the directory is\n> removable (which would not be atomic, anyway), we just rmdir and ignore\n> the error condition.\n>\n> I would argue that your filesystem is broken. Even if we implemented a\n> workaround to opendir() and check for files, it would still have a race\n> condition that could cause this situation to occur.\n>\n> -Peff\n>\n\nOuch.\n\nWould it work to do something like alias rmdir to a script or program\nthat would call /bin/rmdir and then fix up the permissions?\n"},{"id":"162579","messageId":"20110301200805.GA18587@sigill.intra.peff.net","threadId":"26630","inReplyTo":"AANLkTimCzBwsz4TV=jEGeSEScVtgwmGEiDWOomaeTgWD@mail.gmail.com","subject":"Re: Git changes permissions on directories when deleting files.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-03-01T20:08:06Z","receivedAt":"2011-03-01T20:08:06Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 01, 2011 at 02:57:19PM -0500, Chad Joan wrote:\n\n> > Exactly. Rather than spend time figuring out if the directory is\n> > removable (which would not be atomic, anyway), we just rmdir and ignore\n> > the error condition.\n> >\n> > I would argue that your filesystem is broken. Even if we implemented a\n> > workaround to opendir() and check for files, it would still have a race\n> > condition that could cause this situation to occur.\n> \n> Ouch.\n> \n> Would it work to do something like alias rmdir to a script or program\n> that would call /bin/rmdir and then fix up the permissions?\n\nWell, we're using the rmdir system call, so you would need a patch to\ngit either way. If that was something we wanted to support (with a\nconfig option, of course), we could do the permissions check-and-restore\nourselves.\n\nBut it just seems horribly broken to me. This is CIFS to an OpenVMS\nmachine you said? Do the broken permissions appear to other clients or\nacross a remount (i.e., is it broken state in your CIFS client, or has\nthe server actually munged permissions)? If so, have you tried reporting\nthe issue to whoever writes CIFS server on OpenVMS (is it just samba)?\n\n-Peff\n"},{"id":"162582","messageId":"AANLkTint3PARNNN4cpic8XG6HsM3AAGuX5a+oeXfFNx=@mail.gmail.com","threadId":"26630","inReplyTo":"20110301200805.GA18587@sigill.intra.peff.net","subject":"Re: Git changes permissions on directories when deleting files.","fromName":"Chad Joan","fromEmail":"chadjoan@gmail.com","sentAt":"2011-03-01T20:30:39Z","receivedAt":"2011-03-01T20:30:39Z","isPatch":false,"sender":{"key":"chadjoan@gmail.com","avatar":"https://gravatar.com/avatar/7ef94965e8ac203e77176e89aa990a0ab091b0ef08d275f835b308ff24267444?d=mp&s=160"},"body":"On Tue, Mar 1, 2011 at 3:08 PM, Jeff King wrote:\n> On Tue, Mar 01, 2011 at 02:57:19PM -0500, Chad Joan wrote:\n>\n>> > Exactly. Rather than spend time figuring out if the directory is\n>> > removable (which would not be atomic, anyway), we just rmdir and ignore\n>> > the error condition.\n>> >\n>> > I would argue that your filesystem is broken. Even if we implemented a\n>> > workaround to opendir() and check for files, it would still have a race\n>> > condition that could cause this situation to occur.\n>>\n>> Ouch.\n>>\n>> Would it work to do something like alias rmdir to a script or program\n>> that would call /bin/rmdir and then fix up the permissions?\n>\n> Well, we're using the rmdir system call, so you would need a patch to\n> git either way. If that was something we wanted to support (with a\n> config option, of course), we could do the permissions check-and-restore\n> ourselves.\n>\n> But it just seems horribly broken to me. This is CIFS to an OpenVMS\n> machine you said? Do the broken permissions appear to other clients or\n> across a remount (i.e., is it broken state in your CIFS client, or has\n> the server actually munged permissions)? If so, have you tried reporting\n> the issue to whoever writes CIFS server on OpenVMS (is it just samba)?\n>\n> -Peff\n>\n\nYep, CIFS to OpenVMS.\n\nI don't know about other clients because there are none (yet). The\npermission change does survive remounting.\n\nI haven't reported it. I didn't know it existed until now ;)\n\nI'll do that, but it will probably take a long long time for me to see\nthe patch. I'm hoping there's some cheap hack I can use to work\naround it in the meantime.\n\n- Chad\n"},{"id":"162583","messageId":"AANLkTinkjhH6YR7VPC0hji9CK=qeQMAYr+ptJE3szUyR@mail.gmail.com","threadId":"26630","inReplyTo":"AANLkTint3PARNNN4cpic8XG6HsM3AAGuX5a+oeXfFNx=@mail.gmail.com","subject":"Re: Git changes permissions on directories when deleting files.","fromName":"Computer Druid","fromEmail":"computerdruid@gmail.com","sentAt":"2011-03-01T20:39:10Z","receivedAt":"2011-03-01T20:39:10Z","isPatch":false,"sender":{"key":"computerdruid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/34696?v=4"},"body":"On Tue, Mar 1, 2011 at 3:30 PM, Chad Joan wrote:\n> On Tue, Mar 1, 2011 at 3:08 PM, Jeff King wrote:\n>> But it just seems horribly broken to me. This is CIFS to an OpenVMS\n>> machine you said? Do the broken permissions appear to other clients or\n>> across a remount (i.e., is it broken state in your CIFS client, or has\n>> the server actually munged permissions)? If so, have you tried reporting\n>> the issue to whoever writes CIFS server on OpenVMS (is it just samba)?\n>>\n>> -Peff\n>>\n>\n> Yep, CIFS to OpenVMS.\n>\n> I don't know about other clients because there are none (yet).  The\n> permission change does survive remounting.\n>\n> I haven't reported it.  I didn't know it existed until now ;)\n>\n> I'll do that, but it will probably take a long long time for me to see\n> the patch.  I'm hoping there's some cheap hack I can use to work\n> around it in the meantime.\n\nA simple answer to preventing git from calling rmdir would be to run\nrm and git rm separately:\n$ rm file\n$ git rm --cached -f file\n\nBut that doesn't solve the misbehavior of git under the previous\nscenario. I'm not sure if this is something we should fix in git or if\nit should be fixed in cifs.\n"},{"id":"162584","messageId":"vpqmxlea7w1.fsf@bauges.imag.fr","threadId":"26630","inReplyTo":"AANLkTint3PARNNN4cpic8XG6HsM3AAGuX5a+oeXfFNx=@mail.gmail.com","subject":"Re: Git changes permissions on directories when deleting files.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2011-03-01T20:43:58Z","receivedAt":"2011-03-01T20:43:58Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Chad Joan writes:\n\n> I'll do that, but it will probably take a long long time for me to see\n> the patch. I'm hoping there's some cheap hack I can use to work\n> around it in the meantime.\n\nI'd say grep for \"rmdir\" is Git's source code, and replace the calls\nwith a wrapper that does roughly\n\nrmdir_wrapper(dir) {\n\trmdir(dir);\n\tif (stat(dir, &buf))\n\t\tchmod(dir, buf.st_mode | 0777);\n}\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"162585","messageId":"AANLkTi=UX7VNH+biFgn0FQawP-ttCjW2D7SMf2n6XB6w@mail.gmail.com","threadId":"26630","inReplyTo":"vpqmxlea7w1.fsf@bauges.imag.fr","subject":"Re: Git changes permissions on directories when deleting files.","fromName":"Chad Joan","fromEmail":"chadjoan@gmail.com","sentAt":"2011-03-01T20:46:46Z","receivedAt":"2011-03-01T20:46:46Z","isPatch":false,"sender":{"key":"chadjoan@gmail.com","avatar":"https://gravatar.com/avatar/7ef94965e8ac203e77176e89aa990a0ab091b0ef08d275f835b308ff24267444?d=mp&s=160"},"body":"On Tue, Mar 1, 2011 at 3:43 PM, Matthieu Moy\n wrote:\n> Chad Joan writes:\n>\n>> I'll do that, but it will probably take a long long time for me to see\n>> the patch.  I'm hoping there's some cheap hack I can use to work\n>> around it in the meantime.\n>\n> I'd say grep for \"rmdir\" is Git's source code, and replace the calls\n> with a wrapper that does roughly\n>\n> rmdir_wrapper(dir) {\n>        rmdir(dir);\n>        if (stat(dir, &buf))\n>                chmod(dir, buf.st_mode | 0777);\n> }\n>\n> --\n> Matthieu Moy\n> http://www-verimag.imag.fr/~moy/\n>\n\nOK, I'll try that when I get a chance.\n\n- Chad\n"},{"id":"162587","messageId":"20110301205702.GA21429@sigill.intra.peff.net","threadId":"26630","inReplyTo":"AANLkTinkjhH6YR7VPC0hji9CK=qeQMAYr+ptJE3szUyR@mail.gmail.com","subject":"Re: Git changes permissions on directories when deleting files.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-03-01T20:57:02Z","receivedAt":"2011-03-01T20:57:02Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 01, 2011 at 03:39:10PM -0500, Computer Druid wrote:\n\n> > I'll do that, but it will probably take a long long time for me to see\n> > the patch.  I'm hoping there's some cheap hack I can use to work\n> > around it in the meantime.\n> \n> A simple answer to preventing git from calling rmdir would be to run\n> rm and git rm separately:\n> $ rm file\n> $ git rm --cached -f file\n> \n> But that doesn't solve the misbehavior of git under the previous\n> scenario. I'm not sure if this is something we should fix in git or if\n> it should be fixed in cifs.\n\nThat will fix some instances. But git will rmdir to clean up anytime it\nremoves content. That includes during a merge or patch application. So\nyou can't really get around those cases.\n\n-Peff\n"},{"id":"162588","messageId":"20110301210852.GB21429@sigill.intra.peff.net","threadId":"26630","inReplyTo":"AANLkTi=UX7VNH+biFgn0FQawP-ttCjW2D7SMf2n6XB6w@mail.gmail.com","subject":"Re: Git changes permissions on directories when deleting files.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-03-01T21:08:52Z","receivedAt":"2011-03-01T21:08:52Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 01, 2011 at 03:46:46PM -0500, Chad Joan wrote:\n\n> >> I'll do that, but it will probably take a long long time for me to see\n> >> the patch.  I'm hoping there's some cheap hack I can use to work\n> >> around it in the meantime.\n> >\n> > I'd say grep for \"rmdir\" is Git's source code, and replace the calls\n> > with a wrapper that does roughly\n> >\n> > rmdir_wrapper(dir) {\n> >        rmdir(dir);\n> >        if (stat(dir, &buf))\n> >                chmod(dir, buf.st_mode | 0777);\n> > }\n> >\n> OK, I'll try that when I get a chance.\n\nI think this is the cheap hack that you want:\n\ndiff --git a/dir.c b/dir.c\nindex 168dad6..fb6d306 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1236,6 +1236,29 @@ void setup_standard_excludes(struct dir_struct *dir)\n \t\tadd_excludes_from_file(dir, excludes_file);\n }\n \n+static int rmdir_on_broken_cifs(const char *path)\n+{\n+\tstruct stat sb;\n+\tif (stat(path, &sb) < 0) {\n+\t\t/* Oh well, hopefully if we can't stat it\n+\t\t * it is already gone or we don't have\n+\t\t * permissions to screw it up anyway. */\n+\t\treturn rmdir(path);\n+\t}\n+\tif (rmdir(path) == 0) {\n+\t\t/* it worked, nothing to restore */\n+\t\treturn 0;\n+\t}\n+\t/* maybe remove this conditional if you can trigger\n+\t * the problem with other types of errors */\n+\tif (errno != ENOTEMPTY)\n+\t\treturn -1;\n+\tif (chmod(path, sb.st_mode) < 0)\n+\t\twarning(\"we probably just screwed up the permissions of %s\",\n+\t\t\tpath);\n+\treturn -1;\n+}\n+\n int remove_path(const char *name)\n {\n \tchar *slash;\n@@ -1249,7 +1272,7 @@ int remove_path(const char *name)\n \t\tslash = dirs + (slash - name);\n \t\tdo {\n \t\t\t*slash = '\\0';\n-\t\t} while (rmdir(dirs) == 0 && (slash = strrchr(dirs, '/')));\n+\t\t} while (rmdir_on_broken_cifs(dirs) == 0 && (slash = strrchr(dirs, '/')));\n \t\tfree(dirs);\n \t}\n \treturn 0;\n\nTotally untested, of course. But hey, it compiles, so it must be good.\n\n-Peff\n"},{"id":"162698","messageId":"AANLkTi=nFMDHR5WL=TiFmshFkxLMF9N4dNEjqw+r7wyh@mail.gmail.com","threadId":"26630","inReplyTo":"20110301210852.GB21429@sigill.intra.peff.net","subject":"Re: Git changes permissions on directories when deleting files.","fromName":"Chad Joan","fromEmail":"chadjoan@gmail.com","sentAt":"2011-03-03T03:48:00Z","receivedAt":"2011-03-03T03:48:00Z","isPatch":false,"sender":{"key":"chadjoan@gmail.com","avatar":"https://gravatar.com/avatar/7ef94965e8ac203e77176e89aa990a0ab091b0ef08d275f835b308ff24267444?d=mp&s=160"},"body":"On Tue, Mar 1, 2011 at 4:08 PM, Jeff King wrote:\n>\n> I think this is the cheap hack that you want:\n>\n> diff --git a/dir.c b/dir.c\n> index 168dad6..fb6d306 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -1236,6 +1236,29 @@ void setup_standard_excludes(struct dir_struct *dir)\n>                add_excludes_from_file(dir, excludes_file);\n>  }\n>\n> +static int rmdir_on_broken_cifs(const char *path)\n> +{\n> +       struct stat sb;\n> +       if (stat(path, &sb) < 0) {\n> +               /* Oh well, hopefully if we can't stat it\n> +                * it is already gone or we don't have\n> +                * permissions to screw it up anyway. */\n> +               return rmdir(path);\n> +       }\n> +       if (rmdir(path) == 0) {\n> +               /* it worked, nothing to restore */\n> +               return 0;\n> +       }\n> +       /* maybe remove this conditional if you can trigger\n> +        * the problem with other types of errors */\n> +       if (errno != ENOTEMPTY)\n> +               return -1;\n> +       if (chmod(path, sb.st_mode) < 0)\n> +               warning(\"we probably just screwed up the permissions of %s\",\n> +                       path);\n> +       return -1;\n> +}\n> +\n>  int remove_path(const char *name)\n>  {\n>        char *slash;\n> @@ -1249,7 +1272,7 @@ int remove_path(const char *name)\n>                slash = dirs + (slash - name);\n>                do {\n>                        *slash = '\\0';\n> -               } while (rmdir(dirs) == 0 && (slash = strrchr(dirs, '/')));\n> +               } while (rmdir_on_broken_cifs(dirs) == 0 && (slash = strrchr(dirs, '/')));\n>                free(dirs);\n>        }\n>        return 0;\n>\n> Totally untested, of course. But hey, it compiles, so it must be good.\n>\n> -Peff\n>\n\nIt seems to be working! I've tried it with 'git rm' and when pulling\ndeletions.\n\nI imagine that race condition can happen if files in the directory are\nbeing modified while git does an rmdir? If that's the case then I'm\nnot too worried. There is only one other programmer that might be\nworking with me at the same time on an infrequently used directory.\n\nThank you everyone for the excellent help!\n\nI modified the patch with some extra paranoia and replaced the other\nrmdir instance in that file:\n\ndiff -crB git-1.7.3.4/dir.c git-1.7.3.4-new/dir.c\n*** git-1.7.3.4/dir.c\tWed Mar 2 13:00:54 2011\n--- git-1.7.3.4-new/dir.c\tWed Mar 2 14:25:10 2011\n***************\n*** 994,999 ****\n--- 994,1022 ----\n \treturn ret;\n }\n\n+ static int rmdir_on_broken_cifs(const char *path)\n+ {\n+ struct stat sb;\n+ if (stat(path, &sb) < 0) {\n+ /* Oh well, hopefully if we can't stat it\n+ * it is already gone or we don't have\n+ * permissions to screw it up anyway. */\n+ return rmdir(path);\n+ }\n+ if (rmdir(path) == 0) {\n+ /* it worked, nothing to restore */\n+ return 0;\n+ }\n+ /* maybe remove this conditional if you can trigger\n+ * the problem with other types of errors */\n+ if (errno != ENOTEMPTY)\n+ return -1;\n+ if (chmod(path, sb.st_mode) < 0)\n+ warning(\"we probably just screwed up the permissions of %s\",\n+ path);\n+ return -1;\n+ }\n+\n int remove_dir_recursively(struct strbuf *path, int flag)\n {\n \tDIR *dir;\n***************\n*** 1037,1043 ****\n\n \tstrbuf_setlen(path, original_len);\n \tif (!ret)\n! \t\tret = rmdir(path->buf);\n \treturn ret;\n }\n\n--- 1060,1066 ----\n\n \tstrbuf_setlen(path, original_len);\n \tif (!ret)\n! \t\tret = rmdir_on_broken_cifs(path->buf);\n \treturn ret;\n }\n\n***************\n*** 1066,1072 ****\n \t\tslash = dirs + (slash - name);\n \t\tdo {\n \t\t\t*slash = '\\0';\n! \t\t} while (rmdir(dirs) == 0 && (slash = strrchr(dirs, '/')));\n \t\tfree(dirs);\n \t}\n \treturn 0;\n--- 1090,1096 ----\n \t\tslash = dirs + (slash - name);\n \t\tdo {\n \t\t\t*slash = '\\0';\n! \t\t} while (rmdir_on_broken_cifs(dirs) == 0 && (slash = strrchr(dirs, '/')));\n \t\tfree(dirs);\n \t}\n \treturn 0;\n"},{"id":"162724","messageId":"20110303151608.GD1074@sigill.intra.peff.net","threadId":"26630","inReplyTo":"AANLkTi=nFMDHR5WL=TiFmshFkxLMF9N4dNEjqw+r7wyh@mail.gmail.com","subject":"Re: Git changes permissions on directories when deleting files.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-03-03T15:16:09Z","receivedAt":"2011-03-03T15:16:09Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 02, 2011 at 10:48:00PM -0500, Chad Joan wrote:\n\n> It seems to be working! I've tried it with 'git rm' and when pulling\n> deletions.\n\nGreat.\n\n> I imagine that race condition can happen if files in the directory are\n> being modified while git does an rmdir? If that's the case then I'm\n> not too worried. There is only one other programmer that might be\n> working with me at the same time on an infrequently used directory.\n\nThe race condition I mentioned earlier was for a different workaround.\nBasically there are two strategies, each with a difference race:\n\n 1. Don't rmdir on non-empty directories. This means we have to opendir\n the directory and look for entries before rmdir(). If there is file\n activity in the directory while we are looking we may think it is\n empty when it's not and rmdir(), screwing up the permissions.\n\n 2. Before any rmdir, check permissions. Do the rmdir, and then restore\n the permissions if rmdir fails. The race here is if somebody is\n modifying the permissions on a non-empty directory, we may\n overwrite their changes.\n\nObviously the patch does (2), so there is still that race.\n\n> diff -crB git-1.7.3.4/dir.c git-1.7.3.4-new/dir.c\n\nContext diff? Eww. There is this awesome tool called \"git\" that can help\nyou with managing versions of software. :)\n\n-Peff\n"},{"id":"163205","messageId":"AANLkTin1sePpEbzPq5PFnk21BSRSmJf6wi0UhzSsZe7+@mail.gmail.com","threadId":"26630","inReplyTo":"20110303151608.GD1074@sigill.intra.peff.net","subject":"Re: Git changes permissions on directories when deleting files.","fromName":"Chad Joan","fromEmail":"chadjoan@gmail.com","sentAt":"2011-03-11T06:09:58Z","receivedAt":"2011-03-11T06:09:58Z","isPatch":false,"sender":{"key":"chadjoan@gmail.com","avatar":"https://gravatar.com/avatar/7ef94965e8ac203e77176e89aa990a0ab091b0ef08d275f835b308ff24267444?d=mp&s=160"},"body":"On Thu, Mar 3, 2011 at 10:16 AM, Jeff King wrote:\n>\n> ...\n>\n> > diff -crB git-1.7.3.4/dir.c git-1.7.3.4-new/dir.c\n>\n> Context diff? Eww. There is this awesome tool called \"git\" that can help\n> you with managing versions of software. :)\n>\n> -Peff\n\nYes... though I'm just letting Gentoo compile it for me and I'm not\ndoing anything serious enough to justify downloading/installing GIT by\nhand and next to the one that's already there. So I just worked from\nthe tarball that Gentoo uses and added stuff. Any other day and I'd\nbe all over that git usage ;)\n\n\n I did run into more permissions being messed up, so I grepped for\nrmdir and replaced /all/ instances of it in git's C code. I haven't\nhad anymore trouble so far. Here's the newer patch:\n\ndiff -crB git-1.7.3.4/dir.c git-1.7.3.4-new/dir.c\n*** git-1.7.3.4/dir.c\tWed Mar 2 13:00:54 2011\n--- git-1.7.3.4-new/dir.c\tThu Mar 10 11:09:32 2011\n***************\n*** 994,999 ****\n--- 994,1022 ----\n \treturn ret;\n }\n\n+ int rmdir_on_broken_cifs(const char *path)\n+ {\n+ struct stat sb;\n+ if (stat(path, &sb) < 0) {\n+ /* Oh well, hopefully if we can't stat it\n+ * it is already gone or we don't have\n+ * permissions to screw it up anyway. */\n+ return rmdir(path);\n+ }\n+ if (rmdir(path) == 0) {\n+ /* it worked, nothing to restore */\n+ return 0;\n+ }\n+ /* maybe remove this conditional if you can trigger\n+ * the problem with other types of errors */\n+ if (errno != ENOTEMPTY)\n+ return -1;\n+ if (chmod(path, sb.st_mode) < 0)\n+ warning(\"we probably just screwed up the permissions of %s\",\n+ path);\n+ return -1;\n+ }\n+\n int remove_dir_recursively(struct strbuf *path, int flag)\n {\n \tDIR *dir;\n***************\n*** 1037,1043 ****\n\n \tstrbuf_setlen(path, original_len);\n \tif (!ret)\n! \t\tret = rmdir(path->buf);\n \treturn ret;\n }\n\n--- 1060,1066 ----\n\n \tstrbuf_setlen(path, original_len);\n \tif (!ret)\n! \t\tret = rmdir_on_broken_cifs(path->buf);\n \treturn ret;\n }\n\n***************\n*** 1066,1072 ****\n \t\tslash = dirs + (slash - name);\n \t\tdo {\n \t\t\t*slash = '\\0';\n! \t\t} while (rmdir(dirs) == 0 && (slash = strrchr(dirs, '/')));\n \t\tfree(dirs);\n \t}\n \treturn 0;\n--- 1090,1096 ----\n \t\tslash = dirs + (slash - name);\n \t\tdo {\n \t\t\t*slash = '\\0';\n! \t\t} while (rmdir_on_broken_cifs(dirs) == 0 && (slash = strrchr(dirs, '/')));\n \t\tfree(dirs);\n \t}\n \treturn 0;\nOnly in git-1.7.3.4: dir.c~\ndiff -crB git-1.7.3.4/dir.h git-1.7.3.4-new/dir.h\n*** git-1.7.3.4/dir.h\tWed Dec 15 21:52:11 2010\n--- git-1.7.3.4-new/dir.h\tThu Mar 10 11:09:13 2011\n***************\n*** 101,104 ****\n--- 101,106 ----\n /* tries to remove the path with empty directories along it, ignores ENOENT */\n extern int remove_path(const char *path);\n\n+ extern int rmdir_on_broken_cifs(const char *path);\n+\n #endif\ndiff -crB git-1.7.3.4/entry.c git-1.7.3.4-new/entry.c\n*** git-1.7.3.4/entry.c\tWed Dec 15 21:52:11 2010\n--- git-1.7.3.4-new/entry.c\tThu Mar 10 11:12:25 2011\n***************\n*** 68,74 ****\n \t\t\tdie_errno(\"cannot unlink '%s'\", pathbuf);\n \t}\n \tclosedir(dir);\n! \tif (rmdir(path))\n \t\tdie_errno(\"cannot rmdir '%s'\", path);\n }\n\n--- 68,74 ----\n \t\t\tdie_errno(\"cannot unlink '%s'\", pathbuf);\n \t}\n \tclosedir(dir);\n! \tif (rmdir_on_broken_cifs(path))\n \t\tdie_errno(\"cannot rmdir '%s'\", path);\n }\n\ndiff -crB git-1.7.3.4/pack-refs.c git-1.7.3.4-new/pack-refs.c\n*** git-1.7.3.4/pack-refs.c\tWed Dec 15 21:52:11 2010\n--- git-1.7.3.4-new/pack-refs.c\tThu Mar 10 12:34:53 2011\n***************\n*** 2,7 ****\n--- 2,8 ----\n #include \"refs.h\"\n #include \"tag.h\"\n #include \"pack-refs.h\"\n+ #include \"dir.h\"\n\n struct ref_to_prune {\n \tstruct ref_to_prune *next;\n***************\n*** 86,92 ****\n \t\tif (q == p)\n \t\t\tbreak;\n \t\t*q = '\\0';\n! \t\tif (rmdir(git_path(\"%s\", name)))\n \t\t\tbreak;\n \t}\n }\n--- 87,93 ----\n \t\tif (q == p)\n \t\t\tbreak;\n \t\t*q = '\\0';\n! \t\tif (rmdir_on_broken_cifs(git_path(\"%s\", name)))\n \t\t\tbreak;\n \t}\n }\ndiff -crB git-1.7.3.4/symlinks.c git-1.7.3.4-new/symlinks.c\n*** git-1.7.3.4/symlinks.c\tWed Dec 15 21:52:11 2010\n--- git-1.7.3.4-new/symlinks.c\tThu Mar 10 11:24:08 2011\n***************\n*** 1,4 ****\n--- 1,5 ----\n #include \"cache.h\"\n+ #include \"dir.h\"\n\n /*\n * Returns the length (on a path component basis) of the longest\n***************\n*** 255,261 ****\n {\n \twhile (removal.len > new_len) {\n \t\tremoval.path[removal.len] = '\\0';\n! \t\tif (rmdir(removal.path))\n \t\t\tbreak;\n \t\tdo {\n \t\t\tremoval.len--;\n--- 256,262 ----\n {\n \twhile (removal.len > new_len) {\n \t\tremoval.path[removal.len] = '\\0';\n! \t\tif (rmdir_on_broken_cifs(removal.path))\n \t\t\tbreak;\n \t\tdo {\n \t\t\tremoval.len--;\ndiff -crB git-1.7.3.4/wrapper.c git-1.7.3.4-new/wrapper.c\n*** git-1.7.3.4/wrapper.c\tWed Dec 15 21:52:11 2010\n--- git-1.7.3.4-new/wrapper.c\tThu Mar 10 11:12:36 2011\n***************\n*** 2,7 ****\n--- 2,8 ----\n * Various trivial helper wrappers around standard functions\n */\n #include \"cache.h\"\n+ #include \"dir.h\"\n\n static void try_to_free_builtin(size_t size)\n {\n***************\n*** 346,352 ****\n\n int rmdir_or_warn(const char *file)\n {\n! \treturn warn_if_unremovable(\"rmdir\", file, rmdir(file));\n }\n\n int remove_or_warn(unsigned int mode, const char *file)\n--- 347,353 ----\n\n int rmdir_or_warn(const char *file)\n {\n! \treturn warn_if_unremovable(\"rmdir\", file, rmdir_on_broken_cifs(file));\n }\n\n int remove_or_warn(unsigned int mode, const char *file)\n"}]}