Use `fsnotify` for watching file updates

[joelim-work]

Related:

• Continuation of #1548
• Closes #1172
• Closes #1659
• Fixes #1445
• Fixes #1449
• Fixes #1657

This patch uses the fsnotify library for watching filesystem updates so that the UI can be updated accordingly. This offers an alternative to polling for updates by using the configuration set period 1, and should address many of the issues associated with it.

The feature can be enabled by configuring set watch true. I plan to have it disabled by default as it has not yet been tested thoroughly.

List of potential fixes:

• File information (i.e. set info size:time) should be updated accordingly
• Previews should update accordingly
• Created/renamed/deleted files should be reflected
• Directory counts (i.e. set dircounts true) should be updated accordingly
• Copying a large file will show sizes updated in real time
• Updates from external processes (e.g. shell commands), should be reflected

[!IMPORTANT]
Currently attribute updates (e.g. chmod) are being processed, although it is discouraged. Perhaps this can be controlled using an option like watchchmod, in which case it is still possible to refresh the UI after performing chmod, using the load command:

cmd chmod %{{
   chmod "$1" $fx
   touch .
   lf -remote "send $id load"
}}

[!NOTE]
fsnotify may not work properly for remote/FUSE filesystems. If anyone could help verify this, that would be appreciated.

---

TODO

• [ ] Add documentation for watch option
• [ ] Mention in documentation that remote/FUSE filesystems don't work as it is not covered by fsnotify

[valoq]

This feature already works pretty well and I think it can merged now after the recent release to have more people test it. I didn't find any issues so far and while it adds a bit more code (and dependency) to lf, the solution looks a lot cleaner and covers more use cases then what I had in mind with #1671.

Note

fsnotify does not support attribute updates (e.g. chmod)

Actually chmod is listed on the fsnotify repo as a supported attribute and after a quick test with this patch it seems to already cover permission changes even without this 'hack'. (tested on ext4)

Note

fsnotify may not work properly for remote/FUSE filesystems. If anyone could help verify this, that would be appreciated.

A quick test on sshfs seems to confirm that fuse is not covered yet.

[joelim-work]

@valoq Thanks - I updated the description. I have included chmod events for now. Also I very briefly tested with sshfs and found it to be unreliable, sometimes it updates but mostly it doesn't.

[valoq]

#1675 addresses the documentation

Remaining issues I noticed so far:

• When larger directories are copied, the dircount is still unreliable
• Possibly related to https://github.com/gokcehan/lf/issues/1417 which also needs to reopened
• Same with https://github.com/gokcehan/lf/issues/776

Perhaps the solution is to mix the previous patch that tagged a directory as "updated" with this fsnotify implementation to address all use cases related to timing.

[joelim-work]

Remaining issues I noticed so far:

• When larger directories are copied, the dircount is still unreliable
• Possibly related to File size displayed incorrectly after copy #1417 which also needs to reopened
• Same with dircounts is not refreshed with load command #776

Perhaps the solution is to mix the previous patch that tagged a directory as "updated" with this fsnotify implementation to address all use cases related to timing.

Hi @valoq do you have steps to reproduce this? And anything helpful like screen recordings?

[joelim-work]

OK so I found that there are race conditions with mass updates (e.g. touch {1..100} or rm *), so change to use update nav.dirs all in a single thread in 76aacc7.

[valoq]

OK so I found that there are race conditions with mass updates (e.g. touch {1..100} or rm *), so change to use update nav.dirs all in a single thread in 76aacc7.

That seems to address all of the mentioned issues, thanks.

One more issue I noticed (with and without the threading fix) is that copy actions are very slow now. While the current v32 of lf takes about two seconds on my system to copy a 1GB file from one drive to the other, with the fsnotify patch it takes about a minute and the file size is constantly updating. Running time lf with v32 and again with the fsnotify patch shows about the same results though.

[valoq]

