Fix: Update watch paths when renaming directories with sub-watches on Windows #370
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mattn
reviewed
Jan 15, 2022
arp242
force-pushed
the
fix-windows-renames
branch
2 times, most recently
from
ff67a55
to
338aaf0
Compare
arp242
force-pushed
the
fix-windows-renames
branch
from
338aaf0
to
f9eedb9
Compare
arp242
force-pushed
the
fix-windows-renames
branch
from
f9eedb9
to
ccfa501
Compare
arp242
approved these changes
Jul 31, 2022
arp242
added a commit
that referenced
this pull request
Oct 13, 2022
Renaming a watched path is problematic; on inotify we just get a IN_MOVED_SELF event with the old filename and that's it; no more events for you! So if you do: watch one mv one two cat asd >two You still continue to get events for the file "one", even though it's now named "two" (the file descriptor doesn't care about the rename). There is no way we can know the new event as far as I can tell, inotifywait(1) also behaves like this. So instead of continuing in a semi-broken state just remove the watcher, like we do for deletes. On kqueue and FEN the situation is similar, and we actually already removed watchers on renames. On Windows this all works nicely; the watch is preserved and the filename is updated. I decided to keep this as-is for now, even though it's inconsistent. We actually fixed the Windows behaviour for the 1.6.0 release in #370 , so people do seem to care about it and use it, and experience with the symlink change in 1.5.0 shows it's better to keep inconsistent behaviour rather than change it. Fixing this up is something for a v2. Fixes #172 Fixes #503
arp242
added a commit
that referenced
this pull request
Oct 13, 2022
Renaming a watched path is problematic; on inotify we just get a IN_MOVED_SELF event with the old filename and that's it; no more events for you! So if you do: watch one mv one two cat asd >two You still continue to get events for the file "one", even though it's now named "two" (the file descriptor doesn't care about the rename). There is no way we can know the new event as far as I can tell, inotifywait(1) also behaves like this. So instead of continuing in a semi-broken state just remove the watcher, like we do for deletes. On kqueue and FEN the situation is similar, and we actually already removed watchers on renames. On Windows this all works nicely; the watch is preserved and the filename is updated. I decided to keep this as-is for now, even though it's inconsistent. We actually fixed the Windows behaviour for the 1.6.0 release in #370 , so people do seem to care about it and use it, and experience with the symlink change in 1.5.0 shows it's better to keep inconsistent behaviour rather than change it. Fixing this up is something for a v2. Fixes #172 Fixes #503
arp242
added a commit
that referenced
this pull request
Oct 14, 2022
Renaming a watched path is problematic; on inotify we just get a IN_MOVED_SELF event with the old filename and that's it; no more events for you! So if you do: watch one mv one two cat asd >two You still continue to get events for the file "one", even though it's now named "two" (the file descriptor doesn't care about the rename). There is no way we can know the new event as far as I can tell, inotifywait(1) also behaves like this. So instead of continuing in a semi-broken state just remove the watcher, like we do for deletes. On kqueue and FEN the situation is similar, and we actually already removed watchers on renames. On Windows this all works nicely; the watch is preserved and the filename is updated. I decided to keep this as-is for now, even though it's inconsistent. We actually fixed the Windows behaviour for the 1.6.0 release in #370 , so people do seem to care about it and use it, and experience with the symlink change in 1.5.0 shows it's better to keep inconsistent behaviour rather than change it. Fixing this up is something for a v2. Fixes #172 Fixes #503
25 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this pull request do?
Fixes #259
Fixes #243