On Wed, May 17, 2017 at 2:23 AM, Junio C Hamano [off-list ref] wrote:

Samuel Lijin [off-list ref] writes:

quoted

We consider directories containing only untracked and ignored files to
be themselves untracked, which in the usual case means we don't have to
search these directories. This is problematic when we want to collect
ignored files with DIR_SHOW_IGNORED_TOO, though, so we teach
read_directory_recursive() to recurse into untracked directories to find
the ignored files they contain when DIR_SHOW_IGNORED_TOO is set. This
has the side effect of also collecting all untracked files in untracked
directories as well.

Signed-off-by: Samuel Lijin <redacted>
---
 dir.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/dir.c b/dir.c
index f451bfa48..6bd0350e9 100644
--- a/dir.c
+++ b/dir.c

@@ -1784,7 +1784,12 @@ static enum path_treatment read_directory_recursive(struct dir_struct *dir,
                      dir_state = state;

              /* recurse into subdir if instructed by treat_path */
-             if (state == path_recurse) {
+             if ((state == path_recurse) ||
+                             ((get_dtype(cdir.de, path.buf, path.len) == DT_DIR) &&

I see a conditional in treat_path() that is prepared to deal with a
NULL in cdir.de; do we know cdir.de is always non-NULL at this point
in the code, or is get_dtype() prepared to see NULL as its first
parameter?

        ... goes and looks ...

Yes, it falls back to the usual lstat() dance in such a case, so
we'd be OK here.

quoted

+                              (state == path_untracked) &&
+                              (dir->flags & DIR_SHOW_IGNORED_TOO))

If we are told to SHOW_IGNORED_TOO, we'd recurse into an untracked
thing if it is a directory.  No other behaviour change.

Isn't get_dtype() potentially slower than other two checks if it
triggers?  I am wondering if these three conditions in &&-chain
should be reordered to call get_dtype() the last, i.e.

                if ((state == path_recurse) ||
                    ((dir->flags & DIR_SHOW_IGNORED_TOO)) &&
                     (state == path_untracked) &&
                     (get_dtype(cdir.de, path.buf, path.len) == DT_DIR)) {

Ah, I didn't realize that. I figured that get_dtype() was just a
helper to invoke the appropriate macros, but if it's actually mildly
expensive I'll take your reorder.

quoted

+                             )
+             {

It may be just a style, but these new lines are indented overly too
deep.  We don't use a lone "{" on a line to open a block, either.

quoted

                      struct untracked_cache_dir *ud;
                      ud = lookup_untracked(dir->untracked, untracked,
                                            path.buf + baselen,