Actually, with https://github.com/gokcehan/lf/pull/1667/commits/76aacc708b94cf23e8f98055fe534b650b11e1ed the copy action is fast again, but now the file size is not updated at the end and #1417 is back again

[joelim-work]

Regarding the slowness, AFAIK there's nothing that can be done about this. When you copy a large file, the data is written in blocks, and fsnotify sends an update each time a block is written. There's no way to have fsnotify send only a single update at the end because copying a large file isn't an atomic operation.

Also I'm not sure how why the file size doesn't update for you at the end? For me I tested with the below config, the file size updates at all times during the copy. You'll have to provide the steps for me to reproduce.

set watch true
set info size

[valoq]

Also I'm not sure how why the file size doesn't update for you at the end?

That was just a mistake on my end, I tested the wrong version. The file size is correctly updated with the latest commit.

Regarding the slowness, AFAIK there's nothing that can be done about this. When you copy a large file, the data is written in blocks, and fsnotify sends an update each time a block is written. There's no way to have fsnotify send only a single update at the end because copying a large file isn't an atomic operation.

One possible solution might be to mark a directory as "updating" during copy operations and skip this line until the copy operation is finished, similar to the approach here. Ignoring this watcher event seems to fix the problem.

[joelim-work]

OK this is slightly hacky but I pushed a commit 33f7204 to try and throttle the writes updates. Hopefully this improves the copying time.

[valoq]

Well it does work I suppose.

While it does take significantly less time now, this branch still doubles the execution times for copy operations. To copy a 1GB file with v32:

user	0m2.067s
sys	0m3.703s

With fsnotify:

user	0m4.594s
sys	0m6.569s

If I read correctly, the update time is currently set to 10 milliseconds, changing this to 100 milliseconds may also work, though I didn't notice any less cpu circles used with that change. Maybe this is fine as it is, I will test it some time for now.

[joelim-work]

Thanks, I tried 100 milliseconds very briefly as well, the size updates slightly less frequently. But I don't mind changing the time either way. Although I don't think it's worth making this value configurable.

[gokcehan]

Thank you very much @joelim-work for the patch and @valoq for the tests. In general it looks good to me. 100 milliseconds sounds reasonable but I also don't mind it very much. @joelim-work Feel free to merge the patch whenever you feel like it is ready. We will have time to further test it until the next release.

[joelim-work]

Thanks, I will change this from a draft to an actual PR, but leave it out for a week or two before considering merging.

[valoq]

One more bug related to this feature:

After editing a file in vim and returning to lf, sometimes a new empty file named "4913" appears. Apparently this is some kind of testfile and its appearance is common when inotify is used: https://groups.google.com/g/vim_dev/c/sppdpElxY44

Not quite sure how to avoid this other then to filter this filename by default.

[joelim-work]

One more bug related to this feature:

After editing a file in vim and returning to lf, sometimes a new empty file named "4913" appears. Apparently this is some kind of testfile and its appearance is common when inotify is used: https://groups.google.com/g/vim_dev/c/sppdpElxY44

Not quite sure how to avoid this other then to filter this filename by default.

This is an issue with vim, not lf. You can use the hiddenfiles option to hide these kind of files.

[valoq]

One more bug related to this feature: After editing a file in vim and returning to lf, sometimes a new empty file named "4913" appears. Apparently this is some kind of testfile and its appearance is common when inotify is used: https://groups.google.com/g/vim_dev/c/sppdpElxY44 Not quite sure how to avoid this other then to filter this filename by default.

This is an issue with vim, not lf. You can use the hiddenfiles option to hide these kind of files.

You are right, however since this wont be fixed in vim, I would argue that lf should hide this by default like other file managers with inotify support. The hiddenfiles is probably not a safe solution either if we want to avoid side effects.

I'm not sure yet how to solve this, but I believe it should be addressed before a release.

[joelim-work]

@valoq I did some more investigation, turns out this is caused by the fact that write updates are now throttled and processed out of band (delayed). Which means if you write to a file and then delete it immediately afterwards, then processing the write update will happen later and actually add the file back.

