{"thread":{"id":"46561","subject":"[PATCH v1 0/1] Teach status to show ignored directories","startedAt":"2017-08-10T18:50:22Z","lastAt":"2017-09-19T17:52:48Z","messageCount":11,"participants":["Jameson Miller","Stefan Beller","Brandon Williams","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"326065","messageId":"20170810184936.239542-1-jamill@microsoft.com","threadId":"46561","inReplyTo":null,"subject":"[PATCH v1 0/1] Teach status to show ignored directories","fromName":"Jameson Miller","fromEmail":"jameson.miller81@gmail.com","sentAt":"2017-08-10T18:49:35Z","receivedAt":"2017-08-10T18:50:22Z","isPatch":true,"sender":{"key":"jameson.miller81@gmail.com","avatar":null},"body":"Our application (Visual Studio) needs to run git status with options\nto report untracked and ignored files. It needs all untracked files\nreported individually, but would rather not have all individual\nignored files under explicitly ignored directories\nreported. Directories that match an ignore pattern should be reported,\ninstead of all contained files.\n\nIt is not possible to get this output with the current arguments\navailable to git status. You can get ignored files (--ignored), all\nuntracked files (--untracked-files=all), but if you specify both\noptions then you will also get all individual ignored files.\n\nThis change is to add the option to have git status report all\nuntracked files while also reporting directories that match an ignore\npattern. The logic in dir.c was also modified to not iterate over all\nfiles in an ignored directory. This change resulted in a performance\nimprovement in work directories with a large number of files contained\nin ignored directories.  For example, this setup is present when bin\nand obj directories are contained in the repository work dir.\n\nJameson Miller (1):\n  dir: teach status to show ignored directories\n\n Documentation/git-status.txt                      |   5 +\n Documentation/technical/api-directory-listing.txt |   6 +\n builtin/commit.c                                  |   4 +\n dir.c                                             |  48 +++++-\n dir.h                                             |   3 +-\n t/t7519-status-show-ignored-directory.sh          | 189 ++++++++++++++++++++++\n wt-status.c                                       |   4 +\n wt-status.h                                       |   1 +\n 8 files changed, 253 insertions(+), 7 deletions(-)\n create mode 100755 t/t7519-status-show-ignored-directory.sh\n\n-- \n2.11.0\n\n"},{"id":"326066","messageId":"20170810184936.239542-2-jamill@microsoft.com","threadId":"46561","inReplyTo":"20170810184936.239542-1-jamill@microsoft.com","subject":"[PATCH v1 1/1] dir: teach status to show ignored directories","fromName":"Jameson Miller","fromEmail":"jameson.miller81@gmail.com","sentAt":"2017-08-10T18:49:36Z","receivedAt":"2017-08-10T18:50:25Z","isPatch":true,"sender":{"key":"jameson.miller81@gmail.com","avatar":null},"body":"Teach Git to optionally show ignored directories when showing all\nuntracked files. The git status command exposes the options to report\nignored and/or untracked files. However, when reporting all untracked\nfiles (--untracked-files=all), all individual ignored files are reported\nas well. It is not currently possible to get the reporting behavior of\nthe --ignored flag, while also reporting all untracked files. This\nchange exposes a flag to report all untracked files while not showing\nindividual files in ignored directories.\n\nMotivation:\nOur application (Visual Studio) needs all untracked files listed\nindividually, but does not need all ignored files listed individually.\nReporting all ignored files can affect the time it takes for status\nto run. For a representative repository, here are some measurements\nshowing a large perf improvement for this scenario:\n\n| Command | Reported ignored entries | Time (s) |\n| ------- | ------------------------ | -------- |\n| 1       | 0                        | 1.3      |\n| 2       | 1024                     | 4.2      |\n| 3       | 174904                   | 7.5      |\n| 4       | 1046                     | 1.6      |\n\nCommands:\n 1) status\n 2) status --ignored\n 3) status --ignored --untracked-files=all\n 4) status --ignored --untracked-files=all --show-ignored-directory\n\nThis changes exposes a --show-ignored-directory flag to the git status\ncommand. This flag is utilized when running git status with the\n--ignored and --untracked-files options to not list ignored individual\nignored files contained in directories that match an ignore pattern.\n\nPart of the perf improvement comes from the tweak to\nread_directory_recursive to stop scanning the file system after it\nencounters the first file. When a directory is ignored, all it needs to\ndetermine is if the directory is empty or not. The logic currently keeps\nscanning the file system until it finds an untracked file. However, as\nthe directory is ignored, all the contained contents are also marked\nexcluded. For ignored directories that contain a large number of files,\nthis can take some time.\n\nSigned-off-by: Jameson Miller <jamill@microsoft.com>\n---\n Documentation/git-status.txt                      |   5 +\n Documentation/technical/api-directory-listing.txt |   6 +\n builtin/commit.c                                  |   4 +\n dir.c                                             |  48 +++++-\n dir.h                                             |   3 +-\n t/t7519-status-show-ignored-directory.sh          | 189 ++++++++++++++++++++++\n wt-status.c                                       |   4 +\n wt-status.h                                       |   1 +\n 8 files changed, 253 insertions(+), 7 deletions(-)\n create mode 100755 t/t7519-status-show-ignored-directory.sh\n\ndiff --git a/Documentation/git-status.txt b/Documentation/git-status.txt\nindex d47f198f15..48ebe18571 100644\n--- a/Documentation/git-status.txt\n+++ b/Documentation/git-status.txt\n@@ -100,6 +100,11 @@ configuration variable documented in linkgit:git-config[1].\n --ignored::\n \tShow ignored files as well.\n \n+--show-ignored-directory::\n+    Show directories that are ignored, instead of individual files.\n+    Does not recurse into excluded directories when listing all\n+    untracked files.\n+\n -z::\n \tTerminate entries with NUL, instead of LF.  This implies\n \tthe `--porcelain=v1` output format if no other format is given.\ndiff --git a/Documentation/technical/api-directory-listing.txt b/Documentation/technical/api-directory-listing.txt\nindex 6c77b4920c..a9816df44c 100644\n--- a/Documentation/technical/api-directory-listing.txt\n+++ b/Documentation/technical/api-directory-listing.txt\n@@ -33,6 +33,12 @@ The notable options are:\n \tSimilar to `DIR_SHOW_IGNORED`, but return ignored files in `ignored[]`\n \tin addition to untracked files in `entries[]`.\n \n+`DIR_SHOW_IGNORED_DIRECTORY`:::\n+\n+\tIf this is set, non-empty directories that match an ignore pattern are\n+\treturned. The individual files contained in ignored directories are not\n+\tincluded.\n+\n `DIR_KEEP_UNTRACKED_CONTENTS`:::\n \n \tOnly has meaning if `DIR_SHOW_IGNORED_TOO` is also set; if this is set, the\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 8e93802511..9fcf77ae3a 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1333,6 +1333,7 @@ static int git_status_config(const char *k, const char *v, void *cb)\n \n int cmd_status(int argc, const char **argv, const char *prefix)\n {\n+\tstatic int show_ignored_directory = 0;\n \tstatic struct wt_status s;\n \tint fd;\n \tstruct object_id oid;\n@@ -1362,6 +1363,8 @@ int cmd_status(int argc, const char **argv, const char *prefix)\n \t\t  N_(\"ignore changes to submodules, optional when: all, dirty, untracked. (Default: all)\"),\n \t\t  PARSE_OPT_OPTARG, NULL, (intptr_t)\"all\" },\n \t\tOPT_COLUMN(0, \"column\", &s.colopts, N_(\"list untracked files in columns\")),\n+\t\tOPT_BOOL(0, \"show-ignored-directory\", &show_ignored_directory,\n+\t\tN_(\"Only show directories that match an ignore pattern name.\")),\n \t\tOPT_END(),\n \t};\n \n@@ -1394,6 +1397,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)\n \ts.ignore_submodule_arg = ignore_submodule_arg;\n \ts.status_format = status_format;\n \ts.verbose = verbose;\n+\ts.show_ignored_directory = show_ignored_directory;\n \n \twt_status_collect(&s);\n \ndiff --git a/dir.c b/dir.c\nindex ae6f5c9636..b17dfd53bf 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -49,7 +49,7 @@ struct cached_dir {\n static enum path_treatment read_directory_recursive(struct dir_struct *dir,\n \tstruct index_state *istate, const char *path, int len,\n \tstruct untracked_cache_dir *untracked,\n-\tint check_only, const struct pathspec *pathspec);\n+\tint check_only, int stop_at_first_file, const struct pathspec *pathspec);\n static int get_dtype(struct dirent *de, struct index_state *istate,\n \t\t     const char *path, int len);\n \n@@ -1389,6 +1389,21 @@ static enum path_treatment treat_directory(struct dir_struct *dir,\n \tcase index_nonexistent:\n \t\tif (dir->flags & DIR_SHOW_OTHER_DIRECTORIES)\n \t\t\tbreak;\n+\t\tif (exclude &&\n+\t\t\t(dir->flags & DIR_SHOW_IGNORED_TOO) &&\n+\t\t\t(dir->flags & DIR_SHOW_IGNORED_DIRECTORY)) {\n+\n+\t\t\t/*\n+\t\t\t * This is an excluded directory, and we are only\n+\t\t\t * showing the name of a excluded directory.\n+\t\t\t * Check to see if there are any contained files\n+\t\t\t * to determine if the directory is empty or not.\n+\t\t\t */\n+\t\t\tif (read_directory_recursive(dir, istate, dirname, len,\n+\t\t\t\tuntracked, 1, 1, pathspec) == path_excluded)\n+\t\t\t\treturn path_excluded;\n+\t\t\treturn path_none;\n+\t\t}\n \t\tif (!(dir->flags & DIR_NO_GITLINKS)) {\n \t\t\tunsigned char sha1[20];\n \t\t\tif (resolve_gitlink_ref(dirname, \"HEAD\", sha1) == 0)\n@@ -1405,9 +1420,10 @@ static enum path_treatment treat_directory(struct dir_struct *dir,\n \tuntracked = lookup_untracked(dir->untracked, untracked,\n \t\t\t\t     dirname + baselen, len - baselen);\n \treturn read_directory_recursive(dir, istate, dirname, len,\n-\t\t\t\t\tuntracked, 1, pathspec);\n+\t\t\t\t\tuntracked, 1, 0, pathspec);\n }\n \n+\n /*\n  * This is an inexact early pruning of any recursive directory\n  * reading - if the path cannot possibly be in the pathspec,\n@@ -1633,7 +1649,7 @@ static enum path_treatment treat_path_fast(struct dir_struct *dir,\n \t\t * with check_only set.\n \t\t */\n \t\treturn read_directory_recursive(dir, istate, path->buf, path->len,\n-\t\t\t\t\t\tcdir->ucd, 1, pathspec);\n+\t\t\t\t\t\tcdir->ucd, 1, 0, pathspec);\n \t/*\n \t * We get path_recurse in the first run when\n \t * directory_exists_in_index() returns index_nonexistent. We\n@@ -1798,7 +1814,7 @@ static void close_cached_dir(struct cached_dir *cdir)\n static enum path_treatment read_directory_recursive(struct dir_struct *dir,\n \tstruct index_state *istate, const char *base, int baselen,\n \tstruct untracked_cache_dir *untracked, int check_only,\n-\tconst struct pathspec *pathspec)\n+\tint stop_at_first_file, const struct pathspec *pathspec)\n {\n \tstruct cached_dir cdir;\n \tenum path_treatment state, subdir_state, dir_state = path_none;\n@@ -1832,12 +1848,32 @@ static enum path_treatment read_directory_recursive(struct dir_struct *dir,\n \t\t\tsubdir_state =\n \t\t\t\tread_directory_recursive(dir, istate, path.buf,\n \t\t\t\t\t\t\t path.len, ud,\n-\t\t\t\t\t\t\t check_only, pathspec);\n+\t\t\t\t\t\t\t check_only, stop_at_first_file, pathspec);\n \t\t\tif (subdir_state > dir_state)\n \t\t\t\tdir_state = subdir_state;\n \t\t}\n \n \t\tif (check_only) {\n+\t\t\tif (stop_at_first_file) {\n+\t\t\t\t/*\n+\t\t\t\t * In general, if we are stopping at the first found file,\n+\t\t\t\t * We can only signal that a path of at least \"excluded\" was\n+\t\t\t\t * found. If the first file we find is \"excluded\" - there might\n+\t\t\t\t * be other untracked files later on that will not be searched.\n+\t\t\t\t *\n+\t\t\t\t * In current usage of this function, stop_at_first_file will\n+\t\t\t\t * only be set when called from a directory that matches the\n+\t\t\t\t * exclude pattern - there should be no untracked files -\n+\t\t\t\t * all contents should be marked as excluded.\n+\t\t\t\t */\n+\t\t\t\tif (dir_state == path_excluded)\n+\t\t\t\t\tbreak;\n+\t\t\t\telse if (dir_state > path_excluded) {\n+\t\t\t\t\tdir_state = path_excluded;\n+\t\t\t\t\tbreak;\n+\t\t\t\t}\n+\t\t\t}\n+\n \t\t\t/* abort early if maximum state has been reached */\n \t\t\tif (dir_state == path_untracked) {\n \t\t\t\tif (cdir.fdir)\n@@ -2108,7 +2144,7 @@ int read_directory(struct dir_struct *dir, struct index_state *istate,\n \t\t */\n \t\tdir->untracked = NULL;\n \tif (!len || treat_leading_path(dir, istate, path, len, pathspec))\n-\t\tread_directory_recursive(dir, istate, path, len, untracked, 0, pathspec);\n+\t\tread_directory_recursive(dir, istate, path, len, untracked, 0, 0, pathspec);\n \tQSORT(dir->entries, dir->nr, cmp_dir_entry);\n \tQSORT(dir->ignored, dir->ignored_nr, cmp_dir_entry);\n \ndiff --git a/dir.h b/dir.h\nindex e3717055d1..b59eedf633 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -152,7 +152,8 @@ struct dir_struct {\n \t\tDIR_COLLECT_IGNORED = 1<<4,\n \t\tDIR_SHOW_IGNORED_TOO = 1<<5,\n \t\tDIR_COLLECT_KILLED_ONLY = 1<<6,\n-\t\tDIR_KEEP_UNTRACKED_CONTENTS = 1<<7\n+\t\tDIR_KEEP_UNTRACKED_CONTENTS = 1<<7,\n+\t\tDIR_SHOW_IGNORED_DIRECTORY = 1<<8\n \t} flags;\n \tstruct dir_entry **entries;\n \tstruct dir_entry **ignored;\ndiff --git a/t/t7519-status-show-ignored-directory.sh b/t/t7519-status-show-ignored-directory.sh\nnew file mode 100755\nindex 0000000000..3ff0a96cdf\n--- /dev/null\n+++ b/t/t7519-status-show-ignored-directory.sh\n@@ -0,0 +1,189 @@\n+#!/bin/sh\n+#\n+#\n+\n+test_description='git status collapse ignored'\n+\n+. ./test-lib.sh\n+\n+\n+cat >.gitignore <<\\EOF\n+*.ign\n+ignored_dir/\n+!*.unignore\n+EOF\n+\n+# commit initial ignore file\n+test_expect_success 'setup initial commit and ignore file' '\n+\tgit add . &&\n+\ttest_tick &&\n+\tgit commit -m \"Initial commit\"\n+'\n+\n+cat >expect <<\\EOF\n+? expect\n+? output\n+! dir/ignored/ignored_1.ign\n+! dir/ignored/ignored_2.ign\n+! ignored/ignored_1.ign\n+! ignored/ignored_2.ign\n+EOF\n+\n+# Test status behavior on folder with ignored files\n+test_expect_success 'setup folder with ignored files' '\n+\tmkdir -p ignored dir/ignored &&\n+\ttouch ignored/ignored_1.ign ignored/ignored_2.ign \\\n+\t\tdir/ignored/ignored_1.ign dir/ignored/ignored_2.ign\n+'\n+\n+test_expect_success 'Verify behavior of status on folders with ignored files' '\n+\ttest_when_finished \"git clean -fdx\" &&\n+\tgit status --porcelain=v2 --ignored --untracked-files=all --show-ignored-directory >output &&\n+\ttest_i18ncmp expect output\n+'\n+\n+# Test status bahavior on folder with tracked and ignored files\n+cat >expect <<\\EOF\n+? expect\n+? output\n+! dir/tracked_ignored/ignored_1.ign\n+! dir/tracked_ignored/ignored_2.ign\n+! tracked_ignored/ignored_1.ign\n+! tracked_ignored/ignored_2.ign\n+EOF\n+\n+test_expect_success 'setup folder with tracked & ignored files' '\n+\tmkdir -p tracked_ignored dir/tracked_ignored &&\n+\ttouch tracked_ignored/tracked_1 tracked_ignored/tracked_2 \\\n+\t\ttracked_ignored/ignored_1.ign tracked_ignored/ignored_2.ign \\\n+\t\tdir/tracked_ignored/tracked_1 dir/tracked_ignored/tracked_2 \\\n+\t\tdir/tracked_ignored/ignored_1.ign dir/tracked_ignored/ignored_2.ign &&\n+\n+\tgit add tracked_ignored/tracked_1 tracked_ignored/tracked_2 \\\n+\t\tdir/tracked_ignored/tracked_1 dir/tracked_ignored/tracked_2 &&\n+\ttest_tick &&\n+\tgit commit -m \"commit tracked files\"\n+'\n+\n+test_expect_success 'Verify status on folder with tracked & ignored files' '\n+\tgit status --porcelain=v2 --ignored --untracked-files=all --show-ignored-directory >output &&\n+\ttest_i18ncmp expect output\n+'\n+\n+# Test status with modified paths\n+cat >expect <<\\EOF\n+1 .M N... 100644 100644 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 tracked_ignored/tracked_1\n+? expect\n+? output\n+! dir/tracked_ignored/ignored_1.ign\n+! dir/tracked_ignored/ignored_2.ign\n+! tracked_ignored/ignored_1.ign\n+! tracked_ignored/ignored_2.ign\n+EOF\n+\n+test_expect_success 'setup workdir with modified tracked files' '\n+\techo 123 > tracked_ignored/tracked_1\n+'\n+\n+test_expect_success 'Verify status on folder with tracked & ignored files' '\n+\ttest_when_finished \"git clean -fdx && git reset HEAD~1 --hard\" &&\n+\tgit status --porcelain=v2 --ignored --untracked-files=all --show-ignored-directory >output &&\n+\ttest_i18ncmp expect output\n+'\n+\n+# Test status behavior on folder with untracked and ignored files\n+cat >expect <<\\EOF\n+? dir/untracked_ignored/untracked_1\n+? dir/untracked_ignored/untracked_2\n+? expect\n+? output\n+? untracked_ignored/untracked_1\n+? untracked_ignored/untracked_2\n+! dir/untracked_ignored/ignored_1.ign\n+! dir/untracked_ignored/ignored_2.ign\n+! untracked_ignored/ignored_1.ign\n+! untracked_ignored/ignored_2.ign\n+EOF\n+\n+test_expect_success 'setup folder with tracked & ignored files' '\n+\tmkdir -p untracked_ignored dir/untracked_ignored &&\n+\ttouch untracked_ignored/untracked_1 untracked_ignored/untracked_2 \\\n+\t\tuntracked_ignored/ignored_1.ign untracked_ignored/ignored_2.ign \\\n+\t\tdir/untracked_ignored/untracked_1 dir/untracked_ignored/untracked_2 \\\n+\t\tdir/untracked_ignored/ignored_1.ign dir/untracked_ignored/ignored_2.ign\n+'\n+\n+test_expect_success 'Verify status on folder with tracked & ignored files' '\n+\ttest_when_finished \"git clean -fdx\" &&\n+\tgit status --porcelain=v2 --ignored --untracked-files=all --show-ignored-directory >output &&\n+\ttest_i18ncmp expect output\n+'\n+\n+# Test status behavior on ignored folder\n+cat >expect <<\\EOF\n+? expect\n+? output\n+! ignored_dir/\n+EOF\n+\n+test_expect_success 'setup folder with tracked & ignored files' '\n+\tmkdir ignored_dir &&\n+\ttouch ignored_dir/ignored_1 ignored_dir/ignored_2 \\\n+\t\tignored_dir/ignored_1.ign ignored_dir/ignored_2.ign\n+'\n+\n+test_expect_success 'Verify status on folder with tracked & ignored files' '\n+\ttest_when_finished \"git clean -fdx\" &&\n+\tgit status --porcelain=v2 --ignored --untracked-files=all --show-ignored-directory >output &&\n+\ttest_i18ncmp expect output\n+'\n+\n+# Test status behavior on ignored folder with tracked file\n+cat >expect <<\\EOF\n+? expect\n+? output\n+! ignored_dir/ignored_1\n+! ignored_dir/ignored_1.ign\n+! ignored_dir/ignored_2\n+! ignored_dir/ignored_2.ign\n+EOF\n+\n+test_expect_success 'setup folder with tracked & ignored files' '\n+\tmkdir ignored_dir &&\n+\ttouch ignored_dir/ignored_1 ignored_dir/ignored_2 \\\n+\t\tignored_dir/ignored_1.ign ignored_dir/ignored_2.ign \\\n+\t\tignored_dir/tracked &&\n+\tgit add -f ignored_dir/tracked &&\n+\ttest_tick &&\n+\tgit commit -m \"Force add file in ignored directory\"\n+'\n+\n+test_expect_success 'Verify status on folder with tracked & ignored files' '\n+\ttest_when_finished \"git clean -fdx && git reset HEAD~1 --hard\" &&\n+\tgit status --porcelain=v2 --ignored --untracked-files=all --show-ignored-directory >output &&\n+\ttest_i18ncmp expect output\n+'\n+\n+# Test status with more complex paths -\n+# ignored underneath an untracked folder\n+cat >expect <<\\EOF\n+? expect\n+? output\n+? dir/dir/untracked/dir/untracked/a\n+! dir/dir/untracked/dir/ignored/a.ign\n+! dir/dir/untracked/dir/ignored_dir/\n+EOF\n+\n+test_expect_success 'setup deeper folder structure for ignored and untracked' '\n+\tmkdir dir/dir/untracked/dir/ignored dir/dir/untracked/dir/ignored_dir \\\n+\t\tdir/dir/untracked/dir/untracked &&\n+\ttouch dir/dir/untracked/dir/ignored/a.ign dir/dir/untracked/dir/ignored_dir/a \\\n+\t      dir/dir/untracked/dir/untracked/a\n+'\n+test_expect_success 'Verify status on slightly deeper folder structure' '\n+\ttest_when_finished \"git clean -fdx\" &&\n+\tgit status --porcelain=v2 --ignored --untracked-files=all --show-ignored-directory >output &&\n+\ttest_i18ncmp expect output\n+'\n+\n+test_done\ndiff --git a/wt-status.c b/wt-status.c\nindex 77c27c5113..c3d17490a4 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -664,6 +664,10 @@ static void wt_status_collect_untracked(struct wt_status *s)\n \t\tdir.flags |= DIR_SHOW_IGNORED_TOO;\n \telse\n \t\tdir.untracked = the_index.untracked;\n+\n+\tif (s->show_ignored_directory)\n+\t\tdir.flags |= DIR_SHOW_IGNORED_DIRECTORY;\n+\n \tsetup_standard_excludes(&dir);\n \n \tfill_directory(&dir, &the_index, &s->pathspec);\ndiff --git a/wt-status.h b/wt-status.h\nindex 64f4d33ea1..b9c1dffb92 100644\n--- a/wt-status.h\n+++ b/wt-status.h\n@@ -72,6 +72,7 @@ struct wt_status {\n \tint submodule_summary;\n \tint show_ignored_files;\n \tenum untracked_status_type show_untracked_files;\n+\tint show_ignored_directory;\n \tconst char *ignore_submodule_arg;\n \tchar color_palette[WT_STATUS_MAXSLOT][COLOR_MAXLEN];\n \tunsigned colopts;\n-- \n2.11.0\n\n"},{"id":"326085","messageId":"CAGZ79kb8cWR1P=1FSkfZ72Ai63T0WapwtCCN3gwqrWE_=MzOLg@mail.gmail.com","threadId":"46561","inReplyTo":"20170810184936.239542-2-jamill@microsoft.com","subject":"Re: [PATCH v1 1/1] dir: teach status to show ignored directories","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-08-10T20:03:55Z","receivedAt":"2017-08-10T20:04:02Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Aug 10, 2017 at 11:49 AM, Jameson Miller\n<jameson.miller81@gmail.com> wrote:\n\nWelcome to the Git mailing list. :)\n\n> Teach Git to optionally show ignored directories when showing all\n> untracked files. The git status command exposes the options to report\n> ignored and/or untracked files. However, when reporting all untracked\n> files (--untracked-files=all), all individual ignored files are reported\n> as well. It is not currently possible to get the reporting behavior of\n> the --ignored flag, while also reporting all untracked files.\n\nTrying to understand this based off the documentation for\n--untracked=all and --ignored, I realize that the documentation\nfor --ignored seems to be lacking as I do not understand what the\n--ignored behavior is in combination with --untracked=[all, normal, no]\n\n> This\n> change exposes a flag to report all untracked files while not showing\n> individual files in ignored directories.\n\nBy the description up to here, it sounds as if you want to introduce\nmode for --untracked, e.g. \"normal-adjusted-for-ignored\" (it's a bad\nsuggestion)? However the patch seems to add an orthogonal flag,\nsuch that\n\n  status --no-ignored --untracked=no --show-ignored-directory\n\nwould also be possible. What is a reasonable expectation for\nthe output of such?\n\n> Motivation:\n> Our application (Visual Studio) needs all untracked files listed\n> individually, but does not need all ignored files listed individually.\n\nFor parsing output, I would strongly recommend --porcelain[=2],\nbut that is a tangent.\n\n> Reporting all ignored files can affect the time it takes for status\n> to run. For a representative repository, here are some measurements\n> showing a large perf improvement for this scenario:\n>\n> | Command | Reported ignored entries | Time (s) |\n> | ------- | ------------------------ | -------- |\n> | 1       | 0                        | 1.3      |\n> | 2       | 1024                     | 4.2      |\n> | 3       | 174904                   | 7.5      |\n> | 4       | 1046                     | 1.6      |\n>\n> Commands:\n>  1) status\n>  2) status --ignored\n>  3) status --ignored --untracked-files=all\n>  4) status --ignored --untracked-files=all --show-ignored-directory\n\n(2) is --untracked-files=normal I'd presume, such that this flag\ncan be understood as a tweak to \"normal\" based on the similar size\nbetween 2 and 4? (The timing improvement from 2 to 4 is huge though).\n\n> This changes exposes a --show-ignored-directory flag to the git status\n\ns/changes/change/\n\n> command. This flag is utilized when running git status with the\n> --ignored and --untracked-files options to not list ignored individual\n> ignored files contained in directories that match an ignore pattern.\n>\n> Part of the perf improvement comes from the tweak to\n> read_directory_recursive to stop scanning the file system after it\n> encounters the first file. When a directory is ignored, all it needs to\n> determine is if the directory is empty or not. The logic currently keeps\n> scanning the file system until it finds an untracked file. However, as\n> the directory is ignored, all the contained contents are also marked\n> excluded. For ignored directories that contain a large number of files,\n> this can take some time.\n\nI think it is possible to ignore a directory and still track files in it, what\nare the implications of this flag on a tracked (and changed) file in an\nignored dir?\n\nWhat happens to empty directories that match an ignore pattern?\n\n\n> @@ -1362,6 +1363,8 @@ int cmd_status(int argc, const char **argv, const char *prefix)\n>                   N_(\"ignore changes to submodules, optional when: all, dirty, untracked. (Default: all)\"),\n>                   PARSE_OPT_OPTARG, NULL, (intptr_t)\"all\" },\n>                 OPT_COLUMN(0, \"column\", &s.colopts, N_(\"list untracked files in columns\")),\n> +               OPT_BOOL(0, \"show-ignored-directory\", &show_ignored_directory,\n\nIs it possible to directly read into  s.show_ignored_directory here?\n\n> +test_expect_success 'Verify behavior of status on folders with ignored files' '\n> +       test_when_finished \"git clean -fdx\" &&\n> +       git status --porcelain=v2 --ignored --untracked-files=all --show-ignored-directory >output &&\n> +       test_i18ncmp expect output\n> +'\n> +\n> +# Test status bahavior on folder with tracked and ignored files\n\nbehavior\n\n> +cat >expect <<\\EOF\n> +? expect\n> +? output\n> +! dir/tracked_ignored/ignored_1.ign\n> +! dir/tracked_ignored/ignored_2.ign\n> +! tracked_ignored/ignored_1.ign\n> +! tracked_ignored/ignored_2.ign\n> +EOF\n\nI think our latest 'best style' is to include these heredocs into the test.\n"},{"id":"326151","messageId":"20170811173905.GA59325@google.com","threadId":"46561","inReplyTo":"20170810184936.239542-2-jamill@microsoft.com","subject":"Re: [PATCH v1 1/1] dir: teach status to show ignored directories","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-08-11T17:39:05Z","receivedAt":"2017-08-11T17:39:13Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"On 08/10, Jameson Miller wrote:\n> Teach Git to optionally show ignored directories when showing all\n> untracked files. The git status command exposes the options to report\n> ignored and/or untracked files. However, when reporting all untracked\n> files (--untracked-files=all), all individual ignored files are reported\n> as well. It is not currently possible to get the reporting behavior of\n> the --ignored flag, while also reporting all untracked files. This\n> change exposes a flag to report all untracked files while not showing\n> individual files in ignored directories.\n> \n> Motivation:\n> Our application (Visual Studio) needs all untracked files listed\n> individually, but does not need all ignored files listed individually.\n> Reporting all ignored files can affect the time it takes for status\n> to run. For a representative repository, here are some measurements\n> showing a large perf improvement for this scenario:\n> \n> | Command | Reported ignored entries | Time (s) |\n> | ------- | ------------------------ | -------- |\n> | 1       | 0                        | 1.3      |\n> | 2       | 1024                     | 4.2      |\n> | 3       | 174904                   | 7.5      |\n> | 4       | 1046                     | 1.6      |\n> \n> Commands:\n>  1) status\n>  2) status --ignored\n>  3) status --ignored --untracked-files=all\n>  4) status --ignored --untracked-files=all --show-ignored-directory\n> \n> This changes exposes a --show-ignored-directory flag to the git status\n> command. This flag is utilized when running git status with the\n> --ignored and --untracked-files options to not list ignored individual\n> ignored files contained in directories that match an ignore pattern.\n\nI can't help feeling that there is a better way express this with a\nbetter UI.  I'm not saying this is wrong, I'm just not sure how\n--show-ignored-directory would work when not paired with --ignored and\n--untracked-files.  Does it require --ignored to also be given?\n\n> \n> Part of the perf improvement comes from the tweak to\n> read_directory_recursive to stop scanning the file system after it\n> encounters the first file. When a directory is ignored, all it needs to\n> determine is if the directory is empty or not. The logic currently keeps\n> scanning the file system until it finds an untracked file. However, as\n> the directory is ignored, all the contained contents are also marked\n> excluded. For ignored directories that contain a large number of files,\n> this can take some time.\n> \n> Signed-off-by: Jameson Miller <jamill@microsoft.com>\n\n-- \nBrandon Williams\n"},{"id":"326152","messageId":"2bbb1d0f-ae06-1878-d185-112bd51f75c9@gmail.com","threadId":"46561","inReplyTo":"CAGZ79kb8cWR1P=1FSkfZ72Ai63T0WapwtCCN3gwqrWE_=MzOLg@mail.gmail.com","subject":"Re: [PATCH v1 1/1] dir: teach status to show ignored directories","fromName":"Jameson Miller","fromEmail":"jameson.miller81@gmail.com","sentAt":"2017-08-11T17:48:41Z","receivedAt":"2017-08-11T17:48:50Z","isPatch":true,"sender":{"key":"jameson.miller81@gmail.com","avatar":null},"body":"\n\nOn 08/10/2017 04:03 PM, Stefan Beller wrote:\n> On Thu, Aug 10, 2017 at 11:49 AM, Jameson Miller\n> <jameson.miller81@gmail.com> wrote:\n>\n> Welcome to the Git mailing list. :)\n\nThank you for the welcome and the review! I will include the\nsuggested code changes in the next patch version.\n\n>> Teach Git to optionally show ignored directories when showing all\n>> untracked files. The git status command exposes the options to report\n>> ignored and/or untracked files. However, when reporting all untracked\n>> files (--untracked-files=all), all individual ignored files are reported\n>> as well. It is not currently possible to get the reporting behavior of\n>> the --ignored flag, while also reporting all untracked files.\n> Trying to understand this based off the documentation for\n> --untracked=all and --ignored, I realize that the documentation\n> for --ignored seems to be lacking as I do not understand what the\n> --ignored behavior is in combination with --untracked=[all, normal, no]\n>\n\nThe set of files listed by \"--ignored\" changes when different\nvalues are given to \"--untracked-files\".  If would be nice to\nbe able to make the ignored output independent of the untracked\nsettings.  This patch attempts to do that while maintaining\nbackward compatibility with the existing behavior.\n\nWhen \"--ignored\" is used by itself (\"--untracked-files=normal\"),\nignored directories (both directories that explicitly match a\ndirectory ignore pattern -and- directories containing only ignored\nfiles) are listed. Individual files within are *not* listed.\n\nWhen \"--ignored\" is used with \"--untracked-files=all\", git always\nlists the individual files within the ignored directories and DOES\nNOT collapse the output to just the containing (explicitly or\nimplicitly ignored) directory.\n\nThis can cause a massive performance problem when there are\na lot of ignored files in a well-defined set of ignored directories.\nFor example, on Windows, Visual Studio creates a bin/ and\nobj/ directory inside your project where it writes all .obj\nfiles.  Normal usage is to explicitly ignore those 2 directory\nnames in your .gitignores (rather than or in addition to *.obj).\nWe just want to see the \"bin/\" and \"obj/\" paths\nregardless of which \"--untracked\" flag is passed in.\n\nWe want to see the paths that explicitly match an ignore pattern.\nThis means that if a directory only contains ignored files, but the\ndirectory itself is not explicitly ignored, then we want to see the\nindividual files. This is slightly different than the current behavior\nof \"--ignored\".\n\nI am open to suggestions on how to present the options to control\nthis behavior.\n\n>> This\n>> change exposes a flag to report all untracked files while not showing\n>> individual files in ignored directories.\n> By the description up to here, it sounds as if you want to introduce\n> mode for --untracked, e.g. \"normal-adjusted-for-ignored\" (it's a bad\n> suggestion)? However the patch seems to add an orthogonal flag,\n> such that\n>\n>    status --no-ignored --untracked=no --show-ignored-directory\n>\n> would also be possible. What is a reasonable expectation for\n> the output of such?\n\nThe current patch does add another flag. This flag only has meaning if\nthe \"--ignored\" and \"--untracked=all\" flags are also specified. Another\noption I had considered is to let the \"--ignored\" flag take an argument.\nThen, we could express this new behavior through (for example) a\n\"--ignored=exact\" flag to reflect the fact that this new option\nreturns paths that match the ignore pattern, and does not\ncollapse directories that contain only ignored files.\n\n>> Motivation:\n>> Our application (Visual Studio) needs all untracked files listed\n>> individually, but does not need all ignored files listed individually.\n> For parsing output, I would strongly recommend --porcelain[=2],\n> but that is a tangent.\n>\n>> Reporting all ignored files can affect the time it takes for status\n>> to run. For a representative repository, here are some measurements\n>> showing a large perf improvement for this scenario:\n>>\n>> | Command | Reported ignored entries | Time (s) |\n>> | ------- | ------------------------ | -------- |\n>> | 1       | 0                        | 1.3      |\n>> | 2       | 1024                     | 4.2      |\n>> | 3       | 174904                   | 7.5      |\n>> | 4       | 1046                     | 1.6      |\n>>\n>> Commands:\n>>   1) status\n>>   2) status --ignored\n>>   3) status --ignored --untracked-files=all\n>>   4) status --ignored --untracked-files=all --show-ignored-directory\n>> (2) is --untracked-files=normal I'd presume, such that this flag\n>> can be understood as a tweak to \"normal\" based on the similar size\n>> between 2 and 4? (The timing improvement from 2 to 4 is huge though).\n(2) is --untracked-files=normal. Although the count of ignored\nfiles similar between 2 and 4, I consider this flag more of a\ntweak on 3, as we want the untracked files reported with\nthe \"--untracked=all\" flag. The counts between 2 and 4 are\nsimilar in this case because most of the ignored files are\ncontained in ignored directories.\n\nOur application calls status including the following flags:\n\n--porcelain=v2 --ignored --untracked-files=all --ignore-submodules=none\n\nThis means we have bad performance compared to just \"git status\"\nwhen there is a large number of files in ignored directories\nWith this new behavior, our application would move from case 3 to\ncase 4 for this repository.\n\nYou also point out the timing difference between case 2 and 4. I\nthink there is an optimization we can make when running \"git\nstatus --ignored\" logic that will improve the the timing\nhere. Currently, the logic in dir.c is iterating over all files\ncontained in an ignored directory to see if it is empty or\nnot. This is not necessary - we should be able to stop after\nfinding any file. I plan to follow up on this in a different\npatch.\n\n>> This changes exposes a --show-ignored-directory flag to the git status\n> s/changes/change/\n>\n>> command. This flag is utilized when running git status with the\n>> --ignored and --untracked-files options to not list ignored individual\n>> ignored files contained in directories that match an ignore pattern.\n>>\n>> Part of the perf improvement comes from the tweak to\n>> read_directory_recursive to stop scanning the file system after it\n>> encounters the first file. When a directory is ignored, all it needs to\n>> determine is if the directory is empty or not. The logic currently keeps\n>> scanning the file system until it finds an untracked file. However, as\n>> the directory is ignored, all the contained contents are also marked\n>> excluded. For ignored directories that contain a large number of files,\n>> this can take some time.\n> I think it is possible to ignore a directory and still track files in it, what\n> are the implications of this flag on a tracked (and changed) file in an\n> ignored dir?\nIt is possible to have tracked files in an ignored directory. The\nbehavior with this patch is to show individual files in the\nignored directory. This patch includes a test covering this\nscenario. I think this behavior is reasonable for this case - I\nam interested if there are other opinions on this.\n>\n> What happens to empty directories that match an ignore pattern?\n>\nThe \"git status\" command would not show empty directories. From\nthe lower level implementation in dir.c, I think it *should*\ndepend on the DIR_HIDE_EMPTY_DIRECTORIES flag, but does not\ncurrently. I will fix this with the next patch version.\n>> @@ -1362,6 +1363,8 @@ int cmd_status(int argc, const char **argv, const char *prefix)\n>>                    N_(\"ignore changes to submodules, optional when: all, dirty, untracked. (Default: all)\"),\n>>                    PARSE_OPT_OPTARG, NULL, (intptr_t)\"all\" },\n>>                  OPT_COLUMN(0, \"column\", &s.colopts, N_(\"list untracked files in columns\")),\n>> +               OPT_BOOL(0, \"show-ignored-directory\", &show_ignored_directory,\n> Is it possible to directly read into  s.show_ignored_directory here?\n>\n>> +test_expect_success 'Verify behavior of status on folders with ignored files' '\n>> +       test_when_finished \"git clean -fdx\" &&\n>> +       git status --porcelain=v2 --ignored --untracked-files=all --show-ignored-directory >output &&\n>> +       test_i18ncmp expect output\n>> +'\n>> +\n>> +# Test status bahavior on folder with tracked and ignored files\n> behavior\n>\n>> +cat >expect <<\\EOF\n>> +? expect\n>> +? output\n>> +! dir/tracked_ignored/ignored_1.ign\n>> +! dir/tracked_ignored/ignored_2.ign\n>> +! tracked_ignored/ignored_1.ign\n>> +! tracked_ignored/ignored_2.ign\n>> +EOF\n> I think our latest 'best style' is to include these heredocs into the test.\n\n"},{"id":"326156","messageId":"08b17af5-a8cf-ed15-9972-e7d655272a7e@gmail.com","threadId":"46561","inReplyTo":"20170811173905.GA59325@google.com","subject":"Re: [PATCH v1 1/1] dir: teach status to show ignored directories","fromName":"Jameson Miller","fromEmail":"jameson.miller81@gmail.com","sentAt":"2017-08-11T18:29:58Z","receivedAt":"2017-08-11T18:30:06Z","isPatch":true,"sender":{"key":"jameson.miller81@gmail.com","avatar":null},"body":"\n\nOn 08/11/2017 01:39 PM, Brandon Williams wrote:\n> On 08/10, Jameson Miller wrote:\n>> Teach Git to optionally show ignored directories when showing all\n>> untracked files. The git status command exposes the options to report\n>> ignored and/or untracked files. However, when reporting all untracked\n>> files (--untracked-files=all), all individual ignored files are reported\n>> as well. It is not currently possible to get the reporting behavior of\n>> the --ignored flag, while also reporting all untracked files. This\n>> change exposes a flag to report all untracked files while not showing\n>> individual files in ignored directories.\n>>\n>> Motivation:\n>> Our application (Visual Studio) needs all untracked files listed\n>> individually, but does not need all ignored files listed individually.\n>> Reporting all ignored files can affect the time it takes for status\n>> to run. For a representative repository, here are some measurements\n>> showing a large perf improvement for this scenario:\n>>\n>> | Command | Reported ignored entries | Time (s) |\n>> | ------- | ------------------------ | -------- |\n>> | 1       | 0                        | 1.3      |\n>> | 2       | 1024                     | 4.2      |\n>> | 3       | 174904                   | 7.5      |\n>> | 4       | 1046                     | 1.6      |\n>>\n>> Commands:\n>>   1) status\n>>   2) status --ignored\n>>   3) status --ignored --untracked-files=all\n>>   4) status --ignored --untracked-files=all --show-ignored-directory\n>>\n>> This changes exposes a --show-ignored-directory flag to the git status\n>> command. This flag is utilized when running git status with the\n>> --ignored and --untracked-files options to not list ignored individual\n>> ignored files contained in directories that match an ignore pattern.\n> I can't help feeling that there is a better way express this with a\n> better UI.  I'm not saying this is wrong, I'm just not sure how\n> --show-ignored-directory would work when not paired with --ignored and\n> --untracked-files.  Does it require --ignored to also be given?\nYes. This flag only has meaning when --ignored and --untracked=all\nare specified. I am open to other suggestions on how to express this.\n\nAnother option might be to modify the \"--ignored\" flag to take an\nargument, which would allow more explicit control over how ignored\npaths are reported. This way, we could specify (for example)\n\"--ignored=explicit\" to control the output of ignored paths.\n\n>> Part of the perf improvement comes from the tweak to\n>> read_directory_recursive to stop scanning the file system after it\n>> encounters the first file. When a directory is ignored, all it needs to\n>> determine is if the directory is empty or not. The logic currently keeps\n>> scanning the file system until it finds an untracked file. However, as\n>> the directory is ignored, all the contained contents are also marked\n>> excluded. For ignored directories that contain a large number of files,\n>> this can take some time.\n>>\n>> Signed-off-by: Jameson Miller <jamill@microsoft.com>\n\n"},{"id":"326311","messageId":"CAGZ79kaxwKrHJ=00w3=R0xteep3Gy6rfExZmOZxNtc3YLijHLw@mail.gmail.com","threadId":"46561","inReplyTo":"2bbb1d0f-ae06-1878-d185-112bd51f75c9@gmail.com","subject":"Re: [PATCH v1 1/1] dir: teach status to show ignored directories","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-08-14T21:05:04Z","receivedAt":"2017-08-14T21:05:11Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Aug 11, 2017 at 10:48 AM, Jameson Miller\n<jameson.miller81@gmail.com> wrote:\n> The set of files listed by \"--ignored\" changes when different\n> values are given to \"--untracked-files\".  If would be nice to\n> be able to make the ignored output independent of the untracked\n> settings.  This patch attempts to do that while maintaining\n> backward compatibility with the existing behavior.\n\n\nSo after rereading this, maybe we could have\n'--ignored[=as-untracked, new-mode]' with as-untracked as default?\n\nThis would satisfy your goal, while still maintaining backwards\ncompatibility.\n\n> We want to see the paths that explicitly match an ignore pattern.\n> This means that if a directory only contains ignored files, but the\n> directory itself is not explicitly ignored, then we want to see the\n> individual files. This is slightly different than the current behavior\n> of \"--ignored\".\n>\n> I am open to suggestions on how to present the options to control\n> this behavior.\n\nI would strongly favor not introducing yet another flag to tweak the\nbehavior of another command line option. (The UX of Git is already\nbloated, IMHO. But that is just my general stance on things)\n\nSo we'd have two proposals:\n\n(1) The current patch proposes --show-ignored-directory that modifies\n    the behavior of other flags\n(2) extend an already existing option to take another mode (which I\n    propose)\n\nStrong arguments for (1) would be if the flag is orthogonal and apply\nto the other flags in a uniform way, i.e.:\n    status --no-ignored --untracked=no --show-ignored-directory\nwould make sense and produce an output that a user doesn't\nfind confusing.\n\nArguments for (2), are\n* less documentation for users (see below)\n* if the two flags are truly orthogonal, test effort is decreased\n* no exposure of an API that may yield strange results when the user\n  gives strange input.\n\n>>> This\n>>> change exposes a flag to report all untracked files while not showing\n>>> individual files in ignored directories.\n>>\n>> By the description up to here, it sounds as if you want to introduce\n>> mode for --untracked, e.g. \"normal-adjusted-for-ignored\" (it's a bad\n>> suggestion)? However the patch seems to add an orthogonal flag,\n>> such that\n>>\n>>    status --no-ignored --untracked=no --show-ignored-directory\n>>\n>> would also be possible. What is a reasonable expectation for\n>> the output of such?\n>\n>\n> The current patch does add another flag. This flag only has meaning if\n> the \"--ignored\" and \"--untracked=all\" flags are also specified. Another\n> option I had considered is to let the \"--ignored\" flag take an argument.\n\nI would think that is a better solution, as then untracked and ignored\nfiles (and their parameters) will be orthogonal in the long run.\nThings like\n\n  --untracked=all --ignored=none\n  --untracked=none --ignored=only-dirs\n  --untracked=reasonable-default --ignored=other-reasonable-default\n\nwill be possible and eventually the default for ignored files may be\nindependent of untracked settings.\n\n> Then, we could express this new behavior through (for example) a\n> \"--ignored=exact\" flag to reflect the fact that this new option\n> returns paths that match the ignore pattern, and does not\n> collapse directories that contain only ignored files.\n\nYes, and we would not need to worry about how the new\nflag interacts with other flags. I would prefer that solution as it\nseems the API will be cleaner that way.\n\nLess things depend on each other, e.g. as a user I do not need to\nknow about the 'other' flag, be it ignored or untracked to solve my\nproblem. If we'd go with this third flag, we'd need to (a) document it,\nbut (b) also have to add a sentence to untracked and ignored flag\n    \"In the corner case of also having the third flag, we change\n    behavior once again\"\nwhich I'd dislike from a users perspective.\n\n>>> Commands:\n>>>   1) status\n>>>   2) status --ignored\n>>>   3) status --ignored --untracked-files=all\n>>>   4) status --ignored --untracked-files=all --show-ignored-directory\n>>> (2) is --untracked-files=normal I'd presume, such that this flag\n>>> can be understood as a tweak to \"normal\" based on the similar size\n>>> between 2 and 4? (The timing improvement from 2 to 4 is huge though).\n>\n> (2) is --untracked-files=normal. Although the count of ignored\n> files similar between 2 and 4, I consider this flag more of a\n> tweak on 3, as we want the untracked files reported with\n> the \"--untracked=all\" flag. The counts between 2 and 4 are\n> similar in this case because most of the ignored files are\n> contained in ignored directories.\n>\n> Our application calls status including the following flags:\n>\n> --porcelain=v2 --ignored --untracked-files=all --ignore-submodules=none\n>\n> This means we have bad performance compared to just \"git status\"\n> when there is a large number of files in ignored directories\n> With this new behavior, our application would move from case 3 to\n> case 4 for this repository.\n>\n> You also point out the timing difference between case 2 and 4. I\n> think there is an optimization we can make when running \"git\n> status --ignored\" logic that will improve the the timing\n> here. Currently, the logic in dir.c is iterating over all files\n> contained in an ignored directory to see if it is empty or\n> not. This is not necessary - we should be able to stop after\n> finding any file. I plan to follow up on this in a different\n> patch.\n\nThanks!\n"},{"id":"328311","messageId":"1505755473-6720-1-git-send-email-jamill@microsoft.com","threadId":"46561","inReplyTo":"20170810184936.239542-1-jamill@microsoft.com","subject":"[PATCH v2] Improve performance of git status --ignored","fromName":"Jameson Miller","fromEmail":"jameson.miller81@gmail.com","sentAt":"2017-09-18T17:24:32Z","receivedAt":"2017-09-18T17:24:45Z","isPatch":true,"sender":{"key":"jameson.miller81@gmail.com","avatar":null},"body":"This is the second version of my patches to improve handling of\nignored files\n\nI have decided to break the original patch series into two parts:\n\n 1) Perf improvements to handling ignored directories\n\n 2) Expose extra options to control which ignored files are displayed\n by git status.\n\nThis patch will address #1, and I will follow up with another patch\nfor #2.\n\nThis patch improves the performance of 'git status --ignored' by\ncutting out unnecessary work when determining if an ignored directory\nis empty or not. The current logic will recursively enumerate through\nall contents of an ignored directory to determine whether it is empty\nor not. The new logic will return after the 1st file is\nencountered. This can result in a significant speedup in work dirs\nwith a lot of files in ignored directories.\n\nAs an example of the performance improvement, here is a representative\nexample of a repository with ~190,000 files in ~400 ignored\ndirectories:\n\n| Command                    | Time (s) |\n| -------------------------- | -------- |\n| git status                 |    1.2   |\n| git status --ignored (old) |    3.9   |\n| git status --ignored (new) |    1.4   |\n\nJameson Miller (1):\n  Improve performance of git status --ignored\n\n dir.c | 47 +++++++++++++++++++++++++++++++++++++++++------\n 1 file changed, 41 insertions(+), 6 deletions(-)\n\n-- \n2.7.4\n\n"},{"id":"328312","messageId":"1505755473-6720-2-git-send-email-jamill@microsoft.com","threadId":"46561","inReplyTo":"1505755473-6720-1-git-send-email-jamill@microsoft.com","subject":"[PATCH v2] Improve performance of git status --ignored","fromName":"Jameson Miller","fromEmail":"jameson.miller81@gmail.com","sentAt":"2017-09-18T17:24:33Z","receivedAt":"2017-09-18T17:24:46Z","isPatch":true,"sender":{"key":"jameson.miller81@gmail.com","avatar":null},"body":"Improve the performance of the directory listing logic when it wants to list\nnon-empty ignored directories. In order to show non-empty ignored directories,\nthe existing logic will recursively iterate through all contents of an ignored\ndirectory. This change introduces the optimization to stop iterating through\nthe contents once it finds the first file. This can have a significant\nimprovement in 'git status --ignored' performance in repositories with a large\nnumber of files in ignored directories.\n\nFor an example of the performance difference on an example repository with\n196,000 files in 400 ignored directories:\n\n| Command                    |  Time (s) |\n| -------------------------- | --------- |\n| git status                 |   1.2     |\n| git status --ignored (old) |   3.9     |\n| git status --ignored (new) |   1.4     |\n\nSigned-off-by: Jameson Miller <jamill@microsoft.com>\n---\n dir.c | 47 +++++++++++++++++++++++++++++++++++++++++------\n 1 file changed, 41 insertions(+), 6 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex 1c55dc3..1d17b80 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -49,7 +49,7 @@ struct cached_dir {\n static enum path_treatment read_directory_recursive(struct dir_struct *dir,\n \tstruct index_state *istate, const char *path, int len,\n \tstruct untracked_cache_dir *untracked,\n-\tint check_only, const struct pathspec *pathspec);\n+\tint check_only, int stop_at_first_file, const struct pathspec *pathspec);\n static int get_dtype(struct dirent *de, struct index_state *istate,\n \t\t     const char *path, int len);\n \n@@ -1404,8 +1404,13 @@ static enum path_treatment treat_directory(struct dir_struct *dir,\n \n \tuntracked = lookup_untracked(dir->untracked, untracked,\n \t\t\t\t     dirname + baselen, len - baselen);\n+\n+\t/*\n+\t * If this is an excluded directory, then we only need to check if\n+\t * the directory contains any files.\n+\t */\n \treturn read_directory_recursive(dir, istate, dirname, len,\n-\t\t\t\t\tuntracked, 1, pathspec);\n+\t\t\t\t\tuntracked, 1, exclude, pathspec);\n }\n \n /*\n@@ -1633,7 +1638,7 @@ static enum path_treatment treat_path_fast(struct dir_struct *dir,\n \t\t * with check_only set.\n \t\t */\n \t\treturn read_directory_recursive(dir, istate, path->buf, path->len,\n-\t\t\t\t\t\tcdir->ucd, 1, pathspec);\n+\t\t\t\t\t\tcdir->ucd, 1, 0, pathspec);\n \t/*\n \t * We get path_recurse in the first run when\n \t * directory_exists_in_index() returns index_nonexistent. We\n@@ -1793,12 +1798,20 @@ static void close_cached_dir(struct cached_dir *cdir)\n  * Also, we ignore the name \".git\" (even if it is not a directory).\n  * That likely will not change.\n  *\n+ * If 'stop_at_first_file' is specified, 'path_excluded' is returned\n+ * to signal that a file was found. This is the least significant value that\n+ * indicates that a file was encountered that does not depend on the order of\n+ * whether an untracked or exluded path was encountered first.\n+ *\n  * Returns the most significant path_treatment value encountered in the scan.\n+ * If 'stop_at_first_file' is specified, `path_excluded` is the most\n+ * significant path_treatment value that will be returned.\n  */\n+\n static enum path_treatment read_directory_recursive(struct dir_struct *dir,\n \tstruct index_state *istate, const char *base, int baselen,\n \tstruct untracked_cache_dir *untracked, int check_only,\n-\tconst struct pathspec *pathspec)\n+\tint stop_at_first_file, const struct pathspec *pathspec)\n {\n \tstruct cached_dir cdir;\n \tenum path_treatment state, subdir_state, dir_state = path_none;\n@@ -1832,12 +1845,34 @@ static enum path_treatment read_directory_recursive(struct dir_struct *dir,\n \t\t\tsubdir_state =\n \t\t\t\tread_directory_recursive(dir, istate, path.buf,\n \t\t\t\t\t\t\t path.len, ud,\n-\t\t\t\t\t\t\t check_only, pathspec);\n+\t\t\t\t\t\t\t check_only, stop_at_first_file, pathspec);\n \t\t\tif (subdir_state > dir_state)\n \t\t\t\tdir_state = subdir_state;\n \t\t}\n \n \t\tif (check_only) {\n+\t\t\tif (stop_at_first_file) {\n+\t\t\t\t/*\n+\t\t\t\t * If stopping at first file, then\n+\t\t\t\t * signal that a file was found by\n+\t\t\t\t * returning `path_excluded`. This is\n+\t\t\t\t * to return a consistent value\n+\t\t\t\t * regardless of whether an ignored or\n+\t\t\t\t * excluded file happened to be\n+\t\t\t\t * encountered 1st.\n+\t\t\t\t *\n+\t\t\t\t * In current usage, the\n+\t\t\t\t * `stop_at_first_file` is passed when\n+\t\t\t\t * an ancestor directory has matched\n+\t\t\t\t * an exclude pattern, so any found\n+\t\t\t\t * files will be excluded.\n+\t\t\t\t */\n+\t\t\t\tif (dir_state >= path_excluded) {\n+\t\t\t\t\tdir_state = path_excluded;\n+\t\t\t\t\tbreak;\n+\t\t\t\t}\n+\t\t\t}\n+\n \t\t\t/* abort early if maximum state has been reached */\n \t\t\tif (dir_state == path_untracked) {\n \t\t\t\tif (cdir.fdir)\n@@ -2108,7 +2143,7 @@ int read_directory(struct dir_struct *dir, struct index_state *istate,\n \t\t */\n \t\tdir->untracked = NULL;\n \tif (!len || treat_leading_path(dir, istate, path, len, pathspec))\n-\t\tread_directory_recursive(dir, istate, path, len, untracked, 0, pathspec);\n+\t\tread_directory_recursive(dir, istate, path, len, untracked, 0, 0, pathspec);\n \tQSORT(dir->entries, dir->nr, cmp_dir_entry);\n \tQSORT(dir->ignored, dir->ignored_nr, cmp_dir_entry);\n \n-- \n2.7.4\n\n"},{"id":"328341","messageId":"xmqqd16nqsfd.fsf@gitster.mtv.corp.google.com","threadId":"46561","inReplyTo":"1505755473-6720-2-git-send-email-jamill@microsoft.com","subject":"Re: [PATCH v2] Improve performance of git status --ignored","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-09-19T03:27:18Z","receivedAt":"2017-09-19T03:27:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jameson Miller <jameson.miller81@gmail.com> writes:\n\n> Improve the performance of the directory listing logic when it wants to list\n> non-empty ignored directories. In order to show non-empty ignored directories,\n> the existing logic will recursively iterate through all contents of an ignored\n> directory. This change introduces the optimization to stop iterating through\n> the contents once it finds the first file.\n\nWow, such an obviously correct optimization.  Very nicely explained, too.\n\n> This can have a significant\n> improvement in 'git status --ignored' performance in repositories with a large\n> number of files in ignored directories.\n>\n> For an example of the performance difference on an example repository with\n> 196,000 files in 400 ignored directories:\n>\n> | Command                    |  Time (s) |\n> | -------------------------- | --------- |\n> | git status                 |   1.2     |\n> | git status --ignored (old) |   3.9     |\n> | git status --ignored (new) |   1.4     |\n>\n> Signed-off-by: Jameson Miller <jamill@microsoft.com>\n> ---\n\nI wish all the contributions I have to accept are as nicely done as\nthis one ;-)\n\nThanks.\n\n>  dir.c | 47 +++++++++++++++++++++++++++++++++++++++++------\n>  1 file changed, 41 insertions(+), 6 deletions(-)\n>\n> diff --git a/dir.c b/dir.c\n> index 1c55dc3..1d17b80 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -49,7 +49,7 @@ struct cached_dir {\n>  static enum path_treatment read_directory_recursive(struct dir_struct *dir,\n>  \tstruct index_state *istate, const char *path, int len,\n>  \tstruct untracked_cache_dir *untracked,\n> -\tint check_only, const struct pathspec *pathspec);\n> +\tint check_only, int stop_at_first_file, const struct pathspec *pathspec);\n\nWe might want to make check_only and stop_at_first_file into a\nsingle \"unsigned flags\" used as a collection of bits, but we can\nwait until we start feeling the urge to add the third boolean\nparameter to this function (at which point I'd probably demand a\npreliminary clean-up to merge these two into a single flags word\nbefore adding the third one as a new bit in that word).\n\n> @@ -1404,8 +1404,13 @@ static enum path_treatment treat_directory(struct dir_struct *dir,\n>  \n>  \tuntracked = lookup_untracked(dir->untracked, untracked,\n>  \t\t\t\t     dirname + baselen, len - baselen);\n> +\n> +\t/*\n> +\t * If this is an excluded directory, then we only need to check if\n> +\t * the directory contains any files.\n> +\t */\n>  \treturn read_directory_recursive(dir, istate, dirname, len,\n> -\t\t\t\t\tuntracked, 1, pathspec);\n> +\t\t\t\t\tuntracked, 1, exclude, pathspec);\n\nNicely explained in the in-code comment.\n\nI'd assume that you want your microsoft e-mail address used on the\nsigned-off-by line appear as the author, so I'll tweak this a bit to\nmake it so (otherwise, your 81@gmail.com would become the author).\n\nThanks.\n"},{"id":"328397","messageId":"20170919175240.GA94704@google.com","threadId":"46561","inReplyTo":"1505755473-6720-2-git-send-email-jamill@microsoft.com","subject":"Re: [PATCH v2] Improve performance of git status --ignored","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-09-19T17:52:40Z","receivedAt":"2017-09-19T17:52:48Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"On 09/18, Jameson Miller wrote:\n> Improve the performance of the directory listing logic when it wants to list\n> non-empty ignored directories. In order to show non-empty ignored directories,\n> the existing logic will recursively iterate through all contents of an ignored\n> directory. This change introduces the optimization to stop iterating through\n> the contents once it finds the first file. This can have a significant\n> improvement in 'git status --ignored' performance in repositories with a large\n> number of files in ignored directories.\n> \n> For an example of the performance difference on an example repository with\n> 196,000 files in 400 ignored directories:\n> \n> | Command                    |  Time (s) |\n> | -------------------------- | --------- |\n> | git status                 |   1.2     |\n> | git status --ignored (old) |   3.9     |\n> | git status --ignored (new) |   1.4     |\n> \n> Signed-off-by: Jameson Miller <jamill@microsoft.com>\n\nEverything looks good to me.  My only nit (and no need to change it for\nthis patch) is that the first line of the commit msg usually has the\nform:\n\n  <area>: <short summary>\n\nSo that its easy to tell which part of the code a commit is changing.\n\nSeriously, great patch.  Thanks!\n\n> ---\n>  dir.c | 47 +++++++++++++++++++++++++++++++++++++++++------\n>  1 file changed, 41 insertions(+), 6 deletions(-)\n> \n> diff --git a/dir.c b/dir.c\n> index 1c55dc3..1d17b80 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -49,7 +49,7 @@ struct cached_dir {\n>  static enum path_treatment read_directory_recursive(struct dir_struct *dir,\n>  \tstruct index_state *istate, const char *path, int len,\n>  \tstruct untracked_cache_dir *untracked,\n> -\tint check_only, const struct pathspec *pathspec);\n> +\tint check_only, int stop_at_first_file, const struct pathspec *pathspec);\n>  static int get_dtype(struct dirent *de, struct index_state *istate,\n>  \t\t     const char *path, int len);\n>  \n> @@ -1404,8 +1404,13 @@ static enum path_treatment treat_directory(struct dir_struct *dir,\n>  \n>  \tuntracked = lookup_untracked(dir->untracked, untracked,\n>  \t\t\t\t     dirname + baselen, len - baselen);\n> +\n> +\t/*\n> +\t * If this is an excluded directory, then we only need to check if\n> +\t * the directory contains any files.\n> +\t */\n>  \treturn read_directory_recursive(dir, istate, dirname, len,\n> -\t\t\t\t\tuntracked, 1, pathspec);\n> +\t\t\t\t\tuntracked, 1, exclude, pathspec);\n>  }\n>  \n>  /*\n> @@ -1633,7 +1638,7 @@ static enum path_treatment treat_path_fast(struct dir_struct *dir,\n>  \t\t * with check_only set.\n>  \t\t */\n>  \t\treturn read_directory_recursive(dir, istate, path->buf, path->len,\n> -\t\t\t\t\t\tcdir->ucd, 1, pathspec);\n> +\t\t\t\t\t\tcdir->ucd, 1, 0, pathspec);\n>  \t/*\n>  \t * We get path_recurse in the first run when\n>  \t * directory_exists_in_index() returns index_nonexistent. We\n> @@ -1793,12 +1798,20 @@ static void close_cached_dir(struct cached_dir *cdir)\n>   * Also, we ignore the name \".git\" (even if it is not a directory).\n>   * That likely will not change.\n>   *\n> + * If 'stop_at_first_file' is specified, 'path_excluded' is returned\n> + * to signal that a file was found. This is the least significant value that\n> + * indicates that a file was encountered that does not depend on the order of\n> + * whether an untracked or exluded path was encountered first.\n> + *\n>   * Returns the most significant path_treatment value encountered in the scan.\n> + * If 'stop_at_first_file' is specified, `path_excluded` is the most\n> + * significant path_treatment value that will be returned.\n>   */\n> +\n>  static enum path_treatment read_directory_recursive(struct dir_struct *dir,\n>  \tstruct index_state *istate, const char *base, int baselen,\n>  \tstruct untracked_cache_dir *untracked, int check_only,\n> -\tconst struct pathspec *pathspec)\n> +\tint stop_at_first_file, const struct pathspec *pathspec)\n>  {\n>  \tstruct cached_dir cdir;\n>  \tenum path_treatment state, subdir_state, dir_state = path_none;\n> @@ -1832,12 +1845,34 @@ static enum path_treatment read_directory_recursive(struct dir_struct *dir,\n>  \t\t\tsubdir_state =\n>  \t\t\t\tread_directory_recursive(dir, istate, path.buf,\n>  \t\t\t\t\t\t\t path.len, ud,\n> -\t\t\t\t\t\t\t check_only, pathspec);\n> +\t\t\t\t\t\t\t check_only, stop_at_first_file, pathspec);\n>  \t\t\tif (subdir_state > dir_state)\n>  \t\t\t\tdir_state = subdir_state;\n>  \t\t}\n>  \n>  \t\tif (check_only) {\n> +\t\t\tif (stop_at_first_file) {\n> +\t\t\t\t/*\n> +\t\t\t\t * If stopping at first file, then\n> +\t\t\t\t * signal that a file was found by\n> +\t\t\t\t * returning `path_excluded`. This is\n> +\t\t\t\t * to return a consistent value\n> +\t\t\t\t * regardless of whether an ignored or\n> +\t\t\t\t * excluded file happened to be\n> +\t\t\t\t * encountered 1st.\n> +\t\t\t\t *\n> +\t\t\t\t * In current usage, the\n> +\t\t\t\t * `stop_at_first_file` is passed when\n> +\t\t\t\t * an ancestor directory has matched\n> +\t\t\t\t * an exclude pattern, so any found\n> +\t\t\t\t * files will be excluded.\n> +\t\t\t\t */\n> +\t\t\t\tif (dir_state >= path_excluded) {\n> +\t\t\t\t\tdir_state = path_excluded;\n> +\t\t\t\t\tbreak;\n> +\t\t\t\t}\n> +\t\t\t}\n> +\n>  \t\t\t/* abort early if maximum state has been reached */\n>  \t\t\tif (dir_state == path_untracked) {\n>  \t\t\t\tif (cdir.fdir)\n> @@ -2108,7 +2143,7 @@ int read_directory(struct dir_struct *dir, struct index_state *istate,\n>  \t\t */\n>  \t\tdir->untracked = NULL;\n>  \tif (!len || treat_leading_path(dir, istate, path, len, pathspec))\n> -\t\tread_directory_recursive(dir, istate, path, len, untracked, 0, pathspec);\n> +\t\tread_directory_recursive(dir, istate, path, len, untracked, 0, 0, pathspec);\n>  \tQSORT(dir->entries, dir->nr, cmp_dir_entry);\n>  \tQSORT(dir->ignored, dir->ignored_nr, cmp_dir_entry);\n>  \n> -- \n> 2.7.4\n> \n\n-- \nBrandon Williams\n"}]}