From dc76852df2f7bc5a6c79a796954a81f5af284c34 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 7 May 2019 13:10:20 +0200 Subject: [PATCH 1/2] fsmonitor: demonstrate that it is not refreshed after discard_index() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This one is tricky. When `core.fsmonitor` is set, a `refresh_index()` will not perform a full scan of files that might be modified, but will query the fsmonitor and refresh only the ones that have been actually touched. Due to implementation details, the fsmonitor is queried in `refresh_cache_ent()`, but of course it only has to be queried once, so we set a flag when we did that. But when the index was discarded, we did not re-set that flag. So far, this is only covered by our test suite when running with GIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all, and only due to the way the built-in stash interacts with the recursive merge machinery. Let's introduce a straight-forward regression test for this. We simply extend the "read & discard index" loop in `test-tool read-cache` to optionally refresh the index, report on a given file's status, and then modify that file. Due to the bug described above, only the first refresh will actually query the fsmonitor; subsequent loop iterations will not. This problem was reported by Ævar Arnfjörð Bjarmason. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- t/helper/test-read-cache.c | 24 +++++++++++++++++++++++- t/t7519-status-fsmonitor.sh | 8 ++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/t/helper/test-read-cache.c b/t/helper/test-read-cache.c index d674c88ba0..7e79b555de 100644 --- a/t/helper/test-read-cache.c +++ b/t/helper/test-read-cache.c @@ -1,14 +1,36 @@ #include "test-tool.h" #include "cache.h" +#include "config.h" int cmd__read_cache(int argc, const char **argv) { - int i, cnt = 1; + int i, cnt = 1, namelen; + const char *name = NULL; + + if (argc > 1 && skip_prefix(argv[1], "--print-and-refresh=", &name)) { + namelen = strlen(name); + argc--; + argv++; + } + if (argc == 2) cnt = strtol(argv[1], NULL, 0); setup_git_directory(); + git_config(git_default_config, NULL); for (i = 0; i < cnt; i++) { read_cache(); + if (name) { + int pos; + + refresh_index(&the_index, REFRESH_QUIET, + NULL, NULL, NULL); + pos = index_name_pos(&the_index, name, namelen); + if (pos < 0) + die("%s not in index", name); + printf("%s is%s up to date\n", name, + ce_uptodate(the_index.cache[pos]) ? "" : " not"); + write_file(name, "%d\n", i); + } discard_cache(); } return 0; diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh index 3e0a61db23..afd8fa7532 100755 --- a/t/t7519-status-fsmonitor.sh +++ b/t/t7519-status-fsmonitor.sh @@ -346,4 +346,12 @@ test_expect_success UNTRACKED_CACHE 'ignore .git changes when invalidating UNTR' test_cmp before after ' +test_expect_failure 'discard_index() also discards fsmonitor info' ' + test_config core.fsmonitor "$TEST_DIRECTORY/t7519/fsmonitor-all" && + test_might_fail git update-index --refresh && + test-tool read-cache --print-and-refresh=tracked 2 >actual && + printf "tracked is%s up to date\n" "" " not" >expect && + test_cmp expect actual +' + test_done From 398a3b0899dd8a440d4adbcbda38362e3f8359b1 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 7 May 2019 13:10:21 +0200 Subject: [PATCH 2/2] fsmonitor: force a refresh after the index was discarded With this change, the `index_state` struct becomes the new home for the flag that says whether the fsmonitor hook has been run, i.e. it is now per-index. It also gets re-set when the index is discarded, fixing the bug demonstrated by the "test_expect_failure" test added in the preceding commit. In that case fsmonitor-enabled Git would miss updates under certain circumstances, see that preceding commit for details. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- cache.h | 3 ++- fsmonitor.c | 5 ++--- read-cache.c | 1 + t/t7519-status-fsmonitor.sh | 2 +- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/cache.h b/cache.h index 27fe635f62..06ca755c93 100644 --- a/cache.h +++ b/cache.h @@ -338,7 +338,8 @@ struct index_state { struct cache_time timestamp; unsigned name_hash_initialized : 1, initialized : 1, - drop_cache_tree : 1; + drop_cache_tree : 1, + fsmonitor_has_run_once : 1; struct hashmap name_hash; struct hashmap dir_hash; struct object_id oid; diff --git a/fsmonitor.c b/fsmonitor.c index 665bd2d425..1dee0aded1 100644 --- a/fsmonitor.c +++ b/fsmonitor.c @@ -129,7 +129,6 @@ static void fsmonitor_refresh_callback(struct index_state *istate, const char *n void refresh_fsmonitor(struct index_state *istate) { - static int has_run_once = 0; struct strbuf query_result = STRBUF_INIT; int query_success = 0; size_t bol; /* beginning of line */ @@ -137,9 +136,9 @@ void refresh_fsmonitor(struct index_state *istate) char *buf; int i; - if (!core_fsmonitor || has_run_once) + if (!core_fsmonitor || istate->fsmonitor_has_run_once) return; - has_run_once = 1; + istate->fsmonitor_has_run_once = 1; trace_printf_key(&trace_fsmonitor, "refresh fsmonitor"); /* diff --git a/read-cache.c b/read-cache.c index 0e0c93edc9..b298c7f535 100644 --- a/read-cache.c +++ b/read-cache.c @@ -2307,6 +2307,7 @@ int discard_index(struct index_state *istate) free_name_hash(istate); cache_tree_free(&(istate->cache_tree)); istate->initialized = 0; + istate->fsmonitor_has_run_once = 0; FREE_AND_NULL(istate->cache); istate->cache_alloc = 0; discard_split_index(istate); diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh index afd8fa7532..81a375fa0f 100755 --- a/t/t7519-status-fsmonitor.sh +++ b/t/t7519-status-fsmonitor.sh @@ -346,7 +346,7 @@ test_expect_success UNTRACKED_CACHE 'ignore .git changes when invalidating UNTR' test_cmp before after ' -test_expect_failure 'discard_index() also discards fsmonitor info' ' +test_expect_success 'discard_index() also discards fsmonitor info' ' test_config core.fsmonitor "$TEST_DIRECTORY/t7519/fsmonitor-all" && test_might_fail git update-index --refresh && test-tool read-cache --print-and-refresh=tracked 2 >actual &&