I have pushed a commit 089c540 which should fix this. Thanks for this.

---

Regarding the fact that copying large files now takes slightly more time, I'm not sure what can be done about this. The whole point of fsnotify is that you can subscribe to write updates and update the UI accordingly. And while copying files is done by a separate goroutine, I think it is not actually executed by a separate thread running in parallel, so the two operations end up in contention with each other.

[valoq]

Regarding the fact that copying large files now takes slightly more time, I'm not sure what can be done about this. The whole point of fsnotify is that you can subscribe to write updates and update the UI accordingly. And while copying files is done by a separate goroutine, I think it is not actually executed by a separate thread running in parallel, so the two operations end up in contention with each other.

It is probably fine as it is, just something that bothered me since the cpu times only goes up during copy operations in lf and not when a file is copied outside of lf. I believe this is an indicator that it is not the inotify code causing this, which is why I have been looking for the cause, though without much success so far.

Another bug I just found is that with this patch, for some reason the currently selected line jumps to the bottom of the current directory whenever a file is renamed.

[joelim-work]

Another bug I just found is that with this patch, for some reason the currently selected line jumps to the bottom of the current directory whenever a file is renamed.

After trying the rename command out I can reproduce this too. Can you see if d3f7c08 fixes this issue?

[valoq]

Another bug I just found is that with this patch, for some reason the currently selected line jumps to the bottom of the current directory whenever a file is renamed.

Can you see if d3f7c08 fixes this issue?

It seems to be fixed. While not strictly related to this change, one remaining annoyance is that every time a file is renamed, lf redraws with the renamed file at the bottom of the directory and then redraws again with the renamed file at the right position in accordance with the set sorting order. Any idea what causes this? I will open this as a separate issue if I don't find it today.

[joelim-work]

While not strictly related to this change, one remaining annoyance is that every time a file is renamed, lf redraws with the renamed file at the bottom of the directory and then redraws again with the renamed file at the right position in accordance with the set sorting order. Any idea what causes this? I will open this as a separate issue if I don't find it today.

The rename was designed to be asynchronous, so as a hack it just adds a dummy file at the bottom to represent the new name while it reloads the directory again in the background via app.nav.dirChan. https://github.com/gokcehan/lf/blob/ce1211641830029000f7f3c56444dc509487d6b2/nav.go#L1563-L1567

This however means that it's possible for the renamed file to appear at the bottom if the screen is redrawn. c695904 should fix this,

[valoq]

This is better, thank you. I did catch a glimpse of an additional file being listed for a split second after renaming, but I cannot reproduce this anymore.

Can we avoid the redraw of the renamed directory contents? Since the directory name changed, all contents are redrawn even though nothing inside changed.

[joelim-work]

Can we avoid the redraw of the renamed directory contents? Since the directory name changed, all contents are redrawn even though nothing inside changed.

I suspect it's not possible without changing the design of rename to run synchronously (i.e. load the new file, update the internal data structures and then redraw) as opposed to asynchronously (i.e. redraw now, then load in the background and then redraw again).

But anyway this flickering looks like it's not relevant to this watch feature so I am not interested in discussing it here, in the interests of avoiding scope creep.

[valoq]

There appears to be a remaining race condition when updating the number of files inside a directory.

Running lf in /tmp and then executing mkdir /tmp/a && touch /tmp/a/b in a different terminal will show directory a as having 0 files even though the file b is listed inside. If the period option is also used, the dircount will get updated eventually, but since the monitor can replace the period option, using both should not be necessary.

[joelim-work]

There appears to be a remaining race condition when updating the number of files inside a directory.

Running lf in /tmp and then executing mkdir /tmp/a && touch /tmp/a/b in a different terminal will show directory a as having 0 files even though the file b is listed inside. If the period option is also used, the dircount will get updated eventually, but since the monitor can replace the period option, using both should not be necessary.

