dir: if our pathspec might match files under a dir, recurse into it

For git clean, if a directory is entirely untracked and the user did not
specify -d (corresponding to DIR_SHOW_IGNORED_TOO), then we usually do
not want to remove that directory and thus do not recurse into it.
However, if the user manually specified specific (or even globbed) paths
somewhere under that directory to remove, then we need to recurse into
the directory to make sure we remove the relevant paths under that
directory as the user requested.

Note that this does not mean that the recursed-into directory will be
added to dir->entries for later removal; as of a few commits earlier in
this series, there is another more strict match check that is run after
returning from a recursed-into directory before deciding to add it to the
list of entries.  Therefore, this will only result in files underneath
the given directory which match one of the pathspecs being added to the
entries list.

Two notes of potential interest to future readers:

  * If we wanted to only recurse into a directory when it is specifically
    matched rather than matched-via-glob (e.g. '*.c'), then we could do
    so via making the final non-zero return in match_pathspec_item be
    MATCHED_RECURSIVELY instead of MATCHED_RECURSIVELY_LEADING_PATHSPEC.
    (Note that the relative order of MATCHED_RECURSIVELY_LEADING_PATHSPEC
    and MATCHED_RECURSIVELY are important for such a change.)  I was
    leaving open that possibility while writing an RFC asking for the
    behavior we want, but even though we don't want it, that knowledge
    might help you understand the code flow better.

  * There is a growing amount of logic in read_directory_recursive() for
    deciding whether to recurse into a subdirectory.  However, there is a
    comment immediately preceding this logic that says to recurse if
    instructed by treat_path().   It may be better for the logic in
    read_directory_recursive() to ultimately be moved to treat_path() (or
    another function it calls, such as treat_directory()), but I have
    left that for someone else to tackle in the future.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
Elijah Newren 2019-09-17 09:34:58 -07:00 committed by Junio C Hamano
parent a3d89d8f76
commit 89a1f4aaf7
3 changed files with 11 additions and 8 deletions

10
dir.c
View file

@ -360,7 +360,7 @@ static int match_pathspec_item(const struct index_state *istate,
if ((namelen < matchlen) && if ((namelen < matchlen) &&
(match[namelen-offset] == '/') && (match[namelen-offset] == '/') &&
!ps_strncmp(item, match, name, namelen)) !ps_strncmp(item, match, name, namelen))
return MATCHED_RECURSIVELY; return MATCHED_RECURSIVELY_LEADING_PATHSPEC;
/* name" doesn't match up to the first wild character */ /* name" doesn't match up to the first wild character */
if (item->nowildcard_len < item->len && if (item->nowildcard_len < item->len &&
@ -377,7 +377,7 @@ static int match_pathspec_item(const struct index_state *istate,
* The submodules themselves will be able to perform more * The submodules themselves will be able to perform more
* accurate matching to determine if the pathspec matches. * accurate matching to determine if the pathspec matches.
*/ */
return MATCHED_RECURSIVELY; return MATCHED_RECURSIVELY_LEADING_PATHSPEC;
} }
return 0; return 0;
@ -1939,8 +1939,10 @@ static enum path_treatment read_directory_recursive(struct dir_struct *dir,
/* recurse into subdir if instructed by treat_path */ /* recurse into subdir if instructed by treat_path */
if ((state == path_recurse) || if ((state == path_recurse) ||
((state == path_untracked) && ((state == path_untracked) &&
(dir->flags & DIR_SHOW_IGNORED_TOO) && (get_dtype(cdir.de, istate, path.buf, path.len) == DT_DIR) &&
(get_dtype(cdir.de, istate, path.buf, path.len) == DT_DIR))) { ((dir->flags & DIR_SHOW_IGNORED_TOO) ||
do_match_pathspec(istate, pathspec, path.buf, path.len,
baselen, NULL, DO_MATCH_LEADING_PATHSPEC) == MATCHED_RECURSIVELY_LEADING_PATHSPEC))) {
struct untracked_cache_dir *ud; struct untracked_cache_dir *ud;
ud = lookup_untracked(dir->untracked, untracked, ud = lookup_untracked(dir->untracked, untracked,
path.buf + baselen, path.buf + baselen,

5
dir.h
View file

@ -211,8 +211,9 @@ int count_slashes(const char *s);
* when populating the seen[] array. * when populating the seen[] array.
*/ */
#define MATCHED_RECURSIVELY 1 #define MATCHED_RECURSIVELY 1
#define MATCHED_FNMATCH 2 #define MATCHED_RECURSIVELY_LEADING_PATHSPEC 2
#define MATCHED_EXACTLY 3 #define MATCHED_FNMATCH 3
#define MATCHED_EXACTLY 4
int simple_length(const char *match); int simple_length(const char *match);
int no_wildcard(const char *string); int no_wildcard(const char *string);
char *common_prefix(const struct pathspec *pathspec); char *common_prefix(const struct pathspec *pathspec);

View file

@ -691,7 +691,7 @@ test_expect_failure 'git clean -d skips nested repo containing ignored files' '
test_path_is_file nested-repo-with-ignored-file/file test_path_is_file nested-repo-with-ignored-file/file
' '
test_expect_failure 'git clean handles being told what to clean' ' test_expect_success 'git clean handles being told what to clean' '
mkdir -p d1 d2 && mkdir -p d1 d2 &&
touch d1/ut d2/ut && touch d1/ut d2/ut &&
git clean -f */ut && git clean -f */ut &&
@ -707,7 +707,7 @@ test_expect_success 'git clean handles being told what to clean, with -d' '
test_path_is_missing d2/ut test_path_is_missing d2/ut
' '
test_expect_failure 'git clean works if a glob is passed without -d' ' test_expect_success 'git clean works if a glob is passed without -d' '
mkdir -p d1 d2 && mkdir -p d1 d2 &&
touch d1/ut d2/ut && touch d1/ut d2/ut &&
git clean -f "*ut" && git clean -f "*ut" &&