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

macOS: undoing a folder delete does not report folder as added #31

Open
bpasero opened this issue Jul 21, 2017 · 12 comments
Open

macOS: undoing a folder delete does not report folder as added #31

bpasero opened this issue Jul 21, 2017 · 12 comments

Comments

@bpasero
Copy link

bpasero commented Jul 21, 2017

Steps:

  • be on macOS X
  • start the watcher on a folder that includes other folders
  • delete the folder via finder
  • undo this operation

=> the watcher reports the folder as MODIFIED after the undo
=> I would expect the folder event to be reported as CREATED
=> I would also expect CREATED events for each child inside

Output:

[ { action: 1,
    directory: '/Users/bpasero/Desktop/test-js',
    file: 'test' } ]
[ { action: 2,
    directory: '/Users/bpasero/Desktop/test-js',
    file: '.DS_Store' },
  { action: 2,
    directory: '/Users/bpasero/Desktop/test-js',
    file: 'test' } ]

Related issue in VS Code: microsoft/vscode#29766

@implausible
Copy link
Contributor

Will see what I can do. I remember last time I was in this area, we saw bitmasks being reported strangely from the FSEvents API. If I recall correctly, something to do with the bitmask having multiple bits set in the callback something like:

  1. Add a file -> bitmask has add flag
  2. Modify file -> bitmask has add/modify flags
  3. Delete file -> bitmask has add/modify/delete flags

I'd be willing to revisit to see if there's an option to avoid that junk, or even if we could come up with a better strategy for reconciling bad bitmasks.

@bpasero
Copy link
Author

bpasero commented Sep 2, 2017

@implausible any chance you had a look at this?

@bpasero
Copy link
Author

bpasero commented Sep 9, 2017

I think in this case none of the checks within FSEventsServiceCallback return any useful result and we end up in the FSEventsService::demangle method which then has some logic to decide if the node was added or changed:

void FSEventsService::demangle(std::string path) {
  struct stat file;
  if (stat(path.c_str(), &file) != 0) {
    remove(path);
    return;
  }

  if (file.st_birthtimespec.tv_sec != file.st_mtimespec.tv_sec) {
    modify(path);
  } else {
    create(path);
  }
}

When I look at what fsevents does (the library chokidar uses), then I would also assume that fsevents is returning unknown event back to chokidar. In that case, chokidar also has some logic to find out if the node was added or changed (here). Chokidar probably keeps track of all the nodes it encountered during watching and then has a chance of saying if the node existed before or not.

Maybe there is a better way to find out if the folder was created? It seems that when undoing the folder change, the mtime is different from the birthtime, even though the folder was just recreated.

@implausible
Copy link
Contributor

The demangle is definitely the most basic implementation I could think of to clarify some of the events. In GitKraken, it hasn't been a priority to get these events at the level of accuracy that is provided by the other interfaces. Improving the demangle would be a huge bonus for this library as a whole.

@implausible
Copy link
Contributor

I think that we could probably build a reconciliation cache for folders. If we approach it similarly to chokidar, we could keep a record of file events that we can use to demangle better.

@bpasero
Copy link
Author

bpasero commented Sep 13, 2017

@implausible when I was looking into this, I was trying to understand how the native macOS APIs would not fire any event type for the undo case. In other words, are we sure there is no other event type (from the kFSEventStreamEvent* types) that fires for this case? It seems so weird that this case is not exposed as an event type...

@implausible
Copy link
Contributor

Possibly kFSEventStreamEventFlagItemInodeMetaMod?
https://stackoverflow.com/a/13735760

Regardless, there are other mangle issues that would need to be solved, not just this. You'd be surprised by the heartache in the fsevents api.

@bpasero
Copy link
Author

bpasero commented Sep 15, 2017

@implausible no, from my testing this flag was not set because the isModified condition was never met and we use this flag already.

Btw one thing worth mentioning is that for a fresh new folder that I create, then delete and then restore, the events work just fine. But as soon as I add a file to this folder and do the same, it does not work anymore.

+1 for making demangle more robust 👍

@bpasero
Copy link
Author

bpasero commented Oct 24, 2017

@implausible I just had a look at fsevents and I see that they report the delete as "moved-out" and the undo as "moved-in" (via kFSEventStreamEventFlagItemRenamed) indicating that the file is moved to trash and restored from there.

Code is at: https://github.com/strongloop/fsevents/blob/ca07cfdb534f6f85a4a918bc99ef414d191275dc/fsevents.js#L81

@bpasero
Copy link
Author

bpasero commented Jul 2, 2019

Just verified that this still happens with v1.2.5

@DavidGoldman
Copy link

@implausible I just had a look at fsevents and I see that they report the delete as "moved-out" and the undo as "moved-in" (via kFSEventStreamEventFlagItemRenamed) indicating that the file is moved to trash and restored from there.

Code is at: https://github.com/strongloop/fsevents/blob/ca07cfdb534f6f85a4a918bc99ef414d191275dc/fsevents.js#L81

Yep, can confirm that this is the case by testing with FSEvents directly, it reports kFSEventStreamEventFlagItemRenamed and kFSEventStreamEventFlagItemIsDir for both finder deletion and restoration.

@bpasero
Copy link
Author

bpasero commented Mar 30, 2021

This still reproduces in 2.1.2

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

Successfully merging a pull request may close this issue.

3 participants