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

pywatchman raises SystemError on Python 3.10 #970

Open
ian-em opened this issue Nov 2, 2021 · 26 comments
Open

pywatchman raises SystemError on Python 3.10 #970

ian-em opened this issue Nov 2, 2021 · 26 comments

Comments

@ian-em
Copy link

ian-em commented Nov 2, 2021

The latest version of pywatchman (1.4.1) is not working on Python 3.10.

A SystemError occurs which can be reproduced as follows:

import pywatchman
client = pywatchman.client()
client.capabilityCheck()

This code will trigger SystemError: PY_SSIZE_T_CLEAN macro must be defined for '#' formats.

@jwatt
Copy link

jwatt commented Dec 6, 2021

I'm not familiar enough with watchman to even know where I can find pywatchman 1.4.1, but wasn't this issue fixed in the code a year ago in 6813a94?

@ian-em
Copy link
Author

ian-em commented Dec 6, 2021

I'm not familiar enough with watchman to even know where I can find pywatchman 1.4.1, but wasn't this issue fixed in the code a year ago in 6813a94?

It looks like it was fixed in that commit, however for some reason there hasn't been a new release of pywatchman on pypi since September 1st 2017: https://pypi.org/project/pywatchman/#history

@jwatt
Copy link

jwatt commented Dec 6, 2021

@wez Would you still be the person to update pypi, or should we draw this to the attention of someone else nowadays? :-)

@wez
Copy link
Contributor

wez commented Dec 6, 2021

@fanzeyi is more involved these days. I'll sync up with them to ensure that they have the ability to update pypi

@pauloxnet
Copy link

Please @jwatt @wez @fanzeyi release a new version of PyWatchman (in a wheel format) on PyPi, the package is abandoned there since 2017 as already reported in issue #503

@fanzeyi
Copy link
Member

fanzeyi commented Dec 28, 2021

@pauloxnet Apologies for the delay and I will try to get a version out soon.

I have published a pre-release 1.4.2.dev1 of pywatchman to TestPyPi. You can find it at https://test.pypi.org/project/pywatchman/1.4.2.dev1/. I am worried dumping four years of change to PyPi directly might break something somewhere so I'd like to take it slow. I also would like to find sometime to fix our CI and have the package automatically published some time in the future.

That being said, I have built some wheels for Python 3.6 to 3.10 targeting manylinux2014 and macos-arm64 there in TestPyPi (I built them by hand, not CI, thus the lack of Windows and macos-amd64 :( ). Please use the following command to try it out. If everything works fine, I will make a pre-release to the mainline PyPi later this week for more testing.

pip install -i https://test.pypi.org/simple/ pywatchman==1.4.2.dev1

Thanks


Also, other people in this thread please give it a try as well. Thank you :)

@pauloxnet
Copy link

I'll have a look at it as soon as possible. in the meantime i shared your request to test the dev version on twitter. I hope it helps this phase.
https://twitter.com/pauloxnet/status/1475840504180924426?t=PNfe7Q3rGIN3S2ViChCAmA&s=19

@pauloxnet
Copy link

pauloxnet commented Dec 28, 2021

@fanzeyi I have successfully tested pywatchman 1.4.2.dev1 in these environments:

  • Ubuntu 21.10 and Debian 11
  • watchman 4.9.0 (system package installed via apt)
  • Python 3.9.7 and Python 3.10.0
  • Django 3.2.7 and Django 3.2.10

I hope others will test and report here. Thanks.

@niccolomineo
Copy link

v1.4.2 works fine on my end, please release it ASAP.

@adamchainz
Copy link

v1.4.2 also works for me on Python 3.10! I checked with Django and a standalone test script.

@pauloxnet
Copy link

@fanzeyi I think maybe we can release a 1.5.0 version instead of 1.4.2 due all last year changes.

@minusf
Copy link

minusf commented Jan 12, 2022

@fanzeyi I think maybe we can release a 1.5.0 version instead of 1.4.2 due all last year changes.

or even 2.0, the new watchman is nothing like 4.9.0

@adamchainz
Copy link

