keep watching files if they live outside a system path but are symlinked

[mmerickel]

fixes #60

[merwok]

Doesn’t fix #60 because the module files are not symlinked, only the package directory.

Could we turn the check around? Instead of file path startswith prefix from system paths, have file path startswith working directory prefix

[merwok]

Wait I’m not sure I read that right when I printed the paths from the function you changed. Let me look again.

[merwok]

Say my app is a package named $project living in $project_dir, base Python is in /usr and I use a virtualenv in $venv (using virtualenvwrapper, so always activated rather than using $venv/bin/stuff commands). flit install --symlink links $venv/lib/python3.7/site-packages/$project to $project_dir/$project.

Inspecting from watch_paths function:

self.system_paths:

[
 '$venv/lib/python3.7/site-packages',
 '$venv/lib/python3.7',
 '$venv/lib/python3.7',
 '$venv/lib/python3.7/site-packages',
 '$venv/lib/python3.7/site-packages',
]

Some of the paths before filtering:

[
 '$venv/lib/python3.7/__future__.py',  # and many other stdlib
 '$venv/lib/python3.7/lib-dynload/_json.cpython-37m-x86_64-linux-gnu.so',  # lib-dynload is symlink to /usr/lib/python3.7/lib-dynload
 '$venv/lib/python3.7/site-packages/hupper/__init__.py',  # and other hupper, but not other site-packages, and not my project
 '/usr/lib/python3.7/_compat_pickle.py',  # and many other system python stdlib
]

Adding prints to in_system_paths function (return true → in system, false → not system) shows this:

*** $venv/lib/python3.7/_bootlocale.py not system
*** /usr/lib/python3.7/contextlib.py not system

_bootlocale is symlinked from $venv/lib/python3.7 to /usr/lib, but the check from realpath prefix in system_paths fails because get_system_path uses sysconfig.get_path which never returns paths from real Python, only virtualenv paths. So the change you did for the flit use case ends up watching many sytem modules!

I think hupper will need virtualenv awareness, checking sys.real_prefix (virtualenv) or sys.base_prefix (venv) to add these paths to the exclude list. But this is to offer a complete system paths exclusion, not to fix my issue of not excluding package dir symlinked in site-packages. (Also de-duplicating list of system paths could help performance!)

I don’t understand why I see my project modules in the prints from in_system_paths but not when print paths:

*** $venv/lib/python3.7/site-packages/$project/views.py not system

Also this should mean that my project modules are watched (the path before realpath is printed), but I don’t see reload when I edit them.

I’m going to look at what happens with released hupper.

[merwok]

Ok I understand the first mystery: the hupper code is executed twice:

Running reloading file monitor
File monitor backend: watchdog
Starting monitor for PID 4321.
!!! /usr/lib/python3.7/importlib/_bootstrap.py not system

(last line being the print I added to in_system_paths; I changed it to print realpath)

then after that:

2019-11-12 13:23.42 app_ready (log line from my main function)
Starting server in PID 4321.
Serving on http://localhost:6432
Serving on http://localhost:6432
*** $venv/lib/python3.7/site-packages/pyramid/__init__.py in system
!!! /usr/lib/python3.7/argparse.py not system
!!! $project_dir/$project/auth.py not system

This last line is promising; printing kept and ignored paths from watch_paths shows my package in kept:

 '$venv/lib/python3.7/site-packages/$project/auth.py',

But touching or editing the file does not cause a reload.

Could it be that the event is ignored because hupper is watching the symlinked path?

[merwok]

Confirmed that without realpath, all modules from the virtualenv are ignored, but modules imported form /usr/lib are not, so I get hot reload when Debian updates Python but not when I change my code 😄

If this was only a Pyramid problem, I could try comparing prefix of realpath file to prefix of config file, but that’s not generic enough for hupper…

[merwok]

Ah… https://pypi.org/project/watchdog/#about-using-watchdog-with-editors-like-vim

Using this branch + removing watchdog makes reloading work for me!
(the reload message shows $venv/lib/python/site-packages/$project/auth.py changed rather than the local path, but technically it is correct and I think the UI is acceptable)

Adding awareness of real/base prefix to extend the list of ignored system paths (and de-duplicating them) should probably be another PR (which I could be interested in writing).

[mmerickel]

I think what I gather here is that hupper should also configure the file monitors to watch the realpath instead of symlinked path. Do you think that would fix issues with watchdog?

[mmerickel]

I'll try harder to repro this issue later, mainly wanted to see if this quick fix would do it cuz I'm lazy. I suspect we should be invoking realpath instead of abspath on all the paths. Or at least the file monitors should if they can't monitor/inotify on something if there is a symlink'd directory in their path.

[merwok]

I’m +1 to releasing with this quick fix. Without it, there are false positives (/usr/lib paths are watched) and a bug (my app not watched!), and with it only a few more false positives (modules symlinked in venv are not ignored) but a working reload for flit users.

I’m interested in fixing the false positives in another PR.

The problem with watchdog is not related to paths, but to vim implementing edit with edit new file, replace, and I’m not willing to change my options (I also have break-hardlinks and unlimited undo).

I don’t know if switching to watching real paths would only change cosmetics, or cause other issues. With the advent of src layouts, it is actually nice to see that Python and hupper think that your modules lives in $venv/lib/python3.7/site-packages, it confirms that your develop install is effective and there is no direct import from current directory.

I would gladly test/help if you want to pursue that, but in another PR!

[mmerickel]

Ok yeah I'd definitely like to improve what hupper is doing with its virtualenv detection. I never anticipated some of these other scenarios. I added #62 to deal with this a bit more.