On Fri, Apr 1, 2011 at 12:34 AM, Alex Riesen <raa.lkml@xxxxxxxxx> wrote: > > Something like this? > > diff --git a/dir.c b/dir.c > index 325fb56..7251426 100644 > --- a/dir.c > +++ b/dir.c > @@ -1191,8 +1191,11 @@ int remove_dir_recursively(struct strbuf *path, int flag) > return 0; > > dir = opendir(path->buf); > - if (!dir) > + if (!dir) { > + if (rmdir(path->buf) == 0) > + return 0; > return -1; > + } Heh. My minimalist sensibilities go "yuck", and say that you should just do if (!dir) return rmdir(path->buf); instead. But not a big deal. Looks fine, and fixes the trivial annoyance. I'd still like the better error message. With "rm -rf" I get good error messages even for the complex case: [torvalds@i5 git]$ mkdir tmp [torvalds@i5 git]$ mkdir tmp/tmp [torvalds@i5 git]$ chmod -w tmp [torvalds@i5 git]$ rm -rf tmp rm: cannot remove `tmp/tmp': Permission denied but "git clean" says: [torvalds@i5 git]$ git clean -dqfx warning: failed to remove tmp/ Hmm. Adding the obvious "strerror(errno)" to builtin/clean.c just changes it to warning: failed to remove tmp/ (Permission denied) which I guess is better, but not entirely obvious (it's "tmp/tmp" that failed to remove due to permissions, but clean.c only sees/cares about the uppermost level) But it's probably not worth worrying about any more. The simple one-liner rmdir() looks worth it. Linus -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html