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

[Filesystem] When a file gets overwritten, there is no event saying the file has been updated #4838

Open
federicobozzini opened this issue Apr 8, 2019 · 8 comments
Labels
bug bugs found in the application file-watchers issues related to filesystem watchers - nsfw filesystem issues related to the filesystem

Comments

@federicobozzini
Copy link
Contributor

federicobozzini commented Apr 8, 2019

Description

When a file is updated, an event with type FileChangeType.UPDATED is fired, but when a file gets overwritten (for instance by a mv operation) what gets fired is only an event with type FileChangeType.ADDED. This means that when using the event emitter onFilesChanged of the file watcher to check for updated files it's possible to miss a file change.

Is this a known and expected behavior?

Reproduction Steps

To reproduce the issue it is possible to create a watcher on a file and overwrite it with a mv operation.

OS and Theia version:

All the OSs.
Latest Theia version.

@akosyakov
Copy link
Member

It's strange DELETED + ADDED should be translated to UPDATED: https://github.com/theia-ide/theia/blob/master/packages/filesystem/src/node/file-change-collection.ts#L29

We are using nsfw for fs watching, buffer and translate events from it. It would be interesting to record all raw nsfw events on mv action. It could be a bug in nsfw or in our translation.

@akosyakov akosyakov added 🤔 needs more info issues that require more info from the author filesystem issues related to the filesystem labels Apr 8, 2019
@federicobozzini
Copy link
Contributor Author

I did some quick investigation with nsfw, and what seems to be happening is that when a file is overwritten you only get a nsfw.actions.RENAMED event. This event gets translated by Theia in a deletion on the old URI and a creation on the new URI (the overwritten file), hence there is no deletion of the new URI and the rules you linked are not used.

@akosyakov
Copy link
Member

if URIs are different, then it works as expected. UPDATED means that content changed of one URI, with move one URI gets removed and another added.

@federicobozzini
Copy link
Contributor Author

I'm not sure it's clear what I meant.

Let me describe a few uses cases:

  • when creating file a.txt
    nsfw.actions.CREATED[a.txt] => FileChangeType.ADDED[a.txt] (correct, file)

  • when editing file a.txt
    nsfw.actions.MODIFIED[a.txt] => FileChangeType.UDPATED[a.txt] (correct)

  • when moving file b.txt to 'a.txt', overwridding it's content
    nsfw.actions.RENAMED[b.txt, a.txt] => FileChangeType.REMOVED[b.txt], FileChangeType.ADDED[a.txt] (not correct)

The last example is not correct, or at least the behaviour is not what I would expect, because you get an added event but the content of the a.txt file has changed.

The normalization rules that you linked are not applied here because they are only valid when the changes are applied to the same URI, and it's not really the case here.

@akosyakov
Copy link
Member

It's clear. But i don't have an idea how we can reliably detect whether a.txt is ADDED or UPDATED. We don't know whether it existed before. I would say you should open an issue for nsfw as well.

@akosyakov akosyakov added bug bugs found in the application and removed 🤔 needs more info issues that require more info from the author labels Apr 10, 2019
@federicobozzini
Copy link
Contributor Author

I don't think that nsfw should change its behaviour since it's just a wrapper around inotify (or similar tools) and inotify doesn't manage this case. I also think that a RENAME event in this case is more expressive and something that a developer may be expected to manage. Maybe splitting the RENAME event into the two events is the culprit here.

@akosyakov
Copy link
Member

We have to do it since LSP and vscode extensions don’t support renamed events. I can experiment with introducing renamed event in addition for Theia extensions. If they are reported reliably for all mv operations, it could work. Last time when I was implementing updating widgets based on file renaming it was not a case. Sometimes nsfw fails to deliver them at all or delivers add + remove events instead.

@akosyakov
Copy link
Member

Seems to be an issue in vscode as well: microsoft/vscode#30240

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug bugs found in the application file-watchers issues related to filesystem watchers - nsfw filesystem issues related to the filesystem
Projects
None yet
Development

No branches or pull requests

3 participants