@matheusbsbs / matheus-bagliano / commits / 777b420347
Fork 0

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>

Elijah Newren committed Dec 19, 2019 at 21:28 UTC 777b420347649f26022bb1a4bf7afe7c4fe0b090
3 files changed +32 -2
dir.c
+30
@@ -1990,6 +1990,15 @@ static enum path_treatment read_directory_recursive(struct dir_struct *dir,
1990 struct untracked_cache_dir *untracked, int check_only,
1991 int stop_at_first_file, const struct pathspec *pathspec)
1992 {
1993 + /*
1994 + * WARNING WARNING WARNING:
1995 + *
1996 + * Any updates to the traversal logic here may need corresponding
1997 + * updates in treat_leading_path(). See the commit message for the
1998 + * commit adding this warning as well as the commit preceding it
1999 + * for details.
2000 + */
2001 +
2002 struct cached_dir cdir;
2003 enum path_treatment state, subdir_state, dir_state = path_none;
2004 struct strbuf path = STRBUF_INIT;
@@ -2101,6 +2110,15 @@ static int treat_leading_path(struct dir_struct *dir,
2110 const char *path, int len,
2111 const struct pathspec *pathspec)
2112 {
2113 + /*
2114 + * WARNING WARNING WARNING:
2115 + *
2116 + * Any updates to the traversal logic here may need corresponding
2117 + * updates in treat_leading_path(). See the commit message for the
2118 + * commit adding this warning as well as the commit preceding it
2119 + * for details.
2120 + */
2121 +
2122 struct strbuf sb = STRBUF_INIT;
2123 int prevlen, baselen;
2124 const char *cp;
@@ -2166,6 +2184,18 @@ static int treat_leading_path(struct dir_struct *dir,
2184 de->d_name[baselen-prevlen] = '\0';
2185 state = treat_path(dir, NULL, &cdir, istate, &sb, prevlen,
2186 pathspec);
2187 + if (state == path_untracked &&
2188 + get_dtype(cdir.de, istate, sb.buf, sb.len) == DT_DIR &&
2189 + (dir->flags & DIR_SHOW_IGNORED_TOO ||
2190 + do_match_pathspec(istate, pathspec, sb.buf, sb.len,
2191 + baselen, NULL, DO_MATCH_LEADING_PATHSPEC) == MATCHED_RECURSIVELY_LEADING_PATHSPEC)) {
2192 + add_path_to_appropriate_result_list(dir, NULL, &cdir,
2193 + istate,
2194 + &sb, baselen,
2195 + pathspec, state);
2196 + state = path_recurse;
2197 + }
2198 +
2199 if (state != path_recurse)
2200 break; /* do not recurse into it */
2201 if (len <= baselen)
t/t3011-common-prefixes-and-directory-traversal.sh
+1 -1
@@ -195,7 +195,7 @@ test_expect_success 'git ls-files -o consistent between one or two dirs' '
195
196 # ls-files doesn't have a way to request showing both untracked and ignored
197 # files at the same time, so use `git status --ignored`
198 -test_expect_failure 'git status --ignored shows same files under dir with or without pathspec' '
198 +test_expect_success 'git status --ignored shows same files under dir with or without pathspec' '
199 cat <<-EOF >expect &&
200 ?? an_untracked_dir/
201 !! an_untracked_dir/ignored
t/t7061-wtstatus-ignore.sh
+1 -1
@@ -47,7 +47,7 @@ cat >expected <<\EOF
47 !! untracked/ignored
48 EOF
49
50 -test_expect_failure 'status of untracked directory with --ignored works with or without prefix' '
50 +test_expect_success 'status of untracked directory with --ignored works with or without prefix' '
51 git status --porcelain --ignored >tmp &&
52 grep untracked/ tmp >actual &&
53 rm tmp &&