{"thread":{"id":"32280","subject":"[PATCH] git-clean: Display more accurate delete messages","startedAt":"2012-12-06T10:15:38Z","lastAt":"2012-12-11T12:32:31Z","messageCount":11,"participants":["Zoltan Klinger","Junio C Hamano","Soren Brinkmann"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"204556","messageId":"1354788938-26804-1-git-send-email-zoltan.klinger@gmail.com","threadId":"32280","inReplyTo":null,"subject":"[PATCH] git-clean: Display more accurate delete messages","fromName":"Zoltan Klinger","fromEmail":"zoltan.klinger@gmail.com","sentAt":"2012-12-06T10:15:38Z","receivedAt":"2012-12-06T10:15:38Z","isPatch":true,"sender":{"key":"zoltan.klinger@gmail.com","avatar":"https://avatars.githubusercontent.com/u/95923?v=4"},"body":"Only print out the names of the files and directories that got actually\ndeleted.\n\nConsider the following repo layout:\n |-- test.git/\n |-- foo/\n |-- bar/\n |-- bar.txt\n |-- frotz.git/\n |-- frotz.txt\n |-- tracked_file1\n |-- untracked_file1\n\nSuppose the user issues 'git clean -fd' from the test.git directory.\n\nWhen -d option is used and untracked directory 'foo' contains a\nsubdirectory 'frotz.git' that is managed by a different git repository\ntherefore it will not be removed.\n\n $ git clean -fd\n Removing foo/\n Removing untracked_file1\n\nThe message displayed to the user is slightly misleading. The foo/\ndirectory has not been removed because of foo/frotz.git still exists.\nOn the other hand the subdirectory bar has been deleted but it's not\nmentioned anywhere.\n\nThis behaviour is the result of the way the deletion of untracked\ndirectories are reported. In the current implementation they are\ndeleted recursively but only the name of the top most directory is\nprinted out. The calling function does not know about any\nsubdirectories that could not be removed during the recursion.\n\nImprove the way the deleted directories are reported back to\nthe user:\n (1) Modify the recursive delete function to run in both dry_run and\n delete modes.\n (2) During the recursion collect the name of the files and\n directories that will be or have been removed. Also collect the\n names of files and directories that could not be removed.\n (3) After finishing the deletes print out the names of all deleted\n files and any files or directories that failed to delete.\n\nConsider the output of the improved version:\n\n $ git clean -fd\n Removed foo/bar/bar.txt\n Removed foo/bar\n Removed untracked_file1\n\nNow it displays only the file and directory names that got actually\ndeleted.\n\nSigned-off-by: Zoltan Klinger \n---\nHi there,\n\nMy first patch. Hope you find it useful.\n\nLooking forward to your feedback.\n\nCheers,\nZoltan\n\n builtin/clean.c | 64 +++++++++++++++++++++++++++++++------------------------\n dir.c | 58 ++++++++++++++++++++++++++++++++++++++++---------\n dir.h | 3 +++\n 3 files changed, 87 insertions(+), 38 deletions(-)\n\ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex 69c1cda..9b056b9 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -34,22 +34,31 @@ static int exclude_cb(const struct option *opt, const char *arg, int unset)\n \treturn 0;\n }\n \n+static void print_result(const char *msg, struct string_list *lst)\n+{\n+ int i;\n+ for (i = 0; i < lst->nr; i++)\n+\t\tprintf(\"%s %s\\n\", msg, lst->items[i].string);\n+}\n+\n int cmd_clean(int argc, const char **argv, const char *prefix)\n {\n \tint i;\n-\tint show_only = 0, remove_directories = 0, quiet = 0, ignored = 0;\n+\tint dry_run = 0, remove_directories = 0, quiet = 0, ignored = 0;\n \tint ignored_only = 0, config_set = 0, errors = 0;\n \tint rm_flags = REMOVE_DIR_KEEP_NESTED_GIT;\n \tstruct strbuf directory = STRBUF_INIT;\n \tstruct dir_struct dir;\n \tstatic const char **pathspec;\n \tstruct strbuf buf = STRBUF_INIT;\n+\tstruct string_list dels = STRING_LIST_INIT_DUP;\n+\tstruct string_list errs = STRING_LIST_INIT_DUP;\n \tstruct string_list exclude_list = STRING_LIST_INIT_NODUP;\n \tconst char *qname;\n \tchar *seen = NULL;\n \tstruct option options[] = {\n \t\tOPT__QUIET(&quiet, N_(\"do not print names of files removed\")),\n-\t\tOPT__DRY_RUN(&show_only, N_(\"dry run\")),\n+\t\tOPT__DRY_RUN(&dry_run, N_(\"dry run\")),\n \t\tOPT__FORCE(&force, N_(\"force\")),\n \t\tOPT_BOOLEAN('d', NULL, &remove_directories,\n \t\t\t\tN_(\"remove whole directories\")),\n@@ -77,7 +86,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \tif (ignored && ignored_only)\n \t\tdie(_(\"-x and -X cannot be used together\"));\n \n-\tif (!show_only && !force) {\n+\tif (!dry_run && !force) {\n \t\tif (config_set)\n \t\t\tdie(_(\"clean.requireForce set to true and neither -n nor -f given; \"\n \t\t\t\t \"refusing to clean\"));\n@@ -150,43 +159,42 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \t\tif (S_ISDIR(st.st_mode)) {\n \t\t\tstrbuf_addstr(&directory, ent->name);\n \t\t\tqname = quote_path_relative(directory.buf, directory.len, &buf, prefix);\n-\t\t\tif (show_only && (remove_directories ||\n-\t\t\t (matches == MATCHED_EXACTLY))) {\n-\t\t\t\tprintf(_(\"Would remove %s\\n\"), qname);\n-\t\t\t} else if (remove_directories ||\n-\t\t\t\t (matches == MATCHED_EXACTLY)) {\n-\t\t\t\tif (!quiet)\n-\t\t\t\t\tprintf(_(\"Removing %s\\n\"), qname);\n-\t\t\t\tif (remove_dir_recursively(&directory,\n-\t\t\t\t\t\t\t rm_flags) != 0) {\n-\t\t\t\t\twarning(_(\"failed to remove %s\"), qname);\n-\t\t\t\t\terrors++;\n-\t\t\t\t}\n-\t\t\t} else if (show_only) {\n-\t\t\t\tprintf(_(\"Would not remove %s\\n\"), qname);\n-\t\t\t} else {\n-\t\t\t\tprintf(_(\"Not removing %s\\n\"), qname);\n+\t\t\tif (remove_directories || (matches == MATCHED_EXACTLY)) {\n+\t\t\t\tremove_dir_recursively_with_dryrun(&directory, rm_flags, dry_run,\n+\t\t\t\t\t\t&dels, &errs, prefix);\n \t\t\t}\n \t\t\tstrbuf_reset(&directory);\n \t\t} else {\n \t\t\tif (pathspec && !matches)\n \t\t\t\tcontinue;\n \t\t\tqname = quote_path_relative(ent->name, -1, &buf, prefix);\n-\t\t\tif (show_only) {\n-\t\t\t\tprintf(_(\"Would remove %s\\n\"), qname);\n-\t\t\t\tcontinue;\n-\t\t\t} else if (!quiet) {\n-\t\t\t\tprintf(_(\"Removing %s\\n\"), qname);\n-\t\t\t}\n-\t\t\tif (unlink(ent->name) != 0) {\n-\t\t\t\twarning(_(\"failed to remove %s\"), qname);\n-\t\t\t\terrors++;\n+\t\t\tif (dry_run)\n+\t\t\t\tstring_list_append(&dels, qname);\n+\t\t\telse {\n+\t\t\t\tif (unlink(ent->name) != 0)\n+\t\t\t\t\tstring_list_append(&errs, qname);\n+\t\t\t\telse\n+\t\t\t\t\tstring_list_append(&dels, qname);\n \t\t\t}\n \t\t}\n \t}\n+\n+\tif (!quiet) {\n+\t\tif (dry_run)\n+\t\t\tprint_result(\"Would remove\", &dels);\n+\t\telse\n+\t\t\tprint_result(\"Removed\", &dels);\n+\t}\n+\n+\terrors = errs.nr;\n+\tif (errors)\n+\t\tprint_result(\"Failed to remove\", &errs);\n+\n \tfree(seen);\n \n \tstrbuf_release(&directory);\n+\tstring_list_clear(&dels, 0);\n+\tstring_list_clear(&errs, 0);\n \tstring_list_clear(&exclude_list, 0);\n \treturn (errors != 0);\n }\ndiff --git a/dir.c b/dir.c\nindex 5a83aa7..f580c51 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -7,7 +7,9 @@\n */\n #include \"cache.h\"\n #include \"dir.h\"\n+#include \"quote.h\"\n #include \"refs.h\"\n+#include \"string-list.h\"\n \n struct path_simplify {\n \tint len;\n@@ -1294,11 +1296,25 @@ int is_empty_dir(const char *path)\n \treturn ret;\n }\n \n-static int remove_dir_recurse(struct strbuf *path, int flag, int *kept_up)\n+static void append_dir_name(struct string_list *dels, struct string_list *errs,\n+ char *name, const char * prefix, int failed)\n+{\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tconst char *qname;\n+\tqname = quote_path_relative(name, strlen(name), &buf, prefix);\n+\tif (!failed && dels)\n+\t\tstring_list_append(dels, qname);\n+\telse if (errs)\n+\t\tstring_list_append(errs, qname);\n+}\n+\n+static int remove_dir_recurse(struct strbuf *path, int flag, int *kept_up,\n+\tint dry_run, struct string_list *dels, struct string_list *errs,\n+ const char *prefix)\n {\n \tDIR *dir;\n \tstruct dirent *e;\n-\tint ret = 0, original_len = path->len, len, kept_down = 0;\n+\tint ret = 0, original_len = path->len, len, kept_down = 0, res = 0;\n \tint only_empty = (flag & REMOVE_DIR_EMPTY_ONLY);\n \tint keep_toplevel = (flag & REMOVE_DIR_KEEP_TOPLEVEL);\n \tunsigned char submodule_head[20];\n@@ -1315,8 +1331,13 @@ static int remove_dir_recurse(struct strbuf *path, int flag, int *kept_up)\n \tdir = opendir(path->buf);\n \tif (!dir) {\n \t\t/* an empty dir could be removed even if it is unreadble */\n-\t\tif (!keep_toplevel)\n-\t\t\treturn rmdir(path->buf);\n+\t\tif (!keep_toplevel) {\n+\t\t\tres = 0;\n+\t\t\tif (!dry_run)\n+\t\t\t\tres = rmdir(path->buf);\n+\t\t\tappend_dir_name(dels, errs, path->buf, prefix, res);\n+\t\t\treturn res;\n+\t\t}\n \t\telse\n \t\t\treturn -1;\n \t}\n@@ -1334,10 +1355,16 @@ static int remove_dir_recurse(struct strbuf *path, int flag, int *kept_up)\n \t\tif (lstat(path->buf, &st))\n \t\t\t; /* fall thru */\n \t\telse if (S_ISDIR(st.st_mode)) {\n-\t\t\tif (!remove_dir_recurse(path, flag, &kept_down))\n+\t\t\tif (!remove_dir_recurse(path, flag, &kept_down, dry_run, dels, errs, prefix))\n \t\t\t\tcontinue; /* happy */\n-\t\t} else if (!only_empty && !unlink(path->buf))\n-\t\t\tcontinue; /* happy, too */\n+\t\t} else if (!only_empty) {\n+\t\t\tres = 0;\n+\t\t\tif (!dry_run)\n+\t\t\t\tres = unlink(path->buf);\n+\t\t\tappend_dir_name(dels, errs, path->buf, prefix, res);\n+\t\t\tif (!res)\n+\t\t\t\tcontinue; /* happy, too */\n+\t\t}\n \n \t\t/* path too long, stat fails, or non-directory still exists */\n \t\tret = -1;\n@@ -1346,8 +1373,12 @@ static int remove_dir_recurse(struct strbuf *path, int flag, int *kept_up)\n \tclosedir(dir);\n \n \tstrbuf_setlen(path, original_len);\n-\tif (!ret && !keep_toplevel && !kept_down)\n-\t\tret = rmdir(path->buf);\n+\tif (!ret && !keep_toplevel && !kept_down) {\n+\t\tret = 0;\n+\t\tif (!dry_run)\n+\t\t\tret = rmdir(path->buf);\n+\t\tappend_dir_name(dels, errs, path->buf, prefix, res);\n+\t}\n \telse if (kept_up)\n \t\t/*\n \t\t * report the uplevel that it is not an error that we\n@@ -1359,7 +1390,14 @@ static int remove_dir_recurse(struct strbuf *path, int flag, int *kept_up)\n \n int remove_dir_recursively(struct strbuf *path, int flag)\n {\n-\treturn remove_dir_recurse(path, flag, NULL);\n+\treturn remove_dir_recurse(path, flag, NULL, 0, NULL, NULL, NULL);\n+}\n+\n+int remove_dir_recursively_with_dryrun(struct strbuf *path, int flag,\n+\t\tint dry_run, struct string_list *dels, struct string_list *errs,\n+\t\tconst char *prefix)\n+{\n+\treturn remove_dir_recurse(path, flag, NULL, dry_run, dels, errs, prefix);\n }\n \n void setup_standard_excludes(struct dir_struct *dir)\ndiff --git a/dir.h b/dir.h\nindex f5c89e3..828bd49 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -131,6 +131,9 @@ extern void setup_standard_excludes(struct dir_struct *dir);\n #define REMOVE_DIR_KEEP_NESTED_GIT 02\n #define REMOVE_DIR_KEEP_TOPLEVEL 04\n extern int remove_dir_recursively(struct strbuf *path, int flag);\n+extern int remove_dir_recursively_with_dryrun(struct strbuf *path,\n+\t\t\tint flag, int dryrun, struct string_list *dels,\n+\t\t\tstruct string_list *errs, const char *prefix);\n \n /* tries to remove the path with empty directories along it, ignores ENOENT */\n extern int remove_path(const char *path);\n-- \n1.7.9.5\n"},{"id":"204562","messageId":"7v8v9bjd44.fsf@alter.siamese.dyndns.org","threadId":"32280","inReplyTo":"1354788938-26804-1-git-send-email-zoltan.klinger@gmail.com","subject":"Re: [PATCH] git-clean: Display more accurate delete messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-06T17:37:31Z","receivedAt":"2012-12-06T17:37:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Zoltan Klinger writes:\n\n> Only print out the names of the files and directories that got actually\n> deleted.\n>\n> Consider the following repo layout:\n> |-- test.git/\n> |-- foo/\n> |-- bar/\n> |-- bar.txt\n> |-- frotz.git/\n> |-- frotz.txt\n> |-- tracked_file1\n> |-- untracked_file1\n> ...\n> Consider the output of the improved version:\n>\n> $ git clean -fd\n> Removed foo/bar/bar.txt\n> Removed foo/bar\n> Removed untracked_file1\n\nHrm, following your discussion (ellided above), I would have\nexpected that you would show\n\n Removing directory foo/bar\n Removing untracked_file1\n"},{"id":"204567","messageId":"7d290bdc-8654-4526-ba73-89408fa99a16@DB3EHSMHS002.ehs.local","threadId":"32280","inReplyTo":"7v8v9bjd44.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git-clean: Display more accurate delete messages","fromName":"Soren Brinkmann","fromEmail":"soren.brinkmann@xilinx.com","sentAt":"2012-12-07T00:15:59Z","receivedAt":"2012-12-07T00:15:59Z","isPatch":true,"sender":{"key":"soren.brinkmann@xilinx.com","avatar":null},"body":"Hi,\n\nOn Thu, Dec 06, 2012 at 09:37:31AM -0800, Junio C Hamano wrote:\n> Zoltan Klinger writes:\n> \n> > Only print out the names of the files and directories that got actually\n> > deleted.\n> >\n> > Consider the following repo layout:\n> > |-- test.git/\n> > |-- foo/\n> > |-- bar/\n> > |-- bar.txt\n> > |-- frotz.git/\n> > |-- frotz.txt\n> > |-- tracked_file1\n> > |-- untracked_file1\n> > ...\n> > Consider the output of the improved version:\n> >\n> > $ git clean -fd\n> > Removed foo/bar/bar.txt\n> > Removed foo/bar\n> > Removed untracked_file1\n> \n> Hrm, following your discussion (ellided above), I would have\n> expected that you would show\n> \n> Removing directory foo/bar\n> Removing untracked_file1\n\nAlso it would be nice to have warnings about undeleted directories since this git\nclean behavior (or the work around to pass -f twice) is not documented.\nWithout a warning you would probably miss that something was _not_ deleted.\n\nSo something like:\n\tRemoving foo\n\tRemoving bar\n\t...\n\n\tWarning: Not all untracked objects have been deleted:\n\t (optional)\n\tUse git clean --force --force to delete all objects.\n\n\nBut clearly going into a good direction. Thanks.\n\n\tSoren\n"},{"id":"204627","messageId":"CAKJhZwROXsTa4wu-C9rhfGysetL+cZRDECyFUn5VTb833pWzMQ@mail.gmail.com","threadId":"32280","inReplyTo":"7d290bdc-8654-4526-ba73-89408fa99a16@DB3EHSMHS002.ehs.local","subject":"Re: [PATCH] git-clean: Display more accurate delete messages","fromName":"Zoltan Klinger","fromEmail":"zoltan.klinger@gmail.com","sentAt":"2012-12-09T11:18:19Z","receivedAt":"2012-12-09T11:18:19Z","isPatch":true,"sender":{"key":"zoltan.klinger@gmail.com","avatar":"https://avatars.githubusercontent.com/u/95923?v=4"},"body":">> Hrm, following your discussion (ellided above), I would have\n>> expected that you would show\n>>\n>> Removing directory foo/bar\n>> Removing untracked_file1\n>\n> Also it would be nice to have warnings about undeleted directories since this git\n> clean behavior (or the work around to pass -f twice) is not documented.\n> Without a warning you would probably miss that something was _not_ deleted.\n\nThanks for the feedback. I think you're right. Showing 'foo/bar/bar.txt' in\nthe list when 'foo/bar/' directory has been successfully deleted is just noise.\n\nWould like to get some more feedback on the proposed output in case of\n (1) an untracked subdirectory with multiple files where at least one of them\n cannot be removed.\n (2) reporting ignored untracked git subdirectories\n\nSuppose we have a repo like the one below:\n test.git/\n |-- tracked_file\n |-- untracked_file\n |-- untracked_foo/\n | |-- bar/\n | | |-- bar.txt\n | |-- emptydir/\n | |-- frotz.git/\n | | |-- frotx.txt\n | |-- quux/\n | |-- failedquux.txt\n | |-- quux.txt\n |-- untracked_unreadable_dir/\n | |-- afile\n |-- untracked_some.git/\n |-- some.txt\n\n$ git clean -fd\nRemoving untracked_file\nRemoving untracked_foo/bar\nRemoving untracked_foo/emptydir\nRemoving untracked_foo/quux/quux.txt\nwarning: failed to remove untracked_foo/quux/failedquux.txt\nwarning: failed to remove remove untracked_unreadable_dir/\nwarning: ignoring untracked git repository untracked_foo/frotz.git/\nwarning: ignoring untracked git repository untracked_some.git/\nUse git clean --force --force to delete all untracked git repositories\n\n$ # use forced remove\n$ git clean --force --force -d\nRemoving untracked_foo/frotz.git\nRemoving untracked_foo/quux/quux.txt\nRemoving untracked_some.git/\nwarning: failed to remove untracked_foo/quux/failedquux.txt\nwarning: failed to remove untracked_unreadable_dir/\n\nCan you see any issues with the proposed output, wording above? If\neveryone is happy,\nI'm going to prepare patch V2 for it.\n\nThanks,\nZoltan\n"},{"id":"204643","messageId":"7v38zecrqc.fsf@alter.siamese.dyndns.org","threadId":"32280","inReplyTo":"CAKJhZwROXsTa4wu-C9rhfGysetL+cZRDECyFUn5VTb833pWzMQ@mail.gmail.com","subject":"Re: [PATCH] git-clean: Display more accurate delete messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-10T07:04:59Z","receivedAt":"2012-12-10T07:04:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Zoltan Klinger writes:\n\n> Would like to get some more feedback on the proposed output in case of\n> (1) an untracked subdirectory with multiple files where at least one of them\n> cannot be removed.\n> (2) reporting ignored untracked git subdirectories\n>\n> Suppose we have a repo like the one below:\n> test.git/\n> |-- tracked_file\n> |-- untracked_file\n> |-- untracked_foo/\n> | |-- bar/\n> | | |-- bar.txt\n> | |-- emptydir/\n> | |-- frotz.git/\n> | | |-- frotx.txt\n> | |-- quux/\n> | |-- failedquux.txt\n> | |-- quux.txt\n> |-- untracked_unreadable_dir/\n> | |-- afile\n> |-- untracked_some.git/\n> |-- some.txt\n>\n> $ git clean -fd\n> Removing untracked_file\n> Removing untracked_foo/bar\n> Removing untracked_foo/emptydir\n> Removing untracked_foo/quux/quux.txt\n> warning: failed to remove untracked_foo/quux/failedquux.txt\n> warning: failed to remove remove untracked_unreadable_dir/\n\n\"remove remove\" is a typo, I presume.\n\n> warning: ignoring untracked git repository untracked_foo/frotz.git/\n> warning: ignoring untracked git repository untracked_some.git/\n\nIf you mean \"we report the topmost directory and nothing about\n(recursive) contents in it if everything is removed successfully\"\n(in other words, if we had subdirectories and files inside\nuntracked_foo/bar/ and we successfully removed all of them, the\nabove output does not change), it seems quite reasonable.\n\n> Use git clean --force --force to delete all untracked git repositories\n\nBut I am not sure if this is ever sane. Especially the one that\nremoves an embedded repository is suspicious. \"git clean\" should\nnot ever touch it with or without --superforce or any other command.\n\nI do not think trying to remove something that cannot be removed due\nto filesystem permissions is sensible, either. We simply should treat\nsuch a case a grave error and have the user sort things out, instead\nof blindly attempt to \"chmod\" them ourselves (which may still fail).\n\nThanks.\n"},{"id":"204651","messageId":"5e54ef3e-b872-4aa5-9b10-ca05323e73b5@CH1EHSMHS042.ehs.local","threadId":"32280","inReplyTo":"CAKJhZwROXsTa4wu-C9rhfGysetL+cZRDECyFUn5VTb833pWzMQ@mail.gmail.com","subject":"Re: [PATCH] git-clean: Display more accurate delete messages","fromName":"Soren Brinkmann","fromEmail":"soren.brinkmann@xilinx.com","sentAt":"2012-12-10T17:04:33Z","receivedAt":"2012-12-10T17:04:33Z","isPatch":true,"sender":{"key":"soren.brinkmann@xilinx.com","avatar":null},"body":"Hi Zoltan,\n\nOn Sun, Dec 09, 2012 at 10:18:19PM +1100, Zoltan Klinger wrote:\n> >> Hrm, following your discussion (ellided above), I would have\n> >> expected that you would show\n> >>\n> >> Removing directory foo/bar\n> >> Removing untracked_file1\n> >\n> > Also it would be nice to have warnings about undeleted directories since this git\n> > clean behavior (or the work around to pass -f twice) is not documented.\n> > Without a warning you would probably miss that something was _not_ deleted.\n> \n> Thanks for the feedback. I think you're right. Showing 'foo/bar/bar.txt' in\n> the list when 'foo/bar/' directory has been successfully deleted is just noise.\n> \n> Would like to get some more feedback on the proposed output in case of\n> (1) an untracked subdirectory with multiple files where at least one of them\n> cannot be removed.\n> (2) reporting ignored untracked git subdirectories\n> \n> Suppose we have a repo like the one below:\n> test.git/\n> |-- tracked_file\n> |-- untracked_file\n> |-- untracked_foo/\n> | |-- bar/\n> | | |-- bar.txt\n> | |-- emptydir/\n> | |-- frotz.git/\n> | | |-- frotx.txt\n> | |-- quux/\n> | |-- failedquux.txt\n> | |-- quux.txt\n> |-- untracked_unreadable_dir/\n> | |-- afile\n> |-- untracked_some.git/\n> |-- some.txt\n> \n> $ git clean -fd\n> Removing untracked_file\n> Removing untracked_foo/bar\n> Removing untracked_foo/emptydir\n> Removing untracked_foo/quux/quux.txt\n> warning: failed to remove untracked_foo/quux/failedquux.txt\n> warning: failed to remove remove untracked_unreadable_dir/\n> warning: ignoring untracked git repository untracked_foo/frotz.git/\n> warning: ignoring untracked git repository untracked_some.git/\n> Use git clean --force --force to delete all untracked git repositories\n> \n> $ # use forced remove\n> $ git clean --force --force -d\n> Removing untracked_foo/frotz.git\n> Removing untracked_foo/quux/quux.txt\n> Removing untracked_some.git/\n> warning: failed to remove untracked_foo/quux/failedquux.txt\n> warning: failed to remove untracked_unreadable_dir/\n> \n> Can you see any issues with the proposed output, wording above? If\n> everyone is happy,\n> I'm going to prepare patch V2 for it.\nLooks good to me.\n\nThanks,\nSoren\n"},{"id":"204655","messageId":"5b69a9f1-0860-41da-914c-d55a17e54092@TX2EHSMHS026.ehs.local","threadId":"32280","inReplyTo":"7v38zecrqc.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git-clean: Display more accurate delete messages","fromName":"Soren Brinkmann","fromEmail":"soren.brinkmann@xilinx.com","sentAt":"2012-12-10T17:33:56Z","receivedAt":"2012-12-10T17:33:56Z","isPatch":true,"sender":{"key":"soren.brinkmann@xilinx.com","avatar":null},"body":"Hi, \n\nOn Sun, Dec 09, 2012 at 11:04:59PM -0800, Junio C Hamano wrote:\n> Zoltan Klinger writes:\n> \n> > Would like to get some more feedback on the proposed output in case of\n> > (1) an untracked subdirectory with multiple files where at least one of them\n> > cannot be removed.\n> > (2) reporting ignored untracked git subdirectories\n> >\n> > Suppose we have a repo like the one below:\n> > test.git/\n> > |-- tracked_file\n> > |-- untracked_file\n> > |-- untracked_foo/\n> > | |-- bar/\n> > | | |-- bar.txt\n> > | |-- emptydir/\n> > | |-- frotz.git/\n> > | | |-- frotx.txt\n> > | |-- quux/\n> > | |-- failedquux.txt\n> > | |-- quux.txt\n> > |-- untracked_unreadable_dir/\n> > | |-- afile\n> > |-- untracked_some.git/\n> > |-- some.txt\n> >\n> > $ git clean -fd\n> > Removing untracked_file\n> > Removing untracked_foo/bar\n> > Removing untracked_foo/emptydir\n> > Removing untracked_foo/quux/quux.txt\n> > warning: failed to remove untracked_foo/quux/failedquux.txt\n> > warning: failed to remove remove untracked_unreadable_dir/\n> \n> \"remove remove\" is a typo, I presume.\n> \n> > warning: ignoring untracked git repository untracked_foo/frotz.git/\n> > warning: ignoring untracked git repository untracked_some.git/\n> \n> If you mean \"we report the topmost directory and nothing about\n> (recursive) contents in it if everything is removed successfully\"\n> (in other words, if we had subdirectories and files inside\n> untracked_foo/bar/ and we successfully removed all of them, the\n> above output does not change), it seems quite reasonable.\n> \n> > Use git clean --force --force to delete all untracked git repositories\n> \n> But I am not sure if this is ever sane. Especially the one that\n> removes an embedded repository is suspicious. \"git clean\" should\n> not ever touch it with or without --superforce or any other command.\nAs I mentioned in my email where I reported this incorrect git clean output, I\nhave a use case where I want git clean to remove embedded repositories.\nWhether it is a sane one is probably a different discussion.\n\n\tSoren\n"},{"id":"204656","messageId":"7va9tlbx8v.fsf@alter.siamese.dyndns.org","threadId":"32280","inReplyTo":"5b69a9f1-0860-41da-914c-d55a17e54092@TX2EHSMHS026.ehs.local","subject":"Re: [PATCH] git-clean: Display more accurate delete messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-10T18:03:28Z","receivedAt":"2012-12-10T18:03:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Soren Brinkmann writes:\n\n>> > Use git clean --force --force to delete all untracked git repositories\n>> \n>> But I am not sure if this is ever sane. Especially the one that\n>> removes an embedded repository is suspicious. \"git clean\" should\n>> not ever touch it with or without --superforce or any other command.\n> As I mentioned in my email where I reported this incorrect git clean output, I\n> have a use case where I want git clean to remove embedded repositories.\n> Whether it is a sane one is probably a different discussion.\n\nWhy is it a different discussion? If something is not sane, the\ntool shouldn't encourage users to do such an insane thing.\n"},{"id":"204657","messageId":"aecaf65e-2b7f-4309-a7b5-622c7779de17@DB3EHSMHS018.ehs.local","threadId":"32280","inReplyTo":"7va9tlbx8v.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git-clean: Display more accurate delete messages","fromName":"Soren Brinkmann","fromEmail":"soren.brinkmann@xilinx.com","sentAt":"2012-12-10T18:16:23Z","receivedAt":"2012-12-10T18:16:23Z","isPatch":true,"sender":{"key":"soren.brinkmann@xilinx.com","avatar":null},"body":"On Mon, Dec 10, 2012 at 10:03:28AM -0800, Junio C Hamano wrote:\n> Soren Brinkmann writes:\n> \n> >> > Use git clean --force --force to delete all untracked git repositories\n> >> \n> >> But I am not sure if this is ever sane. Especially the one that\n> >> removes an embedded repository is suspicious. \"git clean\" should\n> >> not ever touch it with or without --superforce or any other command.\n> > As I mentioned in my email where I reported this incorrect git clean output, I\n> > have a use case where I want git clean to remove embedded repositories.\n> > Whether it is a sane one is probably a different discussion.\n> \n> Why is it a different discussion? If something is not sane, the\n> tool shouldn't encourage users to do such an insane thing.\n> \nWell, ok. So I have a repository which essentially consists of a bunch of\nscripts which then pull sources via git to build root filesystems, busybox,\nkernel etc.\nSo I have the master repository I'm actually interested in. And then all the\nother projects which are pulled in to build stuff from.\nlooking somehow like this:\n\ttop.git\n\t |-src\n\t | |-proj1.git\n\t | |-proj2.git\n\t | |-projn.git\n\t |-build\n\t |-proj1\n\t |-proj2\n\t ...\n\nSince the scripts are not perfect I usually used 'git clean -xdf' to wipe\neverything and build from scratch. And I had to experience that the git clean\nbehavior somehow changed recently and the 'projn.git' directories were no longer\nremoved anymore, despite git indicating otherwise in its output.\n\nSo, I think having 'git clean -ff' removing embedded git repos is okay. But either\nway, the output of git clean should match what it is doing. And at least tell me\nif it didn't remove certain dirs or files.\n\n\tSoren\n"},{"id":"204660","messageId":"7vvcc9agdt.fsf@alter.siamese.dyndns.org","threadId":"32280","inReplyTo":"aecaf65e-2b7f-4309-a7b5-622c7779de17@DB3EHSMHS018.ehs.local","subject":"Re: [PATCH] git-clean: Display more accurate delete messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-10T18:53:02Z","receivedAt":"2012-12-10T18:53:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Soren Brinkmann writes:\n\n> But either\n> way, the output of git clean should match what it is doing. And at least tell me\n> if it didn't remove certain dirs or files.\n\nOh, no question about that part. I was reacting to --force --force\nin general, and an unrelated git repository inside a working tree is\njust a subset of the issue.\n"},{"id":"204676","messageId":"CAKJhZwTEykP_w7-02YNUmi8D=X7nD8wSyFBD0_uGE6E760zkNQ@mail.gmail.com","threadId":"32280","inReplyTo":"7v38zecrqc.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git-clean: Display more accurate delete messages","fromName":"Zoltan Klinger","fromEmail":"zoltan.klinger@gmail.com","sentAt":"2012-12-11T12:32:31Z","receivedAt":"2012-12-11T12:32:31Z","isPatch":true,"sender":{"key":"zoltan.klinger@gmail.com","avatar":"https://avatars.githubusercontent.com/u/95923?v=4"},"body":">> Use git clean --force --force to delete all untracked git repositories\n>\n> But I am not sure if this is ever sane. Especially the one that\n> removes an embedded repository is suspicious. \"git clean\" should\n> not ever touch it with or without --superforce or any other command.\n\nMy original intention with this patch was to provide more accurate\ndelete messages for the git-clean command when it's used with the\ncurrent set of command line options. I didn't know that --force\n--force was so controversial.\n\nThe --force --force option has been around since v1.6.4.2. Commit\na0f4afbe introduced it. If the consensus is that it is not a sane\noption to have let's remove it by all means. But I think it should be\ndone in a separate patch '[PATCH] git-clean: Never delete any embedded\ngit repository' or such.\n\n> I do not think trying to remove something that cannot be removed due\n> to filesystem permissions is sensible, either. We simply should treat\n> such a case a grave error and have the user sort things out, instead\n> of blindly attempt to \"chmod\" them ourselves (which may still fail).\n\nBut this is not how git-clean works with or without the --force\n--force flag. The recursive delete does the right thing: it tries to\ndelete a file or directory, if that fails for whatever reason it will\nreport the error and move on. That's it. No \"chmod\" or any other\nhackery at all. The --force --force flag only means \"if during\nrecursion you encounter an embedded git directory that is not tracked\nyou are allowed to recurse into it and keep on deleting files and\nsub-directories as per usual\".\n\nCheers,\nZoltan\n"}]}