Thread (2 messages) 2 messages, 2 authors, 2017-05-17

Re: [PATCH v3 3/8] dir: recurse into untracked dirs for ignored files

From: Junio C Hamano <hidden>
Date: 2017-05-17 06:23:37

Samuel Lijin [off-list ref] writes:
quoted hunk
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.
+				 (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)) {
+				)
+		{
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.
 			struct untracked_cache_dir *ud;
 			ud = lookup_untracked(dir->untracked, untracked,
 					      path.buf + baselen,
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help