git/fsmonitor.c, branch v2.40.2

treewide: always have a valid "index_state.repo" member

2023-01-17T22:32:06Z

When the "repo" member was added to "the_index" in [1] the repo_read_index() was made to populate it, but the unpopulated "the_index" variable didn't get the same treatment. Let's do that in initialize_the_repository() when we set it up, and likewise for all of the current callers initialized an empty "struct index_state". This simplifies code that needs to deal with "the_index" or a custom "struct index_state", we no longer need to second-guess this part of the "index_state" deep in the stack. A recent example of such second-guessing is the "istate->repo ? istate->repo : the_repository" code in [2]. We can now simply use "istate->repo". We're doing this by making use of the INDEX_STATE_INIT() macro (and corresponding function) added in [3], which now have mandatory "repo" arguments. Because we now call index_state_init() in repository.c's initialize_the_repository() we don't need to handle the case where we have a "repo->index" whose "repo" member doesn't match the "repo" we're setting up, i.e. the "Complete the double-reference" code in repo_read_index() being altered here. That logic was originally added in [1], and was working around the lack of what we now have in initialize_the_repository(). For "fsmonitor-settings.c" we can remove the initialization of a NULL "r" argument to "the_repository". This was added back in [4], and was needed at the time for callers that would pass us the "r" from an "istate->repo". Before this change such a change to "fsmonitor-settings.c" would segfault all over the test suite (e.g. in t0002-gitfile.sh). This change has wider eventual implications for "fsmonitor-settings.c". The reason the other lazy loading behavior in it is required (starting with "if (!r->settings.fsmonitor) ..." is because of the previously passed "r" being "NULL". I have other local changes on top of this which move its configuration reading to "prepare_repo_settings()" in "repo-settings.c", as we could now start to rely on it being called for our "r". But let's leave all of that for now, and narrowly remove this particular part of the lazy-loading. 1. 1fd9ae517c4 (repository: add repo reference to index_state, 2021-01-23) 2. ee1f0c242ef (read-cache: add index.skipHash config option, 2023-01-06) 3. 2f6b1eb794e (cache API: add a "INDEX_STATE_INIT" macro/function, add release_index(), 2023-01-12) 4. 1e0ea5c4316 (fsmonitor: config settings are repository-specific, 2022-03-25) Signed-off-by: Ævar Arnfjörð Bjarmason Acked-by: Derrick Stolee Signed-off-by: Junio C Hamano

fsmonitor: fix leak of warning message

2022-10-11T05:16:56Z

The fsm_settings__get_incompatible_msg() function returns an allocated string. So we can't pass its result directly to warning(); we must hold on to the pointer and free it to avoid a leak. The leak here is small and fixed size, but Coverity complained, and presumably SANITIZE=leaks would eventually. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

fsmonitor: check for compatability before communicating with fsmonitor

2022-10-05T18:05:23Z

If fsmonitor is not in a compatible state, warn with an appropriate message. Signed-off-by: Eric DeCosta Signed-off-by: Junio C Hamano

fsmonitor: never set CE_FSMONITOR_VALID on submodules

2022-05-26T22:59:27Z

Never set CE_FSMONITOR_VALID on the cache-entry of submodule directories. During a client command like 'git status', we may need to recurse into each submodule to compute a status summary for the submodule. Since the purpose of the ce_flag is to let Git avoid scanning a cache-entry, setting the flag causes the recursive call to be avoided and we report incorrect (no status) for the submodule. We created an OS watch on the root directory of our working directory and we receive events for everything in the cone under it. When submodules are present inside our working directory, we receive events for both our repo (the super) and any subs within it. Since our index doesn't have any information for items within the submodules, we can't use those events. We could try to truncate the paths of those events back to the submodule boundary and mark the GITLINK as dirty, but that feels expensive since we would have to prefix compare every FS event that we receive against a list of submodule roots. And it still wouldn't be sufficient to correctly report status on the submodule, since we don't have any space in the cache-entry to cache the submodule's status (the 'SCMU' bits in porcelain V2 speak). That is, the CE_FSMONITOR_VALID bit just says that we don't need to scan/inspect it because we already know the answer -- it doesn't say that the item is clean -- and we don't have space in the cache-entry to store those answers. So we should always do the recursive scan. Therefore, we should never set the flag on GITLINK cache-entries. Signed-off-by: Jeff Hostetler Signed-off-by: Junio C Hamano

fsmonitor: optimize processing of directory events

