Message ID | b4cb448e8d76ae16f6fa38a773244bb4a8499938.1591858774.git.gitgitgadget@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Clean cleanups | expand |
"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes: > ... > However, in clean's case we don't need to recurse into ignored > directories; that is just a waste of time. Nice. > Rather than relying on other bugs in fill_directory()'s former > logic to provide the behavior of skipping ignored directories, make use > of the DIR_SHOW_IGNORED_TOO_MODE_MATCHING value specifically added in > commit eec0f7f2b7 ("status: add option to show ignored files > differently", 2017-10-30) for this purpose. Thanks.
Catching up on this - thanks Elijah for jumping on this bug and fixing it immediately. I'll let you know if I see similar bugs in the future. Thanks, Brian On Fri, Jun 12, 2020 at 5:28 PM Junio C Hamano <gitster@pobox.com> wrote: > > "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes: > > > ... > > However, in clean's case we don't need to recurse into ignored > > directories; that is just a waste of time. > > Nice. > > > Rather than relying on other bugs in fill_directory()'s former > > logic to provide the behavior of skipping ignored directories, make use > > of the DIR_SHOW_IGNORED_TOO_MODE_MATCHING value specifically added in > > commit eec0f7f2b7 ("status: add option to show ignored files > > differently", 2017-10-30) for this purpose. > > Thanks. >
diff --git a/builtin/clean.c b/builtin/clean.c index 1be437bd5a3..5a9c29a558b 100644 --- a/builtin/clean.c +++ b/builtin/clean.c @@ -955,8 +955,37 @@ int cmd_clean(int argc, const char **argv, const char *prefix) remove_directories = 1; } - if (remove_directories && !ignored_only) - 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"));