Re: [PATCH] ls-files: fix overeager pathspec optimization
On Fri, Jan 08, 2010 at 05:07:46PM -0800, Linus Torvalds wrote:
Show 16 quoted lines
> > 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
Jeff King <peff@peff.net>