On Mon, Feb 02, 2015 at 11:58:33AM -0500, Andrew Wong wrote: > When "git status" recurses a directory that isn't readable (but > executable), it should print out a warning/error. Currently, if there > are untracked files in these directories, git wouldn't be able to > discover them. Ideally, "git status" should return a non-zero exit > code as well. Also, I think "git add ." would silently ignore such directories, which is probably a bad thing if you are relying on it to capture the whole directory's state. Similarly, I think we would ignore transient errors (like EMFILE) or other EACCES problems (like a mode 0700 directory owned by somebody else). > The problem seems to be In read_directory_recursive() from dir.c. When > opendir() returns null, we continue on ignoring any error. Is there a > scenario where returning null is expected? We can simply call perror() > here, but it would be nice if we can propagate the error to the exit > code too. How would we do that? I think we should report an error on EACCES. Perhaps somebody is happy that "git add" ignores unreadable directories, but the right solution is for them to put those directories in their .gitignore (and/or use "--ignore-errors"). People may want to ignore ENOENT in this situation, though. That is a sign that somebody is racily modifying the directory while git is running. That's generally a bad idea, but it is not a big deal for us to skip such a directory (after all, we might racily have missed its existence in the first place, so all bets are off). >From a cursory look, I'd agree that hitting the opendir() in read_directory_recursive is the right place to start. I'd silently ignore ENOENT, and propagate the rest. That code is too low-level to call die() directly, I think, so you will need to propagate the error back. Adding a new error-value to the "enum path_treatment" could work, but it will probably be rather clumsy getting it all the way back up to the original caller. It will probably be much easier to: 1. Give dir_struct an error flag, and set it whenever the traversal sees an error. Callers can check the flag at the appropriate level and ignore or die() as appropriate. 2. Teach dir_struct a "quiet" flag. If not set, emit a warning() deep in the code. Alternatively, you could collect a set of error-producing pathnames (along with their errno values), and the caller could decide whether to print them itself (this is similar to how DIR_COLLECT_IGNORED works). -Peff -- 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