Creating and removing empty files often doesn't register the removal (MacOS)

Byron commented 7 months ago

How to reproduce

Last tested with GB v0.11.1. On MacOS, with any project loaded and a virtual branch visible:

create an empty file like touch file in the project root
remove the empty file with rm file

This might also reproduce on other unixes, but after all, might be heavily dependent on the filesystem, APFS in this case.

Expected

the file file isn't displayed in the virtual branch anymore

Actual

The file file is still listed as newly added, even though it's not present on disk anymore.

Cause

The filesystem monitor doesn't register the removal in case of deleted files as it only triggers a Metadata modification event. Such events are ignored.

[!NOTE] This reproducer only works if the file is indeed empty - adding anything to the file will cause it's removal to be recognized as such.

As of Version 0.11.3 (20240419.070253), the way filesystem changes are monitored has been overhauled. It is, however, still a source of known issues as under some circumstances, file deletions in particular aren't picked up. On MacOS, this happens regularly if a new file is created and removed like so: touch this; rm this - the file this is probably still visible in GitButler after that.

The reason for this is GitButler intentionally doesn't react to metadata modifications as they typically don't affect the file content, which is the only interesting part of the change.

I tried out the following patch which indeed fixed the issue described here:

commit 26c2a40b4e96cd04bd922e9ac9dab4a696dab735
Author: Sebastian Thiel <sebastian.thiel@icloud.com>
Date:   Sun Apr 21 17:14:13 2024 +0200

    also consider metadata modifications interesting to catch more subtle changes (#2711, #3511)

    Sometimes, metadata changes are the only indication of the deletion of an empty file.
    Let's listen to these as well as otherwise, the UI state will be inconsistent, causing even more
    problems down the line.

diff --git a/crates/gitbutler-watcher/src/file_monitor.rs b/crates/gitbutler-watcher/src/file_monitor.rs
index ea362b6c8..f94e860cf 100644
--- a/crates/gitbutler-watcher/src/file_monitor.rs
+++ b/crates/gitbutler-watcher/src/file_monitor.rs
@@ -180,6 +180,7 @@ fn is_interesting_kind(kind: notify::EventKind) -> bool {
     matches!(
         kind,
         notify::EventKind::Create(notify::event::CreateKind::File)
+            | notify::EventKind::Modify(notify::event::ModifyKind::Metadata(_))
             | notify::EventKind::Modify(notify::event::ModifyKind::Data(_))
             | notify::EventKind::Modify(notify::event::ModifyKind::Name(_))
             | notify::EventKind::Remove(notify::event::RemoveKind::File)

However, it also triggers a whole lot more work, and I have a feeing that pretty much anything sets it off now. The problem is that it now listens to Metadata(Extended|Any) as well, and these happen a lot also when there is no file removed.

I do have a feeling that this is related to the way the aggregation works under the hood - probably the aggregator sometimes fails to forward a deletion event, but instead forwards the related metadata event?

To debug this further, one would have to look into the event aggregator, and see which events arrive there.

Byron commented 3 weeks ago

Still an issue.

gitbutlerapp / gitbutler