{"thread":{"id":"40959","subject":"[PATCH 0/8] Untracked cache improvements","startedAt":"2015-12-08T17:15:08Z","lastAt":"2015-12-27T20:21:22Z","messageCount":54,"participants":["Christian Couder","Junio C Hamano","Torsten Bögershausen","Duy Nguyen","Ævar Arnfjörð Bjarmason","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":8},"messages":[{"id":"274175","messageId":"1449594916-21167-1-git-send-email-chriscool@tuxfamily.org","threadId":"40959","inReplyTo":null,"subject":"[PATCH 0/8] Untracked cache improvements","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2015-12-08T17:15:08Z","receivedAt":"2015-12-08T17:15:08Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Following the previous RFC version of this patch series and the\nrelated discussions, here is a new version that tries to improve the\nuntracked cache feature.\n\nThis patch series implements core.untrackedCache as a bool config\nvariable. When it's true git should always try to use the untracked\ncache, and when false git should never use it.\n\nPatchs 1/8 and 3/8 add some features that are missing.\n\nPatch 2/8, which is new, adds an enum as suggested by Duy.\n\nPatchs 4/8, 5/8 and 6/8 are some refactoring to prepare for patch 7/8\nwhich implements core.untrackedCache.\n\nPatch 7/8 is the result of squashing the last 3 patches of the RFC\nseries. As discussed this sacrifies backward compatibility for a\nsimpler interface.\n\nPatch 8/8, which is new, add some tests.\n\nSo the changes compared to the RFC version are just small bug fixes\nand patchs 2/8 and 8/8.\n\nThe patch series is also available there:\n\nhttps://github.com/chriscool/git/tree/uc-notifs14\n\nChristian Couder (8):\n update-index: add untracked cache notifications\n update-index: use enum for untracked cache options\n update-index: add --test-untracked-cache\n update-index: move 'uc' var declaration\n dir: add add_untracked_cache()\n dir: add remove_untracked_cache()\n config: add core.untrackedCache\n t7063: add tests for core.untrackedCache\n\n Documentation/config.txt | 7 +++++\n Documentation/git-update-index.txt | 31 ++++++++++++++------\n builtin/update-index.c | 52 +++++++++++++++++++---------------\n cache.h | 1 +\n config.c | 4 +++\n contrib/completion/git-completion.bash | 1 +\n dir.c | 22 +++++++++++++-\n dir.h | 2 ++\n environment.c | 1 +\n t/t7063-status-untracked-cache.sh | 52 ++++++++++++++++++++++++++++++----\n wt-status.c | 9 ++++++\n 11 files changed, 145 insertions(+), 37 deletions(-)\n\n-- \n2.6.3.478.g9f95483.dirty\n"},{"id":"274177","messageId":"1449594916-21167-2-git-send-email-chriscool@tuxfamily.org","threadId":"40959","inReplyTo":"1449594916-21167-1-git-send-email-chriscool@tuxfamily.org","subject":"[PATCH 1/8] update-index: add untracked cache notifications","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2015-12-08T17:15:09Z","receivedAt":"2015-12-08T17:15:09Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Doing:\n\n cd /tmp\n git --git-dir=/git/somewhere/else/.git update-index --untracked-cache\n\ndoesn't work how one would expect. It hardcodes \"/tmp\" as the directory\nthat \"works\" into the index, so if you use the working tree, you'll\nnever use the untracked cache.\n\nWith this patch \"git update-index --untracked-cache\" tells the user in\nwhich directory tests are performed and in which working directory the\nuntracked cache is allowed.\n\nSigned-off-by: Christian Couder \n---\n builtin/update-index.c | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex 7431938..6f6b289 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -121,7 +121,7 @@ static int test_if_untracked_cache_is_supported(void)\n \tif (!mkdtemp(mtime_dir.buf))\n \t\tdie_errno(\"Could not make temporary directory\");\n \n-\tfprintf(stderr, _(\"Testing \"));\n+\tfprintf(stderr, _(\"Testing mtime in '%s' \"), xgetcwd());\n \tatexit(remove_test_directory);\n \txstat_mtime_dir(&st);\n \tfill_stat_data(&base, &st);\n@@ -1122,9 +1122,11 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n \t\t}\n \t\tadd_untracked_ident(the_index.untracked);\n \t\tthe_index.cache_changed |= UNTRACKED_CHANGED;\n+\t\tfprintf(stderr, _(\"Untracked cache enabled for '%s'\\n\"), get_git_work_tree());\n \t} else if (!untracked_cache && the_index.untracked) {\n \t\tthe_index.untracked = NULL;\n \t\tthe_index.cache_changed |= UNTRACKED_CHANGED;\n+\t\tfprintf(stderr, _(\"Untracked cache disabled\\n\"));\n \t}\n \n \tif (active_cache_changed) {\n-- \n2.6.3.478.g9f95483.dirty\n"},{"id":"274176","messageId":"1449594916-21167-3-git-send-email-chriscool@tuxfamily.org","threadId":"40959","inReplyTo":"1449594916-21167-1-git-send-email-chriscool@tuxfamily.org","subject":"[PATCH 2/8] update-index: use enum for untracked cache options","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2015-12-08T17:15:10Z","receivedAt":"2015-12-08T17:15:10Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Signed-off-by: Christian Couder \n---\n builtin/update-index.c | 18 +++++++++++++-----\n 1 file changed, 13 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex 6f6b289..246b3d3 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -35,6 +35,14 @@ static int mark_skip_worktree_only;\n #define UNMARK_FLAG 2\n static struct strbuf mtime_dir = STRBUF_INIT;\n \n+/* Untracked cache mode */\n+enum uc_mode {\n+\tUNDEF_UC = -1,\n+\tNO_UC = 0,\n+\tUC,\n+\tFORCE_UC\n+};\n+\n __attribute__((format (printf, 1, 2)))\n static void report(const char *fmt, ...)\n {\n@@ -902,7 +910,7 @@ static int reupdate_callback(struct parse_opt_ctx_t *ctx,\n int cmd_update_index(int argc, const char **argv, const char *prefix)\n {\n \tint newfd, entries, has_errors = 0, line_termination = '\\n';\n-\tint untracked_cache = -1;\n+\tenum uc_mode untracked_cache = UNDEF_UC;\n \tint read_from_stdin = 0;\n \tint prefix_length = prefix ? strlen(prefix) : 0;\n \tint preferred_index_format = 0;\n@@ -997,7 +1005,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL(0, \"untracked-cache\", &untracked_cache,\n \t\t\tN_(\"enable/disable untracked cache\")),\n \t\tOPT_SET_INT(0, \"force-untracked-cache\", &untracked_cache,\n-\t\t\t N_(\"enable untracked cache without testing the filesystem\"), 2),\n+\t\t\t N_(\"enable untracked cache without testing the filesystem\"), FORCE_UC),\n \t\tOPT_END()\n \t};\n \n@@ -1104,10 +1112,10 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n \t\tthe_index.split_index = NULL;\n \t\tthe_index.cache_changed |= SOMETHING_CHANGED;\n \t}\n-\tif (untracked_cache > 0) {\n+\tif (untracked_cache > NO_UC) {\n \t\tstruct untracked_cache *uc;\n \n-\t\tif (untracked_cache < 2) {\n+\t\tif (untracked_cache < FORCE_UC) {\n \t\t\tsetup_work_tree();\n \t\t\tif (!test_if_untracked_cache_is_supported())\n \t\t\t\treturn 1;\n@@ -1123,7 +1131,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n \t\tadd_untracked_ident(the_index.untracked);\n \t\tthe_index.cache_changed |= UNTRACKED_CHANGED;\n \t\tfprintf(stderr, _(\"Untracked cache enabled for '%s'\\n\"), get_git_work_tree());\n-\t} else if (!untracked_cache && the_index.untracked) {\n+\t} else if (untracked_cache == NO_UC && the_index.untracked) {\n \t\tthe_index.untracked = NULL;\n \t\tthe_index.cache_changed |= UNTRACKED_CHANGED;\n \t\tfprintf(stderr, _(\"Untracked cache disabled\\n\"));\n-- \n2.6.3.478.g9f95483.dirty\n"},{"id":"274179","messageId":"1449594916-21167-4-git-send-email-chriscool@tuxfamily.org","threadId":"40959","inReplyTo":"1449594916-21167-1-git-send-email-chriscool@tuxfamily.org","subject":"[PATCH 3/8] update-index: add --test-untracked-cache","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2015-12-08T17:15:11Z","receivedAt":"2015-12-08T17:15:11Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"It is nice to just be able to test if untracked cache is\nsupported without enabling it.\n\nSigned-off-by: Christian Couder \n---\n Documentation/git-update-index.txt | 9 ++++++++-\n builtin/update-index.c | 5 +++++\n 2 files changed, 13 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt\nindex 3df9c26..0ff7396 100644\n--- a/Documentation/git-update-index.txt\n+++ b/Documentation/git-update-index.txt\n@@ -17,7 +17,7 @@ SYNOPSIS\n \t [--[no-]assume-unchanged]\n \t [--[no-]skip-worktree]\n \t [--ignore-submodules]\n-\t [--[no-|force-]untracked-cache]\n+\t [--[no-|test-|force-]untracked-cache]\n \t [--really-refresh] [--unresolve] [--again | -g]\n \t [--info-only] [--index-info]\n \t [-z] [--stdin] [--index-version ]\n@@ -179,6 +179,13 @@ may not support it yet.\n \tsystem must change `st_mtime` field of a directory if files\n \tare added or deleted in that directory.\n \n+--test-untracked-cache::\n+\tOnly perform tests on the working directory to make sure\n+\tuntracked cache can be used. You have to manually enable\n+\tuntracked cache using `--force-untracked-cache` (or\n+\t`--untracked-cache` but this will run the tests again)\n+\tafterwards if you really want to use it.\n+\n --force-untracked-cache::\n \tFor safety, `--untracked-cache` performs tests on the working\n \tdirectory to make sure untracked cache can be used. These\ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex 246b3d3..ecb685d 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -40,6 +40,7 @@ enum uc_mode {\n \tUNDEF_UC = -1,\n \tNO_UC = 0,\n \tUC,\n+\tTEST_UC,\n \tFORCE_UC\n };\n \n@@ -1004,6 +1005,8 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n \t\t\tN_(\"enable or disable split index\")),\n \t\tOPT_BOOL(0, \"untracked-cache\", &untracked_cache,\n \t\t\tN_(\"enable/disable untracked cache\")),\n+\t\tOPT_SET_INT(0, \"test-untracked-cache\", &untracked_cache,\n+\t\t\t N_(\"test if the filesystem supports untracked cache\"), TEST_UC),\n \t\tOPT_SET_INT(0, \"force-untracked-cache\", &untracked_cache,\n \t\t\t N_(\"enable untracked cache without testing the filesystem\"), FORCE_UC),\n \t\tOPT_END()\n@@ -1119,6 +1122,8 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n \t\t\tsetup_work_tree();\n \t\t\tif (!test_if_untracked_cache_is_supported())\n \t\t\t\treturn 1;\n+\t\t\tif (untracked_cache == TEST_UC)\n+\t\t\t\treturn 0;\n \t\t}\n \t\tif (!the_index.untracked) {\n \t\t\tuc = xcalloc(1, sizeof(*uc));\n-- \n2.6.3.478.g9f95483.dirty\n"},{"id":"274178","messageId":"1449594916-21167-5-git-send-email-chriscool@tuxfamily.org","threadId":"40959","inReplyTo":"1449594916-21167-1-git-send-email-chriscool@tuxfamily.org","subject":"[PATCH 4/8] update-index: move 'uc' var declaration","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2015-12-08T17:15:12Z","receivedAt":"2015-12-08T17:15:12Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Signed-off-by: Christian Couder \n---\n builtin/update-index.c | 4 +---\n 1 file changed, 1 insertion(+), 3 deletions(-)\n\ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex ecb685d..21f74b2 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -1116,8 +1116,6 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n \t\tthe_index.cache_changed |= SOMETHING_CHANGED;\n \t}\n \tif (untracked_cache > NO_UC) {\n-\t\tstruct untracked_cache *uc;\n-\n \t\tif (untracked_cache < FORCE_UC) {\n \t\t\tsetup_work_tree();\n \t\t\tif (!test_if_untracked_cache_is_supported())\n@@ -1126,7 +1124,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n \t\t\t\treturn 0;\n \t\t}\n \t\tif (!the_index.untracked) {\n-\t\t\tuc = xcalloc(1, sizeof(*uc));\n+\t\t\tstruct untracked_cache *uc = xcalloc(1, sizeof(*uc));\n \t\t\tstrbuf_init(&uc->ident, 100);\n \t\t\tuc->exclude_per_dir = \".gitignore\";\n \t\t\t/* should be the same flags used by git-status */\n-- \n2.6.3.478.g9f95483.dirty\n"},{"id":"274180","messageId":"1449594916-21167-6-git-send-email-chriscool@tuxfamily.org","threadId":"40959","inReplyTo":"1449594916-21167-1-git-send-email-chriscool@tuxfamily.org","subject":"[PATCH 5/8] dir: add add_untracked_cache()","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2015-12-08T17:15:13Z","receivedAt":"2015-12-08T17:15:13Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"This new function will be used in a later patch.\n\nSigned-off-by: Christian Couder \n---\n builtin/update-index.c | 11 +----------\n dir.c | 14 ++++++++++++++\n dir.h | 1 +\n 3 files changed, 16 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex 21f74b2..40530b0 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -1123,16 +1123,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n \t\t\tif (untracked_cache == TEST_UC)\n \t\t\t\treturn 0;\n \t\t}\n-\t\tif (!the_index.untracked) {\n-\t\t\tstruct untracked_cache *uc = xcalloc(1, sizeof(*uc));\n-\t\t\tstrbuf_init(&uc->ident, 100);\n-\t\t\tuc->exclude_per_dir = \".gitignore\";\n-\t\t\t/* should be the same flags used by git-status */\n-\t\t\tuc->dir_flags = DIR_SHOW_OTHER_DIRECTORIES | DIR_HIDE_EMPTY_DIRECTORIES;\n-\t\t\tthe_index.untracked = uc;\n-\t\t}\n-\t\tadd_untracked_ident(the_index.untracked);\n-\t\tthe_index.cache_changed |= UNTRACKED_CHANGED;\n+\t\tadd_untracked_cache();\n \t\tfprintf(stderr, _(\"Untracked cache enabled for '%s'\\n\"), get_git_work_tree());\n \t} else if (untracked_cache == NO_UC && the_index.untracked) {\n \t\tthe_index.untracked = NULL;\ndiff --git a/dir.c b/dir.c\nindex d2a8f06..0f7e293 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1938,6 +1938,20 @@ void add_untracked_ident(struct untracked_cache *uc)\n \tstrbuf_addch(&uc->ident, 0);\n }\n \n+void add_untracked_cache(void)\n+{\n+\tif (!the_index.untracked) {\n+\t\tstruct untracked_cache *uc = xcalloc(1, sizeof(*uc));\n+\t\tstrbuf_init(&uc->ident, 100);\n+\t\tuc->exclude_per_dir = \".gitignore\";\n+\t\t/* should be the same flags used by git-status */\n+\t\tuc->dir_flags = DIR_SHOW_OTHER_DIRECTORIES | DIR_HIDE_EMPTY_DIRECTORIES;\n+\t\tthe_index.untracked = uc;\n+\t}\n+\tadd_untracked_ident(the_index.untracked);\n+\tthe_index.cache_changed |= UNTRACKED_CHANGED;\n+}\n+\n static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct *dir,\n \t\t\t\t\t\t int base_len,\n \t\t\t\t\t\t const struct pathspec *pathspec)\ndiff --git a/dir.h b/dir.h\nindex 7b5855d..ee94c76 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -308,4 +308,5 @@ void free_untracked_cache(struct untracked_cache *);\n struct untracked_cache *read_untracked_extension(const void *data, unsigned long sz);\n void write_untracked_extension(struct strbuf *out, struct untracked_cache *untracked);\n void add_untracked_ident(struct untracked_cache *);\n+void add_untracked_cache(void);\n #endif\n-- \n2.6.3.478.g9f95483.dirty\n"},{"id":"274182","messageId":"1449594916-21167-7-git-send-email-chriscool@tuxfamily.org","threadId":"40959","inReplyTo":"1449594916-21167-1-git-send-email-chriscool@tuxfamily.org","subject":"[PATCH 6/8] dir: add remove_untracked_cache()","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2015-12-08T17:15:14Z","receivedAt":"2015-12-08T17:15:14Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"This new function will be used in a later patch.\n\nSigned-off-by: Christian Couder \n---\n builtin/update-index.c | 3 +--\n dir.c | 6 ++++++\n dir.h | 1 +\n 3 files changed, 8 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex 40530b0..e427657 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -1126,8 +1126,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n \t\tadd_untracked_cache();\n \t\tfprintf(stderr, _(\"Untracked cache enabled for '%s'\\n\"), get_git_work_tree());\n \t} else if (untracked_cache == NO_UC && the_index.untracked) {\n-\t\tthe_index.untracked = NULL;\n-\t\tthe_index.cache_changed |= UNTRACKED_CHANGED;\n+\t\tremove_untracked_cache();\n \t\tfprintf(stderr, _(\"Untracked cache disabled\\n\"));\n \t}\n \ndiff --git a/dir.c b/dir.c\nindex 0f7e293..ffc0286 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1952,6 +1952,12 @@ void add_untracked_cache(void)\n \tthe_index.cache_changed |= UNTRACKED_CHANGED;\n }\n \n+void remove_untracked_cache(void)\n+{\n+\tthe_index.untracked = NULL;\n+\tthe_index.cache_changed |= UNTRACKED_CHANGED;\n+}\n+\n static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct *dir,\n \t\t\t\t\t\t int base_len,\n \t\t\t\t\t\t const struct pathspec *pathspec)\ndiff --git a/dir.h b/dir.h\nindex ee94c76..3e5114d 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -309,4 +309,5 @@ struct untracked_cache *read_untracked_extension(const void *data, unsigned long\n void write_untracked_extension(struct strbuf *out, struct untracked_cache *untracked);\n void add_untracked_ident(struct untracked_cache *);\n void add_untracked_cache(void);\n+void remove_untracked_cache(void);\n #endif\n-- \n2.6.3.478.g9f95483.dirty\n"},{"id":"274181","messageId":"1449594916-21167-8-git-send-email-chriscool@tuxfamily.org","threadId":"40959","inReplyTo":"1449594916-21167-1-git-send-email-chriscool@tuxfamily.org","subject":"[PATCH 7/8] config: add core.untrackedCache","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2015-12-08T17:15:15Z","receivedAt":"2015-12-08T17:15:15Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"When we know that mtime is fully supported by the environment, we\nmight want the untracked cache to be always used by default without\nany mtime test or kernel version check being performed.\n\nAlso when we know that mtime is not supported by the environment,\nfor example because the repo is shared over a network file system,\nthen we might want 'git update-index --untracked-cache' to fail\nimmediately instead of preforming tests (because it might work on\nsome systems using the repo over the network file system but not\nothers).\n\nThe normal way to achieve the above in Git is to use a config\nvariable. That's why this patch introduces \"core.untrackedCache\".\n\nTo keep things simple, this variable is a bool which default to\nfalse.\n\nWhen \"git status\" is run, it now adds or removes the untracked\ncache in the index to respect the value of this variable.\n\nThis means that `git update-index --[no-|force-]untracked-cache`,\nto be compatible with the new config variable, have to set or\nunset it. This new behavior is backward incompatible change, but\nthat is deliberate.\n\nAlso `--untracked-cache` used to check that the underlying\noperating system and file system change `st_mtime` field of a\ndirectory if files are added or deleted in that directory. But\nthose tests take a long time and there is now\n`--test-untracked-cache` to perform them.\n\nSo to be more consistent with other git commands, this patch\nprevents `--untracked-cache` to perform tests. This means that\nafter this patch there is no difference any more between\n`--untracked-cache` and `--force-untracked-cache`.\n\nSigned-off-by: Christian Couder \n---\n Documentation/config.txt | 7 +++++++\n Documentation/git-update-index.txt | 28 ++++++++++++++++++----------\n builtin/update-index.c | 23 +++++++++++++----------\n cache.h | 1 +\n config.c | 4 ++++\n contrib/completion/git-completion.bash | 1 +\n dir.c | 2 +-\n environment.c | 1 +\n t/t7063-status-untracked-cache.sh | 4 +---\n wt-status.c | 9 +++++++++\n 10 files changed, 56 insertions(+), 24 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 2d06b11..94820eb 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -308,6 +308,13 @@ core.trustctime::\n \tcrawlers and some backup systems).\n \tSee linkgit:git-update-index[1]. True by default.\n \n+core.untrackedCache::\n+\tDetermines if untracked cache will be enabled. Using\n+\t'git update-index --[no-|force-]untracked-cache' will set\n+\tthis variable. Before setting it to true, you should check\n+\tthat mtime is working properly on your system.\n+\tSee linkgit:git-update-index[1]. False by default.\n+\n core.checkStat::\n \tDetermines which stat fields to match between the index\n \tand work tree. The user can set this to 'default' or\ndiff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt\nindex 0ff7396..0fb39db 100644\n--- a/Documentation/git-update-index.txt\n+++ b/Documentation/git-update-index.txt\n@@ -175,22 +175,28 @@ may not support it yet.\n --no-untracked-cache::\n \tEnable or disable untracked cache extension. This could speed\n \tup for commands that involve determining untracked files such\n-\tas `git status`. The underlying operating system and file\n-\tsystem must change `st_mtime` field of a directory if files\n-\tare added or deleted in that directory.\n+\tas `git status`.\n++\n+The underlying operating system and file system must change `st_mtime`\n+field of a directory if files are added or deleted in that\n+directory. You can test that using\n+`--test-untracked-cache`. `--untracked-cache` used to test that too\n+but it doesn't anymore.\n++\n+This sets the `core.untrackedCache` configuration variable to 'true'\n+or 'false' in the repo config file, (see linkgit:git-config[1]), so\n+that the untracked cache stays enabled or disabled.\n \n --test-untracked-cache::\n \tOnly perform tests on the working directory to make sure\n \tuntracked cache can be used. You have to manually enable\n-\tuntracked cache using `--force-untracked-cache` (or\n-\t`--untracked-cache` but this will run the tests again)\n-\tafterwards if you really want to use it.\n+\tuntracked cache using `--untracked-cache` or\n+\t`--force-untracked-cache` or the `core.untrackedCache`\n+\tconfiguration variable afterwards if you really want to use\n+\tit.\n \n --force-untracked-cache::\n-\tFor safety, `--untracked-cache` performs tests on the working\n-\tdirectory to make sure untracked cache can be used. These\n-\ttests can take a few seconds. `--force-untracked-cache` can be\n-\tused to skip the tests.\n+\tSame as `--untracked-cache`.\n \n \\--::\n \tDo not interpret any more arguments as options.\n@@ -406,6 +412,8 @@ It can be useful when the inode change time is regularly modified by\n something outside Git (file system crawlers and backup systems use\n ctime for marking files processed) (see linkgit:git-config[1]).\n \n+Untracked cache look at `core.untrackedCache` configuration variable\n+(see linkgit:git-config[1]).\n \n SEE ALSO\n --------\ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex e427657..7fe3a86 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -1115,19 +1115,22 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n \t\tthe_index.split_index = NULL;\n \t\tthe_index.cache_changed |= SOMETHING_CHANGED;\n \t}\n+\tif (untracked_cache == TEST_UC) {\n+\t\tsetup_work_tree();\n+\t\treturn !test_if_untracked_cache_is_supported();\n+\t}\n \tif (untracked_cache > NO_UC) {\n-\t\tif (untracked_cache < FORCE_UC) {\n-\t\t\tsetup_work_tree();\n-\t\t\tif (!test_if_untracked_cache_is_supported())\n-\t\t\t\treturn 1;\n-\t\t\tif (untracked_cache == TEST_UC)\n-\t\t\t\treturn 0;\n-\t\t}\n+\t\tif (!use_untracked_cache && git_config_set(\"core.untrackedCache\", \"true\"))\n+\t\t\tdie(\"could not set core.untrackedCache to true\");\n \t\tadd_untracked_cache();\n \t\tfprintf(stderr, _(\"Untracked cache enabled for '%s'\\n\"), get_git_work_tree());\n-\t} else if (untracked_cache == NO_UC && the_index.untracked) {\n-\t\tremove_untracked_cache();\n-\t\tfprintf(stderr, _(\"Untracked cache disabled\\n\"));\n+\t} else if (untracked_cache == NO_UC) {\n+\t\tif (use_untracked_cache > 0 && git_config_set(\"core.untrackedCache\", \"false\"))\n+\t\t\tdie(\"could not set core.untrackedCache to false\");\n+\t\tif (the_index.untracked) {\n+\t\t\tremove_untracked_cache();\n+\t\t\tfprintf(stderr, _(\"Untracked cache disabled\\n\"));\n+\t\t}\n \t}\n \n \tif (active_cache_changed) {\ndiff --git a/cache.h b/cache.h\nindex 2a9e902..0cc2c2f 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -619,6 +619,7 @@ extern void set_alternate_index_output(const char *);\n /* Environment bits from configuration mechanism */\n extern int trust_executable_bit;\n extern int trust_ctime;\n+extern int use_untracked_cache;\n extern int check_stat;\n extern int quote_path_fully;\n extern int has_symlinks;\ndiff --git a/config.c b/config.c\nindex 248a21a..f023ee7 100644\n--- a/config.c\n+++ b/config.c\n@@ -691,6 +691,10 @@ static int git_default_core_config(const char *var, const char *value)\n \t\ttrust_ctime = git_config_bool(var, value);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(var, \"core.untrackedcache\")) {\n+\t\tuse_untracked_cache = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n \tif (!strcmp(var, \"core.checkstat\")) {\n \t\tif (!strcasecmp(value, \"default\"))\n \t\t\tcheck_stat = 1;\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 111b053..b7e5736 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -2054,6 +2054,7 @@ _git_config ()\n \t\tcore.sparseCheckout\n \t\tcore.symlinks\n \t\tcore.trustctime\n+\t\tcore.untrackedCache\n \t\tcore.warnAmbiguousRefs\n \t\tcore.whitespace\n \t\tcore.worktree\ndiff --git a/dir.c b/dir.c\nindex ffc0286..aa07aca 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -2014,7 +2014,7 @@ static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct *d\n \tif (dir->exclude_list_group[EXC_CMDL].nr)\n \t\treturn NULL;\n \n-\tif (!ident_in_untracked(dir->untracked)) {\n+\tif (use_untracked_cache != 1 && !ident_in_untracked(dir->untracked)) {\n \t\twarning(_(\"Untracked cache is disabled on this system.\"));\n \t\treturn NULL;\n \t}\ndiff --git a/environment.c b/environment.c\nindex 2da7fe2..9ca71b1 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -14,6 +14,7 @@\n \n int trust_executable_bit = 1;\n int trust_ctime = 1;\n+int use_untracked_cache;\n int check_stat = 1;\n int has_symlinks = 1;\n int minimum_abbrev = 4, default_abbrev = 7;\ndiff --git a/t/t7063-status-untracked-cache.sh b/t/t7063-status-untracked-cache.sh\nindex 0e8d0d4..253160a 100755\n--- a/t/t7063-status-untracked-cache.sh\n+++ b/t/t7063-status-untracked-cache.sh\n@@ -8,10 +8,8 @@ avoid_racy() {\n \tsleep 1\n }\n \n-# It's fine if git update-index returns an error code other than one,\n-# it'll be caught in the first test.\n test_lazy_prereq UNTRACKED_CACHE '\n-\t{ git update-index --untracked-cache; ret=$?; } &&\n+\t{ git update-index --test-untracked-cache; ret=$?; } &&\n \ttest $ret -ne 1\n '\n \ndiff --git a/wt-status.c b/wt-status.c\nindex 435fc28..3e0fe02 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -586,6 +586,15 @@ 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 (!dir.untracked && use_untracked_cache == 1) {\n+\t\tadd_untracked_cache();\n+\t\tdir.untracked = the_index.untracked;\n+\t} else if (dir.untracked && use_untracked_cache == 0) {\n+\t\tremove_untracked_cache();\n+\t\tdir.untracked = NULL;\n+\t}\n+\n \tsetup_standard_excludes(&dir);\n \n \tfill_directory(&dir, &s->pathspec);\n-- \n2.6.3.478.g9f95483.dirty\n"},{"id":"274183","messageId":"1449594916-21167-9-git-send-email-chriscool@tuxfamily.org","threadId":"40959","inReplyTo":"1449594916-21167-1-git-send-email-chriscool@tuxfamily.org","subject":"[PATCH 8/8] t7063: add tests for core.untrackedCache","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2015-12-08T17:15:16Z","receivedAt":"2015-12-08T17:15:16Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Signed-off-by: Christian Couder \n---\n t/t7063-status-untracked-cache.sh | 48 +++++++++++++++++++++++++++++++++++++--\n 1 file changed, 46 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t7063-status-untracked-cache.sh b/t/t7063-status-untracked-cache.sh\nindex 253160a..f0af41c 100755\n--- a/t/t7063-status-untracked-cache.sh\n+++ b/t/t7063-status-untracked-cache.sh\n@@ -18,6 +18,10 @@ if ! test_have_prereq UNTRACKED_CACHE; then\n \ttest_done\n fi\n \n+test_expect_success 'core.untrackedCache is unset' '\n+\ttest_must_fail git config --get core.untrackedCache\n+'\n+\n test_expect_success 'setup' '\n \tgit init worktree &&\n \tcd worktree &&\n@@ -28,6 +32,11 @@ test_expect_success 'setup' '\n \tgit update-index --untracked-cache\n '\n \n+test_expect_success 'core.untrackedCache is true' '\n+\tUC=$(git config core.untrackedCache) &&\n+\ttest \"$UC\" = \"true\"\n+'\n+\n test_expect_success 'untracked cache is empty' '\n \ttest-dump-untracked-cache >../actual &&\n \tcat >../expect <../actual &&\n-\tcat >../expect <../expect-from-test-dump <../actual &&\n+\techo \"no untracked cache\" >../expect &&\n+\ttest_cmp ../expect ../actual\n+'\n+\n+test_expect_success 'git status does not change anything' '\n+\tgit status &&\n+\ttest-dump-untracked-cache >../actual &&\n+\ttest_cmp ../expect ../actual &&\n+\tUC=$(git config core.untrackedCache) &&\n+\ttest \"$UC\" = \"false\"\n+'\n+\n+test_expect_success 'setting core.untrackedCache and using git status creates the cache' '\n+\tgit config core.untrackedCache true &&\n+\ttest-dump-untracked-cache >../actual &&\n+\ttest_cmp ../expect ../actual &&\n+\tgit status &&\n+\ttest-dump-untracked-cache >../actual &&\n+\ttest_cmp ../expect-from-test-dump ../actual\n+'\n+\n+test_expect_success 'unsetting core.untrackedCache and using git status removes the cache' '\n+\tgit config --unset core.untrackedCache &&\n+\ttest-dump-untracked-cache >../actual &&\n+\ttest_cmp ../expect-from-test-dump ../actual &&\n+\tgit status &&\n+\ttest-dump-untracked-cache >../actual &&\n+\ttest_cmp ../expect ../actual\n+'\n+\n test_done\n-- \n2.6.3.478.g9f95483.dirty\n"},{"id":"274188","messageId":"xmqq6108lq3w.fsf@gitster.mtv.corp.google.com","threadId":"40959","inReplyTo":"1449594916-21167-2-git-send-email-chriscool@tuxfamily.org","subject":"Re: [PATCH 1/8] update-index: add untracked cache notifications","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-12-08T19:03:47Z","receivedAt":"2015-12-08T19:03:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> Doing:\n>\n> cd /tmp\n> git --git-dir=/git/somewhere/else/.git update-index --untracked-cache\n>\n> doesn't work how one would expect. It hardcodes \"/tmp\" as the directory\n> that \"works\" into the index, so if you use the working tree, you'll\n> never use the untracked cache.\n\nI think your \"expectation\" needs to be more explicitly spelled out.\n\n\"git -C /tmp --git-dir=/git/somewhere/else/.git\" is a valid way to\nuse that repository you have in somewhere else to track things under\n/tmp/ (as you are only passing GIT_DIR but not GIT_WORK_TREE, the\ncwd, i.e. /tmp, is the root level of the working tree), and for such\na usage, the above command works as expected. Perhaps\n\n Attempting to flip the untracked-cache feature on for a random index\n file with\n\n cd /random/unrelated/place\n git --git-dir=/somewhere/else/.git update-index --untracked-cache\n\n would not work as you might expect. Because flipping the\n feature on in the index also records the location of the\n corresponding working tree (/random/unrelated/place in the above\n example), when the index is subsequently used to keep track of\n files in the working tree in /somewhere/else, the feature is\n disabled.\n\nmay be an improvement.\n\nThe index already implicitly records where the working tree was and\nthat is not limited to untracked-cache option. For example, if you\nhave your repository and its working tree in /git/somewhere/else,\nwhich does not have a path X, then doing:\n\n cd /tmp && >tmp/X\n git --git-dir=/git/somewhere/else/.git update-index --add X\n\nwould store X taken from /tmp in the index, so subsequent use of the\nindex \"knows\" about X that was taken outside /git/somewhere/else/\nafter the above command finishes and the subsequent use is made\nwithout the --git-dir parameter, e.g.\n\n cd /git/somewhere/else/ && git diff-index --cached HEAD'\n\nwould say that you added X, even though /git/somewhere/else/ may not\nhave that X at all. And this is not limited to update-index,\neither. You can temporarily use --git-dir with \"git add X\" and the\nresult would persist the same way in the index.\n\nI think the moral of the story is that you are not expected to\nrandomly use git-dir and git-work-tree to point at different places\nwithout knowing what you are doing, and we may need a way to help\npeople understand what is going on when it is done by a mistake.\n\nThe patch to show result from xgetcwd() and get_git_work_tree() may\nbe a step in the right direction, but if the user is not doing\nanything fancy, \"Testing mtime in /long/path/to/the/directory\" would\nbe irritatingly verbose.\n\nI wonder if it is easy to tell when the user is doing such an\nunnatural thing. Off the top of my head, when the working tree is\nanywhere other than one level above $GIT_DIR, the user is doing\nsomething unusual; I do not know if there are cases where the user\nis doing something unnatural if $GIT_WORK_TREE is one level above\n$GIT_DIR, though.\n\n> With this patch \"git update-index --untracked-cache\" tells the user in\n> which directory tests are performed and in which working directory the\n> untracked cache is allowed.\n>\n> Signed-off-by: Christian Couder \n> ---\n> builtin/update-index.c | 4 +++-\n> 1 file changed, 3 insertions(+), 1 deletion(-)\n>\n> diff --git a/builtin/update-index.c b/builtin/update-index.c\n> index 7431938..6f6b289 100644\n> --- a/builtin/update-index.c\n> +++ b/builtin/update-index.c\n> @@ -121,7 +121,7 @@ static int test_if_untracked_cache_is_supported(void)\n> \tif (!mkdtemp(mtime_dir.buf))\n> \t\tdie_errno(\"Could not make temporary directory\");\n> \n> -\tfprintf(stderr, _(\"Testing \"));\n> +\tfprintf(stderr, _(\"Testing mtime in '%s' \"), xgetcwd());\n> \tatexit(remove_test_directory);\n> \txstat_mtime_dir(&st);\n> \tfill_stat_data(&base, &st);\n> @@ -1122,9 +1122,11 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n> \t\t}\n> \t\tadd_untracked_ident(the_index.untracked);\n> \t\tthe_index.cache_changed |= UNTRACKED_CHANGED;\n> +\t\tfprintf(stderr, _(\"Untracked cache enabled for '%s'\\n\"), get_git_work_tree());\n> \t} else if (!untracked_cache && the_index.untracked) {\n> \t\tthe_index.untracked = NULL;\n> \t\tthe_index.cache_changed |= UNTRACKED_CHANGED;\n> +\t\tfprintf(stderr, _(\"Untracked cache disabled\\n\"));\n> \t}\n> \n> \tif (active_cache_changed) {\n"},{"id":"274190","messageId":"xmqq1tawlpqw.fsf@gitster.mtv.corp.google.com","threadId":"40959","inReplyTo":"1449594916-21167-3-git-send-email-chriscool@tuxfamily.org","subject":"Re: [PATCH 2/8] update-index: use enum for untracked cache options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-12-08T19:11:35Z","receivedAt":"2015-12-08T19:11:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> Signed-off-by: Christian Couder \n> ---\n> builtin/update-index.c | 18 +++++++++++++-----\n> 1 file changed, 13 insertions(+), 5 deletions(-)\n>\n> diff --git a/builtin/update-index.c b/builtin/update-index.c\n> index 6f6b289..246b3d3 100644\n> --- a/builtin/update-index.c\n> +++ b/builtin/update-index.c\n> @@ -35,6 +35,14 @@ static int mark_skip_worktree_only;\n> #define UNMARK_FLAG 2\n> static struct strbuf mtime_dir = STRBUF_INIT;\n> \n> +/* Untracked cache mode */\n> +enum uc_mode {\n> +\tUNDEF_UC = -1,\n> +\tNO_UC = 0,\n> +\tUC,\n> +\tFORCE_UC\n> +};\n> +\n\nWith these, the code is much easier to read than with the mystery\nconstants, but did you consider making UC_ a common prefix for\nfurther ease-of-reading? E.g.\n\n UC_UNSPECIFIED\n UC_DONTUSE\n UC_USE\n UC_FORCE\n\nor something?\n\n> __attribute__((format (printf, 1, 2)))\n> static void report(const char *fmt, ...)\n> {\n> @@ -902,7 +910,7 @@ static int reupdate_callback(struct parse_opt_ctx_t *ctx,\n> int cmd_update_index(int argc, const char **argv, const char *prefix)\n> {\n> \tint newfd, entries, has_errors = 0, line_termination = '\\n';\n> -\tint untracked_cache = -1;\n> +\tenum uc_mode untracked_cache = UNDEF_UC;\n> \tint read_from_stdin = 0;\n> \tint prefix_length = prefix ? strlen(prefix) : 0;\n> \tint preferred_index_format = 0;\n> @@ -997,7 +1005,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n> \t\tOPT_BOOL(0, \"untracked-cache\", &untracked_cache,\n> \t\t\tN_(\"enable/disable untracked cache\")),\n> \t\tOPT_SET_INT(0, \"force-untracked-cache\", &untracked_cache,\n> -\t\t\t N_(\"enable untracked cache without testing the filesystem\"), 2),\n> +\t\t\t N_(\"enable untracked cache without testing the filesystem\"), FORCE_UC),\n> \t\tOPT_END()\n> \t};\n> \n> @@ -1104,10 +1112,10 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n> \t\tthe_index.split_index = NULL;\n> \t\tthe_index.cache_changed |= SOMETHING_CHANGED;\n> \t}\n> -\tif (untracked_cache > 0) {\n> +\tif (untracked_cache > NO_UC) {\n> \t\tstruct untracked_cache *uc;\n> \n> -\t\tif (untracked_cache < 2) {\n> +\t\tif (untracked_cache < FORCE_UC) {\n> \t\t\tsetup_work_tree();\n> \t\t\tif (!test_if_untracked_cache_is_supported())\n> \t\t\t\treturn 1;\n> @@ -1123,7 +1131,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n> \t\tadd_untracked_ident(the_index.untracked);\n> \t\tthe_index.cache_changed |= UNTRACKED_CHANGED;\n> \t\tfprintf(stderr, _(\"Untracked cache enabled for '%s'\\n\"), get_git_work_tree());\n> -\t} else if (!untracked_cache && the_index.untracked) {\n> +\t} else if (untracked_cache == NO_UC && the_index.untracked) {\n> \t\tthe_index.untracked = NULL;\n> \t\tthe_index.cache_changed |= UNTRACKED_CHANGED;\n> \t\tfprintf(stderr, _(\"Untracked cache disabled\\n\"));\n"},{"id":"274191","messageId":"xmqqwpsokb04.fsf@gitster.mtv.corp.google.com","threadId":"40959","inReplyTo":"1449594916-21167-7-git-send-email-chriscool@tuxfamily.org","subject":"Re: [PATCH 6/8] dir: add remove_untracked_cache()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-12-08T19:15:23Z","receivedAt":"2015-12-08T19:15:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> This new function will be used in a later patch.\n>\n> Signed-off-by: Christian Couder \n> ---\n\nUp to this step I looked at and they made sense (I am not saying the\nremainder of the series do not make sense).\n\nI however wonder where the memory used for untracked cache goes when\nthis is called?\n\n> builtin/update-index.c | 3 +--\n> dir.c | 6 ++++++\n> dir.h | 1 +\n> 3 files changed, 8 insertions(+), 2 deletions(-)\n>\n> diff --git a/builtin/update-index.c b/builtin/update-index.c\n> index 40530b0..e427657 100644\n> --- a/builtin/update-index.c\n> +++ b/builtin/update-index.c\n> @@ -1126,8 +1126,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n> \t\tadd_untracked_cache();\n> \t\tfprintf(stderr, _(\"Untracked cache enabled for '%s'\\n\"), get_git_work_tree());\n> \t} else if (untracked_cache == NO_UC && the_index.untracked) {\n> -\t\tthe_index.untracked = NULL;\n> -\t\tthe_index.cache_changed |= UNTRACKED_CHANGED;\n> +\t\tremove_untracked_cache();\n> \t\tfprintf(stderr, _(\"Untracked cache disabled\\n\"));\n> \t}\n> \n> diff --git a/dir.c b/dir.c\n> index 0f7e293..ffc0286 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -1952,6 +1952,12 @@ void add_untracked_cache(void)\n> \tthe_index.cache_changed |= UNTRACKED_CHANGED;\n> }\n> \n> +void remove_untracked_cache(void)\n> +{\n> +\tthe_index.untracked = NULL;\n> +\tthe_index.cache_changed |= UNTRACKED_CHANGED;\n> +}\n> +\n> static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct *dir,\n> \t\t\t\t\t\t int base_len,\n> \t\t\t\t\t\t const struct pathspec *pathspec)\n> diff --git a/dir.h b/dir.h\n> index ee94c76..3e5114d 100644\n> --- a/dir.h\n> +++ b/dir.h\n> @@ -309,4 +309,5 @@ struct untracked_cache *read_untracked_extension(const void *data, unsigned long\n> void write_untracked_extension(struct strbuf *out, struct untracked_cache *untracked);\n> void add_untracked_ident(struct untracked_cache *);\n> void add_untracked_cache(void);\n> +void remove_untracked_cache(void);\n> #endif\n"},{"id":"274192","messageId":"xmqqsi3ckadi.fsf@gitster.mtv.corp.google.com","threadId":"40959","inReplyTo":"1449594916-21167-8-git-send-email-chriscool@tuxfamily.org","subject":"Re: [PATCH 7/8] config: add core.untrackedCache","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-12-08T19:28:57Z","receivedAt":"2015-12-08T19:28:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> When we know that mtime is fully supported by the environment, we\n> might want the untracked cache to be always used by default without\n> any mtime test or kernel version check being performed.\n>\n> Also when we know that mtime is not supported by the environment,\n> for example because the repo is shared over a network file system,\n> then we might want 'git update-index --untracked-cache' to fail\n> immediately instead of preforming tests (because it might work on\n> some systems using the repo over the network file system but not\n> others).\n>\n> The normal way to achieve the above in Git is to use a config\n> variable. That's why this patch introduces \"core.untrackedCache\".\n>\n> To keep things simple, this variable is a bool which default to\n> false.\n>\n> When \"git status\" is run, it now adds or removes the untracked\n> cache in the index to respect the value of this variable.\n>\n> This means that `git update-index --[no-|force-]untracked-cache`,\n> to be compatible with the new config variable, have to set or\n> unset it. This new behavior is backward incompatible change, but\n> that is deliberate.\n\nThe logic in this paragraph is fuzzy to me. Shouldn't the config\ngive a sensible default, that is overriden by command line options?\nI agree that it is insane to do a runtime check when the user says\n\"update-index --untracked-cache\" to enable it, as the user _knows_\nthat enabling it would help (or the user _knows_ that she wants to\nplay with it). Similarly, shouldn't the config be ignored when the\nuser says \"update-index --no-untracked-cache\" (hence removing the\nuntracked cache from the resulting index no matter the config is set\nto)? Why is it a good idea to allow an explicit operation from the\ncommand line to muck with the configuration? If the user wants to\nchange the configuration permanently, \"git config\" has been there\nfor the purpose for all the configuration variables to do so for\nalmost forever. Why is it a good idea to make this variable so\nspecial that the user does not have to use \"git config\" but disable\nor enable it in some other way?\n\n> Also `--untracked-cache` used to check that the underlying\n> operating system and file system change `st_mtime` field of a\n> directory if files are added or deleted in that directory. But\n> those tests take a long time and there is now\n> `--test-untracked-cache` to perform them.\n>\n> So to be more consistent with other git commands, this patch\n> prevents `--untracked-cache` to perform tests.\n\nGood change.\n\n> This means that\n> after this patch there is no difference any more between\n> `--untracked-cache` and `--force-untracked-cache`.\n\nA tip to write the explanation. Every time you say \"This means\nthat...\", you are (perhaps unconsciously) admitting that what you\nwrote immedidately before that may not be understandable and what\ncomes after it may be a better explanation. Discard the sentence\nbefore \"This means that\", and try to formulate your explanation\naround what you wrote after it, adding information in the discarded\nearlier sentence back to the later one. Doing this exercise often\nhelps the result read much better.\n\n>\n> Signed-off-by: Christian Couder \n> ---\n> Documentation/config.txt | 7 +++++++\n> Documentation/git-update-index.txt | 28 ++++++++++++++++++----------\n> builtin/update-index.c | 23 +++++++++++++----------\n> cache.h | 1 +\n> config.c | 4 ++++\n> contrib/completion/git-completion.bash | 1 +\n> dir.c | 2 +-\n> environment.c | 1 +\n> t/t7063-status-untracked-cache.sh | 4 +---\n> wt-status.c | 9 +++++++++\n> 10 files changed, 56 insertions(+), 24 deletions(-)\n>\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index 2d06b11..94820eb 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -308,6 +308,13 @@ core.trustctime::\n> \tcrawlers and some backup systems).\n> \tSee linkgit:git-update-index[1]. True by default.\n> \n> +core.untrackedCache::\n> +\tDetermines if untracked cache will be enabled. Using\n> +\t'git update-index --[no-|force-]untracked-cache' will set\n> +\tthis variable. Before setting it to true, you should check\n> +\tthat mtime is working properly on your system.\n> +\tSee linkgit:git-update-index[1]. False by default.\n> +\n> core.checkStat::\n> \tDetermines which stat fields to match between the index\n> \tand work tree. The user can set this to 'default' or\n> diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt\n> index 0ff7396..0fb39db 100644\n> --- a/Documentation/git-update-index.txt\n> +++ b/Documentation/git-update-index.txt\n> @@ -175,22 +175,28 @@ may not support it yet.\n> --no-untracked-cache::\n> \tEnable or disable untracked cache extension. This could speed\n> \tup for commands that involve determining untracked files such\n> -\tas `git status`. The underlying operating system and file\n> -\tsystem must change `st_mtime` field of a directory if files\n> -\tare added or deleted in that directory.\n> +\tas `git status`.\n> ++\n> +The underlying operating system and file system must change `st_mtime`\n> +field of a directory if files are added or deleted in that\n> +directory. You can test that using\n> +`--test-untracked-cache`. `--untracked-cache` used to test that too\n> +but it doesn't anymore.\n> ++\n> +This sets the `core.untrackedCache` configuration variable to 'true'\n> +or 'false' in the repo config file, (see linkgit:git-config[1]), so\n> +that the untracked cache stays enabled or disabled.\n> \n> --test-untracked-cache::\n> \tOnly perform tests on the working directory to make sure\n> \tuntracked cache can be used. You have to manually enable\n> -\tuntracked cache using `--force-untracked-cache` (or\n> -\t`--untracked-cache` but this will run the tests again)\n> -\tafterwards if you really want to use it.\n> +\tuntracked cache using `--untracked-cache` or\n> +\t`--force-untracked-cache` or the `core.untrackedCache`\n> +\tconfiguration variable afterwards if you really want to use\n> +\tit.\n> \n> --force-untracked-cache::\n> -\tFor safety, `--untracked-cache` performs tests on the working\n> -\tdirectory to make sure untracked cache can be used. These\n> -\ttests can take a few seconds. `--force-untracked-cache` can be\n> -\tused to skip the tests.\n> +\tSame as `--untracked-cache`.\n> \n> \\--::\n> \tDo not interpret any more arguments as options.\n> @@ -406,6 +412,8 @@ It can be useful when the inode change time is regularly modified by\n> something outside Git (file system crawlers and backup systems use\n> ctime for marking files processed) (see linkgit:git-config[1]).\n> \n> +Untracked cache look at `core.untrackedCache` configuration variable\n> +(see linkgit:git-config[1]).\n> \n> SEE ALSO\n> --------\n> diff --git a/builtin/update-index.c b/builtin/update-index.c\n> index e427657..7fe3a86 100644\n> --- a/builtin/update-index.c\n> +++ b/builtin/update-index.c\n> @@ -1115,19 +1115,22 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n> \t\tthe_index.split_index = NULL;\n> \t\tthe_index.cache_changed |= SOMETHING_CHANGED;\n> \t}\n> +\tif (untracked_cache == TEST_UC) {\n> +\t\tsetup_work_tree();\n> +\t\treturn !test_if_untracked_cache_is_supported();\n> +\t}\n> \tif (untracked_cache > NO_UC) {\n> -\t\tif (untracked_cache < FORCE_UC) {\n> -\t\t\tsetup_work_tree();\n> -\t\t\tif (!test_if_untracked_cache_is_supported())\n> -\t\t\t\treturn 1;\n> -\t\t\tif (untracked_cache == TEST_UC)\n> -\t\t\t\treturn 0;\n> -\t\t}\n> +\t\tif (!use_untracked_cache && git_config_set(\"core.untrackedCache\", \"true\"))\n> +\t\t\tdie(\"could not set core.untrackedCache to true\");\n> \t\tadd_untracked_cache();\n> \t\tfprintf(stderr, _(\"Untracked cache enabled for '%s'\\n\"), get_git_work_tree());\n> -\t} else if (untracked_cache == NO_UC && the_index.untracked) {\n> -\t\tremove_untracked_cache();\n> -\t\tfprintf(stderr, _(\"Untracked cache disabled\\n\"));\n> +\t} else if (untracked_cache == NO_UC) {\n> +\t\tif (use_untracked_cache > 0 && git_config_set(\"core.untrackedCache\", \"false\"))\n> +\t\t\tdie(\"could not set core.untrackedCache to false\");\n> +\t\tif (the_index.untracked) {\n> +\t\t\tremove_untracked_cache();\n> +\t\t\tfprintf(stderr, _(\"Untracked cache disabled\\n\"));\n> +\t\t}\n> \t}\n> \n> \tif (active_cache_changed) {\n> diff --git a/cache.h b/cache.h\n> index 2a9e902..0cc2c2f 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -619,6 +619,7 @@ extern void set_alternate_index_output(const char *);\n> /* Environment bits from configuration mechanism */\n> extern int trust_executable_bit;\n> extern int trust_ctime;\n> +extern int use_untracked_cache;\n> extern int check_stat;\n> extern int quote_path_fully;\n> extern int has_symlinks;\n> diff --git a/config.c b/config.c\n> index 248a21a..f023ee7 100644\n> --- a/config.c\n> +++ b/config.c\n> @@ -691,6 +691,10 @@ static int git_default_core_config(const char *var, const char *value)\n> \t\ttrust_ctime = git_config_bool(var, value);\n> \t\treturn 0;\n> \t}\n> +\tif (!strcmp(var, \"core.untrackedcache\")) {\n> +\t\tuse_untracked_cache = git_config_bool(var, value);\n> +\t\treturn 0;\n> +\t}\n> \tif (!strcmp(var, \"core.checkstat\")) {\n> \t\tif (!strcasecmp(value, \"default\"))\n> \t\t\tcheck_stat = 1;\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index 111b053..b7e5736 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -2054,6 +2054,7 @@ _git_config ()\n> \t\tcore.sparseCheckout\n> \t\tcore.symlinks\n> \t\tcore.trustctime\n> +\t\tcore.untrackedCache\n> \t\tcore.warnAmbiguousRefs\n> \t\tcore.whitespace\n> \t\tcore.worktree\n> diff --git a/dir.c b/dir.c\n> index ffc0286..aa07aca 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -2014,7 +2014,7 @@ static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct *d\n> \tif (dir->exclude_list_group[EXC_CMDL].nr)\n> \t\treturn NULL;\n> \n> -\tif (!ident_in_untracked(dir->untracked)) {\n> +\tif (use_untracked_cache != 1 && !ident_in_untracked(dir->untracked)) {\n> \t\twarning(_(\"Untracked cache is disabled on this system.\"));\n> \t\treturn NULL;\n> \t}\n> diff --git a/environment.c b/environment.c\n> index 2da7fe2..9ca71b1 100644\n> --- a/environment.c\n> +++ b/environment.c\n> @@ -14,6 +14,7 @@\n> \n> int trust_executable_bit = 1;\n> int trust_ctime = 1;\n> +int use_untracked_cache;\n> int check_stat = 1;\n> int has_symlinks = 1;\n> int minimum_abbrev = 4, default_abbrev = 7;\n> diff --git a/t/t7063-status-untracked-cache.sh b/t/t7063-status-untracked-cache.sh\n> index 0e8d0d4..253160a 100755\n> --- a/t/t7063-status-untracked-cache.sh\n> +++ b/t/t7063-status-untracked-cache.sh\n> @@ -8,10 +8,8 @@ avoid_racy() {\n> \tsleep 1\n> }\n> \n> -# It's fine if git update-index returns an error code other than one,\n> -# it'll be caught in the first test.\n> test_lazy_prereq UNTRACKED_CACHE '\n> -\t{ git update-index --untracked-cache; ret=$?; } &&\n> +\t{ git update-index --test-untracked-cache; ret=$?; } &&\n> \ttest $ret -ne 1\n> '\n> \n> diff --git a/wt-status.c b/wt-status.c\n> index 435fc28..3e0fe02 100644\n> --- a/wt-status.c\n> +++ b/wt-status.c\n> @@ -586,6 +586,15 @@ 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 (!dir.untracked && use_untracked_cache == 1) {\n> +\t\tadd_untracked_cache();\n> +\t\tdir.untracked = the_index.untracked;\n> +\t} else if (dir.untracked && use_untracked_cache == 0) {\n> +\t\tremove_untracked_cache();\n> +\t\tdir.untracked = NULL;\n> +\t}\n> +\n> \tsetup_standard_excludes(&dir);\n> \n> \tfill_directory(&dir, &s->pathspec);\n"},{"id":"274195","messageId":"xmqqfuzck1dp.fsf@gitster.mtv.corp.google.com","threadId":"40959","inReplyTo":"xmqqsi3ckadi.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 7/8] config: add core.untrackedCache","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-12-08T22:43:14Z","receivedAt":"2015-12-08T22:43:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Christian Couder writes:\n>\n>> When we know that mtime is fully supported by the environment, we\n>> might want the untracked cache to be always used by default without\n>> any mtime test or kernel version check being performed.\n>>\n>> Also when we know that mtime is not supported by the environment,\n>> for example because the repo is shared over a network file system,\n>> then we might want 'git update-index --untracked-cache' to fail\n>> immediately instead of preforming tests (because it might work on\n>> some systems using the repo over the network file system but not\n>> others).\n>> ...\n> The logic in this paragraph is fuzzy to me. Shouldn't the config\n> give a sensible default, that is overriden by command line options?\n> I agree that it is insane to do a runtime check when the user says\n> \"update-index --untracked-cache\" to enable it, as the user _knows_\n> that enabling it would help (or the user _knows_ that she wants to\n> play with it). Similarly, shouldn't the config be ignored when the\n> user says \"update-index --no-untracked-cache\" (hence removing the\n> untracked cache from the resulting index no matter the config is set\n> to)? ...\n\nAs I think about this more, it really seems to me that we shouldn't\nneed to make this configuration variable that special. Because I\nthink it is a *BUG* in the current implementation to do the runtime\ncheck even when the user explicitly tells us to use untracked-cache,\nI'd imagine something along the lines of the following would be a\nlot simpler, easier to understand and a generally more useful\nbugfix:\n\n 1 Add one boolean configuration variable, core.untrackedCache, that\n defaults to 'false'.\n\n 2 When creating an index file in an empty repository, enable the\n untracked cache in the index (even without the user runninng\n \"update-index --untracked-cache\") iff the configuration is set to\n 'true'. No runtime check necessary.\n\n 3 When working on an existing index file, unless the operation is\n \"update-index --[no-]untracked-cache\", keep the current setting,\n regardless of this configuration variable. No runtime check\n necessary.\n\n 4 \"update-index --[no-]untracked-cache\" should enable or disable\n the untracked cache as the user tells us, regardless of the\n configuration variable. No runtime check necessary.\n\nIt is OK to then add an \"auto-detect\" on top of the above, that\nwould only affect the second bullet point, like so:\n\n 2a When creating an index file in an empty repository, if the\n configuration is set to 'auto', do the lengthy runtime check and\n enable the untracked cache in the index (even without the user\n runninng \"update-index --untracked-cache\").\n\nwithout changing any other in the first 4-bullet list.\n\nAm I missing some other requirements?\n"},{"id":"274202","messageId":"5667DA2B.3000506@web.de","threadId":"40959","inReplyTo":"1449594916-21167-6-git-send-email-chriscool@tuxfamily.org","subject":"Re: [PATCH 5/8] dir: add add_untracked_cache()","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2015-12-09T07:37:15Z","receivedAt":"2015-12-09T07:37:15Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 08.12.15 18:15, Christian Couder wrote:\n> This new function will be used in a later patch.\nMay be \nFactor out code into add_untracked_cache(), which will be used in the next commit.\n"},{"id":"274203","messageId":"5667DAB4.7000002@web.de","threadId":"40959","inReplyTo":"1449594916-21167-7-git-send-email-chriscool@tuxfamily.org","subject":"Re: [PATCH 6/8] dir: add remove_untracked_cache()","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2015-12-09T07:39:32Z","receivedAt":"2015-12-09T07:39:32Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 08.12.15 18:15, Christian Couder wrote:\n> This new function will be used in a later patch.\nWhy not combine 5/8 with 6/8 into a single patch ?\n\n(And please consider to mark the series with v2)\n"},{"id":"274205","messageId":"56682A75.5090204@web.de","threadId":"40959","inReplyTo":"1449594916-21167-8-git-send-email-chriscool@tuxfamily.org","subject":"Re: [PATCH 7/8] config: add core.untrackedCache","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2015-12-09T13:19:49Z","receivedAt":"2015-12-09T13:19:49Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 08.12.15 18:15, Christian Couder wrote:\n> When we know that mtime is fully supported by the environment, we\n> might want the untracked cache to be always used by default without\n> any mtime test or kernel version check being performed.\n> \nFor the people which didn't follow the discussion, or read this in \na year or 2:\nA short description what \"mtime is fully supported by the environment\" means\nwould be nice:\nWhen a file system object is added or deleted in a directory, \nand stat(dirname, &st) returns an updated st.st_mtime.\n> Also when we know that mtime is not supported by the environment,\n> for example because the repo is shared over a network file system,\n> then we might want 'git update-index --untracked-cache' to fail\n> immediately instead of preforming tests (because it might work on\n performing\n> some systems using the repo over the network file system but not\n> others).\n> \n> The normal way to achieve the above in Git is to use a config\n> variable. That's why this patch introduces \"core.untrackedCache\".\n> \n> To keep things simple, this variable is a bool which default to\n> false.\nIf this is the case, can we remove some code?\ne.g add_untracked_ident() in dir.c, do we still need it?\nAnd probably some other functions as well.\nOr would it be better to say \n\"false\" is false,\n\"true\" is true\n\"unset\" is as before (Where when different machines/OS/mount points\n access the same repo over a network file system, some use the UT, some don't)\n\n> \n> When \"git status\" is run, it now adds or removes the untracked\n> cache in the index to respect the value of this variable.\n> \n> This means that `git update-index --[no-|force-]untracked-cache`,\n> to be compatible with the new config variable, have to set or\n> unset it.\nwhat does unset mean ? Set to false ?\n This new behavior is backward incompatible change, but\n> that is deliberate.\n\n> \n> Also `--untracked-cache` used to check that the underlying\n> operating system and file system change `st_mtime` field of a\n> directory if files are added or deleted in that directory. But\n> those tests take a long time and there is now\n> `--test-untracked-cache` to perform them.\n> \n> So to be more consistent with other git commands, this patch\n> prevents `--untracked-cache` to perform tests. This means that\n> after this patch there is no difference any more between\n> `--untracked-cache` and `--force-untracked-cache`.\n> \n> Signed-off-by: Christian Couder \n> ---\n> Documentation/config.txt | 7 +++++++\n> Documentation/git-update-index.txt | 28 ++++++++++++++++++----------\n> builtin/update-index.c | 23 +++++++++++++----------\n> cache.h | 1 +\n> config.c | 4 ++++\n> contrib/completion/git-completion.bash | 1 +\n> dir.c | 2 +-\n> environment.c | 1 +\n> t/t7063-status-untracked-cache.sh | 4 +---\n> wt-status.c | 9 +++++++++\n> 10 files changed, 56 insertions(+), 24 deletions(-)\n> \n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index 2d06b11..94820eb 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -308,6 +308,13 @@ core.trustctime::\n> \tcrawlers and some backup systems).\n> \tSee linkgit:git-update-index[1]. True by default.\n> \n> +core.untrackedCache::\n> +\tDetermines if untracked cache will be enabled. Using\n> +\t'git update-index --[no-|force-]untracked-cache' will set\n> +\tthis variable. \nset to what ? true ? false ?\n\nBefore setting it to true, you should check\n> +\tthat mtime is working properly on your system.\n> +\tSee linkgit:git-update-index[1]. False by default.\n> +\nisn't this what \"git update-index --test-untracked-cached\" is good for?\n\n> core.checkStat::\n> \tDetermines which stat fields to match between the index\n> \tand work tree. The user can set this to 'default' or\n> diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt\n> index 0ff7396..0fb39db 100644\n> --- a/Documentation/git-update-index.txt\n> +++ b/Documentation/git-update-index.txt\n> @@ -175,22 +175,28 @@ may not support it yet.\n> --no-untracked-cache::\n> \tEnable or disable untracked cache extension. This could speed\n> \tup for commands that involve determining untracked files such\n> +\tas `git status`.\n> ++\n> +The underlying operating system and file system must change `st_mtime`\n> +field of a directory if files are added or deleted in that\n> +directory. You can test that using\n> +`--test-untracked-cache`. \n\n`--untracked-cache` used to test that too\n> +but it doesn't anymore.\nDo we need this historical comment ?\n\n> ++\n> +This sets the `core.untrackedCache` configuration variable to 'true'\n> +or 'false' in the repo config file, (see linkgit:git-config[1]), so\n> +that the untracked cache stays enabled or disabled.\n> \n> --test-untracked-cache::\n> \tOnly perform tests on the working directory to make sure\n> \tuntracked cache can be used. You have to manually enable\n> -\tuntracked cache using `--force-untracked-cache` (or\n> -\t`--untracked-cache` but this will run the tests again)\n> -\tafterwards if you really want to use it.\n> +\tuntracked cache using `--untracked-cache` or\n> +\t`--force-untracked-cache` or the `core.untrackedCache`\n> +\tconfiguration variable afterwards if you really want to use\n> +\tit.\nThis seems confusing, at least to me.\nDo you mean:\n --test-untracked-cache::\n \tPerform tests on the working directory and tells the user if\n \tuntracked cache can be used.\n\n> \n> --force-untracked-cache::\n> -\tFor safety, `--untracked-cache` performs tests on the working\n> -\tdirectory to make sure untracked cache can be used. These\n> -\ttests can take a few seconds. `--force-untracked-cache` can be\n> -\tused to skip the tests.\n> +\tSame as `--untracked-cache`.\n> \n> \\--::\n> \tDo not interpret any more arguments as options.\n> @@ -406,6 +412,8 @@ It can be useful when the inode change time is regularly modified by\n> something outside Git (file system crawlers and backup systems use\n> ctime for marking files processed) (see linkgit:git-config[1]).\n> \n> +Untracked cache look at `core.untrackedCache` configuration variable\n> +(see linkgit:git-config[1]).\n> \n> SEE ALSO\n> --------\n> diff --git a/builtin/update-index.c b/builtin/update-index.c\n> index e427657..7fe3a86 100644\n> --- a/builtin/update-index.c\n> +++ b/builtin/update-index.c\n> @@ -1115,19 +1115,22 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n> \t\tthe_index.split_index = NULL;\n> \t\tthe_index.cache_changed |= SOMETHING_CHANGED;\n> \t}\n> +\tif (untracked_cache == TEST_UC) {\n> +\t\tsetup_work_tree();\n> +\t\treturn !test_if_untracked_cache_is_supported();\n> +\t}\n> \tif (untracked_cache > NO_UC) {\n> -\t\tif (untracked_cache < FORCE_UC) {\n> -\t\t\tsetup_work_tree();\n> -\t\t\tif (!test_if_untracked_cache_is_supported())\n> -\t\t\t\treturn 1;\n> -\t\t\tif (untracked_cache == TEST_UC)\n> -\t\t\t\treturn 0;\n> -\t\t}\n> +\t\tif (!use_untracked_cache && git_config_set(\"core.untrackedCache\", \"true\"))\n> +\t\t\tdie(\"could not set core.untrackedCache to true\");\n> \t\tadd_untracked_cache();\n> \t\tfprintf(stderr, _(\"Untracked cache enabled for '%s'\\n\"), get_git_work_tree());\n> -\t} else if (untracked_cache == NO_UC && the_index.untracked) {\n> -\t\tremove_untracked_cache();\n> -\t\tfprintf(stderr, _(\"Untracked cache disabled\\n\"));\n> +\t} else if (untracked_cache == NO_UC) {\n> +\t\tif (use_untracked_cache > 0 && git_config_set(\"core.untrackedCache\", \"false\"))\n> +\t\t\tdie(\"could not set core.untrackedCache to false\");\n> +\t\tif (the_index.untracked) {\n> +\t\t\tremove_untracked_cache();\n> +\t\t\tfprintf(stderr, _(\"Untracked cache disabled\\n\"));\n> +\t\t}\n> \t}\n> \n> \tif (active_cache_changed) {\n> diff --git a/cache.h b/cache.h\n> index 2a9e902..0cc2c2f 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -619,6 +619,7 @@ extern void set_alternate_index_output(const char *);\n> /* Environment bits from configuration mechanism */\n> extern int trust_executable_bit;\n> extern int trust_ctime;\n> +extern int use_untracked_cache;\n> extern int check_stat;\n> extern int quote_path_fully;\n> extern int has_symlinks;\n> diff --git a/config.c b/config.c\n> index 248a21a..f023ee7 100644\n> --- a/config.c\n> +++ b/config.c\n> @@ -691,6 +691,10 @@ static int git_default_core_config(const char *var, const char *value)\n> \t\ttrust_ctime = git_config_bool(var, value);\n> \t\treturn 0;\n> \t}\n> +\tif (!strcmp(var, \"core.untrackedcache\")) {\n> +\t\tuse_untracked_cache = git_config_bool(var, value);\n> +\t\treturn 0;\n> +\t}\n> \tif (!strcmp(var, \"core.checkstat\")) {\n> \t\tif (!strcasecmp(value, \"default\"))\n> \t\t\tcheck_stat = 1;\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index 111b053..b7e5736 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -2054,6 +2054,7 @@ _git_config ()\n> \t\tcore.sparseCheckout\n> \t\tcore.symlinks\n> \t\tcore.trustctime\n> +\t\tcore.untrackedCache\n> \t\tcore.warnAmbiguousRefs\n> \t\tcore.whitespace\n> \t\tcore.worktree\n> diff --git a/dir.c b/dir.c\n> index ffc0286..aa07aca 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -2014,7 +2014,7 @@ static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct *d\n> \tif (dir->exclude_list_group[EXC_CMDL].nr)\n> \t\treturn NULL;\n> \n> -\tif (!ident_in_untracked(dir->untracked)) {\n> +\tif (use_untracked_cache != 1 && !ident_in_untracked(dir->untracked)) {\n> \t\twarning(_(\"Untracked cache is disabled on this system.\"));\n> \t\treturn NULL;\n> \t}\n> diff --git a/environment.c b/environment.c\n> index 2da7fe2..9ca71b1 100644\n> --- a/environment.c\n> +++ b/environment.c\n> @@ -14,6 +14,7 @@\n> \n> int trust_executable_bit = 1;\n> int trust_ctime = 1;\n> +int use_untracked_cache;\n> int check_stat = 1;\n> int has_symlinks = 1;\n> int minimum_abbrev = 4, default_abbrev = 7;\n> diff --git a/t/t7063-status-untracked-cache.sh b/t/t7063-status-untracked-cache.sh\n> index 0e8d0d4..253160a 100755\n> --- a/t/t7063-status-untracked-cache.sh\n> +++ b/t/t7063-status-untracked-cache.sh\n> @@ -8,10 +8,8 @@ avoid_racy() {\n> \tsleep 1\n> }\n> \n> -# It's fine if git update-index returns an error code other than one,\n> -# it'll be caught in the first test.\n> test_lazy_prereq UNTRACKED_CACHE '\n> -\t{ git update-index --untracked-cache; ret=$?; } &&\n> +\t{ git update-index --test-untracked-cache; ret=$?; } &&\n> \ttest $ret -ne 1\n> '\n> \n> diff --git a/wt-status.c b/wt-status.c\n> index 435fc28..3e0fe02 100644\n> --- a/wt-status.c\n> +++ b/wt-status.c\n> @@ -586,6 +586,15 @@ 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\n> +\tif (!dir.untracked && use_untracked_cache == 1) {\n> +\t\tadd_untracked_cache();\n> +\t\tdir.untracked = the_index.untracked;\n> +\t} else if (dir.untracked && use_untracked_cache == 0) {\n> +\t\tremove_untracked_cache();\n> +\t\tdir.untracked = NULL;\n> +\t}\n\nIf we say core.untrackedCache = unset is the same as false,\nthis code can be simplified:\n\n> +\tif (!dir.untracked && use_untracked_cache) {\n> +\t\tadd_untracked_cache();\n> +\t\tdir.untracked = the_index.untracked;\n> +\t} else if (dir.untracked && !use_untracked_cache) {\n> +\t\tremove_untracked_cache();\n> +\t\tdir.untracked = NULL;\n> +\t}\n"},{"id":"274230","messageId":"CAP8UFD3+gsHZuaBweP83or=rEh-LFnz6=ycBCeuhApxp0PzN1A@mail.gmail.com","threadId":"40959","inReplyTo":"xmqq1tawlpqw.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 2/8] update-index: use enum for untracked cache options","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2015-12-10T10:37:53Z","receivedAt":"2015-12-10T10:37:53Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Tue, Dec 8, 2015 at 8:11 PM, Junio C Hamano wrote:\n> Christian Couder writes:\n>\n>> Signed-off-by: Christian Couder \n>> ---\n>> builtin/update-index.c | 18 +++++++++++++-----\n>> 1 file changed, 13 insertions(+), 5 deletions(-)\n>>\n>> diff --git a/builtin/update-index.c b/builtin/update-index.c\n>> index 6f6b289..246b3d3 100644\n>> --- a/builtin/update-index.c\n>> +++ b/builtin/update-index.c\n>> @@ -35,6 +35,14 @@ static int mark_skip_worktree_only;\n>> #define UNMARK_FLAG 2\n>> static struct strbuf mtime_dir = STRBUF_INIT;\n>>\n>> +/* Untracked cache mode */\n>> +enum uc_mode {\n>> + UNDEF_UC = -1,\n>> + NO_UC = 0,\n>> + UC,\n>> + FORCE_UC\n>> +};\n>> +\n>\n> With these, the code is much easier to read than with the mystery\n> constants, but did you consider making UC_ a common prefix for\n> further ease-of-reading? E.g.\n>\n> UC_UNSPECIFIED\n> UC_DONTUSE\n> UC_USE\n> UC_FORCE\n>\n> or something?\n\nOk, I will use what you suggest.\n\nThanks,\nChristian.\n"},{"id":"274238","messageId":"xmqqh9jqi1ky.fsf@gitster.mtv.corp.google.com","threadId":"40959","inReplyTo":"CAP8UFD3+gsHZuaBweP83or=rEh-LFnz6=ycBCeuhApxp0PzN1A@mail.gmail.com","subject":"Re: [PATCH 2/8] update-index: use enum for untracked cache options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-12-10T18:46:21Z","receivedAt":"2015-12-10T18:46:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n>>> +/* Untracked cache mode */\n>>> +enum uc_mode {\n>>> + UNDEF_UC = -1,\n>>> + NO_UC = 0,\n>>> + UC,\n>>> + FORCE_UC\n>>> +};\n>>> +\n>>\n>> With these, the code is much easier to read than with the mystery\n>> constants, but did you consider making UC_ a common prefix for\n>> further ease-of-reading? E.g.\n>>\n>> UC_UNSPECIFIED\n>> UC_DONTUSE\n>> UC_USE\n>> UC_FORCE\n>>\n>> or something?\n>\n> Ok, I will use what you suggest.\n\nAs you know I am bad at bikeshedding; the only suggestion in the\nabove is to have common UC_ prefix ;-) Don't take what follow UC_\nas my suggestion.\n\nThanks.\n"},{"id":"274266","messageId":"CAP8UFD1F-s_oPEgSXehcTV7UD1uC_Uxf8cMNPfyFro8fj4zZEA@mail.gmail.com","threadId":"40959","inReplyTo":"xmqq6108lq3w.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 1/8] update-index: add untracked cache notifications","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2015-12-11T08:51:53Z","receivedAt":"2015-12-11T08:51:53Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Tue, Dec 8, 2015 at 8:03 PM, Junio C Hamano wrote:\n> Christian Couder writes:\n>\n>> Doing:\n>>\n>> cd /tmp\n>> git --git-dir=/git/somewhere/else/.git update-index --untracked-cache\n>>\n>> doesn't work how one would expect. It hardcodes \"/tmp\" as the directory\n>> that \"works\" into the index, so if you use the working tree, you'll\n>> never use the untracked cache.\n>\n> I think your \"expectation\" needs to be more explicitly spelled out.\n>\n> \"git -C /tmp --git-dir=/git/somewhere/else/.git\" is a valid way to\n> use that repository you have in somewhere else to track things under\n> /tmp/ (as you are only passing GIT_DIR but not GIT_WORK_TREE, the\n> cwd, i.e. /tmp, is the root level of the working tree), and for such\n> a usage, the above command works as expected. Perhaps\n>\n> Attempting to flip the untracked-cache feature on for a random index\n> file with\n>\n> cd /random/unrelated/place\n> git --git-dir=/somewhere/else/.git update-index --untracked-cache\n>\n> would not work as you might expect. Because flipping the\n> feature on in the index also records the location of the\n> corresponding working tree (/random/unrelated/place in the above\n> example), when the index is subsequently used to keep track of\n> files in the working tree in /somewhere/else, the feature is\n> disabled.\n>\n> may be an improvement.\n\nYeah, I agree that it is better. I have included your explanations in\nthe next version I will send. Thanks.\n\n> The index already implicitly records where the working tree was and\n> that is not limited to untracked-cache option. For example, if you\n> have your repository and its working tree in /git/somewhere/else,\n> which does not have a path X, then doing:\n>\n> cd /tmp && >tmp/X\n> git --git-dir=/git/somewhere/else/.git update-index --add X\n>\n> would store X taken from /tmp in the index, so subsequent use of the\n> index \"knows\" about X that was taken outside /git/somewhere/else/\n> after the above command finishes and the subsequent use is made\n> without the --git-dir parameter, e.g.\n>\n> cd /git/somewhere/else/ && git diff-index --cached HEAD'\n>\n> would say that you added X, even though /git/somewhere/else/ may not\n> have that X at all. And this is not limited to update-index,\n> either. You can temporarily use --git-dir with \"git add X\" and the\n> result would persist the same way in the index.\n>\n> I think the moral of the story is that you are not expected to\n> randomly use git-dir and git-work-tree to point at different places\n> without knowing what you are doing, and we may need a way to help\n> people understand what is going on when it is done by a mistake.\n\nYeah, I agree, and I think displaying more information might be a good way.\n\n> The patch to show result from xgetcwd() and get_git_work_tree() may\n> be a step in the right direction, but if the user is not doing\n> anything fancy, \"Testing mtime in /long/path/to/the/directory\" would\n> be irritatingly verbose.\n\nYeah, but after this series only \"--test-untracked-cache\" does any\ntesting, and the 10 seconds time it takes are probably more irritating\nthan its output.\n\n> I wonder if it is easy to tell when the user is doing such an\n> unnatural thing. Off the top of my head, when the working tree is\n> anywhere other than one level above $GIT_DIR, the user is doing\n> something unusual; I do not know if there are cases where the user\n> is doing something unnatural if $GIT_WORK_TREE is one level above\n> $GIT_DIR, though.\n\nYeah, it could only print a warning in case something unusual is done.\nI am not sure it is worth it though.\n\nThanks,\nChristian.\n"},{"id":"274267","messageId":"CAP8UFD367LKTWHX2JdS_63n25n+0k_=OKSGQ1s7SJcJnbHjcCA@mail.gmail.com","threadId":"40959","inReplyTo":"5667DA2B.3000506@web.de","subject":"Re: [PATCH 5/8] dir: add add_untracked_cache()","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2015-12-11T08:54:20Z","receivedAt":"2015-12-11T08:54:20Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Wed, Dec 9, 2015 at 8:37 AM, Torsten Bögershausen wrote:\n> On 08.12.15 18:15, Christian Couder wrote:\n>> This new function will be used in a later patch.\n> May be\n> Factor out code into add_untracked_cache(), which will be used in the next commit.\n\nThanks for the suggestion. I think I will put something like this:\n\n Factor out code into add_untracked_cache(), which will be used\n in a later commit.\n"},{"id":"274269","messageId":"CAP8UFD3AHMEZB+6Ajt8XSsecTp_PSxV_ZxFLbMAYgA3CDhPDng@mail.gmail.com","threadId":"40959","inReplyTo":"xmqqh9jqi1ky.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 2/8] update-index: use enum for untracked cache options","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2015-12-11T09:10:40Z","receivedAt":"2015-12-11T09:10:40Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Thu, Dec 10, 2015 at 7:46 PM, Junio C Hamano wrote:\n> Christian Couder writes:\n>\n>>>> +/* Untracked cache mode */\n>>>> +enum uc_mode {\n>>>> + UNDEF_UC = -1,\n>>>> + NO_UC = 0,\n>>>> + UC,\n>>>> + FORCE_UC\n>>>> +};\n>>>> +\n>>>\n>>> With these, the code is much easier to read than with the mystery\n>>> constants, but did you consider making UC_ a common prefix for\n>>> further ease-of-reading? E.g.\n>>>\n>>> UC_UNSPECIFIED\n>>> UC_DONTUSE\n>>> UC_USE\n>>> UC_FORCE\n>>>\n>>> or something?\n>>\n>> Ok, I will use what you suggest.\n>\n> As you know I am bad at bikeshedding; the only suggestion in the\n> above is to have common UC_ prefix ;-) Don't take what follow UC_\n> as my suggestion.\n\nI am bad at finding names too, so I think what you wrote is pretty good.\n\nI thought about the following:\n\n UC_UNDEF\n UC_DISABLE\n UC_ENABLE\n UC_TEST\n UC_FORCE\n\nbut I am not sure it is much better.\n"},{"id":"274279","messageId":"xmqqio44g9rm.fsf@gitster.mtv.corp.google.com","threadId":"40959","inReplyTo":"CAP8UFD3AHMEZB+6Ajt8XSsecTp_PSxV_ZxFLbMAYgA3CDhPDng@mail.gmail.com","subject":"Re: [PATCH 2/8] update-index: use enum for untracked cache options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-12-11T17:44:45Z","receivedAt":"2015-12-11T17:44:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n>> As you know I am bad at bikeshedding; the only suggestion in the\n>> above is to have common UC_ prefix ;-) Don't take what follow UC_\n>> as my suggestion.\n>\n> I am bad at finding names too, so I think what you wrote is pretty good.\n>\n> I thought about the following:\n>\n> UC_UNDEF\n> UC_DISABLE\n> UC_ENABLE\n> UC_TEST\n> UC_FORCE\n>\n> but I am not sure it is much better.\n\nI hated the DONTUSE/USE I listed, and I think DISABLE/ENABLE is much\nmuch better. I do prefer unspecified over undef, though.\n\nThanks.\n"},{"id":"274314","messageId":"CAP8UFD24Xm6bVYYJShDXui=ot+YpScaGZ9HC3K+Rqem3y1Rn0w@mail.gmail.com","threadId":"40959","inReplyTo":"xmqqio44g9rm.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 2/8] update-index: use enum for untracked cache options","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2015-12-12T09:25:37Z","receivedAt":"2015-12-12T09:25:37Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Fri, Dec 11, 2015 at 6:44 PM, Junio C Hamano wrote:\n> Christian Couder writes:\n>\n>>> As you know I am bad at bikeshedding; the only suggestion in the\n>>> above is to have common UC_ prefix ;-) Don't take what follow UC_\n>>> as my suggestion.\n>>\n>> I am bad at finding names too, so I think what you wrote is pretty good.\n>>\n>> I thought about the following:\n>>\n>> UC_UNDEF\n>> UC_DISABLE\n>> UC_ENABLE\n>> UC_TEST\n>> UC_FORCE\n>>\n>> but I am not sure it is much better.\n>\n> I hated the DONTUSE/USE I listed, and I think DISABLE/ENABLE is much\n> much better. I do prefer unspecified over undef, though.\n\nOk, then it will be:\n\n UC_UNSPECIFIED\n UC_DISABLE\n UC_ENABLE\n UC_TEST\n UC_FORCE\n\nThanks,\nChristian.\n"},{"id":"274370","messageId":"CAP8UFD3at0X9ThpXGTwyPnu_dXFj6x=YzfkCa82m+RsWwhFOOA@mail.gmail.com","threadId":"40959","inReplyTo":"xmqqfuzck1dp.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 7/8] config: add core.untrackedCache","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2015-12-14T12:18:25Z","receivedAt":"2015-12-14T12:18:25Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Tue, Dec 8, 2015 at 11:43 PM, Junio C Hamano wrote:\n> Junio C Hamano writes:\n>\n>> Christian Couder writes:\n>>\n>>> When we know that mtime is fully supported by the environment, we\n>>> might want the untracked cache to be always used by default without\n>>> any mtime test or kernel version check being performed.\n>>>\n>>> Also when we know that mtime is not supported by the environment,\n>>> for example because the repo is shared over a network file system,\n>>> then we might want 'git update-index --untracked-cache' to fail\n>>> immediately instead of preforming tests (because it might work on\n>>> some systems using the repo over the network file system but not\n>>> others).\n>>> ...\n>> The logic in this paragraph is fuzzy to me. Shouldn't the config\n>> give a sensible default, that is overriden by command line options?\n\nThe problem is that \"git update-index --[no-|force-]untracked-cache\"\nis not just changing things for the duration of the current command.\nIt is in fact changing the configuration of the repository, because it\nis adding the UC (untracked cache) in the index and saving the index,\nwhich will persist the use of the UC for the following commands.\n\nSo it is very different from something like \"git status\n--no-untracked-cache\" that would perform a \"git status\" without using\nthe UC.\n\nIn fact \"git update-index --[no-|force-]untracked-cache\" is very bad\nbecause it means that two repositories can be configured differently\neven if they have the same config files.\n\n>> I agree that it is insane to do a runtime check when the user says\n>> \"update-index --untracked-cache\" to enable it, as the user _knows_\n>> that enabling it would help (or the user _knows_ that she wants to\n>> play with it). Similarly, shouldn't the config be ignored when the\n>> user says \"update-index --no-untracked-cache\" (hence removing the\n>> untracked cache from the resulting index no matter the config is set\n>> to)? ...\n>\n> As I think about this more, it really seems to me that we shouldn't\n> need to make this configuration variable that special. Because I\n> think it is a *BUG* in the current implementation to do the runtime\n> check even when the user explicitly tells us to use untracked-cache,\n> I'd imagine something along the lines of the following would be a\n> lot simpler, easier to understand and a generally more useful\n> bugfix:\n>\n> 1 Add one boolean configuration variable, core.untrackedCache, that\n> defaults to 'false'.\n>\n> 2 When creating an index file in an empty repository, enable the\n> untracked cache in the index (even without the user runninng\n> \"update-index --untracked-cache\") iff the configuration is set to\n> 'true'. No runtime check necessary.\n\nI guess this means that when cloning a repo, it would not use the UC\nunless either \"git -c core.untrackedCache=true clone ...\" is used, or\ncore.untrackedCache is set in the global config.\n\n> 3 When working on an existing index file, unless the operation is\n> \"update-index --[no-]untracked-cache\", keep the current setting,\n> regardless of this configuration variable. No runtime check\n> necessary.\n>\n> 4 \"update-index --[no-]untracked-cache\" should enable or disable\n> the untracked cache as the user tells us, regardless of the\n> configuration variable. No runtime check necessary.\n\nIf you want only some repos to use the UC, you will set\ncore.untrackedCache in the repo config. Then after cloning such a\nrepo, you will copy the config file, and this will not be enough to\nenable the UC. The original repo will use the UC but the cloned one\nwill not, and you might wonder why is \"git status\" slower in the\ncloned repo despite the machines and the config being the same for\nboth repos?\n\nAnd if you have set core.untrackedCache in the global config when you\nclone, UC is enabled, but if you have just set it in the repo config\nafter the clone, it is not enabled.\n\nThis is quite bad in my opinion.\n\nAlso think about system administrators who have a lot of machines with\nlots of repos everywhere on these machines and just upgrade Git from\nlet's say v2.4.0 to v2.8.0. They find out that using UC git status\nwill be faster and want to provide that to the machine's users knowing\nthat they only use recent Linux machines where mtime works well.\nShouldn't it be nice if they could just enable core.untrackedCache in\nthe global config files without having to also cd into every repo and\nuse \"git update-index --untracked-cache\" there?\n\nIf we consider that \"git update-index --[no-|force-]untracked-cache\"\nshould not have been created in the first place, and that the good\nmechanism for this is a config variable, and that maybe \"git\nupdate-index --[no-|force-]untracked-cache\" should just be deprecated,\nthen the important thing is to make the core.untrackedCache config\nvariable work in the best possible way.\n\n> It is OK to then add an \"auto-detect\" on top of the above, that\n> would only affect the second bullet point, like so:\n>\n> 2a When creating an index file in an empty repository, if the\n> configuration is set to 'auto', do the lengthy runtime check and\n> enable the untracked cache in the index (even without the user\n> runninng \"update-index --untracked-cache\").\n>\n> without changing any other in the first 4-bullet list.\n>\n> Am I missing some other requirements?\n\nIt is more about what is a good way to use this feature?\n\n\"git update-index --[no-|force-]untracked-cache\" is a bad way, so\nlet's make it easy for people to not use it at all.\n\nIf people can just set core.untrackedCache in an existing repo without\nthe need to use \"git update-index --untracked-cache\" afterwards to\nreally enable it, and without having to remember to use it again after\ncloning (supposing core.untrackedCache is not set globally where they\nclone), it is just simpler, and as an added benefit we can deprecate\n\"git update-index --[no-|force-]untracked-cache\".\n\nThanks,\nChristian.\n"},{"id":"274411","messageId":"xmqq6100ke7v.fsf@gitster.mtv.corp.google.com","threadId":"40959","inReplyTo":"CAP8UFD3at0X9ThpXGTwyPnu_dXFj6x=YzfkCa82m+RsWwhFOOA@mail.gmail.com","subject":"Re: [PATCH 7/8] config: add core.untrackedCache","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-12-14T19:44:04Z","receivedAt":"2015-12-14T19:44:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> In fact \"git update-index --[no-|force-]untracked-cache\" is very bad\n> because it means that two repositories can be configured differently\n> even if they have the same config files.\n\nIf you stop thinking that \"update-index --untracked-cache\" is\nsomehow a \"configuration\", things will get clearer to you.\n\nImagine there is no configuration whatsoever. The index primarily\nkeeps track of what will be in the next \"write-tree\", but optionally\nit can keep track of other kinds of information, such as the last\nunmerged states for paths whose conflicts have been resolved. Duy's\nwork adds another kind of information to the mix--cached stat bits\nfor untracked paths to speed up the next \"git status\", and the option\nis to start keeping track of that information in the index.\n\nBecause it is a \"cache\", once you start keeping track of it, you\nneed to maintain it--otherwise you will be fooled by stale\ninformation still in the cache. So of course the effect has to\npersist. There is nothing _wrong_ with that. If you want to stop\nmaintaining it, you can tell the index \"don't use untracked-cache\".\n\nSo I think the above \"is very bad because\" is total nonsense.\n\n> If you want only some repos to use the UC, you will set\n> core.untrackedCache in the repo config. Then after cloning such a\n> repo, you will copy the config file, and this will not be enough to\n> enable the UC.\n\nSurely. \"Does this index file keeps track of the untracked files'\nstates?\" is a property of the index. Cloning does not propagate the\nconfiguration and copying or not copying is irrelevant. If you want\nto enable, running \"update-index --untracked-cache\" is a way to do\nso. I cannot see what's so hard about it.\n\n> And if you have set core.untrackedCache in the global config when you\n> clone, UC is enabled, but if you have just set it in the repo config\n> after the clone, it is not enabled.\n\nThat's fine. In your patch series, if you set it in the global, you\nwill get the cache in the new one. With the cleaned-up semantics I\nsuggested, the same thing will happen.\n\nAnd with the cleaned-up semantics, the configuration is *ONLY* used\nto give the *DEFAULT* before other things happen, i.e. creation of\nthe index file for the first time. Because the configuration is\nonly the default, an explicit \"update-index --[no-]untracked-cache\"\nwill defeat it, just like any other config/option interaction.\n\nThe biggest issue I had with your patch series, IIRC, is that\nconfiguration will defeat the command line option.\n\n> Shouldn't it be nice if they could just enable core.untrackedCache in\n> the global config files without having to also cd into every repo and\n> use \"git update-index --untracked-cache\" there?\n\nNO. It is bad to change the behaviour behind users' back.\n\nSet that once, _future_ repositories their users will create will\nuse that by default, and tell their users what to do to their\nexisting repositories. Problem solved.\n\n> \"git update-index --[no-|force-]untracked-cache\" is a bad way, so\n> let's make it easy for people to not use it at all.\n\nAs I disagree with that basic premise, I have to disagree with the\nconclusion as well.\n"},{"id":"274437","messageId":"xmqqa8pciuqq.fsf@gitster.mtv.corp.google.com","threadId":"40959","inReplyTo":"xmqq6100ke7v.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 7/8] config: add core.untrackedCache","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-12-14T21:30:05Z","receivedAt":"2015-12-14T21:30:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> If you stop thinking that \"update-index --untracked-cache\" is\n> somehow a \"configuration\", things will get clearer to you.\n> ...\n>> \"git update-index --[no-|force-]untracked-cache\" is a bad way, so\n>> let's make it easy for people to not use it at all.\n>\n> As I disagree with that basic premise, I have to disagree with the\n> conclusion as well.\n\nHaving said all that, I do not think an option to \"update-index\"\nmust be the _only_ interface to tell the index to use (or ignore)\nthe untracked cache. Two obvious places that can also have the same\ncommand line option would be \"git init\" and \"git clone\". If either\nthe per-user configuration (or the per-site one the administrator\nsets) gave the default for these two commands, that would make it\nunnecessary to use \"update-index\", unless you are experimenting or\nworking around bugs in the implementation.\n\nThe primary reason why I do not like your \"configuration decides\" is\nit will be a huge source of confusions and bugs. Imagine what\nshould happen in this sequence, and when should a stale cached\ninformation be discarded?\n\n - the configuration is set to 'yes'.\n - the index is updated and written by various commands.\n - more work is done in the working tree without updating the index.\n - the configuration is set to 'no'.\n - more work is done in the working tree without updating the index.\n - the configuration is set to 'yes'.\n - more work is done in the working tree without updating the index.\n - somebody asks \"what untracked paths are there?\"\n\nIn the \"configuration decides\" world, I am not sure how a sane\nimplementation efficiently invalidates the cache as needed, without\nthe config subsystem having intimate knowledge with the untracked\ncache (so far, the config subsystem is merely a key-value store and\ndoes not care _what_ it stores; you would want to invalidate the\ncache in the index when somebody sets the variable to 'no', which\nmeans the config subsystem needs to know that this variable is\nspecial).\n"},{"id":"274483","messageId":"CAP8UFD0ex4JuU=FMDwnSnf9io16=D2_m3x1ajXtCu25OU_DLAw@mail.gmail.com","threadId":"40959","inReplyTo":"xmqqa8pciuqq.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 7/8] config: add core.untrackedCache","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2015-12-15T09:34:41Z","receivedAt":"2015-12-15T09:34:41Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Mon, Dec 14, 2015 at 10:30 PM, Junio C Hamano wrote:\n> Junio C Hamano writes:\n>\n> The primary reason why I do not like your \"configuration decides\" is\n> it will be a huge source of confusions and bugs. Imagine what\n> should happen in this sequence, and when should a stale cached\n> information be discarded?\n>\n> - the configuration is set to 'yes'.\n> - the index is updated and written by various commands.\n> - more work is done in the working tree without updating the index.\n> - the configuration is set to 'no'.\n> - more work is done in the working tree without updating the index.\n> - the configuration is set to 'yes'.\n> - more work is done in the working tree without updating the index.\n> - somebody asks \"what untracked paths are there?\"\n\nAs far as I understand the UC just stores the mtime of the directories\nin the working tree to avoid the need of lstat'ing all the files in\nthe directories.\n\nWhen somebody asks \"what untracked paths are there\", if the UC has not\nbeen discarded when the configuration was set to no, then git will\njust ask for the mtimes of the directories in the working tree and\ncompare them with what is in the UC.\n\nI don't see how it can create confusion and bugs, as the work done in\nthe working tree should anyway have changed the mtime of the\ndirectories where work has been done.\n\nMaybe as the UC has not been updated for a long time, there will be a\nlot of mtimes that are different, so there will not be a big speed up\nor it could be even slower than if the UC was not used, but that's\nall.\n\n> In the \"configuration decides\" world, I am not sure how a sane\n> implementation efficiently invalidates the cache as needed, without\n> the config subsystem having intimate knowledge with the untracked\n> cache (so far, the config subsystem is merely a key-value store and\n> does not care _what_ it stores; you would want to invalidate the\n> cache in the index when somebody sets the variable to 'no', which\n> means the config subsystem needs to know that this variable is\n> special).\n\nIn the current patch series and in the one I am preparing and will\nprobably send soon, this hunk takes care of removing or addind the UC\nif needed:\n\ndiff --git a/wt-status.c b/wt-status.c\nindex 435fc28..3e0fe02 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -586,6 +586,15 @@ static void wt_status_collect_untracked(struct\nwt_status *s)\n dir.flags |= DIR_SHOW_IGNORED_TOO;\n else\n dir.untracked = the_index.untracked;\n+\n+ if (!dir.untracked && use_untracked_cache == 1) {\n+ add_untracked_cache();\n+ dir.untracked = the_index.untracked;\n+ } else if (dir.untracked && use_untracked_cache == 0) {\n+ remove_untracked_cache();\n+ dir.untracked = NULL;\n+ }\n+\n setup_standard_excludes(&dir);\n\n fill_directory(&dir, &s->pathspec);\n\nSo when the config option is changed, the UC is removed or recreated\nonly the next time \"git status\" (and maybe also \"git commit\" and a few\nother commands) is called.\n\nAnd anyway I don't think people will change the UC config very often.\nMaybe they will play with it a bit when they discover it, but\nafterwards they will just set it or not and be done with it. So I\nthink it is not worth trying to optimize what happens when the config\nis set or unset. We should just make sure that it works and it is well\ndocumented.\n"},{"id":"274484","messageId":"566FE228.8040708@web.de","threadId":"40959","inReplyTo":"CAP8UFD0ex4JuU=FMDwnSnf9io16=D2_m3x1ajXtCu25OU_DLAw@mail.gmail.com","subject":"Re: [PATCH 7/8] config: add core.untrackedCache","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2015-12-15T09:49:28Z","receivedAt":"2015-12-15T09:49:28Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 15.12.15 10:34, Christian Couder wrote:\n> On Mon, Dec 14, 2015 at 10:30 PM, Junio C Hamano wrote:\n>> Junio C Hamano writes:\n>>\n>> The primary reason why I do not like your \"configuration decides\" is\n>> it will be a huge source of confusions and bugs. Imagine what\n>> should happen in this sequence, and when should a stale cached\n>> information be discarded?\n>>\n>> - the configuration is set to 'yes'.\n>> - the index is updated and written by various commands.\n>> - more work is done in the working tree without updating the index.\n>> - the configuration is set to 'no'.\n>> - more work is done in the working tree without updating the index.\n>> - the configuration is set to 'yes'.\n>> - more work is done in the working tree without updating the index.\n>> - somebody asks \"what untracked paths are there?\"\n> \n\n> As far as I understand the UC just stores the mtime of the directories\n> in the working tree to avoid the need of lstat'ing all the files in\n> the directories.\n\nThis is what I understand:\nUC stores the mtime of the directories in the working tree to avoid the need \nopendir() readdir() closedir() to find new, yet untracked, files.\n(including sub-directories)\n"},{"id":"274485","messageId":"CACsJy8CG6LXpUaS09CqaoqLEqzexb14JA02o4HcoxdwM05QQ=g@mail.gmail.com","threadId":"40959","inReplyTo":"CAP8UFD0ex4JuU=FMDwnSnf9io16=D2_m3x1ajXtCu25OU_DLAw@mail.gmail.com","subject":"Re: [PATCH 7/8] config: add core.untrackedCache","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-12-15T10:02:20Z","receivedAt":"2015-12-15T10:02:20Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Dec 15, 2015 at 4:34 PM, Christian Couder\n wrote:\n> On Mon, Dec 14, 2015 at 10:30 PM, Junio C Hamano wrote:\n>> Junio C Hamano writes:\n>>\n>> The primary reason why I do not like your \"configuration decides\" is\n>> it will be a huge source of confusions and bugs. Imagine what\n>> should happen in this sequence, and when should a stale cached\n>> information be discarded?\n>>\n>> - the configuration is set to 'yes'.\n>> - the index is updated and written by various commands.\n>> - more work is done in the working tree without updating the index.\n>> - the configuration is set to 'no'.\n>> - more work is done in the working tree without updating the index.\n>> - the configuration is set to 'yes'.\n>> - more work is done in the working tree without updating the index.\n>> - somebody asks \"what untracked paths are there?\"\n>\n> As far as I understand the UC just stores the mtime of the directories\n> in the working tree to avoid the need of lstat'ing all the files in\n> the directories.\n>\n> When somebody asks \"what untracked paths are there\", if the UC has not\n> been discarded when the configuration was set to no, then git will\n> just ask for the mtimes of the directories in the working tree and\n> compare them with what is in the UC.\n>\n> I don't see how it can create confusion and bugs, as the work done in\n> the working tree should anyway have changed the mtime of the\n> directories where work has been done.\n\nAny operation that can add or remove an entry from the index may lead\nto UC update. For example, if file \"foo\" is tracked, then the user\ndoes \"git rm --cached foo\", \"foo\" may become either untracked or\nignored. So if you disable UC while doing this removal, then re-enable\nUC again, \"git-status\" may show incorrect output. So, as long as UC\nextension exists in the index, it must be updated, or it may become\noutdated and useless.\n-- \nDuy\n"},{"id":"274489","messageId":"CACBZZX6=sU2cb_vRn5DAqVEuNTwsk0m7vQ0_WUp5qPWeQq5JhQ@mail.gmail.com","threadId":"40959","inReplyTo":"xmqqa8pciuqq.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 7/8] config: add core.untrackedCache","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2015-12-15T13:04:16Z","receivedAt":"2015-12-15T13:04:16Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Mon, Dec 14, 2015 at 8:44 PM, Junio C Hamano wrote:\n\nI'm replying to & quoting from two E-Mails of yours at once here for\nclarity & less noise. I'm working wich Christian on getting this\nintegrated, and we both thought it would be good to have some fresh\ninput on the matter from me.\n\n> Christian Couder writes:\n\n>> If you want only some repos to use the UC, you will set\n>> core.untrackedCache in the repo config. Then after cloning such a\n>> repo, you will copy the config file, and this will not be enough to\n>> enable the UC.\n>\n> Surely. \"Does this index file keeps track of the untracked files'\n> states?\" is a property of the index. Cloning does not propagate the\n> configuration and copying or not copying is irrelevant. If you want\n> to enable, running \"update-index --untracked-cache\" is a way to do\n> so. I cannot see what's so hard about it.\n>\n>> And if you have set core.untrackedCache in the global config when you\n>> clone, UC is enabled, but if you have just set it in the repo config\n>> after the clone, it is not enabled.\n>\n> That's fine. In your patch series, if you set it in the global, you\n> will get the cache in the new one. With the cleaned-up semantics I\n> suggested, the same thing will happen.\n>\n> And with the cleaned-up semantics, the configuration is *ONLY* used\n> to give the *DEFAULT* before other things happen, i.e. creation of\n> the index file for the first time. Because the configuration is\n> only the default, an explicit \"update-index --[no-]untracked-cache\"\n> will defeat it, just like any other config/option interaction.\n\nAs you know Christian is working on this for Booking.com to integrate\nfeatures we find useful into git.git in such a way that we don't have\nto maintain some internal fork of Git.\n\nWhat we're trying to do, and what a lot of other big deployments of\nGit elsewhere would also find useful, is to ship a default sensible\nconfiguration for all users on the system in /etc/gitconfig.\n\nI'd like to be able to easily enable some feature that aids Git\nperformance globally on our thousands of machines and for our hundreds\nof users by just tweaking something in puppet to change\n/etc/gitconfig, and more importantly if that change ends up being bad\nreverting that config in /etc/gitconfig should undo the change.\n\nIt's an unacceptable level of complexity for system-level automation\nto have to scour the filesystem for existing Git repositories and run\n\"git update-index\" on each of them, that's why we're submitting\npatches to make this a config option, so we can simply flip a flag in\n/etc/gitconfig.\n\nIt's also unacceptable to have the config simply provide the default\nwhich'll be frozen either at clone time or after an initial \"git\nstatus\".\n\nLet's say I ship a /etc/gitconfig that says \"new clones should use the\nuntracked cache\". Now I roll that out across our fleet of machines and\nit turns out the morning after that the feature doesn't work properly\nfor whatever reason. If it's just a \"default until clone or status\"\ntype of thing even if I revert the configuration a lot of users &\ntheir repositories in the wild will still be broken, and will have to\nbe manually fixed. Which again leads to the scouring the filesystem\nproblem.\n\nSo that gives some more context for why we're pushing for this change.\nI believe this feature breaks no existing use-case and just supports\nnew ones, and I think that your objections to it are based on a simple\nmisunderstanding as will become apparent if you read on below.\n\n> The biggest issue I had with your patch series, IIRC, is that\n> configuration will defeat the command line option.\n\nI think it's a moot point to focus on configuration v.s. command-line\noption. The important question is whether or not this feature can\nstill be configured on a repo-local basis with this series as before.\nThat's still the case since --local git configuration overrides\n--global and --system, so users who want to enable/disable this\nper-repo still can.\n\n>> Shouldn't it be nice if they could just enable core.untrackedCache in\n>> the global config files without having to also cd into every repo and\n>> use \"git update-index --untracked-cache\" there?\n>\n> NO. It is bad to change the behaviour behind users' back.\n\nI'm not quite sure what the objection here is exactly. If you're a\nnormal user you can enable/disable this per-repo just like you can\nnow, and enable/disable it for all your repos in ~/.gitconfig.\n\nIf you mean that the user's configuration shouldn't be changed by the\nglobal config in /etc/gitconfig I do think that's a moot point. If\nyou're a user on a system where I have root and I want to change your\nGit configuration I'm going to be able to do that whatever the\nmechanism is.\n\nThat's indeed that's what we're doing to enable this at Booking.com\ncurrently, we run a job to find some limited set of common checkouts\nand run \"git update-index\" for users as root. The problem with that is\nthat it's needlessly complex, hence this series.\n\nBut in case you mean disabling the config for existing checkouts if\nthere's no configuration, I address that at the end of this mail.\n\n[...]\n> The primary reason why I do not like your \"configuration decides\" is\n> it will be a huge source of confusions and bugs. Imagine what\n> should happen in this sequence, and when should a stale cached\n> information be discarded?\n>\n> - the configuration is set to 'yes'.\n> - the index is updated and written by various commands.\n> - more work is done in the working tree without updating the index.\n> - the configuration is set to 'no'.\n> - more work is done in the working tree without updating the index.\n> - the configuration is set to 'yes'.\n> - more work is done in the working tree without updating the index.\n> - somebody asks \"what untracked paths are there?\"\n>\n> In the \"configuration decides\" world, I am not sure how a sane\n> implementation efficiently invalidates the cache as needed, without\n> the config subsystem having intimate knowledge with the untracked\n> cache (so far, the config subsystem is merely a key-value store and\n> does not care _what_ it stores; you would want to invalidate the\n> cache in the index when somebody sets the variable to 'no', which\n> means the config subsystem needs to know that this variable is\n> special).\n\nI think this is the main misunderstanding about how this works that\nneeds to be clarified.\n\nIt would indeed really suck if changing this to some configuration\noption introduced some race condition where fiddling with the config\noption would render the cache stale & invalid. I fully agree that that\nshould prevent the inclusion of this patch series.\n\nThe way the \"config decides\" patch series deals with this is that if\nyou have the UC information in the index and the configuration is set\nto core.untrackedCache=false the UC will be removed from the index.\n\nOtherwise you would indeed easily end up with a stale cache, since you\ncould disable it, then make some tweaks to the index or your files,\nand then subsequently enable it and end up with nonsensical \"git\nstatus\" output.\n\nThere's a test for this in t/t7063-status-untracked-cache.sh called\n\"unsetting core.untrackedCache and using git status removes the\ncache\".\n\nSumming this up: The only thing that this configuration potentially\n*does* break, and doesn't address, and which could be fixed. Is the\nfollowing scenario. Once this series is applied and git is shipped\nwith it existing users that have set \"git update-index\n--untracked-cache\" will have their UC feature disabled. This is\nbecause we fall back to \"oh no config here? Must have been disabled,\nrm it from the index\" clause which keeps our UC from going stale in\nthe face of config flip-flopping.\n\nWe *could* make even that use-case work by detecting the legacy marker\nfor the UC in the index (the uname info), then we'd do a one-time \"git\nconfig --local core.untrackedCache true\" and remove the marker. Thus\nusers who upgrade git and had the untracked cache enabled already\nwould have their checkouts migrated to whatever their existing setup\nwas.\n\nI don't think that's worth it for two reasons 1) This is a really new\nexperimental feature and I think it's fine to just change how it works\n2) It's just as likely that this surprises users and doesn't do what\nthey want, i.e. someone will think \"neat, I can toggle this in\n~/.gitconfig now\", then they set it to \"false\" because they don't want\nit, but some of their existing checkouts that had it enabled will be\nmigrated to \"core.untrackedCache=true\".\n\nSo I think it makes the most sense to just apply this as-is.\n"},{"id":"274490","messageId":"CAP8UFD2ZUnWWO5T91pYjsDtXgFecZkJuu9fjQ_s8at=01bzxvQ@mail.gmail.com","threadId":"40959","inReplyTo":"CACBZZX6=sU2cb_vRn5DAqVEuNTwsk0m7vQ0_WUp5qPWeQq5JhQ@mail.gmail.com","subject":"Re: [PATCH 7/8] config: add core.untrackedCache","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2015-12-15T13:42:24Z","receivedAt":"2015-12-15T13:42:24Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Tue, Dec 15, 2015 at 2:04 PM, Ævar Arnfjörð Bjarmason\n wrote:\n> On Mon, Dec 14, 2015 at 8:44 PM, Junio C Hamano wrote:\n>\n> I'm replying to & quoting from two E-Mails of yours at once here for\n> clarity & less noise. I'm working wich Christian on getting this\n> integrated, and we both thought it would be good to have some fresh\n> input on the matter from me.\n>\n>> Christian Couder writes:\n>\n>>> If you want only some repos to use the UC, you will set\n>>> core.untrackedCache in the repo config. Then after cloning such a\n>>> repo, you will copy the config file, and this will not be enough to\n>>> enable the UC.\n>>\n>> Surely. \"Does this index file keeps track of the untracked files'\n>> states?\" is a property of the index. Cloning does not propagate the\n>> configuration and copying or not copying is irrelevant. If you want\n>> to enable, running \"update-index --untracked-cache\" is a way to do\n>> so. I cannot see what's so hard about it.\n>>\n>>> And if you have set core.untrackedCache in the global config when you\n>>> clone, UC is enabled, but if you have just set it in the repo config\n>>> after the clone, it is not enabled.\n>>\n>> That's fine. In your patch series, if you set it in the global, you\n>> will get the cache in the new one. With the cleaned-up semantics I\n>> suggested, the same thing will happen.\n>>\n>> And with the cleaned-up semantics, the configuration is *ONLY* used\n>> to give the *DEFAULT* before other things happen, i.e. creation of\n>> the index file for the first time. Because the configuration is\n>> only the default, an explicit \"update-index --[no-]untracked-cache\"\n>> will defeat it, just like any other config/option interaction.\n>\n> As you know Christian is working on this for Booking.com to integrate\n> features we find useful into git.git in such a way that we don't have\n> to maintain some internal fork of Git.\n>\n> What we're trying to do, and what a lot of other big deployments of\n> Git elsewhere would also find useful, is to ship a default sensible\n> configuration for all users on the system in /etc/gitconfig.\n>\n> I'd like to be able to easily enable some feature that aids Git\n> performance globally on our thousands of machines and for our hundreds\n> of users by just tweaking something in puppet to change\n> /etc/gitconfig, and more importantly if that change ends up being bad\n> reverting that config in /etc/gitconfig should undo the change.\n>\n> It's an unacceptable level of complexity for system-level automation\n> to have to scour the filesystem for existing Git repositories and run\n> \"git update-index\" on each of them, that's why we're submitting\n> patches to make this a config option, so we can simply flip a flag in\n> /etc/gitconfig.\n>\n> It's also unacceptable to have the config simply provide the default\n> which'll be frozen either at clone time or after an initial \"git\n> status\".\n>\n> Let's say I ship a /etc/gitconfig that says \"new clones should use the\n> untracked cache\". Now I roll that out across our fleet of machines and\n> it turns out the morning after that the feature doesn't work properly\n> for whatever reason. If it's just a \"default until clone or status\"\n> type of thing even if I revert the configuration a lot of users &\n> their repositories in the wild will still be broken, and will have to\n> be manually fixed. Which again leads to the scouring the filesystem\n> problem.\n>\n> So that gives some more context for why we're pushing for this change.\n> I believe this feature breaks no existing use-case and just supports\n> new ones, and I think that your objections to it are based on a simple\n> misunderstanding as will become apparent if you read on below.\n>\n>> The biggest issue I had with your patch series, IIRC, is that\n>> configuration will defeat the command line option.\n>\n> I think it's a moot point to focus on configuration v.s. command-line\n> option. The important question is whether or not this feature can\n> still be configured on a repo-local basis with this series as before.\n> That's still the case since --local git configuration overrides\n> --global and --system, so users who want to enable/disable this\n> per-repo still can.\n>\n>>> Shouldn't it be nice if they could just enable core.untrackedCache in\n>>> the global config files without having to also cd into every repo and\n>>> use \"git update-index --untracked-cache\" there?\n>>\n>> NO. It is bad to change the behaviour behind users' back.\n>\n> I'm not quite sure what the objection here is exactly. If you're a\n> normal user you can enable/disable this per-repo just like you can\n> now, and enable/disable it for all your repos in ~/.gitconfig.\n>\n> If you mean that the user's configuration shouldn't be changed by the\n> global config in /etc/gitconfig I do think that's a moot point. If\n> you're a user on a system where I have root and I want to change your\n> Git configuration I'm going to be able to do that whatever the\n> mechanism is.\n>\n> That's indeed that's what we're doing to enable this at Booking.com\n> currently, we run a job to find some limited set of common checkouts\n> and run \"git update-index\" for users as root. The problem with that is\n> that it's needlessly complex, hence this series.\n>\n> But in case you mean disabling the config for existing checkouts if\n> there's no configuration, I address that at the end of this mail.\n>\n> [...]\n>> The primary reason why I do not like your \"configuration decides\" is\n>> it will be a huge source of confusions and bugs. Imagine what\n>> should happen in this sequence, and when should a stale cached\n>> information be discarded?\n>>\n>> - the configuration is set to 'yes'.\n>> - the index is updated and written by various commands.\n>> - more work is done in the working tree without updating the index.\n>> - the configuration is set to 'no'.\n>> - more work is done in the working tree without updating the index.\n>> - the configuration is set to 'yes'.\n>> - more work is done in the working tree without updating the index.\n>> - somebody asks \"what untracked paths are there?\"\n>>\n>> In the \"configuration decides\" world, I am not sure how a sane\n>> implementation efficiently invalidates the cache as needed, without\n>> the config subsystem having intimate knowledge with the untracked\n>> cache (so far, the config subsystem is merely a key-value store and\n>> does not care _what_ it stores; you would want to invalidate the\n>> cache in the index when somebody sets the variable to 'no', which\n>> means the config subsystem needs to know that this variable is\n>> special).\n>\n> I think this is the main misunderstanding about how this works that\n> needs to be clarified.\n>\n> It would indeed really suck if changing this to some configuration\n> option introduced some race condition where fiddling with the config\n> option would render the cache stale & invalid. I fully agree that that\n> should prevent the inclusion of this patch series.\n>\n> The way the \"config decides\" patch series deals with this is that if\n> you have the UC information in the index and the configuration is set\n> to core.untrackedCache=false the UC will be removed from the index.\n>\n> Otherwise you would indeed easily end up with a stale cache, since you\n> could disable it, then make some tweaks to the index or your files,\n> and then subsequently enable it and end up with nonsensical \"git\n> status\" output.\n>\n> There's a test for this in t/t7063-status-untracked-cache.sh called\n> \"unsetting core.untrackedCache and using git status removes the\n> cache\".\n>\n> Summing this up: The only thing that this configuration potentially\n> *does* break, and doesn't address, and which could be fixed. Is the\n> following scenario. Once this series is applied and git is shipped\n> with it existing users that have set \"git update-index\n> --untracked-cache\" will have their UC feature disabled. This is\n> because we fall back to \"oh no config here? Must have been disabled,\n> rm it from the index\" clause which keeps our UC from going stale in\n> the face of config flip-flopping.\n>\n> We *could* make even that use-case work by detecting the legacy marker\n> for the UC in the index (the uname info), then we'd do a one-time \"git\n> config --local core.untrackedCache true\" and remove the marker. Thus\n> users who upgrade git and had the untracked cache enabled already\n> would have their checkouts migrated to whatever their existing setup\n> was.\n\nThe above needs a patch, that I haven't sent yet, but will send really\nsoon now, and that removes everything from the \"ident\" field in the\nUC, because this field is useless with the patch series.\n\n> I don't think that's worth it for two reasons 1) This is a really new\n> experimental feature and I think it's fine to just change how it works\n> 2) It's just as likely that this surprises users and doesn't do what\n> they want, i.e. someone will think \"neat, I can toggle this in\n> ~/.gitconfig now\", then they set it to \"false\" because they don't want\n> it, but some of their existing checkouts that had it enabled will be\n> migrated to \"core.untrackedCache=true\".\n>\n> So I think it makes the most sense to just apply this as-is.\n\nOr rather to apply something based on the patch series I will send soon.\n"},{"id":"274507","messageId":"CAP8UFD1gJGASyhkks4BdVwdCdZRRcwWR8inq291ft_3dg2y6XQ@mail.gmail.com","threadId":"40959","inReplyTo":"CACsJy8CG6LXpUaS09CqaoqLEqzexb14JA02o4HcoxdwM05QQ=g@mail.gmail.com","subject":"Re: [PATCH 7/8] config: add core.untrackedCache","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2015-12-15T16:35:51Z","receivedAt":"2015-12-15T16:35:51Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Tue, Dec 15, 2015 at 11:02 AM, Duy Nguyen wrote:\n> On Tue, Dec 15, 2015 at 4:34 PM, Christian Couder\n> wrote:\n>> On Mon, Dec 14, 2015 at 10:30 PM, Junio C Hamano wrote:\n>>> Junio C Hamano writes:\n>>>\n>>> The primary reason why I do not like your \"configuration decides\" is\n>>> it will be a huge source of confusions and bugs. Imagine what\n>>> should happen in this sequence, and when should a stale cached\n>>> information be discarded?\n>>>\n>>> - the configuration is set to 'yes'.\n>>> - the index is updated and written by various commands.\n>>> - more work is done in the working tree without updating the index.\n>>> - the configuration is set to 'no'.\n>>> - more work is done in the working tree without updating the index.\n>>> - the configuration is set to 'yes'.\n>>> - more work is done in the working tree without updating the index.\n>>> - somebody asks \"what untracked paths are there?\"\n>>\n>> As far as I understand the UC just stores the mtime of the directories\n>> in the working tree to avoid the need of lstat'ing all the files in\n>> the directories.\n>>\n>> When somebody asks \"what untracked paths are there\", if the UC has not\n>> been discarded when the configuration was set to no, then git will\n>> just ask for the mtimes of the directories in the working tree and\n>> compare them with what is in the UC.\n>>\n>> I don't see how it can create confusion and bugs, as the work done in\n>> the working tree should anyway have changed the mtime of the\n>> directories where work has been done.\n>\n> Any operation that can add or remove an entry from the index may lead\n> to UC update. For example, if file \"foo\" is tracked, then the user\n> does \"git rm --cached foo\", \"foo\" may become either untracked or\n> ignored. So if you disable UC while doing this removal, then re-enable\n> UC again, \"git-status\" may show incorrect output. So, as long as UC\n> extension exists in the index, it must be updated, or it may become\n> outdated and useless.\n\nWhen UC is disabled, it is removed from the index, and when it is\n(re-)enabled, it is recreated, so I don't think that your example\napplies to the code in this patch.\n\nThanks anyway for this insight,\nChristian.\n"},{"id":"274508","messageId":"CAP8UFD1g_jZuZdrPs4V1w5Pgtaq9_1NeLkqA5gSGmRQz79bsQA@mail.gmail.com","threadId":"40959","inReplyTo":"566FE228.8040708@web.de","subject":"Re: [PATCH 7/8] config: add core.untrackedCache","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2015-12-15T16:42:40Z","receivedAt":"2015-12-15T16:42:40Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Tue, Dec 15, 2015 at 10:49 AM, Torsten Bögershausen wrote:\n> On 15.12.15 10:34, Christian Couder wrote:\n>> On Mon, Dec 14, 2015 at 10:30 PM, Junio C Hamano wrote:\n>>> Junio C Hamano writes:\n>>>\n>>> The primary reason why I do not like your \"configuration decides\" is\n>>> it will be a huge source of confusions and bugs. Imagine what\n>>> should happen in this sequence, and when should a stale cached\n>>> information be discarded?\n>>>\n>>> - the configuration is set to 'yes'.\n>>> - the index is updated and written by various commands.\n>>> - more work is done in the working tree without updating the index.\n>>> - the configuration is set to 'no'.\n>>> - more work is done in the working tree without updating the index.\n>>> - the configuration is set to 'yes'.\n>>> - more work is done in the working tree without updating the index.\n>>> - somebody asks \"what untracked paths are there?\"\n>>\n>\n>> As far as I understand the UC just stores the mtime of the directories\n>> in the working tree to avoid the need of lstat'ing all the files in\n>> the directories.\n>\n> This is what I understand:\n> UC stores the mtime of the directories in the working tree to avoid the need\n> opendir() readdir() closedir() to find new, yet untracked, files.\n> (including sub-directories)\n\nI think you are probably right too.\n\nIn the v2 patch series I just sent, there is:\n\n+This feature works by recording the mtime of the working tree\n+directories and then omitting reading directories and stat calls\n+against files in those directories whose mtime hasn't changed.\n\nI hope it is better.\n\nThanks,\nChristian.\n"},{"id":"274510","messageId":"xmqqh9jjfqk4.fsf@gitster.mtv.corp.google.com","threadId":"40959","inReplyTo":"CACBZZX6=sU2cb_vRn5DAqVEuNTwsk0m7vQ0_WUp5qPWeQq5JhQ@mail.gmail.com","subject":"Re: [PATCH 7/8] config: add core.untrackedCache","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-12-15T19:40:59Z","receivedAt":"2015-12-15T19:40:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> The way the \"config decides\" patch series deals with this is that if\n> you have the UC information in the index and the configuration is set\n> to core.untrackedCache=false the UC will be removed from the index.\n>\n> Otherwise you would indeed easily end up with a stale cache,...\n\nYeah, that's \"correctness\" thing; it goes without saying that it\nwould be unacceptable if the series did not get this right.\n\nI still have a problem with the approach from \"design cleanliness\"\npoint of view, primarily that the index already has a bit to tell us\nif the user already said that she wants to use the feature, but\nbecause you want to make config win, the code needs to always read\nthe config to allow it to disable the feature in the index, just in\ncase the data that is already in the index says otherwise, and the\ncode has to keep doing that even after the data is removed [*1*].\nUnfortunately, I do not think you can solve the \"design cleanliness\"\nproblem unless you give up \"we want /etc/gitconfig override\".\n\nIn any case I think we already have agreed to disagree on this\npoint, so there is no use discussing it any longer from my side. I\nam not closing the door to this series, but I am not convinced,\neither. At least not yet.\n\n> Once this series is applied and git is shipped with it, existing\n> users that have set \"git update-index --untracked-cache\" will have\n> their UC feature disabled.\n\nWell, the fix to that is merely just a one-shot thing away, so it\nwould not be too much of a hassle, no [*2*]? So I do not think it\nwould be such a big issue.\n\n> We *could* make even that use-case work by detecting the legacy marker\n> for the UC in the index (the uname info), then we'd do a one-time \"git\n> config --local core.untrackedCache true\" and remove the marker.\n\nI do not think we want to go there---that would mean you would need\nto revamp the repository format version because the old tools would\nbe now unusable on the index/config combo your version mucked with.\n\n\n[Footnote]\n\n*1* I also do not want to see that design pattern imitated and used\n in other parts of the system, and this gives a precedent for\n people to copy.\n\n*2* Yet, those who are broken by this series may say \"it is\n unacceptable that we have to survey all existing repositories\n and selectively add the configuration to the ones that have\n enabled the feature before updating.\", the same way you complain\n against the \"index already knows\" approach.\n"},{"id":"274514","messageId":"CACBZZX7QW2J6DcMSXTa1y+QdMrqq5DXs1Fu3m8toV5a4yZKNjw@mail.gmail.com","threadId":"40959","inReplyTo":"xmqqh9jjfqk4.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 7/8] config: add core.untrackedCache","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2015-12-15T21:53:09Z","receivedAt":"2015-12-15T21:53:09Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Tue, Dec 15, 2015 at 8:40 PM, Junio C Hamano wrote:\n> Ævar Arnfjörð Bjarmason writes:\n> I still have a problem with the approach from \"design cleanliness\"\n> point of view[...]\n>\n> In any case I think we already have agreed to disagree on this\n> point, so there is no use discussing it any longer from my side. I\n> am not closing the door to this series, but I am not convinced,\n> either. At least not yet.\n\nIn general the fantastic thing about the git configuration facility is\nthat it provides both systems administrators and normal users with\nwhat they want. It's possible to configure things system-wide and\noverride those on a user or repository basis.\n\nOf course hindsight is 20/20, but I think that given what's been\ncovered in this thread it's been established that it's categorically\nbetter that if we introduce features like these that they be\nconfigured through the normal configuration facility rather than the\nconfiguration being sticky to the index. It gives you everything that\nthe per-index configuration gives you and more.\n\nSo assuming that's the case, how do we migrate something that's\nconfigured via the index towards being configured through git-config?\n\nI think there's no general answer to that, but in this case the worst\ncase scenario with accepting this series as-is is that we downgrade\nsome users who've opted in to it to pre-v2.5.0 \"git status\"\nperformance.\n\nSince the change in performance really isn't noticeable except on\nreally large repositories, which are more likely to have someone\ninvolved watching the changelog on upgrades I think that's OK.\n"},{"id":"274518","messageId":"xmqqy4cvco25.fsf@gitster.mtv.corp.google.com","threadId":"40959","inReplyTo":"CACBZZX7QW2J6DcMSXTa1y+QdMrqq5DXs1Fu3m8toV5a4yZKNjw@mail.gmail.com","subject":"Re: [PATCH 7/8] config: add core.untrackedCache","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-12-15T23:03:14Z","receivedAt":"2015-12-15T23:03:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Of course hindsight is 20/20, but I think that given what's been\n> covered in this thread it's been established that it's categorically\n> better that if we introduce features like these that they be\n> configured through the normal configuration facility rather than the\n> configuration being sticky to the index.\n\nI doubt that any such thing has been established at all in this\nthread. It may be true that you and perhaps Christian loudly\nrepeated it, but loudly repeating something and establishing\nsomething as a fact are slightly different.\n\nThe thing is, I do not necessarily view this as \"configuration\".\nThe way I see the feature is that you say \"--untracked\" when you\nwant the states of untracked paths be kept track of in the index,\njust like you say \"git add Makefile\" when you want the state of\n'Makefile' be kept track of in the index. Either the index keeps\ntrack of it, or it doesn't, based solely on user's request, and the\nbit to tell us which is the case is already in the index, exactly\nbecause that is part of the data that is kept track of in the index.\n\n> Since the change in performance really isn't noticeable except on\n> really large repositories, which are more likely to have someone\n> involved watching the changelog on upgrades I think that's OK.\n\nEspecially it is dubious to me that the trade-off you are making\nwith this design is a good one. In order to avoid paying a one-time\ncost to run \"update-index --untracked-cache\" at sites that _do_ want\nto use that feature (and after that, if you teach \"git init\" and\n\"git clone\" to pay attention to the \"give you the default\"\nconfiguration to run it for you, so that your users won't have to),\nyou are forcing all codepaths that makes any write to the index (not\njust \"init\"-time) to make an extra check with the configuration all\nthe time for everybody, because you made the presence of the\nuntracked cache data in the index not usable as a sign that the user\nwants to use that feature. If the feature is something only those\nwith really large repositories care about, is it a good trade-off to\nmake everybody pay the runtime cost and make code more complex and\nfragile? I am not yet convinced.\n"},{"id":"274542","messageId":"CACBZZX4joSbCTGQqN=u6FjG2V1-7T+0P+KDwZYi14a-9OQV=LQ@mail.gmail.com","threadId":"40959","inReplyTo":"xmqqy4cvco25.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 7/8] config: add core.untrackedCache","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2015-12-16T01:10:06Z","receivedAt":"2015-12-16T01:10:06Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Wed, Dec 16, 2015 at 12:03 AM, Junio C Hamano wrote:\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> Of course hindsight is 20/20, but I think that given what's been\n>> covered in this thread it's been established that it's categorically\n>> better that if we introduce features like these that they be\n>> configured through the normal configuration facility rather than the\n>> configuration being sticky to the index.\n>\n> I doubt that any such thing has been established at all in this\n> thread. It may be true that you and perhaps Christian loudly\n> repeated it, but loudly repeating something and establishing\n> something as a fact are slightly different.\n>\n> The thing is, I do not necessarily view this as \"configuration\".\n> The way I see the feature is that you say \"--untracked\" when you\n> want the states of untracked paths be kept track of in the index.\n\nYou probably know this, but the --untracked-cache has no bearing on\nwhat we actually keep track of, it's just an optimization for how\nefficiently we execute \"git status\" commands without the \"-uno\"\noption. We still produce the same output.\n\n> just like you say \"git add Makefile\" when you want the state of\n> 'Makefile' be kept track of in the index. Either the index keeps\n> track of it, or it doesn't, based solely on user's request, and the\n> bit to tell us which is the case is already in the index, exactly\n> because that is part of the data that is kept track of in the index.\n\nWhat I mean by \"[we've] established that it's categorically better [to\ndo this via git-config]\" is that we can still do all that stuff, we\ncan just also do more stuff now.\n\n>> Since the change in performance really isn't noticeable except on\n>> really large repositories, which are more likely to have someone\n>> involved watching the changelog on upgrades I think that's OK.\n>\n> Especially it is dubious to me that the trade-off you are making\n> with this design is a good one. In order to avoid paying a one-time\n> cost to run \"update-index --untracked-cache\" at sites that _do_ want\n> to use that feature (and after that, if you teach \"git init\" and\n> \"git clone\" to pay attention to the \"give you the default\"\n> configuration to run it for you, so that your users won't have to),\n\nIt's not unreasonable to avoid the cost of running \"update-index\n--untracked-cache\", it's the difference between just adjusting\n/etc/gitconfig and continually having to traverse the entire /\nfilesystem if you want to enable this feature on a system-wide basis.\nIt should be easy to enable any Git feature via the configuration\nfacility either on a --system, or --global or --local basis.\n\n> you are forcing all codepaths that makes any write to the index (not\n> just \"init\"-time) to make an extra check with the configuration all\n> the time for everybody, because you made the presence of the\n> untracked cache data in the index not usable as a sign that the user\n> wants to use that feature.\n\nMaybe I'm misunderstanding Christian's patches but don't we already\nparse the git configuration on any commands that update the index\nanyway? See git_default_core_config().\nWe already parse the git configuration to run \"git status\".\n\n> If the feature is something only those\n> with really large repositories care about, is it a good trade-off to\n> make everybody pay the runtime cost and make code more complex and\n> fragile? I am not yet convinced.\n\nI was arguing that only users with really large repositories would\nnotice if we turned this off because the enabling facility had changed\nfrom per-index to config. But it doesn't follow that the expense of\nchecking the git configuration which we're parsing anyway for the\nindex-related commands makes things more complex & fragile.\n"},{"id":"274543","messageId":"20151216024605.GA618@sigill.intra.peff.net","threadId":"40959","inReplyTo":"xmqqy4cvco25.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 7/8] config: add core.untrackedCache","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-12-16T02:46:05Z","receivedAt":"2015-12-16T02:46:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Dec 15, 2015 at 03:03:14PM -0800, Junio C Hamano wrote:\n\n> The thing is, I do not necessarily view this as \"configuration\".\n> The way I see the feature is that you say \"--untracked\" when you\n> want the states of untracked paths be kept track of in the index,\n> just like you say \"git add Makefile\" when you want the state of\n> 'Makefile' be kept track of in the index. Either the index keeps\n> track of it, or it doesn't, based solely on user's request, and the\n> bit to tell us which is the case is already in the index, exactly\n> because that is part of the data that is kept track of in the index.\n\nI know this is a fairly subjective argument, but it feels quite weird\nfor me for such a config to persist in the index and not be mentioned\nanywhere else.\n\nIs there any other user-specified configuration option for which:\n\n rm -f .git/index\n git read-tree HEAD\n\nwill actually _lose_ information?\n\nIt seems to me that all other things being equal, we should be in favor\nof a config option simply because it reduces the cognitive burden on the\nuser: it's one fewer place they need to be aware that git is keeping\npersistent decisions.\n\n> If the feature is something only those with really large repositories\n> care about, is it a good trade-off to make everybody pay the runtime\n> cost and make code more complex and fragile? I am not yet convinced.\n\nI'm not sure I understand the runtime and complexity costs of the config\noption. Isn't it just:\n\n if (core_untracked_cache) {\n\t/* do the thing */\n }\n\nand loading core_untracked_cache in git_default_core_config()? Versus:\n\n if (the_index.has_untracked_cache) {\n /* do the thing */\n }\n\nI ask as somebody who hasn't followed the topic closely. I just don't\nsee what is that different about this versus other config options. What\nam I missing?\n\n-Peff\n"},{"id":"274549","messageId":"xmqqwpsfdl5y.fsf@gitster.mtv.corp.google.com","threadId":"40959","inReplyTo":"20151216024605.GA618@sigill.intra.peff.net","subject":"Re: [PATCH 7/8] config: add core.untrackedCache","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-12-16T05:20:25Z","receivedAt":"2015-12-16T05:20:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n>> If the feature is something only those with really large repositories\n>> care about, is it a good trade-off to make everybody pay the runtime\n>> cost and make code more complex and fragile? I am not yet convinced.\n>\n> I'm not sure I understand the runtime and complexity costs of the config\n> option. Isn't it just:\n>\n> if (core_untracked_cache) {\n> \t/* do the thing */\n> }\n>\n> and loading core_untracked_cache in git_default_core_config()? Versus:\n>\n> if (the_index.has_untracked_cache) {\n> /* do the thing */\n> }\n\nThe latter is pretty much so, but the former needs to be a bit more\ninvolved, e.g. more like:\n\n if (core_untracked_cache) {\n if (!the_index.has_untracked_cache)\n create_and_attach_uc(&the_index);\n /* do the thing */\n } else {\n if (the_index.has_untracked_cache)\n destroy and remove untracked cache;\n }\n\nOtherwise, after adding the cache to the index, flipping the config\noff, doing things with the index and working tree and then filpping\nthe config back on, the index would have a stale cache that does not\nknow what happened to the working tree while config was off, and\nyour \"git status\" will start throwing a wrong result.\n\nThis is why the_index.has_untracked_cache is not just a simple \"Do I\nwant to use this feature?\" boolean configuration. The index also\nstores the real data, and \"Am I using this feature?\" bit goes hand\nin hand with that real data. Thinking that this is merely a boolean\nconfiguration is the real source of the confusion, and introducing a\nconfig that overrules what the user has stored in the index needs to\nadd complexity.\n\nThe additional complexity may (or may not) be justifiable, but in\nany case \"all other things being equal, this is a config\" feels like\na flawed observation.\n"},{"id":"274551","messageId":"xmqqoadrdj22.fsf@gitster.mtv.corp.google.com","threadId":"40959","inReplyTo":"xmqqwpsfdl5y.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 7/8] config: add core.untrackedCache","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-12-16T06:05:57Z","receivedAt":"2015-12-16T06:05:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> This is why the_index.has_untracked_cache is not just a simple \"Do I\n> want to use this feature?\" boolean configuration. The index also\n> stores the real data, and \"Am I using this feature?\" bit goes hand\n> in hand with that real data. Thinking that this is merely a boolean\n> configuration is the real source of the confusion, and introducing a\n> config that overrules what the user has stored in the index needs to\n> add complexity.\n>\n> The additional complexity may (or may not) be justifiable, but in\n> any case \"all other things being equal, this is a config\" feels like\n> a flawed observation.\n\nTo put it another way, the \"bit\" in the index (i.e. the presence of\nthe cached data) is \"Am I using the feature now?\". The effect of\nthe feature has to (and is designed to) persist, as it is a cache\nand you do not want a stale cache to give you wrong answers. There\nis no \"Do I want to use the feature?\" preference, in other words.\n\nAnd I do not mind creating such a preference bit as a configuration.\n\nThat is why I suggested such a configuration to cause the equivalent\nof \"update-index --untracked-cache\" when a new index is created from\nscratch (as opposed to the case where the previously created cache\ndata is carried forward across read_cache() -> do things to the\nindex -> write_cache() flow). Doing it that way will not have to\ninvolve additional complexity that comes from the desire that\nsetting a single configuration on (or off) has to suddenly change\nthe behaviour of an index file that is already using (or not using)\nthe feature.\n"},{"id":"274650","messageId":"20151217074443.GA4830@sigill.intra.peff.net","threadId":"40959","inReplyTo":"xmqqoadrdj22.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 7/8] config: add core.untrackedCache","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-12-17T07:44:43Z","receivedAt":"2015-12-17T07:44:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Dec 15, 2015 at 10:05:57PM -0800, Junio C Hamano wrote:\n\n> Junio C Hamano writes:\n> \n> > This is why the_index.has_untracked_cache is not just a simple \"Do I\n> > want to use this feature?\" boolean configuration. The index also\n> > stores the real data, and \"Am I using this feature?\" bit goes hand\n> > in hand with that real data. Thinking that this is merely a boolean\n> > configuration is the real source of the confusion, and introducing a\n> > config that overrules what the user has stored in the index needs to\n> > add complexity.\n> >\n> > The additional complexity may (or may not) be justifiable, but in\n> > any case \"all other things being equal, this is a config\" feels like\n> > a flawed observation.\n> \n> To put it another way, the \"bit\" in the index (i.e. the presence of\n> the cached data) is \"Am I using the feature now?\". The effect of\n> the feature has to (and is designed to) persist, as it is a cache\n> and you do not want a stale cache to give you wrong answers. There\n> is no \"Do I want to use the feature?\" preference, in other words.\n> \n> And I do not mind creating such a preference bit as a configuration.\n> \n> That is why I suggested such a configuration to cause the equivalent\n> of \"update-index --untracked-cache\" when a new index is created from\n> scratch (as opposed to the case where the previously created cache\n> data is carried forward across read_cache() -> do things to the\n> index -> write_cache() flow). Doing it that way will not have to\n> involve additional complexity that comes from the desire that\n> setting a single configuration on (or off) has to suddenly change\n> the behaviour of an index file that is already using (or not using)\n> the feature.\n\nI think we may actually be thinking of the same thing. Naively, I would\nexpect:\n\n - if there is untracked cache data in the index, we will make use of\n it when enumerating untracked files (and my understanding is that if\n it is stale, we can detect that)\n\n - if core.untrackedCache is set, we will update and write out an\n untracked cache when we are enumerating the untracked files\n\n - if there is cache data in the index but that config flag is not set,\n presumably we would not update it (we could even explicitly drop it,\n but my understanding is that is not necessary for correctness, but\n only as a possible optimization).\n\nYou could have a config option for \"if there is a cache there, pretend\nit isn't and ignore it\", but I don't see much point.\n\n-Peff\n"},{"id":"274665","messageId":"CACsJy8BwARfGmGBXEdWHnDxxXcubZDzjCg7Zy6qD0qzHZWGoFw@mail.gmail.com","threadId":"40959","inReplyTo":"20151217074443.GA4830@sigill.intra.peff.net","subject":"Re: [PATCH 7/8] config: add core.untrackedCache","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-12-17T12:26:45Z","receivedAt":"2015-12-17T12:26:45Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Dec 17, 2015 at 2:44 PM, Jeff King wrote:\n> I think we may actually be thinking of the same thing. Naively, I would\n> expect:\n>\n> ..\n> - if there is cache data in the index but that config flag is not set,\n> presumably we would not update it (we could even explicitly drop it,\n> but my understanding is that is not necessary for correctness, but\n> only as a possible optimization).\n\nNo, if somebody adds or removes something from the index, we either\nupdate or drop it, or it's stale. There's the invalidate_untracked()\nor something in dir.c that we can hook in, check config var and do\nthat. And because config is cached recently, it should be a cheap\noperation.\n\nApart from that the idea is fine.\n\n> You could have a config option for \"if there is a cache there, pretend\n> it isn't and ignore it\", but I don't see much point.\n-- \nDuy\n"},{"id":"274667","messageId":"CACsJy8B2tFqhfKPE17cPpp1uDYPa4OTB9zLHPY0j2QF9zuxAtQ@mail.gmail.com","threadId":"40959","inReplyTo":"CACBZZX7QW2J6DcMSXTa1y+QdMrqq5DXs1Fu3m8toV5a4yZKNjw@mail.gmail.com","subject":"Re: [PATCH 7/8] config: add core.untrackedCache","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-12-17T12:36:31Z","receivedAt":"2015-12-17T12:36:31Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Dec 16, 2015 at 4:53 AM, Ævar Arnfjörð Bjarmason\n wrote:\n> On Tue, Dec 15, 2015 at 8:40 PM, Junio C Hamano wrote:\n>> Ævar Arnfjörð Bjarmason writes:\n>> I still have a problem with the approach from \"design cleanliness\"\n>> point of view[...]\n>>\n>> In any case I think we already have agreed to disagree on this\n>> point, so there is no use discussing it any longer from my side. I\n>> am not closing the door to this series, but I am not convinced,\n>> either. At least not yet.\n>\n> In general the fantastic thing about the git configuration facility is\n> that it provides both systems administrators and normal users with\n> what they want. It's possible to configure things system-wide and\n> override those on a user or repository basis.\n>\n> Of course hindsight is 20/20, but I think that given what's been\n> covered in this thread it's been established that it's categorically\n> better that if we introduce features like these that they be\n> configured through the normal configuration facility rather than the\n> configuration being sticky to the index.\n\nA minor note for implementers. We need to check that config is loaded\nfirst. read-cache.c, being part of the core, does not bother itself\nwith config loading. And I think so far it has not used any config\nvars. If a command forgets (*) to load the config, the cache may be\ndeleted (if we do it the safe way).\n\n(*) is there any command deliberately avoid loading config? git-clone\nand git-init are special, but for this particular case it's probably\nfine.\n-- \nDuy\n"},{"id":"274729","messageId":"CAP8UFD2=hGbUFg_wX-0r6QgW+44ZiX8iJPX5c1FGG-4H8i163A@mail.gmail.com","threadId":"40959","inReplyTo":"CAP8UFD0LAQG+gQ5EhYYLjo5=tpW3_ah6GV-mgRbgTjjgNmdorA@mail.gmail.com","subject":"Fwd: [PATCH 7/8] config: add core.untrackedCache","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2015-12-18T22:38:35Z","receivedAt":"2015-12-18T22:38:35Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Sorry I sent this privately to Peff by mistake (once again).\n\n---------- Forwarded message ----------\nFrom: Christian Couder \nDate: Fri, Dec 18, 2015 at 11:09 PM\nSubject: Re: [PATCH 7/8] config: add core.untrackedCache\nTo: Jeff King \n\n\nOn Thu, Dec 17, 2015 at 8:44 AM, Jeff King wrote:\n> On Tue, Dec 15, 2015 at 10:05:57PM -0800, Junio C Hamano wrote:\n>>\n>> To put it another way, the \"bit\" in the index (i.e. the presence of\n>> the cached data) is \"Am I using the feature now?\". The effect of\n>> the feature has to (and is designed to) persist, as it is a cache\n>> and you do not want a stale cache to give you wrong answers.\n\nSorry if I repeat myself or misunderstood something, but in what I\nimplemented we either use the UC fully or we remove it, so it cannot\nbe stale vs git operations (like other changes in the index Duy talks\nabout).\n\nAnd as we are caching directory mtimes and as we are comparing each\ncached mtime with the current mtime of the original directory before\ntrusting the cached content related to the directory, a UC that is\nstale vs working tree operations could result in time lost but not in\nbad cached content used.\n\nOr maybe if we are very unlucky and have two directories with exactly\nthe same mtime and the same name but not the same content, and if we\nmove away the directory the UC was made from, and then put the other\none at the path where the first one was. Yeah in this case you may get\nbad results because bad UC content is used. But note that this case\ncan already happen now. It is a case that is inherent in using the UC.\n\n>> There\n>> is no \"Do I want to use the feature?\" preference, in other words.\n>>\n>> And I do not mind creating such a preference bit as a configuration.\n>>\n>> That is why I suggested such a configuration to cause the equivalent\n>> of \"update-index --untracked-cache\" when a new index is created from\n>> scratch (as opposed to the case where the previously created cache\n>> data is carried forward across read_cache() -> do things to the\n>> index -> write_cache() flow). Doing it that way will not have to\n>> involve additional complexity that comes from the desire that\n>> setting a single configuration on (or off) has to suddenly change\n>> the behaviour of an index file that is already using (or not using)\n>> the feature.\n>\n> I think we may actually be thinking of the same thing. Naively, I would\n> expect:\n>\n> - if there is untracked cache data in the index, we will make use of\n> it when enumerating untracked files (and my understanding is that if\n> it is stale, we can detect that)\n\nI agree for \"git status\", but I am not sure that the UC is used in all\nthe code paths that enumerate untracked files.\nI recall Duy mentioning that it was not used by \"git add\" and in some\nother cases, but I might be wrong.\n\nI also agree that we can detect when the UC content should not be used\nbecause it is stale vs working tree operations (see above).\n\nNow as Duy says the UC should never be stale vs git operations, but\nthat is easy to do if we just remove the UC when we stop using it.\n\n> - if core.untrackedCache is set, we will update and write out an\n> untracked cache when we are enumerating the untracked files\n\nIn the current patch this happens when \"git status\" is called,\nactually in wt_status_collect_untracked(), again I am not sure about\nall the other code paths.\n\n> - if there is cache data in the index but that config flag is not set,\n> presumably we would not update it (we could even explicitly drop it,\n> but my understanding is that is not necessary for correctness, but\n> only as a possible optimization).\n\nIn the current patch we drop the UC, also in\nwt_status_collect_untracked(), if the config flag is not set (or set\nto false).\n\nIt is necessary to drop it for correctness for the following reasons:\n\n- git operations on (other parts of) the index must update the UC if\nit exists according to Duy who gave the following example in a\nprevious email:\n\n\"... if file \"foo\" is tracked, then the user does \"git rm --cached\nfoo\", \"foo\" may become either untracked or ignored. So if you disable\nUC while doing this removal, then re-enable UC again, \"git-status\" may\nshow incorrect output.\"\n\n- the user may have decided to unset the config flag because the mtime\nfeatures we rely on are in fact not well supported by the system.\n(Though it's true that the user should have used \"git update-index\n--test-untracked-cache\" before setting the config flag in the first\nplace, but maybe it was a mistake, or maybe the file system will be\naccessed for some time by another system that will mount it or\nsomething.)\n\n> You could have a config option for \"if there is a cache there, pretend\n> it isn't and ignore it\", but I don't see much point.\n\nThere is not much point indeed in such an option and it could be dangerous.\n"},{"id":"274730","messageId":"CAP8UFD1kWd9Y9pLfz2aAHNQjtxOrhLk25ExZPU7p0OFfiixuow@mail.gmail.com","threadId":"40959","inReplyTo":"CAP8UFD0S_rWKjWiq_enkN+QVtvnq9fuwAxuuVTXTxu-F1mw4dg@mail.gmail.com","subject":"Fwd: [PATCH 7/8] config: add core.untrackedCache","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2015-12-18T22:40:35Z","receivedAt":"2015-12-18T22:40:35Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"(Sorry I sent this one privately to Duy by mistake too.)\n\n\n---------- Forwarded message ----------\nFrom: Christian Couder \nDate: Fri, Dec 18, 2015 at 11:35 PM\nSubject: Re: [PATCH 7/8] config: add core.untrackedCache\nTo: Duy Nguyen \n\n\nOn Thu, Dec 17, 2015 at 1:26 PM, Duy Nguyen wrote:\n> On Thu, Dec 17, 2015 at 2:44 PM, Jeff King wrote:\n>> I think we may actually be thinking of the same thing. Naively, I would\n>> expect:\n>>\n>> ..\n>> - if there is cache data in the index but that config flag is not set,\n>> presumably we would not update it (we could even explicitly drop it,\n>> but my understanding is that is not necessary for correctness, but\n>> only as a possible optimization).\n>\n> No, if somebody adds or removes something from the index, we either\n> update or drop it, or it's stale. There's the invalidate_untracked()\n> or something in dir.c that we can hook in, check config var and do\n> that. And because config is cached recently, it should be a cheap\n> operation.\n\nI think I understand what you are saying but it took me a long time,\nand I am not sure it is very clear to others.\n\nWhat I understand is that you are talking about\nvalidate_untracked_cache() in dir.c.\nAnd you suggest to check there if the core.untrackedCache config var\nis set, and, if it is not set, then to drop the UC there.\nAnd the reason for that is that git operations on other parts of the\nindex should update the UC if it exists otherwise the UC content could\nbe wrong as you explained previously in your \"git rm --cached foo\"\nexample.\n\nIn the current patch, the code to create or remove the UC to reflect\nthe state of the core.untrackedCache config var is in\nwt_status_collect_untracked().\nI think it works well there, but if you are saying that it's better if\nit is in validate_untracked_cache(), I am willing to consider moving\nit to validate_untracked_cache().\nCould you tell a bit though about why you think it's better in\nvalidate_untracked_cache()?\n\n> Apart from that the idea is fine.\n\nOk so, except maybe the part about wt_status_collect_untracked() vs\nvalidate_untracked_cache(), what the patch does is ok for you?\n"},{"id":"274732","messageId":"CAP8UFD16HbwN71ugTKek_CdT65571M1meisaBajjSFgoZePg8g@mail.gmail.com","threadId":"40959","inReplyTo":"CACsJy8B2tFqhfKPE17cPpp1uDYPa4OTB9zLHPY0j2QF9zuxAtQ@mail.gmail.com","subject":"Re: [PATCH 7/8] config: add core.untrackedCache","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2015-12-18T23:24:10Z","receivedAt":"2015-12-18T23:24:10Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Thu, Dec 17, 2015 at 1:36 PM, Duy Nguyen wrote:\n> On Wed, Dec 16, 2015 at 4:53 AM, Ævar Arnfjörð Bjarmason\n> wrote:\n>> On Tue, Dec 15, 2015 at 8:40 PM, Junio C Hamano wrote:\n>>> Ævar Arnfjörð Bjarmason writes:\n>>> I still have a problem with the approach from \"design cleanliness\"\n>>> point of view[...]\n>>>\n>>> In any case I think we already have agreed to disagree on this\n>>> point, so there is no use discussing it any longer from my side. I\n>>> am not closing the door to this series, but I am not convinced,\n>>> either. At least not yet.\n>>\n>> In general the fantastic thing about the git configuration facility is\n>> that it provides both systems administrators and normal users with\n>> what they want. It's possible to configure things system-wide and\n>> override those on a user or repository basis.\n>>\n>> Of course hindsight is 20/20, but I think that given what's been\n>> covered in this thread it's been established that it's categorically\n>> better that if we introduce features like these that they be\n>> configured through the normal configuration facility rather than the\n>> configuration being sticky to the index.\n>\n> A minor note for implementers. We need to check that config is loaded\n> first. read-cache.c, being part of the core, does not bother itself\n> with config loading. And I think so far it has not used any config\n> vars. If a command forgets (*) to load the config, the cache may be\n> deleted (if we do it the safe way).\n>\n> (*) is there any command deliberately avoid loading config? git-clone\n> and git-init are special, but for this particular case it's probably\n> fine.\n\nThanks for this note.\n\nLooking at the current patch, the global variable in which the value\nof the core.untrackedCache config var is stored is\n\"use_untracked_cache\".\nIt is used in the following places:\n\n- wt_status_collect_untracked() in wt-status.c which is called only by\n\"git status\" and \"git commit\" after the config has been loaded.\n\n- cmd_update_index() in builtin/update-index.c which loads the config\nbefore using it\n\n- validate_untracked_cache() in dir.c where it is used in:\n\n if (use_untracked_cache != 1 && !ident_in_untracked(dir->untracked)) {\n warning(_(\"Untracked cache is disabled on this system.\"));\n return NULL;\n }\n\nbut this \"if\" and its contents are removed by patch 10/10 in v2.\n\nSo at the end of this patch series, there is no risk of\nuse_untracked_cache not being properly setup.\n"},{"id":"274816","messageId":"xmqqy4cnfyds.fsf@gitster.mtv.corp.google.com","threadId":"40959","inReplyTo":"CACsJy8BwARfGmGBXEdWHnDxxXcubZDzjCg7Zy6qD0qzHZWGoFw@mail.gmail.com","subject":"Re: [PATCH 7/8] config: add core.untrackedCache","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-12-21T18:30:07Z","receivedAt":"2015-12-21T18:30:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> On Thu, Dec 17, 2015 at 2:44 PM, Jeff King wrote:\n>> I think we may actually be thinking of the same thing. Naively, I would\n>> expect:\n>>\n>> ..\n>> - if there is cache data in the index but that config flag is not set,\n>> presumably we would not update it (we could even explicitly drop it,\n>> but my understanding is that is not necessary for correctness, but\n>> only as a possible optimization).\n>\n> No, if somebody adds or removes something from the index, we either\n> update or drop it, or it's stale. There's the invalidate_untracked()\n> or something in dir.c that we can hook in, check config var and do\n> that. And because config is cached recently, it should be a cheap\n> operation.\n\nChecking the config may be cheap, but it bothers me a lot that we\nhave to call that \"invalidate\" thing every time we go into the\ncodepath to deal with the index, from code cleanliness point of\nview.\n"},{"id":"274844","messageId":"CACsJy8AF-7ULixapHmTtCia9x0HTtJ1nmnAER9A3BeLVjQM_Mg@mail.gmail.com","threadId":"40959","inReplyTo":"xmqqy4cnfyds.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 7/8] config: add core.untrackedCache","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-12-22T08:27:34Z","receivedAt":"2015-12-22T08:27:34Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Dec 22, 2015 at 1:30 AM, Junio C Hamano wrote:\n> Duy Nguyen writes:\n>\n>> On Thu, Dec 17, 2015 at 2:44 PM, Jeff King wrote:\n>>> I think we may actually be thinking of the same thing. Naively, I would\n>>> expect:\n>>>\n>>> ..\n>>> - if there is cache data in the index but that config flag is not set,\n>>> presumably we would not update it (we could even explicitly drop it,\n>>> but my understanding is that is not necessary for correctness, but\n>>> only as a possible optimization).\n>>\n>> No, if somebody adds or removes something from the index, we either\n>> update or drop it, or it's stale. There's the invalidate_untracked()\n>> or something in dir.c that we can hook in, check config var and do\n>> that. And because config is cached recently, it should be a cheap\n>> operation.\n>\n> Checking the config may be cheap, but it bothers me a lot that we\n> have to call that \"invalidate\" thing every time we go into the\n> codepath to deal with the index, from code cleanliness point of\n> view.\n\nIn that case we can just check config once in read_index_from and\ndestroy UNTR extension. Or the middle ground, we check config once in\nthat place, make a note in struct index_state, and make invalidate_*\ncheck that note instead of config file. The middle ground has an\nadvantage over destroying UNTR: (probably) many operations will touch\nindex but do not add or remove entries. Though I may be wrong,\nreplacing an entry may be implemented as delete then add, I haven't\nchecked the code.\n-- \nDuy\n"},{"id":"274893","messageId":"xmqqh9jae94o.fsf@gitster.mtv.corp.google.com","threadId":"40959","inReplyTo":"CACsJy8AF-7ULixapHmTtCia9x0HTtJ1nmnAER9A3BeLVjQM_Mg@mail.gmail.com","subject":"Re: [PATCH 7/8] config: add core.untrackedCache","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-12-22T16:33:11Z","receivedAt":"2015-12-22T16:33:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> In that case we can just check config once in read_index_from and\n> destroy UNTR extension. Or the middle ground, we check config once in\n> that place, make a note in struct index_state, and make invalidate_*\n> check that note instead of config file. The middle ground has an\n> advantage over destroying UNTR: (probably) many operations will touch\n> index but do not add or remove entries.\n\nOr we can even teach read_index_from() to skip reading the extension\nwithout even parsing when the configuration tells it that the\nfeature is force-disabled. It can also add an empty one when the\nconfiguration tells it that the feature is force-enabled and there\nis no UNTR extension yet.\n"},{"id":"274979","messageId":"xmqqio3obody.fsf@gitster.mtv.corp.google.com","threadId":"40959","inReplyTo":"xmqqh9jae94o.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 7/8] config: add core.untrackedCache","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-12-24T01:56:25Z","receivedAt":"2015-12-24T01:56:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Duy Nguyen writes:\n>\n>> In that case we can just check config once in read_index_from and\n>> destroy UNTR extension. Or the middle ground, we check config once in\n>> that place, make a note in struct index_state, and make invalidate_*\n>> check that note instead of config file. The middle ground has an\n>> advantage over destroying UNTR: (probably) many operations will touch\n>> index but do not add or remove entries.\n>\n> Or we can even teach read_index_from() to skip reading the extension\n> without even parsing when the configuration tells it that the\n> feature is force-disabled. It can also add an empty one when the\n> configuration tells it that the feature is force-enabled and there\n> is no UNTR extension yet.\n\nThinking about this a bit more, I am starting to feel that the\nconfig that can be used to optionally override the presence of\nin-index UNTR extension does not have to be too bad a thing,\nprovided if it is done with a few tweaks to the design I read in\nChristian & Ævar's messages.\n\nOne tweak is to address the following from Ævar's message:\n\n>> Once this series is applied and git is shipped with it existing\n>> users that have set \"git update-index --untracked-cache\" will have\n>> their UC feature disabled. This is because we fall back to \"oh no\n>> config here? Must have been disabled, rm it from the index\" clause\n>> which keeps our UC from going stale in the face of config\n>> flip-flopping.\n\nThe above would happen only if the configuration is a boolean that\ndefaults to false. I'd think we can have this a tristate instead.\nThat is, config.untrackedCache can be explicitly set to 'true',\n'false', or 'keep'. And make a missing config.untrackedCache\ndefault to 'keep'.\n\nWhen read_index_from() reads an existing index:\n\n When the configuration is set to 'true':\n an existing UNTR is kept, otherwise a new UNTR gets added.\n When the configuration is set to 'false:\n an existing UNTR is dropped.\n When the configuration is set to 'keep':\n an existing UNTR is kept, otherwise nothing happens.\n\nWhen write_index() writes out an index that wasn't initialized from\nthe filesystem, a new UNTR gets added only when the configuration is\nset to 'true' and there is no in-core UNTR already.\n\nThat way, those who use /etc/gitconfig to force the feature over\ntheir users would be able to set it to 'true', those who have been\nusing the feature in some but not all of their repositories won't\nsee any different behaviour until they muck with the configuration\n(and if they are paranoid and want to opt out of their administrator's\nsetting, they can set it to 'keep' in their $HOME/.gitconfig to make\nsure their repositories are not affected).\n\nOrthogonal to the \"config overrides the existing users' practice\"\nissue, I still think that [PATCH v2 10/10] goes too far to remove\nthe safety based on the working tree location. Checking kernel\nversion and other thing may be too much, but the check based on the\nworking tree location is a cheap way to make sure that those who do\nunusual things (namely, use $GIT_DIR/$GIT_WORK_TREE or their\nequivalents to tell Git that the working tree for this invocation is\nat a place different from what UNTR data was prepared for) are not\nharmed by incorrectly reusing the cached data for an unrelated\nlocation. So another tweak I'd feel better to see is to resurrect\nthat safety.\n\nI wouldn't have as much issue with such a scheme as I had with the\nearlier description of the design in the Christian's series.\n\nThanks.\n"},{"id":"274985","messageId":"CACsJy8AO0BumZQmwWNVHL-viGLY=ExE3syJOt1h75zeQug95mQ@mail.gmail.com","threadId":"40959","inReplyTo":"xmqqio3obody.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 7/8] config: add core.untrackedCache","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-12-24T09:49:15Z","receivedAt":"2015-12-24T09:49:15Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Dec 24, 2015 at 8:56 AM, Junio C Hamano wrote:\n> Junio C Hamano writes:\n>\n>> Duy Nguyen writes:\n>>\n>>> In that case we can just check config once in read_index_from and\n>>> destroy UNTR extension. Or the middle ground, we check config once in\n>>> that place, make a note in struct index_state, and make invalidate_*\n>>> check that note instead of config file. The middle ground has an\n>>> advantage over destroying UNTR: (probably) many operations will touch\n>>> index but do not add or remove entries.\n>>\n>> Or we can even teach read_index_from() to skip reading the extension\n>> without even parsing when the configuration tells it that the\n>> feature is force-disabled. It can also add an empty one when the\n>> configuration tells it that the feature is force-enabled and there\n>> is no UNTR extension yet.\n>\n> Thinking about this a bit more, I am starting to feel that the\n> config that can be used to optionally override the presence of\n> in-index UNTR extension does not have to be too bad a thing,\n> provided if it is done with a few tweaks to the design I read in\n> Christian & Ævar's messages.\n>\n> One tweak is to address the following from Ævar's message:\n>\n>>> Once this series is applied and git is shipped with it existing\n>>> users that have set \"git update-index --untracked-cache\" will have\n>>> their UC feature disabled. This is because we fall back to \"oh no\n>>> config here? Must have been disabled, rm it from the index\" clause\n>>> which keeps our UC from going stale in the face of config\n>>> flip-flopping.\n>\n> The above would happen only if the configuration is a boolean that\n> defaults to false. I'd think we can have this a tristate instead.\n> That is, config.untrackedCache can be explicitly set to 'true',\n> 'false', or 'keep'. And make a missing config.untrackedCache\n> default to 'keep'.\n>\n> When read_index_from() reads an existing index:\n>\n> When the configuration is set to 'true':\n> an existing UNTR is kept, otherwise a new UNTR gets added.\n> When the configuration is set to 'false:\n> an existing UNTR is dropped.\n> When the configuration is set to 'keep':\n> an existing UNTR is kept, otherwise nothing happens.\n\nSounds good, except..\n\n> When write_index() writes out an index that wasn't initialized from\n> the filesystem, a new UNTR gets added only when the configuration is\n> set to 'true' and there is no in-core UNTR already.\n\nDo we really need this? If the index is created fresh from the core,\nfirst write will not have UNTR. Next time it's loaded up, UNTR is\nadded if config is true, based on the above rules. The new UNTR\nextension (should?) marks the index as dirty as writing out is\nrequired.\n\n> Orthogonal to the \"config overrides the existing users' practice\"\n> issue,\n\nYeah it looks orthogonal to me too. There should be a separate config\nfile like /etc/gitconfig.lockdown, that will override user config vars\n(but we still can't undefine user config vars yet, known missing\nfeature).\n-- \nDuy\n"},{"id":"274989","messageId":"CAP8UFD2riDWmK64UWHyg61YyxLVEmT8d_5qzM7U+Bs-9qCv0mA@mail.gmail.com","threadId":"40959","inReplyTo":"xmqqio3obody.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 7/8] config: add core.untrackedCache","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2015-12-24T20:54:59Z","receivedAt":"2015-12-24T20:54:59Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Thu, Dec 24, 2015 at 2:56 AM, Junio C Hamano wrote:\n> Junio C Hamano writes:\n>\n>> Duy Nguyen writes:\n>>\n>>> In that case we can just check config once in read_index_from and\n>>> destroy UNTR extension. Or the middle ground, we check config once in\n>>> that place, make a note in struct index_state, and make invalidate_*\n>>> check that note instead of config file. The middle ground has an\n>>> advantage over destroying UNTR: (probably) many operations will touch\n>>> index but do not add or remove entries.\n>>\n>> Or we can even teach read_index_from() to skip reading the extension\n>> without even parsing when the configuration tells it that the\n>> feature is force-disabled. It can also add an empty one when the\n>> configuration tells it that the feature is force-enabled and there\n>> is no UNTR extension yet.\n>\n> Thinking about this a bit more, I am starting to feel that the\n> config that can be used to optionally override the presence of\n> in-index UNTR extension does not have to be too bad a thing,\n> provided if it is done with a few tweaks to the design I read in\n> Christian & Ævar's messages.\n\nGreat!\n\n> One tweak is to address the following from Ævar's message:\n>\n>>> Once this series is applied and git is shipped with it existing\n>>> users that have set \"git update-index --untracked-cache\" will have\n>>> their UC feature disabled. This is because we fall back to \"oh no\n>>> config here? Must have been disabled, rm it from the index\" clause\n>>> which keeps our UC from going stale in the face of config\n>>> flip-flopping.\n>\n> The above would happen only if the configuration is a boolean that\n> defaults to false. I'd think we can have this a tristate instead.\n> That is, config.untrackedCache can be explicitly set to 'true',\n> 'false', or 'keep'. And make a missing config.untrackedCache\n> default to 'keep'.\n\nOk. The first RFC patch series about this had a tristate and I\nswitched to a boolean as it seemed that people prefered a boolean, but\nyou are right that a tristate is more backward compatible.\n\n> When read_index_from() reads an existing index:\n>\n> When the configuration is set to 'true':\n> an existing UNTR is kept, otherwise a new UNTR gets added.\n> When the configuration is set to 'false:\n> an existing UNTR is dropped.\n> When the configuration is set to 'keep':\n> an existing UNTR is kept, otherwise nothing happens.\n>\n> When write_index() writes out an index that wasn't initialized from\n> the filesystem, a new UNTR gets added only when the configuration is\n> set to 'true' and there is no in-core UNTR already.\n\nMy current patch series does these changes in\nwt_status_collect_untracked() because currently the UNTR is mostly\nused only in git status, so it feels safer to me to not affect other\ncode paths.\n\n> That way, those who use /etc/gitconfig to force the feature over\n> their users would be able to set it to 'true', those who have been\n> using the feature in some but not all of their repositories won't\n> see any different behaviour until they muck with the configuration\n> (and if they are paranoid and want to opt out of their administrator's\n> setting, they can set it to 'keep' in their $HOME/.gitconfig to make\n> sure their repositories are not affected).\n>\n> Orthogonal to the \"config overrides the existing users' practice\"\n> issue, I still think that [PATCH v2 10/10] goes too far to remove\n> the safety based on the working tree location. Checking kernel\n> version and other thing may be too much, but the check based on the\n> working tree location is a cheap way to make sure that those who do\n> unusual things (namely, use $GIT_DIR/$GIT_WORK_TREE or their\n> equivalents to tell Git that the working tree for this invocation is\n> at a place different from what UNTR data was prepared for) are not\n> harmed by incorrectly reusing the cached data for an unrelated\n> location. So another tweak I'd feel better to see is to resurrect\n> that safety.\n\nThis has been changed in v3, see patch 09/11, and I think it should\nnow work as you suggest.\n\n> I wouldn't have as much issue with such a scheme as I had with the\n> earlier description of the design in the Christian's series.\n\nGreat! I am preparing a v4 that I hope to send in a few days.\nBy the way the v3 does not pass its own tests due to a bug introduced\nin last minute changes.\n\nThanks,\nChristian.\n"},{"id":"275025","messageId":"xmqqege7bq2l.fsf@gitster.mtv.corp.google.com","threadId":"40959","inReplyTo":"CACsJy8AO0BumZQmwWNVHL-viGLY=ExE3syJOt1h75zeQug95mQ@mail.gmail.com","subject":"Re: [PATCH 7/8] config: add core.untrackedCache","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-12-27T20:21:22Z","receivedAt":"2015-12-27T20:21:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> Sounds good, except..\n>\n>> When write_index() writes out an index that wasn't initialized from\n>> the filesystem, a new UNTR gets added only when the configuration is\n>> set to 'true' and there is no in-core UNTR already.\n>\n> Do we really need this?\n\nWe don't; you're right.\n\nThanks.\n"}]}