Thanks, I can reproduce this, but I don't know what can be done about it. This kind of race condition occurs because /tmp/a/b is created before lf has a chance to watch /tmp/a. It's impossible to watch /tmp/a before it gets created, and /tmp can't be watched recursively because fsnotify doesn't support it.

TBH I'm not really sure dircounts (and by extension calcdirsize) mix well with directory watching, because this kind of information can only be obtained by watching the directory itself instead of the containing directory. In other words, if you watch a given directory, you can receive updates about files/directories being added/removed, updates about files being modified, but not directories being modified. Do you actually use dircounts, or are you just testing whether it works?

[valoq]

TBH I'm not really sure dircounts (and by extension calcdirsize) mix well with directory watching, because this kind of information can only be obtained by watching the directory itself instead of the containing directory. In other words, if you watch a given directory, you can receive updates about files/directories being added/removed, updates about files being modified, but not directories being modified.

That does make sense, but perhaps we can trigger an update of the directory if a file inside the directory has changed, similar to the original approach with the updated variable I used here. When a file has been created inside a directory the directory could be marked as updated and refresh its content information after the copy operation has finished.

Do you actually use dircounts, or are you just testing whether it works?

I do and frankly I would expect most users to use it since I would consider it a basic feature for a file manager.

I considered for a moment if this issue is relevant enough since it can only be triggered when files are created quickly after its parent directory is, but I noticed the issue is also present when copying a directory with files inside. The newly copied directory will always be listed as having 0 files. There were also cases where a directory indicated having files inside after all files inside had been deleted, but I was not yet able to recreate that so it may be a separate issue, though I suspect it to be related.

[joelim-work]

That does make sense, but perhaps we can trigger an update of the directory if a file inside the directory has changed, similar to the original approach with the updated variable I used here. When a file has been created inside a directory the directory could be marked as updated and refresh its content information after the copy operation has finished.

I don't think an updated flag is necessary since the directory is modified anyway when a new file is created. Anyway I changed the code to use this approach (i.e. actually reloading the directory via app.nav.renew after a small delay). It's kind of hacky but I don't know what else works if fsnotify can't watch directories recursively.

I do and frankly I would expect most users to use it since I would consider it a basic feature for a file manager.

It's very easy for a user to say this (especially if it their preference), but the fact remains that dircounts is an awkward feature to implement because each pane represents a directory, and any dircounts shown in that pane does not actually relate to that directory itself. This causes problems with the load approach from #776, and also with fsnotify because of they way it notifies about file creations. I'm not saying it's impossible to update dircounts from fsnotify, but it is very fiddly and requires some hacks to get working. I'm worried that if this patch becomes too complex then @gokcehan will decide it is not worth merging.

[valoq]

That does make sense, but perhaps we can trigger an update of the directory if a file inside the directory has changed, similar to the original approach with the updated variable I used here. When a file has been created inside a directory the directory could be marked as updated and refresh its content information after the copy operation has finished.

I don't think an updated flag is necessary since the directory is modified anyway when a new file is created. Anyway I changed the code to use this approach (i.e. actually reloading the directory via app.nav.renew after a small delay). It's kind of hacky but I don't know what else works if fsnotify can't watch directories recursively.

When copying directories with multiple files, the copied directory is now listed with a partial file count. Strangely its appears to be the same wrong number. Probably because of the timing?

If I copy the lf source code to lf2 (using cp) the original directory is listed with 44 files and the copy is always listed with 41 until I hit refresh manually.

I do and frankly I would expect most users to use it since I would consider it a basic feature for a file manager.

dircounts is an awkward feature to implement

It got this impression with a lot of the code and I am wondering if the only solid solution is to rewrite large parts of lf with a different approach to file updates. Not that I would find the time to do this either :)

So far this patch does not look that complex, especially when considering how many bugs it fixes.

[valoq]

Setting the refresh time to 100 milliseconds seems to solve the remaining issue and I was not able to recreate the problem with directories with different file numbers or file sizes.