Hey, sorry to be that developer, but can we have a release? pywatchman is broken on Python 3.10 until this is out.

@pauloxnet
Copy link

We seem to be at a standstill again, despite the positive feedback.
@jwatt @wez @fanzeyi , could any of you release a new pywatchman version on PyPi ?

@jwatt
Copy link

jwatt commented Jan 28, 2022

I'm a user, not a maintainer. @fanzeyi is the only person who might make a release, it seems, but presumably they're currently busy. I'm sure they'll get to it when they can, but shipping a new release (to a certain extent) puts you on the hook for breakage. Right now they may not have the time to risk that.

@shosca
Copy link

shosca commented Feb 27, 2022

a release would be nice, additionally i tried to install it directly from git with:

❯ pip install "git+https://github.com/facebook/watchman.git#subdirectory=watchman/python"

but it seems setup.py is broken:

Collecting git+https://github.com/facebook/watchman.git#subdirectory=watchman/python
  Cloning https://github.com/facebook/watchman.git to /tmp/pip-req-build-67axjmh9
  Running command git clone --filter=blob:none -q https://github.com/facebook/watchman.git /tmp/pip-req-build-67axjmh9
  Resolved https://github.com/facebook/watchman.git to commit dfdccd79a0a232f8b6d7fc153a08864cd2972369
  Preparing metadata (setup.py) ... error
  ERROR: Command errored out with exit status 1:
   command: /home/serkan/.local/share/virtualenvs/project-py3.10/bin/python -c 'import io, os, sys, setuptools, tokenize; sys.argv[0] = '"'"'/tmp/pip-req-build-67axjmh9/watchman/python/setup.py'"'"'; __file__='"'"'/tmp/pip-req-build-67axjmh9/watchman/python/setup.py'"'"';f = getattr(tokenize, '"'"'open'"'"', open)(__file__) if os.path.exists(__file__) else io.StringIO('"'"'from setuptools import setup; setup()'"'"');code = f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, __file__, '"'"'exec'"'"'))' egg_info --egg-base /tmp/pip-pip-egg-info-ee2qurk4
       cwd: /tmp/pip-req-build-67axjmh9/watchman/python
  Complete output (7 lines):
  running egg_info
  creating /tmp/pip-pip-egg-info-ee2qurk4/pywatchman.egg-info
  writing /tmp/pip-pip-egg-info-ee2qurk4/pywatchman.egg-info/PKG-INFO
  writing dependency_links to /tmp/pip-pip-egg-info-ee2qurk4/pywatchman.egg-info/dependency_links.txt
  writing top-level names to /tmp/pip-pip-egg-info-ee2qurk4/pywatchman.egg-info/top_level.txt
  writing manifest file '/tmp/pip-pip-egg-info-ee2qurk4/pywatchman.egg-info/SOURCES.txt'
  error: package directory '/tmp/pip-req-build-67axjmh9/watchman/python/../watchman/python/pywatchman' does not exist
  ----------------------------------------
WARNING: Discarding git+https://github.com/facebook/watchman.git#subdirectory=watchman/python. Command errored out with exit status1: python setup.py egg_info Check the logs for full command output.
ERROR: Command errored out with exit status 1: python setup.py egg_info Check the logs for full command output.
WARNING: You are using pip version 21.3.1; however, version 22.0.3 is available.
You should consider upgrading via the '/home/serkan/.local/share/virtualenvs/project-py3.10/bin/python -m pip install --upgrade pip' command.

it seems like setup.py has a problem trying to figure out the watchman_src_dir around line no 17, replacing that section with:

watchman_src_dir = os.environ.get("CMAKE_CURRENT_SOURCE_DIR")
if watchman_src_dir is None:
    watchman_src_dir = os.path.abspath(
        os.path.join(os.path.dirname(os.path.realpath(__file__)), "..", "..")
    )

should allow using pip to directly install from git, after the change i can install it:

