{"thread":{"id":"57449","subject":"[PATCH v2 1/2] clean: avoid looking for nested repositories when unnecessary","startedAt":"2022-02-21T09:09:15Z","lastAt":"2022-02-22T01:48:55Z","messageCount":3,"participants":["Patrick Marlier","Elijah Newren"],"isPatch":true,"patchVersion":2,"patchTotal":2},"messages":[{"id":"448959","messageId":"20220221090034.4615-1-patrick.marlier@gmail.com","threadId":"57449","inReplyTo":null,"subject":"[PATCH v2 1/2] clean: avoid looking for nested repositories when unnecessary","fromName":"Patrick Marlier","fromEmail":"patrick.marlier@gmail.com","sentAt":"2022-02-21T09:00:33Z","receivedAt":"2022-02-21T09:09:15Z","isPatch":true,"sender":{"key":"patrick.marlier@gmail.com","avatar":null},"body":"With `git clean --ff` we will be deleting nested untracked repositories,\nso there is no need to differentiate them from other untracked files.\nUse the DIR_NO_GITLINKS flag in dir.flags to signify this and avoid the\nis_nonbare_repository_dir() checks.\n---\n builtin/clean.c | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex 3ff02bbbff..18b37e3fd9 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -955,9 +955,10 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \t\t\t\t  \" refusing to clean\"));\n \t}\n \n-\tif (force > 1)\n+\tif (force > 1) {\n \t\trm_flags = 0;\n-\telse\n+\t\tdir.flags |= DIR_NO_GITLINKS;\n+\t} else\n \t\tdir.flags |= DIR_SKIP_NESTED_GIT;\n \n \tdir.flags |= DIR_SHOW_OTHER_DIRECTORIES;\n-- \n2.35.1\n\n"},{"id":"448960","messageId":"20220221090034.4615-2-patrick.marlier@gmail.com","threadId":"57449","inReplyTo":"20220221090034.4615-1-patrick.marlier@gmail.com","subject":"[PATCH v2 2/2] clean: avoid traversing into untracked dirs when unnecessary","fromName":"Patrick Marlier","fromEmail":"patrick.marlier@gmail.com","sentAt":"2022-02-21T09:00:34Z","receivedAt":"2022-02-21T09:09:17Z","isPatch":true,"sender":{"key":"patrick.marlier@gmail.com","avatar":null},"body":"When deleting all untracked and ignored files and any nested\nrepositories (such as with `git clean -ffdx`), we do not need to recurse\ninto an untracked directory to see if any of the entries under it are\nignored or a nested repository.  Special case this condition to avoid\nunnecessary recursion.\n---\n builtin/clean.c  |  4 +++-\n t/t7300-clean.sh | 24 ++++++++++++++++++++++++\n 2 files changed, 27 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex 18b37e3fd9..1b1454d052 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -978,7 +978,9 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \t\tremove_directories = 1;\n \t}\n \n-\tif (remove_directories && !ignored_only) {\n+\tif (remove_directories && ignored && !exclude_list.nr && force > 1)\n+\t\t; /* No need to recurse to look for ignored files */\n+\telse if (remove_directories && !ignored_only) {\n \t\t/*\n \t\t * We need to know about ignored files too:\n \t\t *\ndiff --git a/t/t7300-clean.sh b/t/t7300-clean.sh\nindex 0399701e62..ceab7c4883 100755\n--- a/t/t7300-clean.sh\n+++ b/t/t7300-clean.sh\n@@ -788,4 +788,28 @@ test_expect_success 'traverse into directories that may have ignored entries' '\n \t)\n '\n \n+test_expect_success 'avoid traversing into untracked directories' '\n+\ttest_when_finished rm -f output error trace.* &&\n+\tgit init avoid-traversing-untracked-hierarchy &&\n+\t(\n+\t\tcd avoid-traversing-untracked-hierarchy &&\n+\n+\t\tmkdir -p untracked/subdir/with/b &&\n+\t\tmkdir -p untracked/subdir/with/a &&\n+\t\t>untracked/subdir/with/a/random-file.txt &&\n+\n+\t\tGIT_TRACE2_PERF=\"$TRASH_DIRECTORY/trace.output\" \\\n+\t\tgit clean -ffdx\n+\t) &&\n+\n+\t# Make sure we only visited into the top-level directory, and did\n+\t# not traverse into the \"untracked\" subdirectory since it was excluded\n+\tgrep data.*read_directo.*directories-visited trace.output |\n+\t\tcut -d \"|\" -f 9 >trace.relevant &&\n+\tcat >trace.expect <<-EOF &&\n+\t ..directories-visited:1\n+\tEOF\n+\ttest_cmp trace.expect trace.relevant\n+'\n+\n test_done\n-- \n2.35.1\n\n"},{"id":"449080","messageId":"CABPp-BGdkgaRC+wBobhzyge=di8uE3kzHiO8v26bi2v0kTXerw@mail.gmail.com","threadId":"57449","inReplyTo":"20220221090034.4615-2-patrick.marlier@gmail.com","subject":"Re: [PATCH v2 2/2] clean: avoid traversing into untracked dirs when unnecessary","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-02-22T01:48:37Z","receivedAt":"2022-02-22T01:48:55Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Mon, Feb 21, 2022 at 1:00 AM Patrick Marlier\n<patrick.marlier@gmail.com> wrote:\n>\n> When deleting all untracked and ignored files and any nested\n> repositories (such as with `git clean -ffdx`), we do not need to recurse\n> into an untracked directory to see if any of the entries under it are\n> ignored or a nested repository.  Special case this condition to avoid\n> unnecessary recursion.\n> ---\n>  builtin/clean.c  |  4 +++-\n>  t/t7300-clean.sh | 24 ++++++++++++++++++++++++\n>  2 files changed, 27 insertions(+), 1 deletion(-)\n>\n> diff --git a/builtin/clean.c b/builtin/clean.c\n> index 18b37e3fd9..1b1454d052 100644\n> --- a/builtin/clean.c\n> +++ b/builtin/clean.c\n> @@ -978,7 +978,9 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n>                 remove_directories = 1;\n>         }\n>\n> -       if (remove_directories && !ignored_only) {\n> +       if (remove_directories && ignored && !exclude_list.nr && force > 1)\n> +               ; /* No need to recurse to look for ignored files */\n> +       else if (remove_directories && !ignored_only) {\n>                 /*\n>                  * We need to know about ignored files too:\n>                  *\n> diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh\n> index 0399701e62..ceab7c4883 100755\n> --- a/t/t7300-clean.sh\n> +++ b/t/t7300-clean.sh\n> @@ -788,4 +788,28 @@ test_expect_success 'traverse into directories that may have ignored entries' '\n>         )\n>  '\n>\n> +test_expect_success 'avoid traversing into untracked directories' '\n> +       test_when_finished rm -f output error trace.* &&\n> +       git init avoid-traversing-untracked-hierarchy &&\n> +       (\n> +               cd avoid-traversing-untracked-hierarchy &&\n> +\n> +               mkdir -p untracked/subdir/with/b &&\n> +               mkdir -p untracked/subdir/with/a &&\n> +               >untracked/subdir/with/a/random-file.txt &&\n> +\n> +               GIT_TRACE2_PERF=\"$TRASH_DIRECTORY/trace.output\" \\\n> +               git clean -ffdx\n> +       ) &&\n> +\n> +       # Make sure we only visited into the top-level directory, and did\n> +       # not traverse into the \"untracked\" subdirectory since it was excluded\n> +       grep data.*read_directo.*directories-visited trace.output |\n> +               cut -d \"|\" -f 9 >trace.relevant &&\n> +       cat >trace.expect <<-EOF &&\n> +        ..directories-visited:1\n> +       EOF\n> +       test_cmp trace.expect trace.relevant\n> +'\n> +\n>  test_done\n> --\n> 2.35.1\n\nThanks, this round looks good to me.  Both patches are:\n\nReviewed-by: Elijah Newren <newren@gmail.com>\n"}]}