On Fri, Jan 08, 2010 at 05:07:46PM -0800, Linus Torvalds wrote: > > But I am starting to wonder if we might be better off restructuring > > read_directory_recursive(). Currently it assumes that the path it was > > given _must_ be of interest (i.e. not ignored) and runs excluded() on > > subdirectories it finds to make that same decision before recursing into > > them or skipping them. It might make more sense if it first checked if > > the path given by the caller should be ignored and act accordingly. > > Hmm. I can't make myself care one way or the other, I have to admit. I > assume you mean basically taking the path and using the first component of > it _instead_ of doing a readdir() - and getting rid of the simplification > up front? > > I agree that that should work. Would it be simpler and cleaner? Perhaps. > I'd have to see both patches to be able to tell. I do admit that while I > acked your patch, it sure ain't _pretty_ to do that special odd > "has_leading_ignored_dir()" thing. It would look something like this: diff --git a/dir.c b/dir.c index 3a8d3e6..306d354 100644 --- a/dir.c +++ b/dir.c @@ -811,12 +811,19 @@ static void free_simplify(struct path_simplify *simplify) int read_directory(struct dir_struct *dir, const char *path, int len, const char **pathspec) { struct path_simplify *simplify; + int d_type = DT_DIR; + int exclude; if (has_symlink_leading_path(path, len)) return dir->nr; simplify = create_simplify(pathspec); - read_directory_recursive(dir, path, len, 0, simplify); + exclude = excluded(dir, path, &d_type); + if (exclude && (dir->flags & DIR_COLLECT_IGNORED) && + in_pathspec(path, len, simplify)) + dir_add_ignored(dir, path, len); + if (!exclude || (dir->flags & DIR_SHOW_IGNORED)) + read_directory_recursive(dir, path, len, 0, simplify); free_simplify(simplify); qsort(dir->entries, dir->nr, sizeof(struct dir_entry *), cmp_name); qsort(dir->ignored, dir->ignored_nr, sizeof(struct dir_entry *), cmp_name); But unfortunately excluded() is not happy with the trailing slash on the path given to read_directory, so we also need on top: diff --git a/dir.c b/dir.c index 306d354..6045a84 100644 --- a/dir.c +++ b/dir.c @@ -813,12 +813,17 @@ int read_directory(struct dir_struct *dir, const char *path, int len, const char struct path_simplify *simplify; int d_type = DT_DIR; int exclude; + char *path_without_slash; if (has_symlink_leading_path(path, len)) return dir->nr; simplify = create_simplify(pathspec); - exclude = excluded(dir, path, &d_type); + path_without_slash = xstrdup(path); + if (path_without_slash[strlen(path_without_slash)-1] == '/') + path_without_slash[strlen(path_without_slash)-1] = '\0'; + exclude = excluded(dir, path_without_slash, &d_type); + free(path_without_slash); if (exclude && (dir->flags & DIR_COLLECT_IGNORED) && in_pathspec(path, len, simplify)) dir_add_ignored(dir, path, len); And that does fix the case that triggered this whole discussion, but I haven't tested thoroughly to make sure we are not adversely affecting other cases. -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