Re: Git changes permissions on directories when deleting files.
- From
Chad Joan <chadjoan@gmail.com>
- Date
- Mar 3, 2011, 03:48 UTC
- Message-ID
- <AANLkTi=nFMDHR5WL=TiFmshFkxLMF9N4dNEjqw+r7wyh@mail.gmail.com>
- In-Reply-To
- <20110301210852.GB21429@sigill.intra.peff.net>
On Tue, Mar 1, 2011 at 4:08 PM, Jeff King <peff@peff.net> wrote:
Show 51 quoted lines
>
> 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;