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

Fix remove_watch+ base tree.event gen #61

Closed

Conversation

Elias481
Copy link

Includes pull-requests #57 and #58 from XaF as changes are very related.
Fixes the issue blocking #57 (superficial mode for IN_MOVED_FROM must be False as soon as remove_watch does not ignore this argument). And indirectly fixes #58 which is depending on #57 because both commits are now together...
Additionally removed duplicate IN_MOVED_TO in same function.

Further fixes the issue that remove_watch_with_fd which is a public method can now be called standalone without causing inconsitencies.

And enhances the remove_watch so that it could ne used to remove only the fd from inotify. (There are use-cases for this.) Third state for superficial (None instead of True or False) used for that.

These fixes should obsolete issue #46 and #51 and my #59 (beside the idea for automatic unregistration on IN_IGNORE event but that just a nice to have idea and has no priority in my opinion).

XaF and others added 6 commits July 29, 2018 01:22
The remove_watch method takes a superficial parameter but never used
it, which made the remove_watch_with_id function set it as False by default.
This fixes that behavior by propagating the parameter.

Signed-off-by: Raphaël Beamonte <[email protected]>
The previous behavior was duplicating a block of code and making
one of the two useless. This fixes that by replacing the constant to
IN_DELETE instead of IN_MOVED_FROM in one of the two blocks.

Signed-off-by: Raphaël Beamonte <[email protected]>
Propagate superficial parameter to remove_watch_with_id
Fix constant in adapter to match with DELETE events
…ith_fd] functions

_BaseTree.event_gen(): remove duplicate IN_MOVED_TO, change superficial to False for IN_MOVED_TO
Inotify.remove_watch_with_fd(): as this is a public function it should be possible to call this function without causing inconsitencies which are recoverable only with some dirty try/catch construct... there is also never a need to only remove the wfd from the adaptors list while keeping th path name in list.
 .. now functions remove_watch and remove_watch_with_fd are the same, only difference is parameter eighter pathname or watch-fd
addtionally bot functions now accept a thirst state (None) for superficial - used to skip the "superficial" actions and just remove the wd from inotify (there are use-cases for this, for example with threading to be able to disable wathing from one thread while watching thread get the notification about watch end from inotify an clean up the inotify "superficial").
@coveralls
Copy link

coveralls commented Sep 19, 2018

Coverage Status

Coverage decreased (-2.2%) to 82.886% when pulling 9650e30 on Elias481:fix-remove_watch+_BaseTree.event_gen into 288a2b0 on dsoprea:master.

… by slow)

luckily it pointed me to another bug
…the block from my script that currently need to subclass INotify for different reasons...)
@Elias481 Elias481 force-pushed the fix-remove_watch+_BaseTree.event_gen branch from 0c7f836 to 2e4c0d4 Compare September 22, 2018 17:08
…_FROM/-TO handling, as this branch shall fix all the bugs introduced by that change..)
…g' into fix-remove_watch+_BaseTree.event_gen
@Elias481
Copy link
Author

Added testcase for issue #46 .
Original result on current base can be found here: https://travis-ci.com/Elias481/PyInotify/builds/85639642

@Elias481
Copy link
Author

Added testcase for issue #51 .
Original result on current base can be found here: https://travis-ci.com/Elias481/PyInotify/builds/85643184

@Elias481 Elias481 closed this May 4, 2019
@Elias481 Elias481 deleted the fix-remove_watch+_BaseTree.event_gen branch May 4, 2019 01:16
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 this pull request may close these issues.

3 participants