dir: synchronize treat_leading_path() and read_directory_recursive()

Our optimization to avoid calling into read_directory_recursive() when
all pathspecs have a common leading directory mean that we need to match
the logic that read_directory_recursive() would use if we had just
called it from the root.  Since it does more than call treat_path() we
need to copy that same logic.

Alternatively, we could try to change treat_path to return path_recurse
for an untracked directory under the given special circumstances that
this logic checks for, but a simple switch results in many test failures
such as 'git clean -d' not wiping out untracked but empty directories.
To work around that, we'd need the caller of treat_path to check for
path_recurse and sometimes special case it into path_untracked.  In
other words, we'd still have extra logic in both places.

Needing to duplicate logic like this means it is guaranteed someone will
eventually need to make further changes and forget to update both
locations.  It is tempting to just nuke the leading_directory special
casing to avoid such bugs and simplify the code, but unpack_trees'
verify_clean_subdirectory() also calls read_directory() and does so with
a non-empty leading path, so I'm hesitant to try to restructure further.
Add obnoxious warnings to treat_leading_path() and
read_directory_recursive() to try to warn people of such problems.

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

diff --git a/dir.c b/dir.c
index a42cc2aa8ce150f5059383662ce9a8acc1d72131..357f9593c41e972f810888f7fcc29e8dee9e848a 100644 (file)
--- a/dir.c
+++ b/dir.c
@@ -1990,6 +1990,15 @@ static enum path_treatment read_directory_recursive(struct dir_struct *dir,
        struct untracked_cache_dir *untracked, int check_only,
        int stop_at_first_file, const struct pathspec *pathspec)
 {
+       /*
+        * WARNING WARNING WARNING:
+        *
+        * Any updates to the traversal logic here may need corresponding
+        * updates in treat_leading_path().  See the commit message for the
+        * commit adding this warning as well as the commit preceding it
+        * for details.
+        */
+
        struct cached_dir cdir;
        enum path_treatment state, subdir_state, dir_state = path_none;
        struct strbuf path = STRBUF_INIT;
@@ -2101,6 +2110,15 @@ static int treat_leading_path(struct dir_struct *dir,
                              const char *path, int len,
                              const struct pathspec *pathspec)
 {
+       /*
+        * WARNING WARNING WARNING:
+        *
+        * Any updates to the traversal logic here may need corresponding
+        * updates in treat_leading_path().  See the commit message for the
+        * commit adding this warning as well as the commit preceding it
+        * for details.
+        */
+
        struct strbuf sb = STRBUF_INIT;
        int prevlen, baselen;
        const char *cp;
@@ -2166,6 +2184,18 @@ static int treat_leading_path(struct dir_struct *dir,
                de->d_name[baselen-prevlen] = '\0';
                state = treat_path(dir, NULL, &cdir, istate, &sb, prevlen,
                                    pathspec);
+               if (state == path_untracked &&
+                   get_dtype(cdir.de, istate, sb.buf, sb.len) == DT_DIR &&
+                   (dir->flags & DIR_SHOW_IGNORED_TOO ||
+                    do_match_pathspec(istate, pathspec, sb.buf, sb.len,
+                                      baselen, NULL, DO_MATCH_LEADING_PATHSPEC) == MATCHED_RECURSIVELY_LEADING_PATHSPEC)) {
+                       add_path_to_appropriate_result_list(dir, NULL, &cdir,
+                                                           istate,
+                                                           &sb, baselen,
+                                                           pathspec, state);
+                       state = path_recurse;
+               }
+
                if (state != path_recurse)
                        break; /* do not recurse into it */
                if (len <= baselen)

diff --git a/t/t3011-common-prefixes-and-directory-traversal.sh b/t/t3011-common-prefixes-and-directory-traversal.sh
index 098fddc75b0fe1a6009f1ada6cd3dcf616e14949..3da5b2b6e795ec4587608ead8bc10abf0c6d9039 100755 (executable)
--- a/t/t3011-common-prefixes-and-directory-traversal.sh
+++ b/t/t3011-common-prefixes-and-directory-traversal.sh
@@ -195,7 +195,7 @@ test_expect_success 'git ls-files -o consistent between one or two dirs' '
 
 # ls-files doesn't have a way to request showing both untracked and ignored
 # files at the same time, so use `git status --ignored`
-test_expect_failure 'git status --ignored shows same files under dir with or without pathspec' '
+test_expect_success 'git status --ignored shows same files under dir with or without pathspec' '
        cat <<-EOF >expect &&
        ?? an_untracked_dir/
        !! an_untracked_dir/ignored

diff --git a/t/t7061-wtstatus-ignore.sh b/t/t7061-wtstatus-ignore.sh
index 84366050dabbbfe29c284a6b18e79f99a6723ee7..e4cf5484f97a570da8225ca501d3b395e13604e3 100755 (executable)
--- a/t/t7061-wtstatus-ignore.sh
+++ b/t/t7061-wtstatus-ignore.sh
@@ -47,7 +47,7 @@ cat >expected <<\EOF
 !! untracked/ignored
 EOF
 
-test_expect_failure 'status of untracked directory with --ignored works with or without prefix' '
+test_expect_success 'status of untracked directory with --ignored works with or without prefix' '
        git status --porcelain --ignored >tmp &&
        grep untracked/ tmp >actual &&
        rm tmp &&