2022-05-26T22:59:27Z

Teach Git to perform binary search over the cache-entries for a directory notification and then linearly scan forward to find the immediate children. Previously, when the FSMonitor reported a modified directory Git would perform a linear search on the entire cache-entry array for all entries matching that directory prefix and invalidate them. Since the cache-entry array is already sorted, we can use a binary search to find the first matching entry and then only linearly walk forward and invalidate entries until the prefix changes. Also, the original code would invalidate anything having the same directory prefix. Since a directory event should only be received for items that are immediately within the directory (and not within sub-directories of it), only invalidate those entries and not the whole subtree. Signed-off-by: Jeff Hostetler Signed-off-by: Junio C Hamano

fsmonitor: force update index after large responses

2022-03-25T23:04:17Z

Measure the time taken to apply the FSMonitor query result to the index and the untracked-cache. Set the `FSMONITOR_CHANGED` bit on `istate->cache_changed` when FSMonitor returns a very large repsonse to ensure that the index is written to disk. Normally, when the FSMonitor response includes a tracked file, the index is always updated. Similarly, the index might be updated when the response alters the untracked-cache (when enabled). However, in cases where neither of those cause the index to be considered changed, the FSMonitor response is wasted. Subsequent Git commands will make requests with the same token and receive the same response. If that response is very large, performance may suffer. It would be more efficient to force update the index now (and the token in the index extension) in order to reduce the size of the response received by future commands. This was observed on Windows after a large checkout. On Windows, the kernel emits events for the files that are changed as they are changed. However, it might delay events for the containing directories until the system is more idle (or someone scans the directory (so it seems)). The first status following a checkout would get the list of files. The subsequent status commands would get the list of directories as the events trickled out. But they would never catch up because the token was not advanced because the index wasn't updated. This list of directories caused `wt_status_collect_untracked()` to unnecessarily spend time actually scanning them during each command. Signed-off-by: Jeff Hostetler Signed-off-by: Junio C Hamano

fsmonitor: use IPC to query the builtin FSMonitor daemon

2022-03-25T23:04:15Z

Use simple IPC to directly communicate with the new builtin file system monitor daemon when `core.fsmonitor` is set to true. Signed-off-by: Johannes Schindelin Signed-off-by: Jeff Hostetler Signed-off-by: Junio C Hamano

fsmonitor: config settings are repository-specific

2022-03-25T23:04:15Z

Move fsmonitor config settings to a new and opaque `struct fsmonitor_settings` structure. Add a lazily-loaded pointer to this into `struct repo_settings` Create an `enum fsmonitor_mode` type in `struct fsmonitor_settings` to represent the state of fsmonitor. This lets us represent which, if any, fsmonitor provider (hook or IPC) is enabled. Create `fsm_settings__get_*()` getters to lazily look up fsmonitor- related config settings. Get rid of the `core_fsmonitor` global variable. Move the code to lookup the existing `core.fsmonitor` config value into the fsmonitor settings. Create a hook pathname variable in `struct fsmonitor-settings` and only set it when in hook mode. Extend the definition of `core.fsmonitor` to be either a boolean or a hook pathname. When true, the builtin FSMonitor is used. When false or unset, no FSMonitor (neither builtin nor hook) is used. The existing `core_fsmonitor` global variable was used to store the pathname to the fsmonitor hook *and* it was used as a boolean to see if fsmonitor was enabled. This dual usage and global visibility leads to confusion when we add the IPC-based provider. So lets hide the details in fsmonitor-settings.c and let it decide which provider to use in the case of multiple settings. This avoids cluttering up repo-settings.c with these private details. A future commit in builtin-fsmonitor series will add the ability to disqualify worktrees for various reasons, such as being mounted from a remote volume, where fsmonitor should not be started. Having the config settings hidden in fsmonitor-settings.c allows such worktree restrictions to override the config values used. Signed-off-by: Jeff Hostetler Signed-off-by: Junio C Hamano

fsmonitor: enhance existing comments, clarify trivial response handling

2022-03-25T23:04:14Z

Signed-off-by: Jeff Hostetler Signed-off-by: Junio C Hamano

Merge branch 'jh/fsmonitor-prework'

2021-03-19T22:25:37Z

The fsmonitor interface read from its input without making sure there is something to read from. This bug is new in 2.31 timeframe. * jh/fsmonitor-prework: fsmonitor: avoid global-buffer-overflow READ when checking trivial response