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

Why no :modify event in Linux adapter? #450

Closed
ms-ati opened this issue Jul 11, 2018 · 11 comments
Closed

Why no :modify event in Linux adapter? #450

ms-ati opened this issue Jul 11, 2018 · 11 comments
Assignees
Milestone

Comments

@ms-ati
Copy link

ms-ati commented Jul 11, 2018

In debugging why I no longer get any resonse from Listen when touching files in a Ubuntu docker container, I've discovered that rb-inotify sees a :modify event, but this is NOT in the list of events that the Listen::Adapter::Linux listens to according to linux.rb#8 and linux.rb#87.

What is the reasoning behind excluding the :modify event? And is this affecting anyone else, or is there something unique to my setup where I am only actually seeing those events generated?

@ms-ati
Copy link
Author

ms-ati commented Jul 11, 2018

I've confirmed that, by monkey-patching Listen to add the :modify event to the Linux adapter, I am now, finally, picking up events when I touch a file again. Please see this gist for the details.

@ms-ati
Copy link
Author

ms-ati commented Jul 11, 2018

Workaround until this issue is resolved:

We've added the following snippet to our Guardfile:

# BEGIN MONKEY PATCH OF LISTEN GEM
Listen::Adapter::Linux::DEFAULTS.tap do |defaults|
  # Add :modify to the inotify events monitored by the Linux adapter
  new_defaults = defaults.merge(events: defaults[:events] + [:modify])
  Listen::Adapter::Linux.const_set(:DEFAULTS, new_defaults.freeze)
end
# END MONKEY PATCH OF LISTEN GEM

@ms-ati
Copy link
Author

ms-ati commented Jul 11, 2018

Ah, here's a simpler version of the workaround:

Listen::Adapter::Linux::DEFAULTS[:events] << :modify

@ColinDKelley
Copy link
Collaborator

(Catching up here.) I'm happy to make a PR that adds :modify to the constant, assuming that wouldn't ever hurt.
Can anyone think of any problems that might cause?

@ms-ati
Copy link
Author

ms-ati commented Nov 3, 2020

@ColinDKelley thanks! I hadn't expected any response here after so much time passing.

The Linux man page for inotify events: https://man7.org/linux/man-pages/man7/inotify.7.html

IN_MODIFY (+)
File was modified (e.g., write(2), truncate(2)).

To flip the question: is there any reason why Listen would ever want to ignore IN_MODIFY events on purpose?

@ColinDKelley
Copy link
Collaborator

@ms-ati I volunteered to be a listen maintainer a few months ago. I got the credentials last week, so now it's official. 😄

Thanks for the pointer to the docs! That does seem very clear.
I should have a PR today. Just have to spin up a Linux container so I can write/run a test.
I'd like to add this to v3.3.0.

@ColinDKelley ColinDKelley added this to the v3.3.0 milestone Nov 3, 2020
@ms-ati
Copy link
Author

ms-ati commented Nov 3, 2020

Awesome @ColinDKelley ! Major 👏 for taking this project maintenance on.

One tests case it occurs to me that might be worth investigating is whether there is a duplicate events issue? Is it possible that on Linux but outside of Docker, edits to files were generating multiple events, and that's why it was working to ignore modify? Maybe on non-Docker Linux, when you add Modify, you'll see duplicate events for the same file and need to filter them or accept duplicated response handling?

@ColinDKelley
Copy link
Collaborator

@ms-ati Can you review the PR: #502 ?

@ColinDKelley
Copy link
Collaborator

ColinDKelley commented Nov 3, 2020

Is it possible that on Linux but outside of Docker, edits to files were generating multiple events, and that's why it was working to ignore modify? Maybe on non-Docker Linux, when you add Modify, you'll see duplicate events for the same file and need to filter them or accept duplicated response handling?

I guess that's possible, but I don't think it's likely. listen has a de-duping layer that is designed to suppress any problems like that. I ran an integration test on Linux in Docker and it worked perfectly. I put those notes on the PR.

If anyone can run that same test on Linux outside of Docker, that would be great.

@ColinDKelley
Copy link
Collaborator

Closing this out as it's included in v3.3.0.pre.3.

@ColinDKelley
Copy link
Collaborator

BTW, I followed the git history back, and it turns out that modify was previously in the event list... and was removed here guard/guard#297 because it sometimes caused double events to be reported. It seems that the event list is sensitive to the particular file system driver in the OS.

I hope that the de-dup layer in listen would suppress any double notifications. I'm guessing that layer was missing back when modify was removed the first time?

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

No branches or pull requests

4 participants
@ioquatix @ColinDKelley @ms-ati and others