❯ python setup.py egg_info
running egg_info
creating /tmp/pip-req-build-67axjmh9/watchman/python/pywatchman.egg-info
writing /tmp/pip-req-build-67axjmh9/watchman/python/pywatchman.egg-info/PKG-INFO
writing dependency_links to /tmp/pip-req-build-67axjmh9/watchman/python/pywatchman.egg-info/dependency_links.txt
writing top-level names to /tmp/pip-req-build-67axjmh9/watchman/python/pywatchman.egg-info/top_level.txt
writing manifest file '/tmp/pip-req-build-67axjmh9/watchman/python/pywatchman.egg-info/SOURCES.txt'
reading manifest file '/tmp/pip-req-build-67axjmh9/watchman/python/pywatchman.egg-info/SOURCES.txt'
adding license file 'LICENSE'
writing manifest file '/tmp/pip-req-build-67axjmh9/watchman/python/pywatchman.egg-info/SOURCES.txt'

❯ pip install .
Processing /tmp/pip-req-build-67axjmh9/watchman/python
  Preparing metadata (setup.py) ... done
Building wheels for collected packages: pywatchman
  Building wheel for pywatchman (setup.py) ... done
  Created wheel for pywatchman: filename=pywatchman-1.4.1-cp310-cp310-linux_x86_64.whl size=67222 sha256=fab05327deff80a36a84cc4dfe8553abef8035430b7b664a82cb21a62dfcf51a
  Stored in directory: /tmp/pip-ephem-wheel-cache-7lrvh1pg/wheels/70/8f/9a/31f1d2ec182be71f26f7a9093e2cd4c5c178ef5724bd6c0e89
Successfully built pywatchman
Installing collected packages: pywatchman
  Attempting uninstall: pywatchman
    Found existing installation: pywatchman 1.4.1
    Not uninstalling pywatchman at /tmp/pip-req-build-67axjmh9/watchman/python, outside environment /home/serkan/.local/share/virtualenvs/project-py3.10
    Can't uninstall 'pywatchman'. No files were found to uninstall.
Successfully installed pywatchman-1.4.1
WARNING: You are using pip version 21.3.1; however, version 22.0.3 is available.
You should consider upgrading via the '/home/serkan/.local/share/virtualenvs/project-py3.10/bin/python -m pip install --upgrade pip' command.

@pauloxnet
Copy link

I'm a user, not a maintainer. @fanzeyi is the only person who might make a release, it seems, but presumably they're currently busy. I'm sure they'll get to it when they can, but shipping a new release (to a certain extent) puts you on the hook for breakage. Right now they may not have the time to risk that.

@jwatt I add my opinion here but it is not a direct answer to you, indeed I thank you for the hint.

The last python release of watchman was in 2017, I don't think a company like Facebook hasn't had the time or other people to make a release in these, rather I think we have no interest in maintaining this python package.

The people who are commenting in this thread are writing and releasing free software everyday for the community and no one is making unreasonable requests.

The biggest problem, however, I do not think is a breakage, but the abandonment of the package by the users, after that of the developers themselves.

Personally I have long since removed the support of watchman in all my projects waiting for this release which by now I know it will not come or it will come too late.

The only regret is that Django supports out of the box watchman, I hope there will soon be Django support for a valid and maintained alternative such as watchfiles.

@ulgens
Copy link

ulgens commented May 11, 2022

I don't want to be that guy but, any updates on the release? @fanzeyi 👋🏻

@ThiefMaster
Copy link

@fanzeyi Could we please have a release? TestPyPI and similar sites are simply not an option if you have code that depends on this - you can't just pull in stuff from TestPyPI in a requirements.txt/setup.cfg for example.

@heyman
Copy link

heyman commented Jun 17, 2022

For anyone who comes here looking to use pywatchman with the Django dev server to reduce idle CPU usage, I can instead recommend using django-extensions's runserver_plus command with Werkzeug[watchdog] installed (pinned at version 2.0.3 until a new version of django-extensions is released).

@ThiefMaster
Copy link

AFAIK werkzeug[watchdog] isn't as nice though since it doesn't detect e.g. git operations and perform just a single reload instead of potentially many reloads for each file changed.

BTW, I pinged the developer on twitter and got some feedback on what's blocking an updated release: https://twitter.com/fanzeyi/status/1537369006713217025

@samuelcolvin
Copy link

