{"thread":{"id":"38483","subject":"\"git status\" should warn/error when it cannot lists a directory","startedAt":"2015-02-02T16:58:33Z","lastAt":"2015-02-03T05:36:43Z","messageCount":2,"participants":["Andrew Wong","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"255515","messageId":"CADgNjamcR+b-_DKzScU=35idAgG542B7CaJC2AqAE9Srvsq17g@mail.gmail.com","threadId":"38483","inReplyTo":null,"subject":"\"git status\" should warn/error when it cannot lists a directory","fromName":"Andrew Wong","fromEmail":"andrew.kw.w@gmail.com","sentAt":"2015-02-02T16:58:33Z","receivedAt":"2015-02-02T16:58:33Z","isPatch":false,"sender":{"key":"andrew.kw.w@gmail.com","avatar":"https://avatars.githubusercontent.com/u/489311?v=4"},"body":"When \"git status\" recurses a directory that isn't readable (but\nexecutable), it should print out a warning/error. Currently, if there\nare untracked files in these directories, git wouldn't be able to\ndiscover them. Ideally, \"git status\" should return a non-zero exit\ncode as well.\n\nThe problem seems to be In read_directory_recursive() from dir.c. When\nopendir() returns null, we continue on ignoring any error. Is there a\nscenario where returning null is expected? We can simply call perror()\nhere, but it would be nice if we can propagate the error to the exit\ncode too. How would we do that?\n"},{"id":"255560","messageId":"20150203053642.GB1262@peff.net","threadId":"38483","inReplyTo":"CADgNjamcR+b-_DKzScU=35idAgG542B7CaJC2AqAE9Srvsq17g@mail.gmail.com","subject":"Re: \"git status\" should warn/error when it cannot lists a directory","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-02-03T05:36:43Z","receivedAt":"2015-02-03T05:36:43Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 02, 2015 at 11:58:33AM -0500, Andrew Wong wrote:\n\n> When \"git status\" recurses a directory that isn't readable (but\n> executable), it should print out a warning/error. Currently, if there\n> are untracked files in these directories, git wouldn't be able to\n> discover them. Ideally, \"git status\" should return a non-zero exit\n> code as well.\n\nAlso, I think \"git add .\" would silently ignore such directories, which\nis probably a bad thing if you are relying on it to capture the whole\ndirectory's state. Similarly, I think we would ignore transient\nerrors (like EMFILE) or other EACCES problems (like a mode 0700\ndirectory owned by somebody else).\n\n> The problem seems to be In read_directory_recursive() from dir.c. When\n> opendir() returns null, we continue on ignoring any error. Is there a\n> scenario where returning null is expected? We can simply call perror()\n> here, but it would be nice if we can propagate the error to the exit\n> code too. How would we do that?\n\nI think we should report an error on EACCES. Perhaps somebody is happy\nthat \"git add\" ignores unreadable directories, but the right solution is\nfor them to put those directories in their .gitignore (and/or use\n\"--ignore-errors\").\n\nPeople may want to ignore ENOENT in this situation, though. That is a\nsign that somebody is racily modifying the directory while git is\nrunning. That's generally a bad idea, but it is not a big deal for us to\nskip such a directory (after all, we might racily have missed its\nexistence in the first place, so all bets are off).\n\n>From a cursory look, I'd agree that hitting the opendir() in\nread_directory_recursive is the right place to start. I'd silently\nignore ENOENT, and propagate the rest.\n\nThat code is too low-level to call die() directly, I think, so you will\nneed to propagate the error back. Adding a new error-value to the \"enum\npath_treatment\" could work, but it will probably be rather clumsy\ngetting it all the way back up to the original caller. It will probably\nbe much easier to:\n\n  1. Give dir_struct an error flag, and set it whenever the traversal\n     sees an error. Callers can check the flag at the appropriate level\n     and ignore or die() as appropriate.\n\n  2. Teach dir_struct a \"quiet\" flag. If not set, emit a warning()\n     deep in the code. Alternatively, you could collect a set of\n     error-producing pathnames (along with their errno values), and\n     the caller could decide whether to print them itself (this is\n     similar to how DIR_COLLECT_IGNORED works).\n\n-Peff\n"}]}