rjeczalik / notify

File system event notification library on steroids.
MIT License
903 stars 128 forks source link

Event types differ between linux and windows on rename/move #117

Open imsodin opened 7 years ago

imsodin commented 7 years ago

This might be a bug, but just as well just a support request. I assume that the idea is, that the 4 platform independent events should be as consistent between platforms as possible.

What I need to do is to separated events into two types: "remove" (whenever a path goes away, either delete or source of move) and "non-remove" (for everything else). Looking and testing on linux (inotify) I found that during a move/rename, the source path gets a Rename event and the destination path a Create event. So the mapping is easy: Remove || Rename -> "remove" and Create || Write -> "non-remove". However this doesn't work on windows, where both source and destination of a move/rename get a Rename event.

The following test-case shows the difference:

os.Remove(filepath.Clean("./testfile"))
os.Remove(filepath.Clean("./newtestfile"))
c := make(chan notify.EventInfo, 5)

if err := notify.Watch(".", c, notify.All); err != nil {
    log.Fatal(err)
}
defer notify.Stop(c)

cancel := make(chan struct{})

go func() {
outer:
    for {
        select {
        case ei := <-c:
            log.Println("Got event:", ei)
        case <-cancel:
            break outer
        }
    }
}()

handle, err := os.Create(filepath.Clean("./testfile"))
if err != nil {
    panic(err)
}
handle.Close()

if err := os.Rename(filepath.Clean("./testfile"), filepath.Clean("./newtestfile")); err != nil {
    panic(err)
}

time.Sleep(time.Second)
close(cancel)
rjeczalik commented 7 years ago

However this doesn't work on windows, where both source and destination of a move/rename get a Rename event.

How do you remove files on Windows? I believe Windows may move them to bin, thus the rename.

imsodin commented 7 years ago

I run the code I provided above, so everything works trough the "os" package of Go. Bin shouldn't be involved anyway, as I am renaming not deleting. Rename is expected, as I am renaming, but what is problematic for me is the different behaviour on Linux vs Windows (destination of rename on Linux gets a Create event, on Windows a Rename event).

rjeczalik commented 7 years ago

@imsodin I just realised you posted a repro, sorry, ignore my comment. The package does not translate yet os-specific behaviour to some unified event model, as we don't have one.

