Fix error-prone fill_directory() API; make it only return matches

Traditionally, the expected calling convention for the dir.c API was:

    fill_directory(&dir, ..., pathspec)
    foreach entry in dir->entries:
        if (dir_path_match(entry, pathspec))
            process_or_display(entry)

This may have made sense once upon a time, because the fill_directory() call
could use cheap checks to avoid doing full pathspec matching, and an external
caller may have wanted to do other post-processing of the results anyway.
However:

    * this structure makes it easy for users of the API to get it wrong

    * this structure actually makes it harder to understand
      fill_directory() and the functions it uses internally.  It has
      tripped me up several times while trying to fix bugs and
      restructure things.

    * relying on post-filtering was already found to produce wrong
      results; pathspec matching had to be added internally for multiple
      cases in order to get the right results (see commits 404ebceda01c
      (dir: also check directories for matching pathspecs, 2019-09-17)
      and 89a1f4aaf765 (dir: if our pathspec might match files under a
      dir, recurse into it, 2019-09-17))

    * it's bad for performance: fill_directory() already has to do lots
      of checks and knows the subset of cases where it still needs to do
      more checks.  Forcing external callers to do full pathspec
      matching means they must re-check _every_ path.

So, add the pathspec matching within the fill_directory() internals, and
remove it from external callers.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

1 files changed, 8 insertions, 1 deletions

diff --git a/dir.c b/dir.c
index a67930dcff..2de6491040 100644
--- a/dir.c
+++ b/dir.c
@@ -2117,7 +2117,14 @@ static enum path_treatment treat_path(struct dir_struct *dir,
 				       baselen, excluded, pathspec);
 	case DT_REG:
 	case DT_LNK:
-		return excluded ? path_excluded : path_untracked;
+		if (excluded)
+			return path_excluded;
+		if (pathspec &&
+		    !do_match_pathspec(istate, pathspec, path->buf, path->len,
+				       0 /* prefix */, NULL /* seen */,
+				       0 /* flags */))
+			return path_none;
+		return path_untracked;
 	}
 }