samuelcolvin commented Jul 21, 2022

At the risk of sounding like "that person" who suggests an alternative library that they maintain:

Those frustrated by watchman and watchdog might consider watchfiles.

A few advantages of watchfiles:

  • It's underlying file watching uses the heavily tested notify crate, when inotify / FSEvents / ReadDirectoryChangesW / kqueue are not available notify falls back to polling
  • changes are denounced so changing branch results in one reload
  • watchfiles is actively maintained and supports python >3.7, tests are already run with python 3.11
  • it's the default reloading library for uvicorn and there's django-watchfiles for django users

@vnagendra
Copy link

vnagendra commented Sep 23, 2022

For anyone else that is waiting on the 1.4.2 release specifically for Django, you can "somewhat easily fix this" locally. I can't imagine a case of doing this on a CI Server or something similar.

Locally you just edit the file .../site-packages/pywatchman/__init__.py and change line 1071.

Specifically the following

    def capabilityCheck(self, optional=None, required=None):
        """ Perform a server capability check """
        res = self.query('list-capabilities', {.  # <-- This says 'version' in the original 
            'optional': optional or [],
            'required': required or []
        })

Basically looks like watchman (the service) used to send capabilities when queried for version and it no longer does. Instead of you query for capabilities with list-capabilities the version is also sent. Subsequently there is some funny dictionary access that causes issues. This is why with Django 3.x and Python 3.9 and above WatchmanReloader doesn't launch automatically.

I spent a bit of time following the rabbit down the hole to chase this issue.. Hopefully you won't have to... FWIW, this is a fairly trivial and harmless change IMO.

Small update
I had to start a new service with Python 3.10 and this method almost works. It required further changes, which I was "curious" about and followed the rabbit. Sometimes on Mac it seems like the function _resolvesockname doesn't quite work right. The cleanest thing you can do is just export the things it is looking for. Please remember these need to be exported into the process that is running your runserver (meaning if you are running it in IntelliJ or similar -- you need to make sure these are available in that context)..

ps aux | grep watchman

This should give you the sock file something like this...

vnagendra         1632   0.0  0.2 37021908 136420   ??  S<   11Oct22   6:16.67 /usr/local/bin/watchman --foreground --logfile=/usr/local/var/run/watchman/vnagendra-state/log --log-level=1 --sockname=/usr/local/var/run/watchman/vnagendra-state/sock --statefile=/usr/local/var/run/watchman/vnagendra-state/state --pidfile=/usr/local/var/run/watchman/vnagendra-state/pid

Using that export the following

WATCHMAN_SOCK=/usr/local/var/run/watchman/vnagendra-state/sock
WATCHMAN_ENCODING=json

Once you resolve the socket problem, you'll run into bser (their binary serialization method) having "issues" with python 3.10 -- I didn't quite troubleshoot that problem as much as just work around it.

knuton added a commit to knuton/playos that referenced this issue Sep 13, 2023
python37Packages is no longer defined in today's Nixpkgs, pywatchman
does not work with Python 3.10[^1], and we need to be more explicit
about which inputs we need as packages in the shell.

[^1]: facebook/watchman#970
guyonvarch pushed a commit to guyonvarch/playos that referenced this issue Sep 18, 2023
`python37Packages` is no longer defined in today's Nixpkgs, pywatchman
does not work with Python 3.10[^1],

Use `watchexec`, that we already use in other places, which is simpler
to use.

Additionally, we need to be more explicit about which inputs we need as
packages in the shell.

[^1]: facebook/watchman#970
@amirkdv-stripe
Copy link

FYI pywatchman 2.0.0 has been released and the original issue no longer reproduces on Python 3.11

$ python3.11 -m venv venv
$ . venv/bin/activate
$ pip install pywatchman==2.0.0
$ python3 -c "import pywatchman; print(pywatchman.client().capabilityCheck())"
# {'version': '2024.01.22.00', 'capabilities': {}}

@samuelcolvin
Copy link

3.12 had been out for ages, 3.13 is in alpha...

@adamchainz
Copy link

The same test passes on 3.12 and 3.13.0a4 as well.

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