{"thread":{"id":"50754","subject":"[PATCH 0/2] Fix fsmonitor after discard_index()","startedAt":"2019-03-16T09:49:10Z","lastAt":"2019-05-23T20:49:30Z","messageCount":17,"participants":["Johannes Schindelin via GitGitGadget","Johannes Sixt","Ævar Arnfjörð Bjarmason","Johannes Schindelin","SZEDER Gábor","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"371645","messageId":"pull.165.git.gitgitgadget@gmail.com","threadId":"50754","inReplyTo":null,"subject":"[PATCH 0/2] Fix fsmonitor after discard_index()","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-03-16T09:49:05Z","receivedAt":"2019-03-16T09:49:10Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"It was reported by Ævar Arnfjörð Bjarmason\n[https://public-inbox.org/git/nycvar.QRO.7.76.6.1903142058130.41@tvgsbejvaqbjf.bet/T/#mb8718fe52e4721dacd3b143a09187ff9090ef4e3] \nthat there were problems with the fsmonitor feature in conjunction with the\nnewly built-in git stash/git rebase.\n\nThe culprit really is that the fsmonitor flag that says whether it was\nqueried already was not re-set after discard_index() was called by mistake.\n\nThis fixes that, and apparently also other long-standing fsmonitor issues.\n\n(Note that there is still a flakiness around t7519\n[https://github.com/git-for-windows/git/pull/2127#pullrequestreview-215010574] \nwhere it tries to make sure that the fsmonitor hook can prevent unnecessary\nlstat() calls, but that seems to be unrelated to this here bug.)\n\nJohannes Schindelin (2):\n  fsmonitor: demonstrate that it is not refreshed after discard_index()\n  fsmonitor: force a refresh after the index was discarded\n\n cache.h                     |  3 ++-\n fsmonitor.c                 |  5 ++---\n read-cache.c                |  1 +\n t/helper/test-read-cache.c  | 24 +++++++++++++++++++++++-\n t/t7519-status-fsmonitor.sh | 10 ++++++++++\n 5 files changed, 38 insertions(+), 5 deletions(-)\n\n\nbase-commit: e902e9bcae2010bc42648c80ab6adc6c5a16a4a5\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-165%2Fdscho%2Ffix-fsmonitor-after-discard-index-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-165/dscho/fix-fsmonitor-after-discard-index-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/165\n-- \ngitgitgadget\n"},{"id":"371646","messageId":"f451752656ea5d1e9898e5fa0b2d41de6550781b.1552729745.git.gitgitgadget@gmail.com","threadId":"50754","inReplyTo":"pull.165.git.gitgitgadget@gmail.com","subject":"[PATCH 1/2] fsmonitor: demonstrate that it is not refreshed after discard_index()","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-03-16T09:49:06Z","receivedAt":"2019-03-16T09:49:10Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nThis one is tricky.\n\nWhen `core.fsmonitor` is set, a `refresh_index()` will not perform a\nfull scan of files that might be modified, but will query the fsmonitor\nand refresh only the ones that have been actually touched.\n\nDue to implementation details, the fsmonitor is queried in\n`refresh_cache_ent()`, but of course it only has to be queried once, so\nwe set a flag when we did that. But when the index was discarded, we did\nnot re-set that flag.\n\nSo far, this is only covered by our test suite when running with\nGIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all, and only due to the way the\nbuilt-in stash interacts with the recursive merge machinery.\n\nLet's introduce a straight-forward regression test for this.\n\nWe simply extend the \"read & discard index\" loop in `test-tool\nread-cache` to optionally refresh the index, report on a given file's\nstatus, and then modify that file. Due to the bug described above, only\nthe first refresh will actually query the fsmonitor; subsequent loop\niterations will not.\n\nThis problem was reported by Ævar Arnfjörð Bjarmason.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n t/helper/test-read-cache.c  | 24 +++++++++++++++++++++++-\n t/t7519-status-fsmonitor.sh | 10 ++++++++++\n 2 files changed, 33 insertions(+), 1 deletion(-)\n\ndiff --git a/t/helper/test-read-cache.c b/t/helper/test-read-cache.c\nindex d674c88ba0..7e79b555de 100644\n--- a/t/helper/test-read-cache.c\n+++ b/t/helper/test-read-cache.c\n@@ -1,14 +1,36 @@\n #include \"test-tool.h\"\n #include \"cache.h\"\n+#include \"config.h\"\n \n int cmd__read_cache(int argc, const char **argv)\n {\n-\tint i, cnt = 1;\n+\tint i, cnt = 1, namelen;\n+\tconst char *name = NULL;\n+\n+\tif (argc > 1 && skip_prefix(argv[1], \"--print-and-refresh=\", &name)) {\n+\t\tnamelen = strlen(name);\n+\t\targc--;\n+\t\targv++;\n+\t}\n+\n \tif (argc == 2)\n \t\tcnt = strtol(argv[1], NULL, 0);\n \tsetup_git_directory();\n+\tgit_config(git_default_config, NULL);\n \tfor (i = 0; i < cnt; i++) {\n \t\tread_cache();\n+\t\tif (name) {\n+\t\t\tint pos;\n+\n+\t\t\trefresh_index(&the_index, REFRESH_QUIET,\n+\t\t\t\t      NULL, NULL, NULL);\n+\t\t\tpos = index_name_pos(&the_index, name, namelen);\n+\t\t\tif (pos < 0)\n+\t\t\t\tdie(\"%s not in index\", name);\n+\t\t\tprintf(\"%s is%s up to date\\n\", name,\n+\t\t\t       ce_uptodate(the_index.cache[pos]) ? \"\" : \" not\");\n+\t\t\twrite_file(name, \"%d\\n\", i);\n+\t\t}\n \t\tdiscard_cache();\n \t}\n \treturn 0;\ndiff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh\nindex 3e0a61db23..918bc323ab 100755\n--- a/t/t7519-status-fsmonitor.sh\n+++ b/t/t7519-status-fsmonitor.sh\n@@ -346,4 +346,14 @@ test_expect_success UNTRACKED_CACHE 'ignore .git changes when invalidating UNTR'\n \ttest_cmp before after\n '\n \n+test_expect_failure 'discard_index() also discards fsmonitor info' '\n+\ttest_when_finished \\\n+\t\t\"git config core.monitor .git/hooks/fsmonitor-test\" &&\n+\ttest_config core.fsmonitor \"$TEST_DIRECTORY/t7519/fsmonitor-all\" &&\n+\ttest_might_fail git update-index --refresh &&\n+\ttest-tool read-cache --print-and-refresh=tracked 2 >actual &&\n+\tprintf \"tracked is%s up to date\\n\" \"\" \" not\" >expect &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"371647","messageId":"1d626237762f29d6f617f706a06a024811993185.1552729745.git.gitgitgadget@gmail.com","threadId":"50754","inReplyTo":"pull.165.git.gitgitgadget@gmail.com","subject":"[PATCH 2/2] fsmonitor: force a refresh after the index was discarded","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-03-16T09:49:07Z","receivedAt":"2019-03-16T09:49:10Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nWith this change, the `index_state` struct becomes the new home for the\nflag that says whether the fsmonitor hook has been run, i.e. it is now\nper-index.\n\nIt also gets re-set when the index is discarded, fixing the bug where\nfsmonitor-enabled Git would miss updates under certain circumstances.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n cache.h                     | 3 ++-\n fsmonitor.c                 | 5 ++---\n read-cache.c                | 1 +\n t/t7519-status-fsmonitor.sh | 2 +-\n 4 files changed, 6 insertions(+), 5 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex abd518a9a2..8d07c29c2a 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -339,7 +339,8 @@ struct index_state {\n \tstruct cache_time timestamp;\n \tunsigned name_hash_initialized : 1,\n \t\t initialized : 1,\n-\t\t drop_cache_tree : 1;\n+\t\t drop_cache_tree : 1,\n+\t\t fsmonitor_has_run_once : 1;\n \tstruct hashmap name_hash;\n \tstruct hashmap dir_hash;\n \tstruct object_id oid;\ndiff --git a/fsmonitor.c b/fsmonitor.c\nindex 665bd2d425..1dee0aded1 100644\n--- a/fsmonitor.c\n+++ b/fsmonitor.c\n@@ -129,7 +129,6 @@ static void fsmonitor_refresh_callback(struct index_state *istate, const char *n\n \n void refresh_fsmonitor(struct index_state *istate)\n {\n-\tstatic int has_run_once = 0;\n \tstruct strbuf query_result = STRBUF_INIT;\n \tint query_success = 0;\n \tsize_t bol; /* beginning of line */\n@@ -137,9 +136,9 @@ void refresh_fsmonitor(struct index_state *istate)\n \tchar *buf;\n \tint i;\n \n-\tif (!core_fsmonitor || has_run_once)\n+\tif (!core_fsmonitor || istate->fsmonitor_has_run_once)\n \t\treturn;\n-\thas_run_once = 1;\n+\tistate->fsmonitor_has_run_once = 1;\n \n \ttrace_printf_key(&trace_fsmonitor, \"refresh fsmonitor\");\n \t/*\ndiff --git a/read-cache.c b/read-cache.c\nindex 4dc6de1b55..0bf39e177a 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -2335,6 +2335,7 @@ int discard_index(struct index_state *istate)\n \tfree_name_hash(istate);\n \tcache_tree_free(&(istate->cache_tree));\n \tistate->initialized = 0;\n+\tistate->fsmonitor_has_run_once = 0;\n \tFREE_AND_NULL(istate->cache);\n \tistate->cache_alloc = 0;\n \tdiscard_split_index(istate);\ndiff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh\nindex 918bc323ab..72b9ed3e45 100755\n--- a/t/t7519-status-fsmonitor.sh\n+++ b/t/t7519-status-fsmonitor.sh\n@@ -346,7 +346,7 @@ test_expect_success UNTRACKED_CACHE 'ignore .git changes when invalidating UNTR'\n \ttest_cmp before after\n '\n \n-test_expect_failure 'discard_index() also discards fsmonitor info' '\n+test_expect_success 'discard_index() also discards fsmonitor info' '\n \ttest_when_finished \\\n \t\t\"git config core.monitor .git/hooks/fsmonitor-test\" &&\n \ttest_config core.fsmonitor \"$TEST_DIRECTORY/t7519/fsmonitor-all\" &&\n-- \ngitgitgadget\n"},{"id":"371648","messageId":"680444e1-d670-607b-e84b-c1eb4355b047@kdbg.org","threadId":"50754","inReplyTo":"f451752656ea5d1e9898e5fa0b2d41de6550781b.1552729745.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/2] fsmonitor: demonstrate that it is not refreshed after discard_index()","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2019-03-16T10:26:54Z","receivedAt":"2019-03-16T10:27:00Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 16.03.19 um 10:49 schrieb Johannes Schindelin via GitGitGadget:\n> diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh\n> index 3e0a61db23..918bc323ab 100755\n> --- a/t/t7519-status-fsmonitor.sh\n> +++ b/t/t7519-status-fsmonitor.sh\n> @@ -346,4 +346,14 @@ test_expect_success UNTRACKED_CACHE 'ignore .git changes when invalidating UNTR'\n>  \ttest_cmp before after\n>  '\n>  \n> +test_expect_failure 'discard_index() also discards fsmonitor info' '\n> +\ttest_when_finished \\\n> +\t\t\"git config core.monitor .git/hooks/fsmonitor-test\" &&\n\nDid you mean\n\t\t\"git config core.fsmonitor ...\n?\n\n> +\ttest_config core.fsmonitor \"$TEST_DIRECTORY/t7519/fsmonitor-all\" &&\n\nAnd then, does this not unset core.fsmonitor after the test anyway?\n\n> +\ttest_might_fail git update-index --refresh &&\n> +\ttest-tool read-cache --print-and-refresh=tracked 2 >actual &&\n> +\tprintf \"tracked is%s up to date\\n\" \"\" \" not\" >expect &&\n> +\ttest_cmp expect actual\n> +'\n> +\n>  test_done\n\n-- Hannes\n"},{"id":"371803","messageId":"87ef74a1dv.fsf@evledraar.gmail.com","threadId":"50754","inReplyTo":"pull.165.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/2] Fix fsmonitor after discard_index()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-03-18T11:05:32Z","receivedAt":"2019-03-18T11:05:37Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sat, Mar 16 2019, Johannes Schindelin via GitGitGadget wrote:\n\n> It was reported by Ævar Arnfjörð Bjarmason\n> [https://public-inbox.org/git/nycvar.QRO.7.76.6.1903142058130.41@tvgsbejvaqbjf.bet/T/#mb8718fe52e4721dacd3b143a09187ff9090ef4e3]\n> that there were problems with the fsmonitor feature in conjunction with the\n> newly built-in git stash/git rebase.\n>\n> The culprit really is that the fsmonitor flag that says whether it was\n> queried already was not re-set after discard_index() was called by mistake.\n>\n> This fixes that, and apparently also other long-standing fsmonitor issues.\n\nI've added this to my internal build & now the test suite passes in the\nfsmonitor mode without any test skipping.\n\n> (Note that there is still a flakiness around t7519\n> [https://github.com/git-for-windows/git/pull/2127#pullrequestreview-215010574]\n> where it tries to make sure that the fsmonitor hook can prevent unnecessary\n> lstat() calls, but that seems to be unrelated to this here bug.)\n\nFWIW Since February 1st, 2018 I've run my builds on CentOS [67] through\nan GIT_FSMONITOR_TEST=$PWD/t7519/fsmonitor-all test and have never\nencountered this flakyness, and I built pretty much on every \"next\"\npush-out.\n\nThe fix sounds good, just one data point on the rarity of the race in\npractice. I hadn't noticed this being flaky.\n"},{"id":"372092","messageId":"nycvar.QRO.7.76.6.1903211329500.41@tvgsbejvaqbjf.bet","threadId":"50754","inReplyTo":"680444e1-d670-607b-e84b-c1eb4355b047@kdbg.org","subject":"Re: [PATCH 1/2] fsmonitor: demonstrate that it is not refreshed after discard_index()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-03-21T12:32:28Z","receivedAt":"2019-03-21T12:32:55Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Hannes,\n\nOn Sat, 16 Mar 2019, Johannes Sixt wrote:\n\n> Am 16.03.19 um 10:49 schrieb Johannes Schindelin via GitGitGadget:\n> > diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh\n> > index 3e0a61db23..918bc323ab 100755\n> > --- a/t/t7519-status-fsmonitor.sh\n> > +++ b/t/t7519-status-fsmonitor.sh\n> > @@ -346,4 +346,14 @@ test_expect_success UNTRACKED_CACHE 'ignore .git changes when invalidating UNTR'\n> >  \ttest_cmp before after\n> >  '\n> >\n> > +test_expect_failure 'discard_index() also discards fsmonitor info' '\n> > +\ttest_when_finished \\\n> > +\t\t\"git config core.monitor .git/hooks/fsmonitor-test\" &&\n>\n> Did you mean\n> \t\t\"git config core.fsmonitor ...\n> ?\n\nD'oh, yes.\n\n> > +\ttest_config core.fsmonitor \"$TEST_DIRECTORY/t7519/fsmonitor-all\" &&\n>\n> And then, does this not unset core.fsmonitor after the test anyway?\n\nIt will unset it, as you point out, not *re-set* it to the value it had\nbefore.\n\nBut I guess that's par for the course. I am just worried about side\neffects in our test suite. I run into those all the time, it is not even\nfunny: when a prereq is not met, or when you just want to\n`--run=<this-one>`, you can't, because it is almost as if more than half\nof our test cases depend on the output of *some* previous test case.\n\nBut you're right, I should not worry so much. After all, I am adding a\ntest case to the *end* of the test script.\n\nWill send out a fixed version soon.\n\nThanks,\nDscho\n\n>\n> > +\ttest_might_fail git update-index --refresh &&\n> > +\ttest-tool read-cache --print-and-refresh=tracked 2 >actual &&\n> > +\tprintf \"tracked is%s up to date\\n\" \"\" \" not\" >expect &&\n> > +\ttest_cmp expect actual\n> > +'\n> > +\n> >  test_done\n>\n> -- Hannes\n>\n"},{"id":"372123","messageId":"pull.165.v2.git.gitgitgadget@gmail.com","threadId":"50754","inReplyTo":"pull.165.git.gitgitgadget@gmail.com","subject":"[PATCH v2 0/2] Fix fsmonitor after discard_index()","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-03-21T13:57:32Z","receivedAt":"2019-03-21T13:57:36Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"It was reported by Ævar Arnfjörð Bjarmason\n[https://public-inbox.org/git/nycvar.QRO.7.76.6.1903142058130.41@tvgsbejvaqbjf.bet/T/#mb8718fe52e4721dacd3b143a09187ff9090ef4e3] \nthat there were problems with the fsmonitor feature in conjunction with the\nnewly built-in git stash/git rebase.\n\nThe culprit really is that the fsmonitor flag that says whether it was\nqueried already was not re-set after discard_index() was called by mistake.\n\nThis fixes that, and apparently also other long-standing fsmonitor issues.\n\n(Note that there is still a flakiness around t7519\n[https://github.com/git-for-windows/git/pull/2127#pullrequestreview-215010574] \nwhere it tries to make sure that the fsmonitor hook can prevent unnecessary\nlstat() calls, but that seems to be unrelated to this here bug.)\n\nChanges since v1:\n\n * Removed unnecessary test_when_finished \"test_config ...\" line, pointed\n   out by Hannes Sixt.\n\nJohannes Schindelin (2):\n  fsmonitor: demonstrate that it is not refreshed after discard_index()\n  fsmonitor: force a refresh after the index was discarded\n\n cache.h                     |  3 ++-\n fsmonitor.c                 |  5 ++---\n read-cache.c                |  1 +\n t/helper/test-read-cache.c  | 24 +++++++++++++++++++++++-\n t/t7519-status-fsmonitor.sh |  8 ++++++++\n 5 files changed, 36 insertions(+), 5 deletions(-)\n\n\nbase-commit: 041f5ea1cf987a4068ef5f39ba0a09be85952064\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-165%2Fdscho%2Ffix-fsmonitor-after-discard-index-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-165/dscho/fix-fsmonitor-after-discard-index-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/165\n\nRange-diff vs v1:\n\n 1:  f451752656 ! 1:  51a7edf22a fsmonitor: demonstrate that it is not refreshed after discard_index()\n     @@ -79,8 +79,6 @@\n       '\n       \n      +test_expect_failure 'discard_index() also discards fsmonitor info' '\n     -+\ttest_when_finished \\\n     -+\t\t\"git config core.monitor .git/hooks/fsmonitor-test\" &&\n      +\ttest_config core.fsmonitor \"$TEST_DIRECTORY/t7519/fsmonitor-all\" &&\n      +\ttest_might_fail git update-index --refresh &&\n      +\ttest-tool read-cache --print-and-refresh=tracked 2 >actual &&\n 2:  1d62623776 ! 2:  79fdd0d586 fsmonitor: force a refresh after the index was discarded\n     @@ -70,6 +70,6 @@\n       \n      -test_expect_failure 'discard_index() also discards fsmonitor info' '\n      +test_expect_success 'discard_index() also discards fsmonitor info' '\n     - \ttest_when_finished \\\n     - \t\t\"git config core.monitor .git/hooks/fsmonitor-test\" &&\n       \ttest_config core.fsmonitor \"$TEST_DIRECTORY/t7519/fsmonitor-all\" &&\n     + \ttest_might_fail git update-index --refresh &&\n     + \ttest-tool read-cache --print-and-refresh=tracked 2 >actual &&\n\n-- \ngitgitgadget\n"},{"id":"372124","messageId":"51a7edf22a22e86367893e77f2eccf70e0f91cb0.1553176651.git.gitgitgadget@gmail.com","threadId":"50754","inReplyTo":"pull.165.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 1/2] fsmonitor: demonstrate that it is not refreshed after discard_index()","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-03-21T13:57:33Z","receivedAt":"2019-03-21T13:57:37Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nThis one is tricky.\n\nWhen `core.fsmonitor` is set, a `refresh_index()` will not perform a\nfull scan of files that might be modified, but will query the fsmonitor\nand refresh only the ones that have been actually touched.\n\nDue to implementation details, the fsmonitor is queried in\n`refresh_cache_ent()`, but of course it only has to be queried once, so\nwe set a flag when we did that. But when the index was discarded, we did\nnot re-set that flag.\n\nSo far, this is only covered by our test suite when running with\nGIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all, and only due to the way the\nbuilt-in stash interacts with the recursive merge machinery.\n\nLet's introduce a straight-forward regression test for this.\n\nWe simply extend the \"read & discard index\" loop in `test-tool\nread-cache` to optionally refresh the index, report on a given file's\nstatus, and then modify that file. Due to the bug described above, only\nthe first refresh will actually query the fsmonitor; subsequent loop\niterations will not.\n\nThis problem was reported by Ævar Arnfjörð Bjarmason.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n t/helper/test-read-cache.c  | 24 +++++++++++++++++++++++-\n t/t7519-status-fsmonitor.sh |  8 ++++++++\n 2 files changed, 31 insertions(+), 1 deletion(-)\n\ndiff --git a/t/helper/test-read-cache.c b/t/helper/test-read-cache.c\nindex d674c88ba0..7e79b555de 100644\n--- a/t/helper/test-read-cache.c\n+++ b/t/helper/test-read-cache.c\n@@ -1,14 +1,36 @@\n #include \"test-tool.h\"\n #include \"cache.h\"\n+#include \"config.h\"\n \n int cmd__read_cache(int argc, const char **argv)\n {\n-\tint i, cnt = 1;\n+\tint i, cnt = 1, namelen;\n+\tconst char *name = NULL;\n+\n+\tif (argc > 1 && skip_prefix(argv[1], \"--print-and-refresh=\", &name)) {\n+\t\tnamelen = strlen(name);\n+\t\targc--;\n+\t\targv++;\n+\t}\n+\n \tif (argc == 2)\n \t\tcnt = strtol(argv[1], NULL, 0);\n \tsetup_git_directory();\n+\tgit_config(git_default_config, NULL);\n \tfor (i = 0; i < cnt; i++) {\n \t\tread_cache();\n+\t\tif (name) {\n+\t\t\tint pos;\n+\n+\t\t\trefresh_index(&the_index, REFRESH_QUIET,\n+\t\t\t\t      NULL, NULL, NULL);\n+\t\t\tpos = index_name_pos(&the_index, name, namelen);\n+\t\t\tif (pos < 0)\n+\t\t\t\tdie(\"%s not in index\", name);\n+\t\t\tprintf(\"%s is%s up to date\\n\", name,\n+\t\t\t       ce_uptodate(the_index.cache[pos]) ? \"\" : \" not\");\n+\t\t\twrite_file(name, \"%d\\n\", i);\n+\t\t}\n \t\tdiscard_cache();\n \t}\n \treturn 0;\ndiff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh\nindex 3e0a61db23..afd8fa7532 100755\n--- a/t/t7519-status-fsmonitor.sh\n+++ b/t/t7519-status-fsmonitor.sh\n@@ -346,4 +346,12 @@ test_expect_success UNTRACKED_CACHE 'ignore .git changes when invalidating UNTR'\n \ttest_cmp before after\n '\n \n+test_expect_failure 'discard_index() also discards fsmonitor info' '\n+\ttest_config core.fsmonitor \"$TEST_DIRECTORY/t7519/fsmonitor-all\" &&\n+\ttest_might_fail git update-index --refresh &&\n+\ttest-tool read-cache --print-and-refresh=tracked 2 >actual &&\n+\tprintf \"tracked is%s up to date\\n\" \"\" \" not\" >expect &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"372125","messageId":"79fdd0d586e9086b68af1b68dc5e194a566d2240.1553176651.git.gitgitgadget@gmail.com","threadId":"50754","inReplyTo":"pull.165.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 2/2] fsmonitor: force a refresh after the index was discarded","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-03-21T13:57:34Z","receivedAt":"2019-03-21T13:57:39Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nWith this change, the `index_state` struct becomes the new home for the\nflag that says whether the fsmonitor hook has been run, i.e. it is now\nper-index.\n\nIt also gets re-set when the index is discarded, fixing the bug where\nfsmonitor-enabled Git would miss updates under certain circumstances.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n cache.h                     | 3 ++-\n fsmonitor.c                 | 5 ++---\n read-cache.c                | 1 +\n t/t7519-status-fsmonitor.sh | 2 +-\n 4 files changed, 6 insertions(+), 5 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex ac92421f3a..7434740c99 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -339,7 +339,8 @@ struct index_state {\n \tstruct cache_time timestamp;\n \tunsigned name_hash_initialized : 1,\n \t\t initialized : 1,\n-\t\t drop_cache_tree : 1;\n+\t\t drop_cache_tree : 1,\n+\t\t fsmonitor_has_run_once : 1;\n \tstruct hashmap name_hash;\n \tstruct hashmap dir_hash;\n \tstruct object_id oid;\ndiff --git a/fsmonitor.c b/fsmonitor.c\nindex 665bd2d425..1dee0aded1 100644\n--- a/fsmonitor.c\n+++ b/fsmonitor.c\n@@ -129,7 +129,6 @@ static void fsmonitor_refresh_callback(struct index_state *istate, const char *n\n \n void refresh_fsmonitor(struct index_state *istate)\n {\n-\tstatic int has_run_once = 0;\n \tstruct strbuf query_result = STRBUF_INIT;\n \tint query_success = 0;\n \tsize_t bol; /* beginning of line */\n@@ -137,9 +136,9 @@ void refresh_fsmonitor(struct index_state *istate)\n \tchar *buf;\n \tint i;\n \n-\tif (!core_fsmonitor || has_run_once)\n+\tif (!core_fsmonitor || istate->fsmonitor_has_run_once)\n \t\treturn;\n-\thas_run_once = 1;\n+\tistate->fsmonitor_has_run_once = 1;\n \n \ttrace_printf_key(&trace_fsmonitor, \"refresh fsmonitor\");\n \t/*\ndiff --git a/read-cache.c b/read-cache.c\nindex 4dc6de1b55..0bf39e177a 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -2335,6 +2335,7 @@ int discard_index(struct index_state *istate)\n \tfree_name_hash(istate);\n \tcache_tree_free(&(istate->cache_tree));\n \tistate->initialized = 0;\n+\tistate->fsmonitor_has_run_once = 0;\n \tFREE_AND_NULL(istate->cache);\n \tistate->cache_alloc = 0;\n \tdiscard_split_index(istate);\ndiff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh\nindex afd8fa7532..81a375fa0f 100755\n--- a/t/t7519-status-fsmonitor.sh\n+++ b/t/t7519-status-fsmonitor.sh\n@@ -346,7 +346,7 @@ test_expect_success UNTRACKED_CACHE 'ignore .git changes when invalidating UNTR'\n \ttest_cmp before after\n '\n \n-test_expect_failure 'discard_index() also discards fsmonitor info' '\n+test_expect_success 'discard_index() also discards fsmonitor info' '\n \ttest_config core.fsmonitor \"$TEST_DIRECTORY/t7519/fsmonitor-all\" &&\n \ttest_might_fail git update-index --refresh &&\n \ttest-tool read-cache --print-and-refresh=tracked 2 >actual &&\n-- \ngitgitgadget\n"},{"id":"372134","messageId":"nycvar.QRO.7.76.6.1903211532140.41@tvgsbejvaqbjf.bet","threadId":"50754","inReplyTo":"87ef74a1dv.fsf@evledraar.gmail.com","subject":"Re: [PATCH 0/2] Fix fsmonitor after discard_index()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-03-21T14:36:11Z","receivedAt":"2019-03-21T14:36:36Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Ævar,\n\nOn Mon, 18 Mar 2019, Ævar Arnfjörð Bjarmason wrote:\n\n> On Sat, Mar 16 2019, Johannes Schindelin via GitGitGadget wrote:\n>\n> > It was reported by Ævar Arnfjörð Bjarmason\n> > [https://public-inbox.org/git/nycvar.QRO.7.76.6.1903142058130.41@tvgsbejvaqbjf.bet/T/#mb8718fe52e4721dacd3b143a09187ff9090ef4e3]\n> > that there were problems with the fsmonitor feature in conjunction with the\n> > newly built-in git stash/git rebase.\n> >\n> > The culprit really is that the fsmonitor flag that says whether it was\n> > queried already was not re-set after discard_index() was called by mistake.\n> >\n> > This fixes that, and apparently also other long-standing fsmonitor issues.\n>\n> I've added this to my internal build & now the test suite passes in the\n> fsmonitor mode without any test skipping.\n\nAwesome.\n\n> > (Note that there is still a flakiness around t7519\n> > [https://github.com/git-for-windows/git/pull/2127#pullrequestreview-215010574]\n> > where it tries to make sure that the fsmonitor hook can prevent unnecessary\n> > lstat() calls, but that seems to be unrelated to this here bug.)\n>\n> FWIW Since February 1st, 2018 I've run my builds on CentOS [67] through\n> an GIT_FSMONITOR_TEST=$PWD/t7519/fsmonitor-all test and have never\n> encountered this flakyness, and I built pretty much on every \"next\"\n> push-out.\n>\n> The fix sounds good, just one data point on the rarity of the race in\n> practice. I hadn't noticed this being flaky.\n\nThat makes me think that my bug fix may bring this racy problem to the\nlight of day. After all, now we will query the fsmonitor a lot more times\nthan before, and in quicker succession (because there are no slow shell\ncommands running in between fsmonitor calls, i.e. the test suite).\n\nI guess I will see more of that now, and maybe eventually the pain in my\nnotification mails will be high enough that I'll allocate time to\ninvestigate.\n\nThank you for your feedback, though!\nDscho\n"},{"id":"372234","messageId":"20190322110422.GB22459@szeder.dev","threadId":"50754","inReplyTo":"79fdd0d586e9086b68af1b68dc5e194a566d2240.1553176651.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 2/2] fsmonitor: force a refresh after the index was discarded","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-03-22T11:04:22Z","receivedAt":"2019-03-22T11:04:28Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Thu, Mar 21, 2019 at 06:57:34AM -0700, Johannes Schindelin via GitGitGadget wrote:\n> From: Johannes Schindelin <johannes.schindelin@gmx.de>\n> \n> With this change, the `index_state` struct becomes the new home for the\n> flag that says whether the fsmonitor hook has been run, i.e. it is now\n> per-index.\n> \n> It also gets re-set when the index is discarded, fixing the bug where\n> fsmonitor-enabled Git would miss updates under certain circumstances.\n\nUnder what circumstances would a fsmonitor-enabled Git miss updates?\n\n"},{"id":"375021","messageId":"20190507111021.12840-1-avarab@gmail.com","threadId":"50754","inReplyTo":"pull.165.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v3 0/2] Fix fsmonitor after discard_index()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-05-07T11:10:19Z","receivedAt":"2019-05-07T11:10:36Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This v3 is all Johannes's patches. The outstanding review on v2 could\nbe clarified with a commit message change, which I've addressed, and\nv2 conflicted with a cache.h change that's since landed in \"master\",\nwhich I've rebased this on.\n\nJunio: We're getting closer to the release so it would be great to\nhave this. It's been broken for a long time, but having this finaly\nfixed in v2.22 would be great. The functional code changes are also\nisolated to the fsmonitor code path, which reduces the risk and makes\nthis easier to review/reason about.\n\nJohannes Schindelin (2):\n  fsmonitor: demonstrate that it is not refreshed after discard_index()\n  fsmonitor: force a refresh after the index was discarded\n\n cache.h                     |  3 ++-\n fsmonitor.c                 |  5 ++---\n read-cache.c                |  1 +\n t/helper/test-read-cache.c  | 24 +++++++++++++++++++++++-\n t/t7519-status-fsmonitor.sh |  8 ++++++++\n 5 files changed, 36 insertions(+), 5 deletions(-)\n\nRange-diff:\n1:  51a7edf22a = 1:  c31f834b07 fsmonitor: demonstrate that it is not refreshed after discard_index()\n2:  79fdd0d586 ! 2:  7bf5f9f610 fsmonitor: force a refresh after the index was discarded\n    @@ -6,8 +6,10 @@\n         flag that says whether the fsmonitor hook has been run, i.e. it is now\n         per-index.\n     \n    -    It also gets re-set when the index is discarded, fixing the bug where\n    -    fsmonitor-enabled Git would miss updates under certain circumstances.\n    +    It also gets re-set when the index is discarded, fixing the bug\n    +    demonstrated by the \"test_expect_failure\" test added in the preceding\n    +    commit. In that case fsmonitor-enabled Git would miss updates under\n    +    certain circumstances, see that preceding commit for details.\n     \n         Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n     \n    @@ -15,11 +17,11 @@\n      --- a/cache.h\n      +++ b/cache.h\n     @@\n    - \tstruct cache_time timestamp;\n    - \tunsigned name_hash_initialized : 1,\n      \t\t initialized : 1,\n    --\t\t drop_cache_tree : 1;\n    -+\t\t drop_cache_tree : 1,\n    + \t\t drop_cache_tree : 1,\n    + \t\t updated_workdir : 1,\n    +-\t\t updated_skipworktree : 1;\n    ++\t\t updated_skipworktree : 1,\n     +\t\t fsmonitor_has_run_once : 1;\n      \tstruct hashmap name_hash;\n      \tstruct hashmap dir_hash;\n-- \n2.21.0.593.g511ec345e18\n\n"},{"id":"375022","messageId":"20190507111021.12840-2-avarab@gmail.com","threadId":"50754","inReplyTo":"pull.165.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v3 1/2] fsmonitor: demonstrate that it is not refreshed after discard_index()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-05-07T11:10:20Z","receivedAt":"2019-05-07T11:10:38Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nThis one is tricky.\n\nWhen `core.fsmonitor` is set, a `refresh_index()` will not perform a\nfull scan of files that might be modified, but will query the fsmonitor\nand refresh only the ones that have been actually touched.\n\nDue to implementation details, the fsmonitor is queried in\n`refresh_cache_ent()`, but of course it only has to be queried once, so\nwe set a flag when we did that. But when the index was discarded, we did\nnot re-set that flag.\n\nSo far, this is only covered by our test suite when running with\nGIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all, and only due to the way the\nbuilt-in stash interacts with the recursive merge machinery.\n\nLet's introduce a straight-forward regression test for this.\n\nWe simply extend the \"read & discard index\" loop in `test-tool\nread-cache` to optionally refresh the index, report on a given file's\nstatus, and then modify that file. Due to the bug described above, only\nthe first refresh will actually query the fsmonitor; subsequent loop\niterations will not.\n\nThis problem was reported by Ævar Arnfjörð Bjarmason.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n t/helper/test-read-cache.c  | 24 +++++++++++++++++++++++-\n t/t7519-status-fsmonitor.sh |  8 ++++++++\n 2 files changed, 31 insertions(+), 1 deletion(-)\n\ndiff --git a/t/helper/test-read-cache.c b/t/helper/test-read-cache.c\nindex d674c88ba0..7e79b555de 100644\n--- a/t/helper/test-read-cache.c\n+++ b/t/helper/test-read-cache.c\n@@ -1,14 +1,36 @@\n #include \"test-tool.h\"\n #include \"cache.h\"\n+#include \"config.h\"\n \n int cmd__read_cache(int argc, const char **argv)\n {\n-\tint i, cnt = 1;\n+\tint i, cnt = 1, namelen;\n+\tconst char *name = NULL;\n+\n+\tif (argc > 1 && skip_prefix(argv[1], \"--print-and-refresh=\", &name)) {\n+\t\tnamelen = strlen(name);\n+\t\targc--;\n+\t\targv++;\n+\t}\n+\n \tif (argc == 2)\n \t\tcnt = strtol(argv[1], NULL, 0);\n \tsetup_git_directory();\n+\tgit_config(git_default_config, NULL);\n \tfor (i = 0; i < cnt; i++) {\n \t\tread_cache();\n+\t\tif (name) {\n+\t\t\tint pos;\n+\n+\t\t\trefresh_index(&the_index, REFRESH_QUIET,\n+\t\t\t\t      NULL, NULL, NULL);\n+\t\t\tpos = index_name_pos(&the_index, name, namelen);\n+\t\t\tif (pos < 0)\n+\t\t\t\tdie(\"%s not in index\", name);\n+\t\t\tprintf(\"%s is%s up to date\\n\", name,\n+\t\t\t       ce_uptodate(the_index.cache[pos]) ? \"\" : \" not\");\n+\t\t\twrite_file(name, \"%d\\n\", i);\n+\t\t}\n \t\tdiscard_cache();\n \t}\n \treturn 0;\ndiff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh\nindex 3e0a61db23..afd8fa7532 100755\n--- a/t/t7519-status-fsmonitor.sh\n+++ b/t/t7519-status-fsmonitor.sh\n@@ -346,4 +346,12 @@ test_expect_success UNTRACKED_CACHE 'ignore .git changes when invalidating UNTR'\n \ttest_cmp before after\n '\n \n+test_expect_failure 'discard_index() also discards fsmonitor info' '\n+\ttest_config core.fsmonitor \"$TEST_DIRECTORY/t7519/fsmonitor-all\" &&\n+\ttest_might_fail git update-index --refresh &&\n+\ttest-tool read-cache --print-and-refresh=tracked 2 >actual &&\n+\tprintf \"tracked is%s up to date\\n\" \"\" \" not\" >expect &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.21.0.593.g511ec345e18\n\n"},{"id":"375023","messageId":"20190507111021.12840-3-avarab@gmail.com","threadId":"50754","inReplyTo":"pull.165.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v3 2/2] fsmonitor: force a refresh after the index was discarded","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-05-07T11:10:21Z","receivedAt":"2019-05-07T11:10:39Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nWith this change, the `index_state` struct becomes the new home for the\nflag that says whether the fsmonitor hook has been run, i.e. it is now\nper-index.\n\nIt also gets re-set when the index is discarded, fixing the bug\ndemonstrated by the \"test_expect_failure\" test added in the preceding\ncommit. In that case fsmonitor-enabled Git would miss updates under\ncertain circumstances, see that preceding commit for details.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n cache.h                     | 3 ++-\n fsmonitor.c                 | 5 ++---\n read-cache.c                | 1 +\n t/t7519-status-fsmonitor.sh | 2 +-\n 4 files changed, 6 insertions(+), 5 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex e928fe9d3b..08607ca7f2 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -341,7 +341,8 @@ struct index_state {\n \t\t initialized : 1,\n \t\t drop_cache_tree : 1,\n \t\t updated_workdir : 1,\n-\t\t updated_skipworktree : 1;\n+\t\t updated_skipworktree : 1,\n+\t\t fsmonitor_has_run_once : 1;\n \tstruct hashmap name_hash;\n \tstruct hashmap dir_hash;\n \tstruct object_id oid;\ndiff --git a/fsmonitor.c b/fsmonitor.c\nindex 665bd2d425..1dee0aded1 100644\n--- a/fsmonitor.c\n+++ b/fsmonitor.c\n@@ -129,7 +129,6 @@ static void fsmonitor_refresh_callback(struct index_state *istate, const char *n\n \n void refresh_fsmonitor(struct index_state *istate)\n {\n-\tstatic int has_run_once = 0;\n \tstruct strbuf query_result = STRBUF_INIT;\n \tint query_success = 0;\n \tsize_t bol; /* beginning of line */\n@@ -137,9 +136,9 @@ void refresh_fsmonitor(struct index_state *istate)\n \tchar *buf;\n \tint i;\n \n-\tif (!core_fsmonitor || has_run_once)\n+\tif (!core_fsmonitor || istate->fsmonitor_has_run_once)\n \t\treturn;\n-\thas_run_once = 1;\n+\tistate->fsmonitor_has_run_once = 1;\n \n \ttrace_printf_key(&trace_fsmonitor, \"refresh fsmonitor\");\n \t/*\ndiff --git a/read-cache.c b/read-cache.c\nindex d5a74b1861..c66b5c6b26 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -2323,6 +2323,7 @@ int discard_index(struct index_state *istate)\n \tfree_name_hash(istate);\n \tcache_tree_free(&(istate->cache_tree));\n \tistate->initialized = 0;\n+\tistate->fsmonitor_has_run_once = 0;\n \tFREE_AND_NULL(istate->cache);\n \tistate->cache_alloc = 0;\n \tdiscard_split_index(istate);\ndiff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh\nindex afd8fa7532..81a375fa0f 100755\n--- a/t/t7519-status-fsmonitor.sh\n+++ b/t/t7519-status-fsmonitor.sh\n@@ -346,7 +346,7 @@ test_expect_success UNTRACKED_CACHE 'ignore .git changes when invalidating UNTR'\n \ttest_cmp before after\n '\n \n-test_expect_failure 'discard_index() also discards fsmonitor info' '\n+test_expect_success 'discard_index() also discards fsmonitor info' '\n \ttest_config core.fsmonitor \"$TEST_DIRECTORY/t7519/fsmonitor-all\" &&\n \ttest_might_fail git update-index --refresh &&\n \ttest-tool read-cache --print-and-refresh=tracked 2 >actual &&\n-- \n2.21.0.593.g511ec345e18\n\n"},{"id":"375028","messageId":"xmqqh8a6l5zh.fsf@gitster-ct.c.googlers.com","threadId":"50754","inReplyTo":"20190507111021.12840-1-avarab@gmail.com","subject":"Re: [PATCH v3 0/2] Fix fsmonitor after discard_index()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-05-07T14:00:18Z","receivedAt":"2019-05-07T14:00:27Z","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> This v3 is all Johannes's patches. The outstanding review on v2 could\n> be clarified with a commit message change, which I've addressed, and\n> v2 conflicted with a cache.h change that's since landed in \"master\",\n> which I've rebased this on.\n>\n> Junio: We're getting closer to the release so it would be great to\n> have this. It's been broken for a long time, but having this finaly\n> fixed in v2.22 would be great. The functional code changes are also\n> isolated to the fsmonitor code path, which reduces the risk and makes\n> this easier to review/reason about.\n\nThanks.  With this many topic backlog in flight (and not yet in\n'pu'), a resend like this, which is much easier to handle than mere\nreference to the previous discussion thread, is really appreciated.\n\n"},{"id":"375087","messageId":"xmqqtve5iqr0.fsf@gitster-ct.c.googlers.com","threadId":"50754","inReplyTo":"xmqqh8a6l5zh.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3 0/2] Fix fsmonitor after discard_index()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-05-08T03:12:19Z","receivedAt":"2019-05-08T03:12:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:\n>\n>> This v3 is all Johannes's patches. The outstanding review on v2 could\n>> be clarified with a commit message change, which I've addressed, and\n>> v2 conflicted with a cache.h change that's since landed in \"master\",\n>> which I've rebased this on.\n>>\n>> Junio: We're getting closer to the release so it would be great to\n>> have this. It's been broken for a long time, but having this finaly\n>> fixed in v2.22 would be great. The functional code changes are also\n>> isolated to the fsmonitor code path, which reduces the risk and makes\n>> this easier to review/reason about.\n>\n> Thanks.  With this many topic backlog in flight (and not yet in\n> 'pu'), a resend like this, which is much easier to handle than mere\n> reference to the previous discussion thread, is really appreciated.\n\nHmph, surprisingly, 1/2 alone did not reproduce any breakage.\n\n    ... goes and looks ...\n\nAh, that's false alarm.\n\nAs I use prove, \"make test\" treats known breakage as a non-event and\ndoes not show, which made me lose a few minutes scratching my head\nwondering what I did wrong.  It would have been nicer if it were\neasy to use 1/2 alone with the new test marked to expect success,\nbut with a patch split into two like this, it requires going in to\nthe test script and changing s/_failure/_success/, which is a bit\nsuboptimial.\n\nAnyway, will queue this to 'pu' and hopefully we can merge it down\nsoonish.\n\nThanks.\n"},{"id":"376121","messageId":"nycvar.QRO.7.76.6.1905232245390.46@tvgsbejvaqbjf.bet","threadId":"50754","inReplyTo":"87ef74a1dv.fsf@evledraar.gmail.com","subject":"Re: [PATCH 0/2] Fix fsmonitor after discard_index()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-05-23T20:49:19Z","receivedAt":"2019-05-23T20:49:30Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Ævar,\n\nOn Mon, 18 Mar 2019, Ævar Arnfjörð Bjarmason wrote:\n\n>\n> On Sat, Mar 16 2019, Johannes Schindelin via GitGitGadget wrote:\n>\n> > It was reported by Ævar Arnfjörð Bjarmason\n> > [https://public-inbox.org/git/nycvar.QRO.7.76.6.1903142058130.41@tvgsbejvaqbjf.bet/T/#mb8718fe52e4721dacd3b143a09187ff9090ef4e3]\n> > that there were problems with the fsmonitor feature in conjunction\n> > with the newly built-in git stash/git rebase.\n> >\n> > The culprit really is that the fsmonitor flag that says whether it was\n> > queried already was not re-set after discard_index() was called by\n> > mistake.\n> >\n> > This fixes that, and apparently also other long-standing fsmonitor\n> > issues.\n>\n> I've added this to my internal build & now the test suite passes in the\n> fsmonitor mode without any test skipping.\n>\n> > (Note that there is still a flakiness around t7519\n> > [https://github.com/git-for-windows/git/pull/2127#pullrequestreview-215010574]\n> > where it tries to make sure that the fsmonitor hook can prevent unnecessary\n> > lstat() calls, but that seems to be unrelated to this here bug.)\n>\n> FWIW Since February 1st, 2018 I've run my builds on CentOS [67] through\n> an GIT_FSMONITOR_TEST=$PWD/t7519/fsmonitor-all test and have never\n> encountered this flakyness, and I built pretty much on every \"next\"\n> push-out.\n>\n> The fix sounds good, just one data point on the rarity of the race in\n> practice. I hadn't noticed this being flaky.\n\nI finally found some time to track this down (it only took me two\ndays...): it is a racy condition that *hides* the bug.\n\nDetails and a fix in https://github.com/gitgitgadget/git/pull/223\n\nCiao,\nDscho\n"}]}