* Git changes permissions on directories when deleting files. @ 2011-03-01 1:42 Chad Joan 2011-03-01 1:45 ` Chad Joan 2011-03-01 2:19 ` Computer Druid 0 siblings, 2 replies; 19+ messages in thread From: Chad Joan @ 2011-03-01 1:42 UTC (permalink / raw) To: git Hello, What I'm experiencing is this: $ cd ~/project $ ls -dl somedir drwxrwx--- 1 cjoan cjoan 0 Feb 28 19:57 somedir $ echo "some text" > somedir/somefile.txt $ git add somedir/somefile.txt $ git rm -f somedir/somefile.txt rm 'somedir/somefile.txt' $ ls -dl somedir drw------- 1 cjoan cjoan 0 Feb 28 19:57 somedir $ echo "some text" > somedir/somefile.txt bash: somedir/somefile.txt: Permission denied ~/project is actually a CIFS mount, with the host being an OpenVMS machine. If I use the normal rm command without using git then the permissions will remain the same on 'somedir'. This is why I suspect (and hope) this isn't OpenVMS related. It seems that execute bit is important for CIFS mounted files, because after this happens I am no longer able to do /anything/ within the 'somedir' directory. This also affects branches via "git checkout branchname": if the checkout happens to delete files then this will happen, and it will salt the wound by failing to sync a bunch of files in 'somedir' (because I can't access them anymore) while still moving HEAD to the new branch. The share on the CIFS host looks like this: [homes] comment = Home Directories read only = No create mask = 0770 browseable = No vfs objects = varvfc vms path names = No case sensitive = Yes The fstab entry for the mount looks like this: //vms/homes /home/cjoan/project cifs credentials=/home/cjoan/.cifs_credentials,_netdev,uid=cjoan,gid=cjoan 0 0 I'd really like my directories to keep their permissions. Any idea what might cause this? - Chad ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Git changes permissions on directories when deleting files. 2011-03-01 1:42 Git changes permissions on directories when deleting files Chad Joan @ 2011-03-01 1:45 ` Chad Joan 2011-03-01 2:19 ` Computer Druid 1 sibling, 0 replies; 19+ messages in thread From: Chad Joan @ 2011-03-01 1:45 UTC (permalink / raw) To: git I should probably also mention that my git version is 1.7.3.4-r1 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Git changes permissions on directories when deleting files. 2011-03-01 1:42 Git changes permissions on directories when deleting files Chad Joan 2011-03-01 1:45 ` Chad Joan @ 2011-03-01 2:19 ` Computer Druid 2011-03-01 4:00 ` Chad Joan 1 sibling, 1 reply; 19+ messages in thread From: Computer Druid @ 2011-03-01 2:19 UTC (permalink / raw) To: Chad Joan; +Cc: git On Mon, Feb 28, 2011 at 8:42 PM, Chad Joan <chadjoan@gmail.com> wrote: > Hello, > > What I'm experiencing is this: > > $ cd ~/project > $ ls -dl somedir > drwxrwx--- 1 cjoan cjoan 0 Feb 28 19:57 somedir > $ echo "some text" > somedir/somefile.txt > $ git add somedir/somefile.txt > $ git rm -f somedir/somefile.txt > rm 'somedir/somefile.txt' > $ ls -dl somedir > drw------- 1 cjoan cjoan 0 Feb 28 19:57 somedir > $ echo "some text" > somedir/somefile.txt > bash: somedir/somefile.txt: Permission denied After you remove the file, is "somedir" empty? Git doesn't track empty directories, and therefore git rm on the last file in a directory deletes it: % git init Initialized empty Git repository in /home/cdruid/testrepo/.git/ % mkdir dir % ls -l total 4 drwxr-xr-x 2 cdruid cdruid 4096 Feb 28 21:14 dir % touch dir/test.txt % git add dir/test.txt % git rm -f dir/test.txt rm 'dir/test.txt' % ls -l total 0 My guess is git is somehow failing to delete the directory, thus causing your changed permissions issue. -Dan Johnson ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Git changes permissions on directories when deleting files. 2011-03-01 2:19 ` Computer Druid @ 2011-03-01 4:00 ` Chad Joan 2011-03-01 15:51 ` Chad Joan 0 siblings, 1 reply; 19+ messages in thread From: Chad Joan @ 2011-03-01 4:00 UTC (permalink / raw) To: Computer Druid; +Cc: git On Mon, Feb 28, 2011 at 9:19 PM, Computer Druid <computerdruid@gmail.com> wrote: > On Mon, Feb 28, 2011 at 8:42 PM, Chad Joan <chadjoan@gmail.com> wrote: >> Hello, >> >> What I'm experiencing is this: >> >> $ cd ~/project >> $ ls -dl somedir >> drwxrwx--- 1 cjoan cjoan 0 Feb 28 19:57 somedir >> $ echo "some text" > somedir/somefile.txt >> $ git add somedir/somefile.txt >> $ git rm -f somedir/somefile.txt >> rm 'somedir/somefile.txt' >> $ ls -dl somedir >> drw------- 1 cjoan cjoan 0 Feb 28 19:57 somedir >> $ echo "some text" > somedir/somefile.txt >> bash: somedir/somefile.txt: Permission denied > > After you remove the file, is "somedir" empty? > Nope. > Git doesn't track empty directories, and therefore git rm on the last > file in a directory deletes it: > > % git init > Initialized empty Git repository in /home/cdruid/testrepo/.git/ > % mkdir dir > % ls -l > total 4 > drwxr-xr-x 2 cdruid cdruid 4096 Feb 28 21:14 dir > % touch dir/test.txt > % git add dir/test.txt > % git rm -f dir/test.txt > rm 'dir/test.txt' > % ls -l > total 0 > > My guess is git is somehow failing to delete the directory, thus > causing your changed permissions issue. > > -Dan Johnson > 'somedir' still has plenty of files in it after the deletion, so I'm afraid this isn't the case. - Chad ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Git changes permissions on directories when deleting files. 2011-03-01 4:00 ` Chad Joan @ 2011-03-01 15:51 ` Chad Joan 2011-03-01 17:11 ` Computer Druid 0 siblings, 1 reply; 19+ messages in thread From: Chad Joan @ 2011-03-01 15:51 UTC (permalink / raw) To: git More info: $ mkdir foo $ mkdir foo/bar $ echo "test" > foo/bar/baz.txt $ echo "somestuff" > foo/bar/somefile.txt $ git add foo/bar/* $ ls -dl foo drwxr-x--x 1 cjoan cjoan 0 Mar 1 10:46 foo $ ls -dl foo/bar drwxr-x--x 1 cjoan cjoan 0 Mar 1 10:46 foo/bar $ git rm -f foo/bar/somefile.txt rm 'foo/bar/somefile.txt' $ ls -dl foo drwxr-x--x 1 cjoan cjoan 0 Mar 1 10:46 foo $ ls -dl foo/bar drw------- 1 cjoan cjoan 0 Mar 1 10:47 foo/bar $ git rm -f foo/bar/baz.txt rm 'foo/bar/baz.txt' fatal: git rm: 'foo/bar/baz.txt': Permission denied This time I tried it with git 1.7.4. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Git changes permissions on directories when deleting files. 2011-03-01 15:51 ` Chad Joan @ 2011-03-01 17:11 ` Computer Druid 2011-03-01 19:35 ` Chad Joan 0 siblings, 1 reply; 19+ messages in thread From: Computer Druid @ 2011-03-01 17:11 UTC (permalink / raw) To: Chad Joan; +Cc: git On Tue, Mar 1, 2011 at 10:51 AM, Chad Joan <chadjoan@gmail.com> wrote: > More info: > > $ mkdir foo > $ mkdir foo/bar > $ echo "test" > foo/bar/baz.txt > $ echo "somestuff" > foo/bar/somefile.txt What happens if you "rmdir foo/bar" here? (while there are files still in it) -Dan Johnson ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Git changes permissions on directories when deleting files. 2011-03-01 17:11 ` Computer Druid @ 2011-03-01 19:35 ` Chad Joan 2011-03-01 19:44 ` Jeff King 0 siblings, 1 reply; 19+ messages in thread From: Chad Joan @ 2011-03-01 19:35 UTC (permalink / raw) To: Computer Druid; +Cc: git On Tue, Mar 1, 2011 at 12:11 PM, Computer Druid <computerdruid@gmail.com> wrote: > On Tue, Mar 1, 2011 at 10:51 AM, Chad Joan <chadjoan@gmail.com> wrote: >> More info: >> >> $ mkdir foo >> $ mkdir foo/bar >> $ echo "test" > foo/bar/baz.txt >> $ echo "somestuff" > foo/bar/somefile.txt > What happens if you "rmdir foo/bar" here? (while there are files still in it) > > -Dan Johnson > Something fairly interesting: $ mkdir foo $ mkdir foo/bar $ ls -dl foo/bar drwxr-x--x 1 cjoan cjoan 0 Mar 1 14:31 foo/bar $ ls -dl foo drwxr-x--x 1 cjoan cjoan 0 Mar 1 14:31 foo $ echo "test" > foo/bar/baz.txt $ echo "somestuff" > foo/bar/somefile.txt $ ls -dl foo/bar drwxr-x--x 1 cjoan cjoan 0 Mar 1 14:31 foo/bar $ ls -dl foo drwxr-x--x 1 cjoan cjoan 0 Mar 1 14:31 foo $ rmdir foo/bar rmdir: failed to remove `foo/bar': Directory not empty $ ls -dl foo/bar drw------- 1 cjoan cjoan 0 Mar 1 14:32 foo/bar $ ls -dl foo drwxr-x--x 1 cjoan cjoan 0 Mar 1 14:31 foo The rmdir fails of course, but it also changes the permissions. So I take it that git always runs an rmdir on the parent directory when it removes a file? Seems like it would be a sensible way to do it on a system without this behavior. - Chad ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Git changes permissions on directories when deleting files. 2011-03-01 19:35 ` Chad Joan @ 2011-03-01 19:44 ` Jeff King 2011-03-01 19:57 ` Chad Joan 0 siblings, 1 reply; 19+ messages in thread From: Jeff King @ 2011-03-01 19:44 UTC (permalink / raw) To: Chad Joan; +Cc: Computer Druid, git On Tue, Mar 01, 2011 at 02:35:41PM -0500, Chad Joan wrote: > Something fairly interesting: > > $ mkdir foo > $ mkdir foo/bar > $ ls -dl foo/bar > drwxr-x--x 1 cjoan cjoan 0 Mar 1 14:31 foo/bar > $ ls -dl foo > drwxr-x--x 1 cjoan cjoan 0 Mar 1 14:31 foo > $ echo "test" > foo/bar/baz.txt > $ echo "somestuff" > foo/bar/somefile.txt > $ ls -dl foo/bar > drwxr-x--x 1 cjoan cjoan 0 Mar 1 14:31 foo/bar > $ ls -dl foo > drwxr-x--x 1 cjoan cjoan 0 Mar 1 14:31 foo > $ rmdir foo/bar > rmdir: failed to remove `foo/bar': Directory not empty > $ ls -dl foo/bar > drw------- 1 cjoan cjoan 0 Mar 1 14:32 foo/bar > $ ls -dl foo > drwxr-x--x 1 cjoan cjoan 0 Mar 1 14:31 foo > > > The rmdir fails of course, but it also changes the permissions. > So I take it that git always runs an rmdir on the parent directory > when it removes a file? Seems like it would be a sensible way to do > it on a system without this behavior. Exactly. Rather than spend time figuring out if the directory is removable (which would not be atomic, anyway), we just rmdir and ignore the error condition. I would argue that your filesystem is broken. Even if we implemented a workaround to opendir() and check for files, it would still have a race condition that could cause this situation to occur. -Peff ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Git changes permissions on directories when deleting files. 2011-03-01 19:44 ` Jeff King @ 2011-03-01 19:57 ` Chad Joan 2011-03-01 20:08 ` Jeff King 0 siblings, 1 reply; 19+ messages in thread From: Chad Joan @ 2011-03-01 19:57 UTC (permalink / raw) Cc: Computer Druid, git On Tue, Mar 1, 2011 at 2:44 PM, Jeff King <peff@peff.net> wrote: > On Tue, Mar 01, 2011 at 02:35:41PM -0500, Chad Joan wrote: > >> Something fairly interesting: >> >> $ mkdir foo >> $ mkdir foo/bar >> $ ls -dl foo/bar >> drwxr-x--x 1 cjoan cjoan 0 Mar 1 14:31 foo/bar >> $ ls -dl foo >> drwxr-x--x 1 cjoan cjoan 0 Mar 1 14:31 foo >> $ echo "test" > foo/bar/baz.txt >> $ echo "somestuff" > foo/bar/somefile.txt >> $ ls -dl foo/bar >> drwxr-x--x 1 cjoan cjoan 0 Mar 1 14:31 foo/bar >> $ ls -dl foo >> drwxr-x--x 1 cjoan cjoan 0 Mar 1 14:31 foo >> $ rmdir foo/bar >> rmdir: failed to remove `foo/bar': Directory not empty >> $ ls -dl foo/bar >> drw------- 1 cjoan cjoan 0 Mar 1 14:32 foo/bar >> $ ls -dl foo >> drwxr-x--x 1 cjoan cjoan 0 Mar 1 14:31 foo >> >> >> The rmdir fails of course, but it also changes the permissions. >> So I take it that git always runs an rmdir on the parent directory >> when it removes a file? Seems like it would be a sensible way to do >> it on a system without this behavior. > > Exactly. Rather than spend time figuring out if the directory is > removable (which would not be atomic, anyway), we just rmdir and ignore > the error condition. > > I would argue that your filesystem is broken. Even if we implemented a > workaround to opendir() and check for files, it would still have a race > condition that could cause this situation to occur. > > -Peff > Ouch. Would it work to do something like alias rmdir to a script or program that would call /bin/rmdir and then fix up the permissions? ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Git changes permissions on directories when deleting files. 2011-03-01 19:57 ` Chad Joan @ 2011-03-01 20:08 ` Jeff King 2011-03-01 20:30 ` Chad Joan 0 siblings, 1 reply; 19+ messages in thread From: Jeff King @ 2011-03-01 20:08 UTC (permalink / raw) To: Chad Joan; +Cc: Computer Druid, git On Tue, Mar 01, 2011 at 02:57:19PM -0500, Chad Joan wrote: > > Exactly. Rather than spend time figuring out if the directory is > > removable (which would not be atomic, anyway), we just rmdir and ignore > > the error condition. > > > > I would argue that your filesystem is broken. Even if we implemented a > > workaround to opendir() and check for files, it would still have a race > > condition that could cause this situation to occur. > > Ouch. > > Would it work to do something like alias rmdir to a script or program > that would call /bin/rmdir and then fix up the permissions? Well, we're using the rmdir system call, so you would need a patch to git either way. If that was something we wanted to support (with a config option, of course), we could do the permissions check-and-restore ourselves. But it just seems horribly broken to me. This is CIFS to an OpenVMS machine you said? Do the broken permissions appear to other clients or across a remount (i.e., is it broken state in your CIFS client, or has the server actually munged permissions)? If so, have you tried reporting the issue to whoever writes CIFS server on OpenVMS (is it just samba)? -Peff ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Git changes permissions on directories when deleting files. 2011-03-01 20:08 ` Jeff King @ 2011-03-01 20:30 ` Chad Joan 2011-03-01 20:39 ` Computer Druid 2011-03-01 20:43 ` Matthieu Moy 0 siblings, 2 replies; 19+ messages in thread From: Chad Joan @ 2011-03-01 20:30 UTC (permalink / raw) To: Jeff King; +Cc: Computer Druid, git On Tue, Mar 1, 2011 at 3:08 PM, Jeff King <peff@peff.net> wrote: > On Tue, Mar 01, 2011 at 02:57:19PM -0500, Chad Joan wrote: > >> > Exactly. Rather than spend time figuring out if the directory is >> > removable (which would not be atomic, anyway), we just rmdir and ignore >> > the error condition. >> > >> > I would argue that your filesystem is broken. Even if we implemented a >> > workaround to opendir() and check for files, it would still have a race >> > condition that could cause this situation to occur. >> >> Ouch. >> >> Would it work to do something like alias rmdir to a script or program >> that would call /bin/rmdir and then fix up the permissions? > > Well, we're using the rmdir system call, so you would need a patch to > git either way. If that was something we wanted to support (with a > config option, of course), we could do the permissions check-and-restore > ourselves. > > But it just seems horribly broken to me. This is CIFS to an OpenVMS > machine you said? Do the broken permissions appear to other clients or > across a remount (i.e., is it broken state in your CIFS client, or has > the server actually munged permissions)? If so, have you tried reporting > the issue to whoever writes CIFS server on OpenVMS (is it just samba)? > > -Peff > Yep, CIFS to OpenVMS. I don't know about other clients because there are none (yet). The permission change does survive remounting. I haven't reported it. I didn't know it existed until now ;) I'll do that, but it will probably take a long long time for me to see the patch. I'm hoping there's some cheap hack I can use to work around it in the meantime. - Chad ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Git changes permissions on directories when deleting files. 2011-03-01 20:30 ` Chad Joan @ 2011-03-01 20:39 ` Computer Druid 2011-03-01 20:57 ` Jeff King 2011-03-01 20:43 ` Matthieu Moy 1 sibling, 1 reply; 19+ messages in thread From: Computer Druid @ 2011-03-01 20:39 UTC (permalink / raw) To: Chad Joan; +Cc: Jeff King, git On Tue, Mar 1, 2011 at 3:30 PM, Chad Joan <chadjoan@gmail.com> wrote: > On Tue, Mar 1, 2011 at 3:08 PM, Jeff King <peff@peff.net> wrote: >> But it just seems horribly broken to me. This is CIFS to an OpenVMS >> machine you said? Do the broken permissions appear to other clients or >> across a remount (i.e., is it broken state in your CIFS client, or has >> the server actually munged permissions)? If so, have you tried reporting >> the issue to whoever writes CIFS server on OpenVMS (is it just samba)? >> >> -Peff >> > > Yep, CIFS to OpenVMS. > > I don't know about other clients because there are none (yet). The > permission change does survive remounting. > > I haven't reported it. I didn't know it existed until now ;) > > I'll do that, but it will probably take a long long time for me to see > the patch. I'm hoping there's some cheap hack I can use to work > around it in the meantime. A simple answer to preventing git from calling rmdir would be to run rm and git rm separately: $ rm file $ git rm --cached -f file But that doesn't solve the misbehavior of git under the previous scenario. I'm not sure if this is something we should fix in git or if it should be fixed in cifs. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Git changes permissions on directories when deleting files. 2011-03-01 20:39 ` Computer Druid @ 2011-03-01 20:57 ` Jeff King 0 siblings, 0 replies; 19+ messages in thread From: Jeff King @ 2011-03-01 20:57 UTC (permalink / raw) To: Computer Druid; +Cc: Chad Joan, git On Tue, Mar 01, 2011 at 03:39:10PM -0500, Computer Druid wrote: > > I'll do that, but it will probably take a long long time for me to see > > the patch. I'm hoping there's some cheap hack I can use to work > > around it in the meantime. > > A simple answer to preventing git from calling rmdir would be to run > rm and git rm separately: > $ rm file > $ git rm --cached -f file > > But that doesn't solve the misbehavior of git under the previous > scenario. I'm not sure if this is something we should fix in git or if > it should be fixed in cifs. That will fix some instances. But git will rmdir to clean up anytime it removes content. That includes during a merge or patch application. So you can't really get around those cases. -Peff ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Git changes permissions on directories when deleting files. 2011-03-01 20:30 ` Chad Joan 2011-03-01 20:39 ` Computer Druid @ 2011-03-01 20:43 ` Matthieu Moy 2011-03-01 20:46 ` Chad Joan 1 sibling, 1 reply; 19+ messages in thread From: Matthieu Moy @ 2011-03-01 20:43 UTC (permalink / raw) To: Chad Joan; +Cc: Jeff King, Computer Druid, git Chad Joan <chadjoan@gmail.com> writes: > I'll do that, but it will probably take a long long time for me to see > the patch. I'm hoping there's some cheap hack I can use to work > around it in the meantime. I'd say grep for "rmdir" is Git's source code, and replace the calls with a wrapper that does roughly rmdir_wrapper(dir) { rmdir(dir); if (stat(dir, &buf)) chmod(dir, buf.st_mode | 0777); } -- Matthieu Moy http://www-verimag.imag.fr/~moy/ ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Git changes permissions on directories when deleting files. 2011-03-01 20:43 ` Matthieu Moy @ 2011-03-01 20:46 ` Chad Joan 2011-03-01 21:08 ` Jeff King 0 siblings, 1 reply; 19+ messages in thread From: Chad Joan @ 2011-03-01 20:46 UTC (permalink / raw) To: Matthieu Moy; +Cc: Jeff King, Computer Druid, git On Tue, Mar 1, 2011 at 3:43 PM, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote: > Chad Joan <chadjoan@gmail.com> writes: > >> I'll do that, but it will probably take a long long time for me to see >> the patch. I'm hoping there's some cheap hack I can use to work >> around it in the meantime. > > I'd say grep for "rmdir" is Git's source code, and replace the calls > with a wrapper that does roughly > > rmdir_wrapper(dir) { > rmdir(dir); > if (stat(dir, &buf)) > chmod(dir, buf.st_mode | 0777); > } > > -- > Matthieu Moy > http://www-verimag.imag.fr/~moy/ > OK, I'll try that when I get a chance. - Chad ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Git changes permissions on directories when deleting files. 2011-03-01 20:46 ` Chad Joan @ 2011-03-01 21:08 ` Jeff King 2011-03-03 3:48 ` Chad Joan 0 siblings, 1 reply; 19+ messages in thread From: Jeff King @ 2011-03-01 21:08 UTC (permalink / raw) To: Chad Joan; +Cc: Matthieu Moy, Computer Druid, git On Tue, Mar 01, 2011 at 03:46:46PM -0500, Chad Joan wrote: > >> I'll do that, but it will probably take a long long time for me to see > >> the patch. I'm hoping there's some cheap hack I can use to work > >> around it in the meantime. > > > > I'd say grep for "rmdir" is Git's source code, and replace the calls > > with a wrapper that does roughly > > > > rmdir_wrapper(dir) { > > rmdir(dir); > > if (stat(dir, &buf)) > > chmod(dir, buf.st_mode | 0777); > > } > > > OK, I'll try that when I get a chance. I think this is the cheap hack that you want: diff --git a/dir.c b/dir.c index 168dad6..fb6d306 100644 --- a/dir.c +++ b/dir.c @@ -1236,6 +1236,29 @@ void setup_standard_excludes(struct dir_struct *dir) add_excludes_from_file(dir, excludes_file); } +static int rmdir_on_broken_cifs(const char *path) +{ + struct stat sb; + if (stat(path, &sb) < 0) { + /* Oh well, hopefully if we can't stat it + * it is already gone or we don't have + * permissions to screw it up anyway. */ + return rmdir(path); + } + if (rmdir(path) == 0) { + /* it worked, nothing to restore */ + return 0; + } + /* maybe remove this conditional if you can trigger + * the problem with other types of errors */ + if (errno != ENOTEMPTY) + return -1; + if (chmod(path, sb.st_mode) < 0) + warning("we probably just screwed up the permissions of %s", + path); + return -1; +} + int remove_path(const char *name) { char *slash; @@ -1249,7 +1272,7 @@ int remove_path(const char *name) slash = dirs + (slash - name); do { *slash = '\0'; - } while (rmdir(dirs) == 0 && (slash = strrchr(dirs, '/'))); + } while (rmdir_on_broken_cifs(dirs) == 0 && (slash = strrchr(dirs, '/'))); free(dirs); } return 0; Totally untested, of course. But hey, it compiles, so it must be good. -Peff ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: Git changes permissions on directories when deleting files. 2011-03-01 21:08 ` Jeff King @ 2011-03-03 3:48 ` Chad Joan 2011-03-03 15:16 ` Jeff King 0 siblings, 1 reply; 19+ messages in thread From: Chad Joan @ 2011-03-03 3:48 UTC (permalink / raw) To: Jeff King; +Cc: Matthieu Moy, Computer Druid, git On Tue, Mar 1, 2011 at 4:08 PM, Jeff King <peff@peff.net> wrote: > > I think this is the cheap hack that you want: > > diff --git a/dir.c b/dir.c > index 168dad6..fb6d306 100644 > --- a/dir.c > +++ b/dir.c > @@ -1236,6 +1236,29 @@ void setup_standard_excludes(struct dir_struct *dir) > add_excludes_from_file(dir, excludes_file); > } > > +static int rmdir_on_broken_cifs(const char *path) > +{ > + struct stat sb; > + if (stat(path, &sb) < 0) { > + /* Oh well, hopefully if we can't stat it > + * it is already gone or we don't have > + * permissions to screw it up anyway. */ > + return rmdir(path); > + } > + if (rmdir(path) == 0) { > + /* it worked, nothing to restore */ > + return 0; > + } > + /* maybe remove this conditional if you can trigger > + * the problem with other types of errors */ > + if (errno != ENOTEMPTY) > + return -1; > + if (chmod(path, sb.st_mode) < 0) > + warning("we probably just screwed up the permissions of %s", > + path); > + return -1; > +} > + > int remove_path(const char *name) > { > char *slash; > @@ -1249,7 +1272,7 @@ int remove_path(const char *name) > slash = dirs + (slash - name); > do { > *slash = '\0'; > - } while (rmdir(dirs) == 0 && (slash = strrchr(dirs, '/'))); > + } while (rmdir_on_broken_cifs(dirs) == 0 && (slash = strrchr(dirs, '/'))); > free(dirs); > } > return 0; > > Totally untested, of course. But hey, it compiles, so it must be good. > > -Peff > It seems to be working! I've tried it with 'git rm' and when pulling deletions. I imagine that race condition can happen if files in the directory are being modified while git does an rmdir? If that's the case then I'm not too worried. There is only one other programmer that might be working with me at the same time on an infrequently used directory. Thank you everyone for the excellent help! I modified the patch with some extra paranoia and replaced the other rmdir instance in that file: diff -crB git-1.7.3.4/dir.c git-1.7.3.4-new/dir.c *** git-1.7.3.4/dir.c Wed Mar 2 13:00:54 2011 --- git-1.7.3.4-new/dir.c Wed Mar 2 14:25:10 2011 *************** *** 994,999 **** --- 994,1022 ---- return ret; } + static int rmdir_on_broken_cifs(const char *path) + { + struct stat sb; + if (stat(path, &sb) < 0) { + /* Oh well, hopefully if we can't stat it + * it is already gone or we don't have + * permissions to screw it up anyway. */ + return rmdir(path); + } + if (rmdir(path) == 0) { + /* it worked, nothing to restore */ + return 0; + } + /* maybe remove this conditional if you can trigger + * the problem with other types of errors */ + if (errno != ENOTEMPTY) + return -1; + if (chmod(path, sb.st_mode) < 0) + warning("we probably just screwed up the permissions of %s", + path); + return -1; + } + int remove_dir_recursively(struct strbuf *path, int flag) { DIR *dir; *************** *** 1037,1043 **** strbuf_setlen(path, original_len); if (!ret) ! ret = rmdir(path->buf); return ret; } --- 1060,1066 ---- strbuf_setlen(path, original_len); if (!ret) ! ret = rmdir_on_broken_cifs(path->buf); return ret; } *************** *** 1066,1072 **** slash = dirs + (slash - name); do { *slash = '\0'; ! } while (rmdir(dirs) == 0 && (slash = strrchr(dirs, '/'))); free(dirs); } return 0; --- 1090,1096 ---- slash = dirs + (slash - name); do { *slash = '\0'; ! } while (rmdir_on_broken_cifs(dirs) == 0 && (slash = strrchr(dirs, '/'))); free(dirs); } return 0; ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Git changes permissions on directories when deleting files. 2011-03-03 3:48 ` Chad Joan @ 2011-03-03 15:16 ` Jeff King 2011-03-11 6:09 ` Chad Joan 0 siblings, 1 reply; 19+ messages in thread From: Jeff King @ 2011-03-03 15:16 UTC (permalink / raw) To: Chad Joan; +Cc: Matthieu Moy, Computer Druid, git On Wed, Mar 02, 2011 at 10:48:00PM -0500, Chad Joan wrote: > It seems to be working! I've tried it with 'git rm' and when pulling > deletions. Great. > I imagine that race condition can happen if files in the directory are > being modified while git does an rmdir? If that's the case then I'm > not too worried. There is only one other programmer that might be > working with me at the same time on an infrequently used directory. The race condition I mentioned earlier was for a different workaround. Basically there are two strategies, each with a difference race: 1. Don't rmdir on non-empty directories. This means we have to opendir the directory and look for entries before rmdir(). If there is file activity in the directory while we are looking we may think it is empty when it's not and rmdir(), screwing up the permissions. 2. Before any rmdir, check permissions. Do the rmdir, and then restore the permissions if rmdir fails. The race here is if somebody is modifying the permissions on a non-empty directory, we may overwrite their changes. Obviously the patch does (2), so there is still that race. > diff -crB git-1.7.3.4/dir.c git-1.7.3.4-new/dir.c Context diff? Eww. There is this awesome tool called "git" that can help you with managing versions of software. :) -Peff ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Git changes permissions on directories when deleting files. 2011-03-03 15:16 ` Jeff King @ 2011-03-11 6:09 ` Chad Joan 0 siblings, 0 replies; 19+ messages in thread From: Chad Joan @ 2011-03-11 6:09 UTC (permalink / raw) To: Jeff King; +Cc: Matthieu Moy, Computer Druid, git On Thu, Mar 3, 2011 at 10:16 AM, Jeff King <peff@peff.net> wrote: > > ... > > > diff -crB git-1.7.3.4/dir.c git-1.7.3.4-new/dir.c > > Context diff? Eww. There is this awesome tool called "git" that can help > you with managing versions of software. :) > > -Peff Yes... though I'm just letting Gentoo compile it for me and I'm not doing anything serious enough to justify downloading/installing GIT by hand and next to the one that's already there. So I just worked from the tarball that Gentoo uses and added stuff. Any other day and I'd be all over that git usage ;) I did run into more permissions being messed up, so I grepped for rmdir and replaced /all/ instances of it in git's C code. I haven't had anymore trouble so far. Here's the newer patch: diff -crB git-1.7.3.4/dir.c git-1.7.3.4-new/dir.c *** git-1.7.3.4/dir.c Wed Mar 2 13:00:54 2011 --- git-1.7.3.4-new/dir.c Thu Mar 10 11:09:32 2011 *************** *** 994,999 **** --- 994,1022 ---- return ret; } + int rmdir_on_broken_cifs(const char *path) + { + struct stat sb; + if (stat(path, &sb) < 0) { + /* Oh well, hopefully if we can't stat it + * it is already gone or we don't have + * permissions to screw it up anyway. */ + return rmdir(path); + } + if (rmdir(path) == 0) { + /* it worked, nothing to restore */ + return 0; + } + /* maybe remove this conditional if you can trigger + * the problem with other types of errors */ + if (errno != ENOTEMPTY) + return -1; + if (chmod(path, sb.st_mode) < 0) + warning("we probably just screwed up the permissions of %s", + path); + return -1; + } + int remove_dir_recursively(struct strbuf *path, int flag) { DIR *dir; *************** *** 1037,1043 **** strbuf_setlen(path, original_len); if (!ret) ! ret = rmdir(path->buf); return ret; } --- 1060,1066 ---- strbuf_setlen(path, original_len); if (!ret) ! ret = rmdir_on_broken_cifs(path->buf); return ret; } *************** *** 1066,1072 **** slash = dirs + (slash - name); do { *slash = '\0'; ! } while (rmdir(dirs) == 0 && (slash = strrchr(dirs, '/'))); free(dirs); } return 0; --- 1090,1096 ---- slash = dirs + (slash - name); do { *slash = '\0'; ! } while (rmdir_on_broken_cifs(dirs) == 0 && (slash = strrchr(dirs, '/'))); free(dirs); } return 0; Only in git-1.7.3.4: dir.c~ diff -crB git-1.7.3.4/dir.h git-1.7.3.4-new/dir.h *** git-1.7.3.4/dir.h Wed Dec 15 21:52:11 2010 --- git-1.7.3.4-new/dir.h Thu Mar 10 11:09:13 2011 *************** *** 101,104 **** --- 101,106 ---- /* tries to remove the path with empty directories along it, ignores ENOENT */ extern int remove_path(const char *path); + extern int rmdir_on_broken_cifs(const char *path); + #endif diff -crB git-1.7.3.4/entry.c git-1.7.3.4-new/entry.c *** git-1.7.3.4/entry.c Wed Dec 15 21:52:11 2010 --- git-1.7.3.4-new/entry.c Thu Mar 10 11:12:25 2011 *************** *** 68,74 **** die_errno("cannot unlink '%s'", pathbuf); } closedir(dir); ! if (rmdir(path)) die_errno("cannot rmdir '%s'", path); } --- 68,74 ---- die_errno("cannot unlink '%s'", pathbuf); } closedir(dir); ! if (rmdir_on_broken_cifs(path)) die_errno("cannot rmdir '%s'", path); } diff -crB git-1.7.3.4/pack-refs.c git-1.7.3.4-new/pack-refs.c *** git-1.7.3.4/pack-refs.c Wed Dec 15 21:52:11 2010 --- git-1.7.3.4-new/pack-refs.c Thu Mar 10 12:34:53 2011 *************** *** 2,7 **** --- 2,8 ---- #include "refs.h" #include "tag.h" #include "pack-refs.h" + #include "dir.h" struct ref_to_prune { struct ref_to_prune *next; *************** *** 86,92 **** if (q == p) break; *q = '\0'; ! if (rmdir(git_path("%s", name))) break; } } --- 87,93 ---- if (q == p) break; *q = '\0'; ! if (rmdir_on_broken_cifs(git_path("%s", name))) break; } } diff -crB git-1.7.3.4/symlinks.c git-1.7.3.4-new/symlinks.c *** git-1.7.3.4/symlinks.c Wed Dec 15 21:52:11 2010 --- git-1.7.3.4-new/symlinks.c Thu Mar 10 11:24:08 2011 *************** *** 1,4 **** --- 1,5 ---- #include "cache.h" + #include "dir.h" /* * Returns the length (on a path component basis) of the longest *************** *** 255,261 **** { while (removal.len > new_len) { removal.path[removal.len] = '\0'; ! if (rmdir(removal.path)) break; do { removal.len--; --- 256,262 ---- { while (removal.len > new_len) { removal.path[removal.len] = '\0'; ! if (rmdir_on_broken_cifs(removal.path)) break; do { removal.len--; diff -crB git-1.7.3.4/wrapper.c git-1.7.3.4-new/wrapper.c *** git-1.7.3.4/wrapper.c Wed Dec 15 21:52:11 2010 --- git-1.7.3.4-new/wrapper.c Thu Mar 10 11:12:36 2011 *************** *** 2,7 **** --- 2,8 ---- * Various trivial helper wrappers around standard functions */ #include "cache.h" + #include "dir.h" static void try_to_free_builtin(size_t size) { *************** *** 346,352 **** int rmdir_or_warn(const char *file) { ! return warn_if_unremovable("rmdir", file, rmdir(file)); } int remove_or_warn(unsigned int mode, const char *file) --- 347,353 ---- int rmdir_or_warn(const char *file) { ! return warn_if_unremovable("rmdir", file, rmdir_on_broken_cifs(file)); } int remove_or_warn(unsigned int mode, const char *file) ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2011-03-11 6:10 UTC | newest] Thread overview: 19+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-03-01 1:42 Git changes permissions on directories when deleting files Chad Joan 2011-03-01 1:45 ` Chad Joan 2011-03-01 2:19 ` Computer Druid 2011-03-01 4:00 ` Chad Joan 2011-03-01 15:51 ` Chad Joan 2011-03-01 17:11 ` Computer Druid 2011-03-01 19:35 ` Chad Joan 2011-03-01 19:44 ` Jeff King 2011-03-01 19:57 ` Chad Joan 2011-03-01 20:08 ` Jeff King 2011-03-01 20:30 ` Chad Joan 2011-03-01 20:39 ` Computer Druid 2011-03-01 20:57 ` Jeff King 2011-03-01 20:43 ` Matthieu Moy 2011-03-01 20:46 ` Chad Joan 2011-03-01 21:08 ` Jeff King 2011-03-03 3:48 ` Chad Joan 2011-03-03 15:16 ` Jeff King 2011-03-11 6:09 ` Chad Joan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).