Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing file update after paste #1894

Open
valoq opened this issue Jan 15, 2025 · 6 comments
Open

Missing file update after paste #1894

valoq opened this issue Jan 15, 2025 · 6 comments

Comments

@valoq
Copy link
Contributor

valoq commented Jan 15, 2025

I noticed another issue with the watch feature, similar to #1881 (comment)_ but not quite the same.
While the previously discussed issue was caused by fsnotify failing to register the double mounted path, the current version seems to only miss new pasted files while deleting those files will always trigger an update.

To reproduce:
create a file "tmp/x"
copy the file
paste the file in home, while a second instance of ls shows home
The second instance will immediately show the pasted file, while the pasting lf instance will not
Deleting a file in home will update the view in both instances.

@joelim-work
Copy link
Collaborator

I can't reproduce the issue, but I did notice that you mentioned pasting into the home directory, so it may just be a similar issue with mounted directories. Have you tried logging the fsnotify events to see what is happening? I guess you could also try setting up a VM without a mounted home directory and see if the same issue happens there.

@valoq
Copy link
Contributor Author

valoq commented Jan 16, 2025

Yes this seems to be related to the fsnotify double mount bug, but in a somewhat strange way it seems.

When I open lf twice and then execute touch newfile in a terminal, both instances will show newfile, with the log containing:

fsnotify event: CREATE "/home/testuser/newfileinhome"
fsnotify event: CHMOD "/home/testuser/newfileinhome"

When I execute execute touch /tmp/newfile2 and then use on of the lf instances to go to /tmp, copy the file, then go back to $HOME and paste it, the lf instance that pasted the file will not show the paste file, but the second lf instance will.

The log of the pasting instance contains

fsnotify event: CREATE "/home/testuser.homedir/newfile2"

Before the recent changes, this never happened, lf version 33 does not have this bug, which means that there must have been another recent change aside from the previously identified one, that triggers this fsnotify bug.

@joelim-work
Copy link
Collaborator

It's the same issue as the double mount bug if the events are coming under either /home/testuser.homedir or /home/testuser but not both.

So to investigate why r33 works but not the latest master, I guess you can try bisecting the commits, including the individual commits in #1881. Possibly it might be because the version of fsnotify was bumped as part of that change.

Otherwise I ended up writing a workaround patch #1895, hopefully it can solve your issue. But I consider it an ugly solution because I don't think mapping the paths like this belongs in application code.

@valoq
Copy link
Contributor Author

valoq commented Jan 17, 2025

Thank you for the support in dealing with this edge case. While I much appreciate the workaround, I also agree that it should be avoided, even though it looks like fixed in fsnotify and a consequent release may take some time.

After experimenting with the changes in #1881 I found that it was the exception for /dev that caused the double mount bug in fsnotify to be triggered in lf.

The bug seems to disappear when I revert this change in watch.go https://github.com/gokcehan/lf/pull/1881/files#diff-ec9532ea799bf2b59a5fa15126d75b84fc4f429d6c8f561785fc380f41d18564

@joelim-work
Copy link
Collaborator

Right, I saw you raise fsnotify/fsnotify#667 but I'm not sure how long it will take to address, if at all.

So does #1895 fix the mount issue for you? I was planning to release a new version of lf in the coming weeks, but since this is a bug fix I think I don't mind to delay the release.

@valoq
Copy link
Contributor Author

valoq commented Jan 18, 2025

So does #1895 fix the mount issue for you? I was planning to release a new version of lf in the coming weeks, but since this is a bug fix I think I don't mind to delay the release.

It does seem to fix the issue. It seems ready to be merged but I will test this a bit more in the coming days to make sure the next release does not have this bug.

I was wondering though why this specific commit caused this bug to appear in lf. I do not see why not removing the watcher would trigger this and I was hoping that understanding it could reveal how to avoid it again without adding a workaround, though the later may prove the better solution for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants