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

Hupper doesn't work with watchman installed #70

Closed
tonthon opened this issue May 11, 2021 · 6 comments · Fixed by #71
Closed

Hupper doesn't work with watchman installed #70

tonthon opened this issue May 11, 2021 · 6 comments · Fixed by #71
Labels

Comments

@tonthon
Copy link

tonthon commented May 11, 2021

If watchman is installed on the system, hupper fails to monitor files.

watchman's binary version : 20210409.063814.0
hupper version : hupper==1.10.2

Steps to reproduce (using a Pyramid application)

mkvirtualenv test_hupper_reload
pip install pyramid cookiecutter 
cookiecutter gh:Pylons/pyramid-cookiecutter-starter
# Add an app test_pyramid_app
cd test_pyramid_app
python setup.py develop
pserve development.ini --reload

Generates the following errors (returned by the watchman service)

[ERROR] watchman error=RootResolveError: RootResolveError: unable to resolve root /home/gas/test/test_pyramid_app: RootResolveError: directory /home/gas/test/test_pyramid_app is not watched
Starting server in PID 12261.
2021-05-11 11:33:29,793 INFO  [waitress:485][MainThread] Serving on http://[::1]:6543
2021-05-11 11:33:29,793 INFO  [waitress:485][MainThread] Serving on http://127.0.0.1:6543
[ERROR] watchman error=RootResolveError: RootResolveError: unable to resolve root /home/gas/test/test_pyramid_app/test_pyramid_app: RootResolveError: directory /home/gas/test/test_pyramid_app/test_pyramid_app is not watched
[ERROR] watchman error=RootResolveError: RootResolveError: unable to resolve root /home/gas/test/test_pyramid_app/test_pyramid_app/models: RootResolveError: directory /home/gas/test/test_pyramid_app/test_pyramid_app/models is not watched
[ERROR] watchman error=RootResolveError: RootResolveError: unable to resolve root /home/gas/test/test_pyramid_app/test_pyramid_app/scripts: RootResolveError: directory /home/gas/test/test_pyramid_app/test_pyramid_app/scripts is not watched

Modifications to any file (.py or .ini file) won't fire any reload action.

@mmerickel mmerickel added the bug label May 11, 2021
@mmerickel
Copy link
Member

Thanks for opening this issue. I use watchman all the time with hupper and after some quick sleuthing I discovered that homebrew is shipping a crazy old version (4.9.0 released 2017). It looks like I'll need to test against the newer versions that you are using.

It looks like realistically we probably need to support both versions because homebrew has failed to come up with a solution to ship the newer versions. See Homebrew/homebrew-core#60693 for some info on that.

It's probably worth noting in the short term that you can override the reloader to get things working again for you. https://docs.pylonsproject.org/projects/hupper/en/latest/#overriding-the-default-file-monitor

@tonthon
Copy link
Author

tonthon commented May 11, 2021

@mmerickel Let me know if you need me to test, I'm a linux user so no homeebrew here ;-)

To override the file monitor and get back to the default one, I added

export HUPPER_DEFAULT_MONITOR=hupper.polling.PollingFileMonitor 

to the postactivate script of my virtualenv (.virtualenvs/pyramid/bin/postactivate).

Thx for your quick feedback.

@mmerickel
Copy link
Member

I'm able to repro this issue on macos using the facebook tap to get version 20200816.222424.0:

$ brew tap facebook/fb
$ brew install facebook/fb/watchman

@mmerickel
Copy link
Member

The fix is trickier than I was hoping for, I'll need to spend some actual time to get into it. I'd love it if someone else could though.

From what I can tell, watchman's newer versions now require defining a root path that is watched via watch or watch-project, and subscriptions must be created relative to the root path. It seems like it's no longer valid to simply subscribe to paths without defining a watch first.

Required changes seem to be:

  • maintain a list of watched directories
  • when a new path is registered, if it is inside of a root, then just append it to the list of paths
  • when a new path is registered and it is outside of the existing roots then
    • register a new watch for that directory using the watch command
    • register a new subscription for that directory
    • append the path to the list of paths

For bonus points watch is deprecated and they really want you to use watch-project but this requires a more complicated flow where you invoke watch-project, wait for the response and pull out the root folder from the response. It may choose to watch a parent folder as the root instead of the one you requested. After you get that root, then you can register a subscription for it.

@mmerickel mmerickel mentioned this issue May 13, 2021
@mmerickel
Copy link
Member

@tonthon It'd be handy if you would get a chance to clone and test #71. I think it'll fix the issues.

@tonthon
Copy link
Author

tonthon commented May 17, 2021

I confirm, it works like a charm. Thanks for your reactivity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants