Merge branch 'en/clean-cleanups'

Code clean-up of "git clean" resulted in a fix of recent
performance regression.

* en/clean-cleanups:
  clean: optimize and document cases where we recurse into subdirectories
  clean: consolidate handling of ignored parameters
  dir, clean: avoid disallowed behavior
  dir: fix a few confusing comments

diff --git a/builtin/clean.c b/builtin/clean.c
index 4ca12bc0c0ba2c3bc3084a320835cf83600173ce..5a9c29a558bb1efc383b41256972b3113845be4e 100644 (file)
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -924,12 +924,6 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
                             0);
 
        memset(&dir, 0, sizeof(dir));
-       if (ignored_only)
-               dir.flags |= DIR_SHOW_IGNORED;
-
-       if (ignored && ignored_only)
-               die(_("-x and -X cannot be used together"));
-
        if (!interactive && !dry_run && !force) {
                if (config_set)
                        die(_("clean.requireForce set to true and neither -i, -n, nor -f given; "
@@ -946,6 +940,13 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 
        dir.flags |= DIR_SHOW_OTHER_DIRECTORIES;
 
+       if (ignored && ignored_only)
+               die(_("-x and -X cannot be used together"));
+       if (!ignored)
+               setup_standard_excludes(&dir);
+       if (ignored_only)
+               dir.flags |= DIR_SHOW_IGNORED;
+
        if (argc) {
                /*
                 * Remaining args implies pathspecs specified, and we should
@@ -954,15 +955,41 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
                remove_directories = 1;
        }
 
-       if (remove_directories)
-               dir.flags |= DIR_SHOW_IGNORED_TOO | DIR_KEEP_UNTRACKED_CONTENTS;
+       if (remove_directories && !ignored_only) {
+               /*
+                * We need to know about ignored files too:
+                *
+                * If (ignored), then we will delete ignored files as well.
+                *
+                * If (!ignored), then even though we not are doing
+                * anything with ignored files, we need to know about them
+                * so that we can avoid deleting a directory of untracked
+                * files that also contains an ignored file within it.
+                *
+                * For the (!ignored) case, since we only need to avoid
+                * deleting ignored files, we can set
+                * DIR_SHOW_IGNORED_TOO_MODE_MATCHING in order to avoid
+                * recursing into a directory which is itself ignored.
+                */
+               dir.flags |= DIR_SHOW_IGNORED_TOO;
+               if (!ignored)
+                       dir.flags |= DIR_SHOW_IGNORED_TOO_MODE_MATCHING;
+
+               /*
+                * Let the fill_directory() machinery know that we aren't
+                * just recursing to collect the ignored files; we want all
+                * the untracked ones so that we can delete them.  (Note:
+                * we could also set DIR_KEEP_UNTRACKED_CONTENTS when
+                * ignored_only is true, since DIR_KEEP_UNTRACKED_CONTENTS
+                * only has effect in combination with DIR_SHOW_IGNORED_TOO.  It makes
+                * the code clearer to exclude it, though.
+                */
+               dir.flags |= DIR_KEEP_UNTRACKED_CONTENTS;
+       }
 
        if (read_cache() < 0)
                die(_("index file corrupt"));
 
-       if (!ignored)
-               setup_standard_excludes(&dir);
-
        pl = add_pattern_list(&dir, EXC_CMDL, "--exclude option");
        for (i = 0; i < exclude_list.nr; i++)
                add_pattern(exclude_list.items[i].string, "", 0, pl, -(i+1));

diff --git a/dir.c b/dir.c
index 9ecd349e27dae10d204b36c549540c5d370c62b2..1045cc9c6f58702aba6e9036a8bce91a431c108a 100644 (file)
--- a/dir.c
+++ b/dir.c
@@ -193,6 +193,10 @@ int fill_directory(struct dir_struct *dir,
        const char *prefix;
        size_t prefix_len;
 
+       unsigned exclusive_flags = DIR_SHOW_IGNORED | DIR_SHOW_IGNORED_TOO;
+       if ((dir->flags & exclusive_flags) == exclusive_flags)
+               BUG("DIR_SHOW_IGNORED and DIR_SHOW_IGNORED_TOO are exclusive");
+
        /*
         * Calculate common prefix for the pathspec, and
         * use that to optimize the directory walk
@@ -1836,7 +1840,7 @@ static enum path_treatment treat_directory(struct dir_struct *dir,
         * to recurse into untracked/ignored directories if either of the
         * following bits is set:
         *   - DIR_SHOW_IGNORED_TOO (because then we need to determine if
-        *                           there are ignored directories below)
+        *                           there are ignored entries below)
         *   - DIR_HIDE_EMPTY_DIRECTORIES (because we have to determine if
         *                                 the directory is empty)
         */
@@ -1854,10 +1858,11 @@ static enum path_treatment treat_directory(struct dir_struct *dir,
                return path_excluded;
 
        /*
-        * If we have we don't want to know the all the paths under an
-        * untracked or ignored directory, we still need to go into the
-        * directory to determine if it is empty (because an empty directory
-        * should be path_none instead of path_excluded or path_untracked).
+        * Even if we don't want to know all the paths under an untracked or
+        * ignored directory, we may still need to go into the directory to
+        * determine if it is empty (because with DIR_HIDE_EMPTY_DIRECTORIES,
+        * an empty directory should be path_none instead of path_excluded or
+        * path_untracked).
         */
        check_only = ((dir->flags & DIR_HIDE_EMPTY_DIRECTORIES) &&
                      !(dir->flags & DIR_SHOW_IGNORED_TOO));