Under Linux it should be possible to match these two events (Rename/Create) with (EventInfo).Sys().(*notify.InotifyEvent).Cookie (http://man7.org/linux/man-pages/man7/inotify.7.html).

imsodin commented 7 years ago

Ok, that's a pity, I hoped there was. Maybe I can explain the use case and you have some idea how to solve it: I am implementing filesystem notifications for Syncthing (decentralized syncing), which is cross-plattform (every (?) plattform go builds on). If files are removed first and then created during a rename/move, they need to be re-transferred over the network. So the idea is to delay events corresponding to a file being removed or being moved away from a location. Essentially I am looking for the easiest way to detect such "remove" events on all possible platforms.

I don't care (too much, at the moment) about linking rename events or anything.

rjeczalik commented 7 years ago

If files are removed first and then created during a rename/move, they need to be re-transferred over the network. So the idea is to delay events corresponding to a file being removed or being moved away from a location.

If I understood correctly, you want to implement an optimisation, that if I rename a file locally, your syncthing will detect such rename and instead of transferring the moved file again to the remote, it'll just replay the rename operation on a remote instead.

We had an idea to implement set of utilities for notify, to ease dealing with translating system events to what actually happened on the filesystem. E.g. to not only cover os-specific rename behaviour, but also handling write events (normal file write vs atomic write to a temporary file that some IDEs do).

I'd implement a middle-man that would listen on your event chan and coalesce all events with some interval (say 1s) into some buffer (slice). The middle-man would then look at the buffered events, if it finds rename-related ones (linux) it'd squash them into single one and emit it instead. I can't think of a different solution, that would be cross-platform.

Sadly we did not manage yet to create such helper tools. If you happen to end up with taking that approach, we'd be happy to accept such contribution and maintain it for broader usage.

imsodin commented 7 years ago

Yes it is an optimisation. Detecting a rename would be ideal, but isn't actually needed. And on linux there is only one Rename event produced during renaming, so how would you match them anyway?

In our case something simpler would suffice: There is a database of indexed blocks for all files. So we just need to announce the new path first, thus the blocks from the old file can be recycled. If we announce the old path (removed) first, these blocks are removed from the database and the "created" (moved) file will not be recycled but be pulled over the network.

We already have a "middle-man" that does buffering of events and aggregating events to the parent directory once a limit is reached. In addition I added the ability for this middle man to detect what kind of event it is handling. I did this by matching Rename and Remove events to an internal remove type and Write and Create to nonRemove. This is correct for inotify/Linux but not for Windows.

rjeczalik commented 7 years ago

@imsodin Just to recap, what you need is a mean to match two rename events on Windows, to be able to say whether they describe the same move op or not, correct?

imsodin commented 7 years ago

Not exactly: I need either one of these two things:

ppknap commented 7 years ago

A means to detect all events that correspond to a removed file (either due to deletion or renaming/moving to another path/filename).

@imsodin you may create a workaround, specific only to Windows platform.

When you initialize watcher with system specific events, you will get system specific responses. Eg. using:

// +build windows

if err := notify.Watch(".", c, notify.FileNotifyChangeFileName, notify.FileNotifyChangeDirName); err != nil {
    log.Fatal(err)
}
defer notify.Stop(c)

will not send you notify.Create, notify.Remove, ... events, but rather the ones specyfic to windows:

// ReadDirectoryChangesW actions.
const (
    FileActionAdded          = Event(syscall.FILE_ACTION_ADDED) << 12
    FileActionRemoved        = Event(syscall.FILE_ACTION_REMOVED) << 12
    FileActionModified       = Event(syscall.FILE_ACTION_MODIFIED) << 14
    FileActionRenamedOldName = Event(syscall.FILE_ACTION_RENAMED_OLD_NAME) << 15
    FileActionRenamedNewName = Event(syscall.FILE_ACTION_RENAMED_NEW_NAME) << 16
)

now, you are able to aggregate deletions by using notify.FileActionRemoved || notify.FileActionRenamedOldName and creations by using notify.FileActionAdded || notify.FileActionRenamedNewName events.

ppknap commented 7 years ago

A means to match two rename events on all supported platforms.

This is also possible on windows but hasn't been implemented in notify yet.

We can tie rename file names by using _FILE_NOTIFY_INFORMATION and then store it in Event's Sys. See: here.

imsodin commented 7 years ago

Thanks for your help, You have me convinced that I need to introduce platform-specific code. I was hoping to be able to go the lazy way by using your platform agnostic events.

Is the definition of the actions of readdcw (<< 12 for both Added and Removed) intentional?

imsodin commented 7 years ago

One more question: InMovedTo is mapped to Create in https://github.com/rjeczalik/notify/blob/master/watcher_inotify.go#L329 and FILE_ACTION_RENAMED_NEW_NAME is mapped to Rename in https://github.com/rjeczalik/notify/blob/master/watcher_readdcw.go#L553 Is my assumption incorrect that these two actions are equivalent?

ppknap commented 7 years ago

Is my assumption incorrect that these two actions are equivalent?

They are,

We probably need to make them consistent in some future. The reason why this was done in that way, is that we can check if linux Create event is move(using Sys method and, as @rjeczalik wrote, we were planning to introduce helpers for that). However, we can't(for now) do this on windows.

Is the definition of the actions of readdcw (<< 12 for both Added and Removed) intentional?

Yes, it is. Windows file actions are not bit masks, so:

FILE_ACTION_ADDED = 0x01 -> 0001b
FILE_ACTION_REMOVED = 0x02 -> 0010b 

^- these two events don't require bit shifting

imsodin commented 7 years ago

Ok, so I can't assume the Event type can be bitmasked? I wanted to define my own removeEventsMask for different platforms like this

const removeEventMask = notify.FileActionRemoved | notify.FileActionRenamedOldName

and then check on incoming events like

if ev.Event()&removeEventMask != 0 {

which apparently is incorrect. So alternatively I would store all events in a map (map[notify.Event]struct{}) and check by key lookup. Is this a/the correct way to do this?

ppknap commented 7 years ago

@imsodin all events can be bitmasked - that's the reason why we have these bit shifts there. You can safely:

const removeEventMask = notify.FileActionRemoved | notify.FileActionRenamedOldName
imsodin commented 7 years ago

@ppknap Thanks again for your help, my limited experience shows here...

Now I have a problem with passing native windows events to Watch:

if err := notify.Watch("./...", c, notify.FileActionAdded); err != nil {
    log.Fatal(err)
}

results in

2017/04/14 13:51:40 notify: unknown event

ppknap commented 7 years ago

Just to explain mapping of Windows file actions to notify events:

FileActionAdded          = 1 << 0 = 001b << 0 = 0000_0001b
FileActionRemoved        = 2 << 0 = 010b << 0 = 0000_0010b
FileActionModified       = 3 << 2 = 011b << 2 = 0000_1100b
FileActionRenamedOldName = 4 << 3 = 100b << 3 = 0010_0000b
FileActionRenamedNewName = 5 << 4 = 101b << 4 = 0101_0000b
                                             |= 0111_1111b

I hope this is clear now.

ppknap commented 7 years ago

@imsodin yes, windows has different logic than other watchers, you must register filters eg. notify.FileNotifyChangeFileName, notify.FileNotifyChangeDirName in order to get actions.

More info here

imsodin commented 7 years ago

@ppknap Now everything is up and running, thanks again for your patience. If you are interested, the code I am working on is https://github.com/syncthing/syncthing/pull/3986.

Would you welcome a PR where I add some of the information, that wasn't obvious to me, to the comments on the windows constants? And maybe a sentence in the main description directing to the event_*.go files for more information (just because on godoc only source files with +build linux are included). And mention syncthing-inotify under projects using notify (technically it uses the zillode/notify fork, but that has just minor tweaks that weren't merge here).

ppknap commented 7 years ago

This is rather a question for @rjeczalik . Personally, I think it would be a nice doc improvement and, of course, any kind of help on this project is always appreciated.

If you are interested, here is my answer that has all mappings from Windows filters to actions. As you see, you can remove ChangeSize and ChangeCreation events from your code.

rjeczalik commented 7 years ago

Would you welcome a PR where I add some of the information, that wasn't obvious to me, to the comments on the windows constants?

This is rather a question for @rjeczalik

I like the idea 👍

imsodin commented 7 years ago

If you are interested, here is my answer that has all mappings from Windows filters to actions. As you see, you can remove ChangeSize and ChangeCreation events from your code.

I also need to detect changes to a file (e.g. writing to a textfile), which is not activated by only ChangeFileName. The rest I chose heuristically: ChangeLastWrite also returns an event for the parent directory, when any child is modified, so I ditched it. ChangeCreation did not have any apparent effect, so I kept it for no good reason. ChangeSize did just what I wanted (however it probably won't pick up on an edit of a file that doesn't change its size). Do you know which is the intended filter to pick up file modifications?

That table is neat, I will link to it as well when modifying comments.