{"thread":{"id":"40877","subject":"[RFC/PATCH] config: add core.trustmtime","startedAt":"2015-11-25T06:35:23Z","lastAt":"2015-12-07T05:42:22Z","messageCount":13,"participants":["Christian Couder","Ævar Arnfjörð Bjarmason","Johannes Schindelin","Duy Nguyen","Junio C Hamano","Torsten Bögershausen"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"273701","messageId":"1448433323-21037-1-git-send-email-chriscool@tuxfamily.org","threadId":"40877","inReplyTo":null,"subject":"[RFC/PATCH] config: add core.trustmtime","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2015-11-25T06:35:23Z","receivedAt":"2015-11-25T06:35:23Z","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\ndon't want any slow down because we used --untracked-cache rather\nthan --force-untracked-cache, and we don't want untracked cache to\nstop working because we updated a kernel.\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 it testing if it works (because it might\nwork on some systems using the repo over the network file system\nbut not others).\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\nAt Booking.com we know that mtime works everywhere and we don't\nwant the untracked cache to stop working when a kernel is upgraded\nor when the repo is copied to a machine with a different kernel.\nI will add tests later if people are ok with this.\n\n Documentation/config.txt               | 8 ++++++++\n Documentation/git-update-index.txt     | 6 +++++-\n builtin/update-index.c                 | 6 +++++-\n cache.h                                | 1 +\n config.c                               | 7 +++++++\n contrib/completion/git-completion.bash | 1 +\n dir.c                                  | 2 +-\n environment.c                          | 1 +\n 8 files changed, 29 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex b4b0194..186ad17 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -308,6 +308,14 @@ core.trustctime::\n \tcrawlers and some backup systems).\n \tSee linkgit:git-update-index[1]. True by default.\n \n+core.trustmtime::\n+\tIf unset or set to 'default' or 'check', Git will test if\n+\tmtime is working properly before using features that need a\n+\tworking mtime. If false, Git will refuse to use such\n+\tfeatures. If set to true, Git will blindly use such features\n+\twithout testing if they work.\n+\tSee linkgit:git-update-index[1].\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 1a296bc..8b4c5af 100644\n--- a/Documentation/git-update-index.txt\n+++ b/Documentation/git-update-index.txt\n@@ -182,7 +182,9 @@ may not support it yet.\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+\tused to skip the tests. If you always want to skip those\n+\ttests, you can set the `core.trustmtime` configuration\n+\tvariable to 'true', (see linkgit:git-config[1]).\n \n \\--::\n \tDo not interpret any more arguments as options.\n@@ -398,6 +400,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 needs a fully working mtime, so it will look at\n+`core.trustmtime` configuration variable (see linkgit:git-config[1]).\n \n SEE ALSO\n --------\ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex 7431938..c64439f 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -1107,9 +1107,13 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n \tif (untracked_cache > 0) {\n \t\tstruct untracked_cache *uc;\n \n+\t\tif (trust_mtime == 0) {\n+\t\t\tfprintf_ln(stderr,_(\"core.trustmtime is set to false\"));\n+\t\t\treturn 1;\n+\t\t}\n \t\tif (untracked_cache < 2) {\n \t\t\tsetup_work_tree();\n-\t\t\tif (!test_if_untracked_cache_is_supported())\n+\t\t\tif (trust_mtime != 1 && !test_if_untracked_cache_is_supported())\n \t\t\t\treturn 1;\n \t\t}\n \t\tif (!the_index.untracked) {\ndiff --git a/cache.h b/cache.h\nindex 736abc0..69a6197 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -601,6 +601,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 trust_mtime;\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..d720b1f 100644\n--- a/config.c\n+++ b/config.c\n@@ -691,6 +691,13 @@ 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.trustmtime\")) {\n+\t\tif (!strcasecmp(value, \"default\") || !strcasecmp(value, \"check\"))\n+\t\t\ttrust_mtime = -1;\n+\t\telse\n+\t\t\ttrust_mtime = git_config_maybe_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 482ca84..39d1c9b 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -2038,6 +2038,7 @@ _git_config ()\n \t\tcore.sparseCheckout\n \t\tcore.symlinks\n \t\tcore.trustctime\n+\t\tcore.trustmtime\n \t\tcore.warnAmbiguousRefs\n \t\tcore.whitespace\n \t\tcore.worktree\ndiff --git a/dir.c b/dir.c\nindex d2a8f06..b06df1b 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1994,7 +1994,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 (trust_mtime != 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..c899947 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 trust_mtime = -1;\n int check_stat = 1;\n int has_symlinks = 1;\n int minimum_abbrev = 4, default_abbrev = 7;\n-- \n2.6.3.380.g494b52d\n"},{"id":"273709","messageId":"CACBZZX4YSvfA=BRayk8CC8ipk_nDdvJfznxjyb-upwP9PKRBSA@mail.gmail.com","threadId":"40877","inReplyTo":"1448433323-21037-1-git-send-email-chriscool@tuxfamily.org","subject":"Re: [RFC/PATCH] config: add core.trustmtime","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2015-11-25T09:00:23Z","receivedAt":"2015-11-25T09:00:23Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Wed, Nov 25, 2015 at 7:35 AM, Christian Couder\n<christian.couder@gmail.com> wrote:\n> At Booking.com we know that mtime works everywhere and we don't\n> want the untracked cache to stop working when a kernel is upgraded\n> or when the repo is copied to a machine with a different kernel.\n> I will add tests later if people are ok with this.\n\nI bit more info: I rolled Git out internally with this patch:\nhttps://github.com/avar/git/commit/c63f7c12c2664631961add7cf3da901b0b6aa2f2\n\nThe --untracked-cache feature hardcodes the equivalent of:\n\n    pwd; uname --kernel-name --kernel-release --kernel-version\n\nInto the index. If any of those change it prints out the \"cache is\ndisabled\" warning.\n\nThis patch will make it stop being so afraid of itself to the point of\ndisabling itself on minor kernel upgrades :)\n\nA few other issues with this feature I've noticed:\n\n * There's no way to just enable it globally via the config. Makes it\na bit of a hassle to use it. I wanted to have a config option to\nenable it via the config, how about \"index.untracked_cache = true\" for\nthe config variable name?\n\n * Doing \"cd /tmp: git --git-dir=/git/somewhere/else/.git update-index\n--untracked-cache\" doesn't work how I'd expect. It hardcodes \"/tmp\" as\nthe directory that \"works\" into the index, so if you use the working\ntree you'll never use the untracked cache. I spotted this because I\ncarry out a bunch of git maintenance commands with --git-dir instead\nof cd-ing to the relevant directories. This works for most other\nthings in git, is it a bug that it doesn't work here?\n\n * If you \"ctrl+c\" git update-index --untracked-cache at an\ninopportune time you'll end up with a mtime-test-XXXXXX directory in\nyour working tree. Perhaps this tempdir should be created in the .git\ndirectory instead?\n\n * Maybe we should have a --test-untracked-cache option, so you can\nrun the tests without enabling it.\n\nAside from the slight hassle of enabling this and keeping it enabled\nthis feature is great. It's sped up \"git status\" across the board by\nabout 40%. Slightly less than that on faster spinning disks, slightly\nmore than that on slower ones.\n"},{"id":"273722","messageId":"alpine.DEB.1.00.1511251121390.1686@s15462909.onlinehome-server.info","threadId":"40877","inReplyTo":"1448433323-21037-1-git-send-email-chriscool@tuxfamily.org","subject":"Re: [RFC/PATCH] config: add core.trustmtime","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-11-25T10:25:49Z","receivedAt":"2015-11-25T10:25:49Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Christian,\n\nOn Wed, 25 Nov 2015, Christian Couder wrote:\n\n> diff --git a/config.c b/config.c\n> index 248a21a..d720b1f 100644\n> --- a/config.c\n> +++ b/config.c\n> @@ -691,6 +691,13 @@ 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.trustmtime\")) {\n> +\t\tif (!strcasecmp(value, \"default\") || !strcasecmp(value, \"check\"))\n> +\t\t\ttrust_mtime = -1;\n> +\t\telse\n> +\t\t\ttrust_mtime = git_config_maybe_bool(var, value);\n\nIf `core.trustmtime` is set to `OMG, never!`, `git_config_maybe_bool()`\nwill set trust_mtime to -1, i.e. the exact same value as if you had set\nthe variable to `default` or `check`...\n\nMaybe you want to be a bit stricter here and either test the return value\nof `git_config_maybe_bool()` for -1 (and warn in that case), or just\ndelete the `maybe_`?\n\nCiao,\nDscho\n"},{"id":"273723","messageId":"CAP8UFD18KkH25S3RmyuBQrvGYELSTR1FfZAYKzr-Yzd6OJvmxQ@mail.gmail.com","threadId":"40877","inReplyTo":"alpine.DEB.1.00.1511251121390.1686@s15462909.onlinehome-server.info","subject":"Re: [RFC/PATCH] config: add core.trustmtime","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2015-11-25T10:39:30Z","receivedAt":"2015-11-25T10:39:30Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Hi Johannes,\n\nOn Wed, Nov 25, 2015 at 11:25 AM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> Hi Christian,\n>\n> On Wed, 25 Nov 2015, Christian Couder wrote:\n>\n>> diff --git a/config.c b/config.c\n>> index 248a21a..d720b1f 100644\n>> --- a/config.c\n>> +++ b/config.c\n>> @@ -691,6 +691,13 @@ static int git_default_core_config(const char *var, const char *value)\n>>               trust_ctime = git_config_bool(var, value);\n>>               return 0;\n>>       }\n>> +     if (!strcmp(var, \"core.trustmtime\")) {\n>> +             if (!strcasecmp(value, \"default\") || !strcasecmp(value, \"check\"))\n>> +                     trust_mtime = -1;\n>> +             else\n>> +                     trust_mtime = git_config_maybe_bool(var, value);\n>\n> If `core.trustmtime` is set to `OMG, never!`, `git_config_maybe_bool()`\n> will set trust_mtime to -1, i.e. the exact same value as if you had set\n> the variable to `default` or `check`...\n\nYeah, I think returning -1 is the safe thing to do in this case.\n\n> Maybe you want to be a bit stricter here and either test the return value\n> of `git_config_maybe_bool()` for -1 (and warn in that case), or just\n> delete the `maybe_`?\n\nI thought that if we ever add more options in the future then people\nmight be annoyed by older git versions warning about the new options.\nBut I am ok to add a warning.\n\nThanks,\nChristian.\n"},{"id":"273758","messageId":"CACsJy8DhAfu7J=WpPAp8HYGLuFQC5+DZyZj6Hs6vruEJEeVKig@mail.gmail.com","threadId":"40877","inReplyTo":"CACBZZX4YSvfA=BRayk8CC8ipk_nDdvJfznxjyb-upwP9PKRBSA@mail.gmail.com","subject":"Re: [RFC/PATCH] config: add core.trustmtime","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-11-25T19:51:57Z","receivedAt":"2015-11-25T19:51:57Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Nov 25, 2015 at 10:00 AM, Ævar Arnfjörð Bjarmason\n<avarab@gmail.com> wrote:\n> On Wed, Nov 25, 2015 at 7:35 AM, Christian Couder\n> <christian.couder@gmail.com> wrote:\n>> At Booking.com we know that mtime works everywhere and we don't\n>> want the untracked cache to stop working when a kernel is upgraded\n>> or when the repo is copied to a machine with a different kernel.\n>> I will add tests later if people are ok with this.\n>\n> I bit more info: I rolled Git out internally with this patch:\n> https://github.com/avar/git/commit/c63f7c12c2664631961add7cf3da901b0b6aa2f2\n>\n> The --untracked-cache feature hardcodes the equivalent of:\n>\n>     pwd; uname --kernel-name --kernel-release --kernel-version\n>\n> Into the index. If any of those change it prints out the \"cache is\n> disabled\" warning.\n>\n> This patch will make it stop being so afraid of itself to the point of\n> disabling itself on minor kernel upgrades :)\n\nThe problem is, there's no way to teach git to know it's a \"minor\"\nupgrade.. but if there is a config key to say \"don't be paranoid, I\nknow what I'm doing\", then we can skip that check, or just warn\ninstead of disabling the cache.\n\n> A few other issues with this feature I've noticed:\n>\n>  * There's no way to just enable it globally via the config. Makes it\n> a bit of a hassle to use it. I wanted to have a config option to\n> enable it via the config, how about \"index.untracked_cache = true\" for\n> the config variable name?\n\nIf you haven't noticed, all these experimental features have no real\nUI (update-index is plumbing). I have been waiting for someone like\nyou to start using it and figure out the best UI (then implement it)\n;)\n\n>  * Doing \"cd /tmp: git --git-dir=/git/somewhere/else/.git update-index\n> --untracked-cache\" doesn't work how I'd expect. It hardcodes \"/tmp\" as\n> the directory that \"works\" into the index, so if you use the working\n> tree you'll never use the untracked cache. I spotted this because I\n> carry out a bunch of git maintenance commands with --git-dir instead\n> of cd-ing to the relevant directories. This works for most other\n> things in git, is it a bug that it doesn't work here?\n\nIt needs the current directory at --untrack-cache time to test if the\ndirectory satisfies the requirements. So either you cd to that\nworktree, or you have to specify --worktree as well. Or am I missing\nsomething?\n\n>  * If you \"ctrl+c\" git update-index --untracked-cache at an\n> inopportune time you'll end up with a mtime-test-XXXXXX directory in\n> your working tree. Perhaps this tempdir should be created in the .git\n> directory instead?\n\nNo because in theory .git could be on a separate file system with\ndifferent semantics. But we should probably clean those files at ^C.\n\n>  * Maybe we should have a --test-untracked-cache option, so you can\n> run the tests without enabling it.\n\nI'd say patches welcome.\n\n> Aside from the slight hassle of enabling this and keeping it enabled\n> this feature is great. It's sped up \"git status\" across the board by\n> about 40%. Slightly less than that on faster spinning disks, slightly\n> more than that on slower ones.\n\nI'm still waiting for the day when watchman support gets merged and\nmaybe poke Facebook guys to compare performance with Mercurial :)\nWell, we are probably still behind Mercurial on that day.\n\nAlso, there's still work to be done. Right now it's optimized for\nwhole-tree \"git status\", Doing \"git status -- abc\" will not benefit\nfrom untracked cache, similarly \"git add\" with pathspec..\n-- \nDuy\n"},{"id":"273780","messageId":"CAP8UFD2V3nbY2-abW6cGDtB2PR9Q+sN+d0RgTVJORCPg6TPMcQ@mail.gmail.com","threadId":"40877","inReplyTo":"CACsJy8DhAfu7J=WpPAp8HYGLuFQC5+DZyZj6Hs6vruEJEeVKig@mail.gmail.com","subject":"Re: [RFC/PATCH] config: add core.trustmtime","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2015-11-26T05:21:40Z","receivedAt":"2015-11-26T05:21:40Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Hi Duy,\n\nOn Wed, Nov 25, 2015 at 8:51 PM, Duy Nguyen <pclouds@gmail.com> wrote:\n> On Wed, Nov 25, 2015 at 10:00 AM, Ævar Arnfjörð Bjarmason\n> <avarab@gmail.com> wrote:\n>> On Wed, Nov 25, 2015 at 7:35 AM, Christian Couder\n>> <christian.couder@gmail.com> wrote:\n>>> At Booking.com we know that mtime works everywhere and we don't\n>>> want the untracked cache to stop working when a kernel is upgraded\n>>> or when the repo is copied to a machine with a different kernel.\n>>> I will add tests later if people are ok with this.\n>>\n>> I bit more info: I rolled Git out internally with this patch:\n>> https://github.com/avar/git/commit/c63f7c12c2664631961add7cf3da901b0b6aa2f2\n>>\n>> The --untracked-cache feature hardcodes the equivalent of:\n>>\n>>     pwd; uname --kernel-name --kernel-release --kernel-version\n>>\n>> Into the index. If any of those change it prints out the \"cache is\n>> disabled\" warning.\n>>\n>> This patch will make it stop being so afraid of itself to the point of\n>> disabling itself on minor kernel upgrades :)\n>\n> The problem is, there's no way to teach git to know it's a \"minor\"\n> upgrade.. but if there is a config key to say \"don't be paranoid, I\n> know what I'm doing\", then we can skip that check, or just warn\n> instead of disabling the cache.\n\nYeah, in my patch if core.trustmtime is set to true or false the check\nis skipped.\n\nI am wondering why you didn't make it by default run the mtime checks\nwhen a kernel change is detected. Maybe that would be better than\ndisabling itself.\n\n>> A few other issues with this feature I've noticed:\n>>\n>>  * There's no way to just enable it globally via the config. Makes it\n>> a bit of a hassle to use it. I wanted to have a config option to\n>> enable it via the config, how about \"index.untracked_cache = true\" for\n>> the config variable name?\n>\n> If you haven't noticed, all these experimental features have no real\n> UI (update-index is plumbing). I have been waiting for someone like\n> you to start using it and figure out the best UI (then implement it)\n> ;)\n\nOk, we are happy to do that (including implementing it) :-)\n\nI will take a look at something like index.untracked_cache. It will\nprobably also be a tristate like this:\n\n- true: always enable it; die if core.trustmtime is false otherwise\nwarn if it is not true\n- default/unset: same as current behavior\n- false: die if it is enabled or when trying to enable to it\n\n>>  * Doing \"cd /tmp: git --git-dir=/git/somewhere/else/.git update-index\n>> --untracked-cache\" doesn't work how I'd expect. It hardcodes \"/tmp\" as\n>> the directory that \"works\" into the index, so if you use the working\n>> tree you'll never use the untracked cache. I spotted this because I\n>> carry out a bunch of git maintenance commands with --git-dir instead\n>> of cd-ing to the relevant directories. This works for most other\n>> things in git, is it a bug that it doesn't work here?\n>\n> It needs the current directory at --untrack-cache time to test if the\n> directory satisfies the requirements. So either you cd to that\n> worktree, or you have to specify --worktree as well. Or am I missing\n> something?\n\nMaybe it could print out a message saying \"Testing mtime in directory\n$(pwd)\" and if that works then \"Untracked cache is enabled for\n$(pwd)\". That would make it clear that it will not work in other\ndirectories.\n\nAlso maybe the mtime checks could be run when a directory change is detected.\n\n>>  * If you \"ctrl+c\" git update-index --untracked-cache at an\n>> inopportune time you'll end up with a mtime-test-XXXXXX directory in\n>> your working tree. Perhaps this tempdir should be created in the .git\n>> directory instead?\n>\n> No because in theory .git could be on a separate file system with\n> different semantics. But we should probably clean those files at ^C.\n\nOk, I will have a look at cleaning the files at ^C.\n\n>>  * Maybe we should have a --test-untracked-cache option, so you can\n>> run the tests without enabling it.\n>\n> I'd say patches welcome.\n\nOk, I wll have a look at that too.\n\n>> Aside from the slight hassle of enabling this and keeping it enabled\n>> this feature is great. It's sped up \"git status\" across the board by\n>> about 40%. Slightly less than that on faster spinning disks, slightly\n>> more than that on slower ones.\n>\n> I'm still waiting for the day when watchman support gets merged and\n> maybe poke Facebook guys to compare performance with Mercurial :)\n> Well, we are probably still behind Mercurial on that day.\n\nYeah, it could be interesting to compare performance with Mercurial as\nwe move forward :-)\n\n> Also, there's still work to be done. Right now it's optimized for\n> whole-tree \"git status\", Doing \"git status -- abc\" will not benefit\n> from untracked cache, similarly \"git add\" with pathspec..\n\nThanks for these details. Yeah, it might be interesting to look at\n\"git add\" too.\n\nBest,\nChristian.\n"},{"id":"273785","messageId":"CACsJy8CNRbvsDY+ucFZGcO4+tE9Ud_QNr1k4D=+utawSCrc+Sg@mail.gmail.com","threadId":"40877","inReplyTo":"CAP8UFD2V3nbY2-abW6cGDtB2PR9Q+sN+d0RgTVJORCPg6TPMcQ@mail.gmail.com","subject":"Re: [RFC/PATCH] config: add core.trustmtime","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-11-26T17:53:40Z","receivedAt":"2015-11-26T17:53:40Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Nov 26, 2015 at 6:21 AM, Christian Couder\n<christian.couder@gmail.com> wrote:\n> Hi Duy,\n>\n> On Wed, Nov 25, 2015 at 8:51 PM, Duy Nguyen <pclouds@gmail.com> wrote:\n>> On Wed, Nov 25, 2015 at 10:00 AM, Ævar Arnfjörð Bjarmason\n>> <avarab@gmail.com> wrote:\n>>> On Wed, Nov 25, 2015 at 7:35 AM, Christian Couder\n>>> <christian.couder@gmail.com> wrote:\n>>>> At Booking.com we know that mtime works everywhere and we don't\n>>>> want the untracked cache to stop working when a kernel is upgraded\n>>>> or when the repo is copied to a machine with a different kernel.\n>>>> I will add tests later if people are ok with this.\n>>>\n>>> I bit more info: I rolled Git out internally with this patch:\n>>> https://github.com/avar/git/commit/c63f7c12c2664631961add7cf3da901b0b6aa2f2\n>>>\n>>> The --untracked-cache feature hardcodes the equivalent of:\n>>>\n>>>     pwd; uname --kernel-name --kernel-release --kernel-version\n>>>\n>>> Into the index. If any of those change it prints out the \"cache is\n>>> disabled\" warning.\n>>>\n>>> This patch will make it stop being so afraid of itself to the point of\n>>> disabling itself on minor kernel upgrades :)\n>>\n>> The problem is, there's no way to teach git to know it's a \"minor\"\n>> upgrade.. but if there is a config key to say \"don't be paranoid, I\n>> know what I'm doing\", then we can skip that check, or just warn\n>> instead of disabling the cache.\n>\n> Yeah, in my patch if core.trustmtime is set to true or false the check\n> is skipped.\n>\n> I am wondering why you didn't make it by default run the mtime checks\n> when a kernel change is detected. Maybe that would be better than\n> disabling itself.\n\nIt takes about 10 seconds to go through the mtime check. Imagine you\nhave to wait 10s for some random \"git status\".. Plus I didn't want to\ndo anything fancy.\n\n>\n>>> A few other issues with this feature I've noticed:\n>>>\n>>>  * There's no way to just enable it globally via the config. Makes it\n>>> a bit of a hassle to use it. I wanted to have a config option to\n>>> enable it via the config, how about \"index.untracked_cache = true\" for\n>>> the config variable name?\n>>\n>> If you haven't noticed, all these experimental features have no real\n>> UI (update-index is plumbing). I have been waiting for someone like\n>> you to start using it and figure out the best UI (then implement it)\n>> ;)\n>\n> Ok, we are happy to do that (including implementing it) :-)\n>\n> I will take a look at something like index.untracked_cache. It will\n\nNit. untrackedCache (underscores are avoided for config keys) and it's\nprobably core.untrackedCache instead..\n\n> probably also be a tristate like this:\n>\n> - true: always enable it; die if core.trustmtime is false otherwise\n> warn if it is not true\n> - default/unset: same as current behavior\n> - false: die if it is enabled or when trying to enable to it\n\nThere could be a fourth option: let a hook do the checking. If the\nhook returns ok, all is good.\n\n>>>  * Doing \"cd /tmp: git --git-dir=/git/somewhere/else/.git update-index\n>>> --untracked-cache\" doesn't work how I'd expect. It hardcodes \"/tmp\" as\n>>> the directory that \"works\" into the index, so if you use the working\n>>> tree you'll never use the untracked cache. I spotted this because I\n>>> carry out a bunch of git maintenance commands with --git-dir instead\n>>> of cd-ing to the relevant directories. This works for most other\n>>> things in git, is it a bug that it doesn't work here?\n>>\n>> It needs the current directory at --untrack-cache time to test if the\n>> directory satisfies the requirements. So either you cd to that\n>> worktree, or you have to specify --worktree as well. Or am I missing\n>> something?\n>\n> Maybe it could print out a message saying \"Testing mtime in directory\n> $(pwd)\" and if that works then \"Untracked cache is enabled for\n> $(pwd)\". That would make it clear that it will not work in other\n> directories.\n>\n> Also maybe the mtime checks could be run when a directory change is detected.\n\nYeah.. and we could also have a hook to do this test your own way too,\nif you already know your system supports it, so you wait no time at\nall.\n-- \nDuy\n"},{"id":"273787","messageId":"CACBZZX4jJ_b-5sbor6YibXGASeOa0gx8nEBiv8=54yoHfWL8Fw@mail.gmail.com","threadId":"40877","inReplyTo":"CACsJy8CNRbvsDY+ucFZGcO4+tE9Ud_QNr1k4D=+utawSCrc+Sg@mail.gmail.com","subject":"Re: [RFC/PATCH] config: add core.trustmtime","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2015-11-27T01:35:46Z","receivedAt":"2015-11-27T01:35:46Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Thu, Nov 26, 2015 at 6:53 PM, Duy Nguyen <pclouds@gmail.com> wrote:\n> On Thu, Nov 26, 2015 at 6:21 AM, Christian Couder\n> <christian.couder@gmail.com> wrote:\n>> I am wondering why you didn't make it by default run the mtime checks\n>> when a kernel change is detected. Maybe that would be better than\n>> disabling itself.\n>\n> It takes about 10 seconds to go through the mtime check. Imagine you\n> have to wait 10s for some random \"git status\".. Plus I didn't want to\n> do anything fancy.\n\nI browsed through the commits that added the --untracked-cache and\ntried to find the original mailing list discussion, but I couldn't\nfind the reason for why the default interface for enabling it is doing\nthese exhaustive tests.\n\nMaybe I'm missing some really common breakage with st_mtime on some\nsystem, but having a feature the user explicitly enables turn itself\noff and doing FS-testing that takes 10 seconds when it's enabled seems\nlike the wrong default to me.\n\nWe don't do it with core.fileMode, core.ignorecase or core.trustctime\nor core.symlinks. Do we really need to be treating this differently?\n\nIf that's a \"no\" then the default interface to this could be much\nsimpler. Rather than being a change you apply to .git/index (going\naway if you nuke it etc.) it could just be a config option like the\nrest.\n"},{"id":"273834","messageId":"xmqq610j8fzt.fsf@gitster.mtv.corp.google.com","threadId":"40877","inReplyTo":"CACBZZX4jJ_b-5sbor6YibXGASeOa0gx8nEBiv8=54yoHfWL8Fw@mail.gmail.com","subject":"Re: [RFC/PATCH] config: add core.trustmtime","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-11-30T19:05:42Z","receivedAt":"2015-11-30T19:05:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n> Maybe I'm missing some really common breakage with st_mtime on some\n> system, but having a feature the user explicitly enables turn itself\n> off and doing FS-testing that takes 10 seconds when it's enabled seems\n> like the wrong default to me.\n>\n> We don't do it with core.fileMode, core.ignorecase or core.trustctime\n> or core.symlinks. Do we really need to be treating this differently?\n\nI share the exact thought.  I was looking the other way when\nuntracked-cache was done originally ;-), and I would also want to\nknow the answers to the above questions.\n\nThanks.\n"},{"id":"273835","messageId":"CACsJy8BZo4hFz+mQKNxrmPQaGforvguFmjL90g=Hf698okFs2A@mail.gmail.com","threadId":"40877","inReplyTo":"xmqq610j8fzt.fsf@gitster.mtv.corp.google.com","subject":"Re: [RFC/PATCH] config: add core.trustmtime","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-11-30T19:12:14Z","receivedAt":"2015-11-30T19:12:14Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Nov 30, 2015 at 8:05 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n>\n>> Maybe I'm missing some really common breakage with st_mtime on some\n>> system, but having a feature the user explicitly enables turn itself\n>> off and doing FS-testing that takes 10 seconds when it's enabled seems\n>> like the wrong default to me.\n>>\n>> We don't do it with core.fileMode, core.ignorecase or core.trustctime\n>> or core.symlinks. Do we really need to be treating this differently?\n>\n> I share the exact thought.  I was looking the other way when\n> untracked-cache was done originally ;-), and I would also want to\n> know the answers to the above questions.\n\nOK I was just paranoid (and having to look at filesystem source code\nto determine if it supported this didn't help either). So I guess that\nmeans we can make the test a separate option, only invoked by the\nuser, then.\n-- \nDuy\n"},{"id":"273853","messageId":"565D36B1.9050104@web.de","threadId":"40877","inReplyTo":"xmqq610j8fzt.fsf@gitster.mtv.corp.google.com","subject":"Re: [RFC/PATCH] config: add core.trustmtime","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2015-12-01T05:57:05Z","receivedAt":"2015-12-01T05:57:05Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 11/30/2015 08:05 PM, Junio C Hamano wrote:\n> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n>\n>> Maybe I'm missing some really common breakage with st_mtime on some\n>> system, but having a feature the user explicitly enables turn itself\n>> off and doing FS-testing that takes 10 seconds when it's enabled seems\n>> like the wrong default to me.\n>>\n>> We don't do it with core.fileMode, core.ignorecase or core.trustctime\n>> or core.symlinks. Do we really need to be treating this differently?\n> I share the exact thought.  I was looking the other way when\n> untracked-cache was done originally ;-), and I would also want to\n> know the answers to the above questions.\nMaybe somewhat off topic, but:\n\nCreate a repo under Linux:\nLinux will probe the FS and will set\ncore.filemode true\ncore.ignorecase false.\n\nExport it using SAMBA to Windows or Mac OS X.\nIn some magical way the mounted repo becomes case-insensitive under\nboth Mac and Windows, even if core.ignorecase is true in the config file.\n\nSame for filemode:\n(Git for Windows doesn't see the execute bit at all, but relies on \ncore.filemode anyway)\nDepending how the repo is mounted on the Mac, the execute bits may work, \nbe always 0 or always 1.\n\nRelying on this kind of config files is not ideal in a networked \nenvironment.\nIt is not ideal when different implementations of Git access the same repo,\n(Git for Windows vs cygwin vs Egit/Jgit)\n\nSo I think that the original patch could make Git unreliable under some \ncircumstances.\n\nAnd some day I may send a patch which does a quick auto-probe for \nfilemode and ignorecase,\nto adjust Git to what the underlying \"local OS - network - remote OS - \nremote FS\" really achieves.\n"},{"id":"273932","messageId":"CACsJy8CT=TV8iqMsj7mU2hGn=EJ77fXqqGGdE+P7G-axC+G4YA@mail.gmail.com","threadId":"40877","inReplyTo":"CACBZZX4YSvfA=BRayk8CC8ipk_nDdvJfznxjyb-upwP9PKRBSA@mail.gmail.com","subject":"Re: [RFC/PATCH] config: add core.trustmtime","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-12-02T19:28:13Z","receivedAt":"2015-12-02T19:28:13Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Nov 25, 2015 at 10:00 AM, Ævar Arnfjörð Bjarmason\n<avarab@gmail.com> wrote:\n> Aside from the slight hassle of enabling this and keeping it enabled\n> this feature is great. It's sped up \"git status\" across the board by\n> about 40%. Slightly less than that on faster spinning disks, slightly\n> more than that on slower ones.\n\nBefore I forget again, you should also enable split-index. That\nfeature was added because index update time cut so much into the\nsaving from untracked cache (unless you have small indexes, unlikely).\nAnd untracked cache can update the index often. Then maybe you can\nalso think about improving the usability for it to ;-)\n-- \nDuy\n"},{"id":"274099","messageId":"CAP8UFD1p7kjqOnyn5Qfw5z2q0SXRnccvh1WS-+ZU+0eSkKS_2g@mail.gmail.com","threadId":"40877","inReplyTo":"CACsJy8CT=TV8iqMsj7mU2hGn=EJ77fXqqGGdE+P7G-axC+G4YA@mail.gmail.com","subject":"Re: [RFC/PATCH] config: add core.trustmtime","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2015-12-07T05:42:22Z","receivedAt":"2015-12-07T05:42:22Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Wed, Dec 2, 2015 at 8:28 PM, Duy Nguyen <pclouds@gmail.com> wrote:\n> On Wed, Nov 25, 2015 at 10:00 AM, Ævar Arnfjörð Bjarmason\n> <avarab@gmail.com> wrote:\n>> Aside from the slight hassle of enabling this and keeping it enabled\n>> this feature is great. It's sped up \"git status\" across the board by\n>> about 40%. Slightly less than that on faster spinning disks, slightly\n>> more than that on slower ones.\n>\n> Before I forget again, you should also enable split-index. That\n> feature was added because index update time cut so much into the\n> saving from untracked cache (unless you have small indexes, unlikely).\n> And untracked cache can update the index often. Then maybe you can\n> also think about improving the usability for it to ;-)\n\nYeah, we will probably have a look at split-index next.\n\nThanks for developing it too,\nChristian.\n"}]}