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

Replace listen gem with fswatch-rb #37269

Closed
tgxworld opened this issue Sep 22, 2019 · 8 comments
Closed

Replace listen gem with fswatch-rb #37269

tgxworld opened this issue Sep 22, 2019 · 8 comments

Comments

@tgxworld
Copy link
Contributor

tgxworld commented Sep 22, 2019

Background story #26158

I recently switched to a macbook pro as my primary development machine and noticed 40+ fsevent_watch processes running after starting the Rails server for one of my projects. This is primarily due to the listen gem spinning up a fsevent_watch process for each directory that it is asked to monitor.

While looking into the alternatives, I discovered https://github.com/emcrisostomo/fswatch/ which has cross platform support. Also, there is a Ruby bindings gem available https://github.com/t3hk0d3/fswatch-rb. With fswatch-rb, it is quite trivial for us to replace the use of the listen gem in ActiveSupport and at the same time resolve the problem of having too many processes run on macOS machines.

The only downside here is that fswatch becomes a development dependency for all platforms but I don't see that as too big of a problem.

@tgxworld
Copy link
Contributor Author

@fxn @matthewd @rafaelfranca I'll love to get your thoughts on this.

@tgxworld
Copy link
Contributor Author

FWIW, I'm ready to do the work but just wanted to see if the Rails team is open this change.

@matthewd
Copy link
Member

I'm torn.

The only downside here is that fswatch becomes a development dependency for all platforms but I don't see that as too big of a problem.

I do... the current list of dependencies in https://guides.rubyonrails.org/getting_started.html#installing-rails is: Ruby, SQLite3. Admittedly, that seems to be missing Webpack... nonetheless, adding an entry to that list seems like a pretty big deal to me.

OTOH, I would like to use a stronger library for this feature -- the current implementation has not been without problems. And given the platform weirdnesses inherent in the problem-space, it does seem safer to lean on a library that's not ruby-specific.

OTOOH, I have some questions around the licensing of these libraries, given that libfswatch is GPL.

Given how narrow our usage of Listen is, perhaps we could support both, using fswatch if it seems to be present in the bundle, and listen otherwise.

A different, somewhat dirtier, thought: as we do effectively require NodeJS to be present for development, and NodeJS has a built-in file watching API... 🤷🏻‍♂️

@tgxworld
Copy link
Contributor Author

tgxworld commented Sep 28, 2019

Admittedly, that seems to be missing Webpack... nonetheless, adding an entry to that list seems like a pretty big deal to me.

There are other dependencies like libxml2 for nokogiri and selenium for selenium-webdriver. One thing we could do is to bundle fswatch as a native extension which is similar to what rb-fsevent is doing at the moment.

OTOOH, I have some questions around the licensing of these libraries, given that libfswatch is GPL.

I think it should be OK since Rails will not actually be distributing it unless we pull it in as a native extension? fswatch-rb itself is MIT licensed. If the license is a blocker, I guess watchman by Facebook is the other alternative.

Given how narrow our usage of Listen is, perhaps we could support both, using fswatch if it seems to be present in the bundle, and listen otherwise.

I thought about making the notifier in EventedFileUpdateChecker configurable but I'm hoping to explore a solution which makes the experience on macOS better by default instead of having to opt in.

@ioquatix
Copy link
Contributor

ioquatix commented Dec 5, 2019

I would like to ensure listen is working well. I don't mind how you choose to resolve this issue, but if you have the capacity to help with the listen gem, please work with me.

@kamipo
Copy link
Member

kamipo commented Dec 6, 2019

Resolved by #37896.

@kamipo kamipo closed this as completed Dec 6, 2019
@tgxworld
Copy link
Contributor Author

tgxworld commented Dec 6, 2019

@ioquatix Sorry for creating the issue initially. I couldn't figure out a good solution for the Listen's darwin adapter so I focused on alternatives. It turns out, it was much easier to resolve the problem with the darwin adapter than to introduce a new dependency within Rails.

@ioquatix
Copy link
Contributor

ioquatix commented Dec 6, 2019

The listen gem definitely needs more maintainers, so feel free to help out! :)

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