Bundle `pynvim` into repo to avoid user having to install it with talon pip?

[C-Loftus]

The user needs to pip install into the talon python interpreter. It might make sense to vendor the dependencies and bundle them somehow like the way gazeocr does it.

[saidelike]

Interesting idea about packaging pynvim directly in neovim-talon.

• From https://github.com/wolfmanstout/talon-gaze-ocr/tree/main?tab=readme-ov-file#installation

Required Python packages: All Python package dependencies are present in the .subtrees directory so that no pip installations are needed. If packages are have been installed manually through pip, these will be preferred (e.g. so that faster binary packages can be used.)

• https://github.com/wolfmanstout/talon-gaze-ocr/blob/main/gaze_ocr_talon.py#L21 is where it is implemented

I think we have 2 options:

1. either we have them require to run talon/pip install pynvim
2. include it in the repo similarly to what gaze-ocr does and if people have installed it with Talon pip, the latter will be used as a priority (similar to gaze-ocr)

NOTE: we do need people to run npm install -g neovim so install a node/npm neovim client package for Cursorless. In that sense, the requirement for installing pynvm is similar in that it is a python neovim client package for neovim-talon.

Any preference @fidgetingbits?

[pokey]

How big is pynvim and how often does it change? And how good is neovim about backwards compatibility with old clients?

[fidgetingbits]

Originally I think we planned to just pip install it on the fly if we couldn't import it, but that would likely be considered bad manners?

One thing to note is that pynvim has quite a few dependencies iirc. One dependency, greenlet, I've never gotten to work on MacBook M1 due to aarch64 build failures, which the greenlet author doesn't seem to care much about iirc.

Not sure if we'd have to vendor all it's dependencies but if so it might be a bit annoying, but I'm open to the idea anyway. Worth exploring further.

[fidgetingbits]

https://github.com/neovim/pynvim/blob/master/setup.py

actually not many dependencies I guess

[C-Loftus]

Main downside to bundling dependencies I think would be the fact the user directory then ends up getting a ton of extra Python files and thus makes it harder to search through. Obviously you can ignore subdirectories when searching, but it does add some friction in general. Otherwise probably doesn't make a big difference. I feel like pip installing is probably better honestly, since if the dependency does something bad to the Python environment I don't think it really matters if it is pip installed or just uses the manually copied files.

[saidelike]

TL;DR

It is a bit of a pain to package pynvim into neovim-talon but doable. I was able to use local import pynvim in Talon, similar to how talon-gaze-ocr does it

The most challenging dependency seems to be greenlet dependency. @fidgetingbits mentioned not being able to use pynvim on OS X at all due to greenlet not being supported. On Windows, I had to compile greenlet for Python 3.11 specifically to be able to use it. So we might have to include greenlet binaries for every OS.

One thing about pynvim not being supported on Mac makes me wonder if we should try to fix it (or otherwise none of the OS X users will be able to use neovim-talon (!?). Another idea that @pokey mentioned would be to require node and use the node client (neovim npm package) instead of the python client (pynvim python package) for all the RPC communication. Would mean re-rewriting a lot of the neovim-talon code though.

Requirements

Make sure none of pynvim, msgpack or greenlet are installed in Talon Python using:

C:\Users\User\AppData\Roaming\talon\venv\3.11\Scripts\python.exe -s -m pip list

And to uninstall:

C:\Users\User\AppData\Roaming\talon\venv\3.11\Scripts\python.exe -s -m pip uninstall msgpack greenlet pynvim

C:\neovim-talon\.subtrees>git clone https://github.com/neovim/pynvim
C:\neovim-talon\.subtrees>git clone https://github.com/msgpack/msgpack-python
C:\neovim-talon\.subtrees>git clone https://github.com/python-greenlet/greenlet

I have used that version but we might be able to use the latest greenlet version.

C:\neovim-talon\.subtrees\greenlet>git checkout 2.0.2
C:\neovim-talon\.subtrees\greenlet>C:\Users\User\AppData\Roaming\talon\venv\3.11\Scripts\python.exe setup.py build
...
LINK : fatal error LNK1104: cannot open file 'python311.lib'
error: command 'C:\\Program Files (x86)\\Microsoft Visual Studio\\2019\\Professional\\VC\\Tools\\MSVC\\14.29.30133\\bin\\HostX86\\x64\\link.exe' failed with exit code 1104

Workaround was to download Python 3.11 Windows 64-bit from https://www.python.org/downloads/release/python-3117/, install it and get python311.lib and copy it into C:\neovim-talon\.subtrees\greenlet then it builds properly

C:\neovim-talon\.subtrees\greenlet>C:\Users\User\AppData\Roaming\talon\venv\3.11\Scripts\python.exe setup.py build
...
Generating code
Finished generating code

Import from Talon

diff --git a/core/rpc/rpc.py b/core/rpc/rpc.py
index b342b58..7a7afd8 100644
--- a/core/rpc/rpc.py
+++ b/core/rpc/rpc.py
@@ -2,10 +2,33 @@ from talon import Context, Module, actions, app, settings, ui

 from .direct_input import VimDirectInput

-import logging
+import logging, sys
+from pathlib import Path

 # TODO: make sure pynvim is installed in talon python environment
-import pynvim
+
+# Adjust path to search adjacent package directories. Prefixed with dot to avoid
+# Talon running them itself. Append to search path so that faster binary
+# packages can be used instead if available.
+subtree_dir = Path(__file__).parent / ".subtrees"
+package_paths = [
+    str(subtree_dir / "msgpack-python"),
+    # str(subtree_dir / "greenlet/src"),
+    str(subtree_dir / "greenlet/build/lib.win-amd64-cpython-311"),
+    str(subtree_dir / "pynvim"),
+]
+saved_path = sys.path.copy()
+try:
+    sys.path.extend([path for path in package_paths if path not in sys.path])
+    # sys.path = package_paths + sys.path
+    print(sys.path)
+    import pynvim
+
+finally:
+    # Restore the unmodified path.
+    sys.path = saved_path.copy()
+
+print(pynvim.__version__)

 class VimRPC:

And then we can import it and use neovim-talon commands like tab new, tab next new term, go term:

2024-04-28 06:35:57.972 DEBUG [~] c:\users\user\appdata\roaming\talon\user\neovim-talon\core\rpc\rpc.py
2024-04-28 06:35:57.974    IO ['C:\\program files\\talon\\python311.zip', 'C:\\program files\\talon\\DLLs', 'C:\\program files\\talon\\Lib', 'C:\\program files\\talon', 'C:\\program files\\talon\\Lib\\site-packages', 'C:\\program files\\talon\\Lib\\site-packages\\win32', 'C:\\program files\\talon\\Lib\\site-packages\\win32\\lib', 'C:\\Users\\User\\AppData\\Roaming\\talon\\venv\\3.11', 'C:\\Users\\User\\AppData\\Roaming\\talon\\venv\\3.11\\Lib\\site-packages', 'C:\\Users\\User\\AppData\\Roaming\\talon\\venv\\3.11\\Lib\\site-packages', 'C:\\program files\\talon\\Lib\\site-packages', 'C:\\Users\\User\\AppData\\Roaming\\talon\\user\\neovim-talon\\core\\rpc\\..\\..\\.subtrees\\msgpack-python', 'C:\\Users\\User\\AppData\\Roaming\\talon\\user\\neovim-talon\\core\\rpc\\..\\..\\.subtrees\\greenlet\\build\\lib.win-amd64-cpython-311', 'C:\\Users\\User\\AppData\\Roaming\\talon\\user\\neovim-talon\\core\\rpc\\..\\..\\.subtrees\\pynvim']
2024-04-28 06:35:58.086    IO 0.5.1.dev0

[fidgetingbits]

To be clear it's not that greenlet doesn't support macOS, it's that at some point aarch64 builds started failing and the author doesn't seem interested in investigating/fixing. Afaict working greenlet macOS x86_64 builds do still exist.

If going the vendoring route, we'd have to package both macOS arch versions; and ideally always package greenlet versions for all OS/arch combinations talon supports.

It's been months since I looked at greenlet on macOS so forget what the exact issue was.. will try again when I have time.

[pokey]

Huh that all sounds a bit brutal / fragile. I wonder if if would make sense to switch to command client. Talon-side, the only changes would be in rpc.py, no? And neovim side, it's fairly straightforward, more or less just read some json when you get a keypress. We could do it in lua to avoid needing node

[pokey]

I'm trying to get cursorless-neovim working, and ran into this greenlet issue. I worked around it by wrapping the import in a try-catch (https://github.com/pokey/neovim-talon/commit/953487bcc2de99381470afa53a232179fc748b04), because I believe it's not actually necessary for cursorless-neovim, which uses the command server, but I think we should prob figure something out before considering cursorless-neovim ready for users to try out? cc/ @saidelike

[pokey]

Dropping a couple links here for future reference:

• https://github.com/neovim/pynvim/issues/473
• https://github.com/python-greenlet/greenlet/issues/234

[pokey]

Can we get a workaround for this one? This is a blocker on Mac, right? Even just something like https://github.com/pokey/neovim-talon/commit/953487bcc2de99381470afa53a232179fc748b04

[fidgetingbits]

I briefly looked into this a little bit more, as far as just getting it to work on macos at all, and it seems like there are now working greenlet wheels for arm64, but we still run into another problem which I think is probably due to talon application having some default settings which are enforcing library codesigning.

If I try to load the new greenlight library I get the following error:

❯ ./repl
Talon REPL | Python 3.11.8 (main, Apr 21 2024, 20:55:39) [Clang 15.0.0 (clang-1500.1.0.2.5
)] on darwin)
>>> import pynvim
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "app/resources/loader.py", line 356, in import_hook
  File "/Users/aa/.talon/.venv/lib/python3.11/site-packages/pynvim/__init__.py", line 13, 
in <module>
    from pynvim.msgpack_rpc import (ErrorResponse, Session, TTransportType,
  File "app/resources/loader.py", line 356, in import_hook
  File "/Users/aa/.talon/.venv/lib/python3.11/site-packages/pynvim/msgpack_rpc/__init__.py
", line 12, in <module>
    from pynvim.msgpack_rpc.session import ErrorResponse, Session
  File "app/resources/loader.py", line 356, in import_hook
  File "/Users/aa/.talon/.venv/lib/python3.11/site-packages/pynvim/msgpack_rpc/session.py"
, line 10, in <module>
    import greenlet
  File "app/resources/loader.py", line 356, in import_hook
  File "/Users/aa/.talon/.venv/lib/python3.11/site-packages/greenlet/__init__.py", line 29
, in <module>
    from ._greenlet import _C_API # pylint:disable=no-name-in-module
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "app/resources/loader.py", line 356, in import_hook
ImportError: dlopen(/Users/aa/.talon/.venv/lib/python3.11/site-packages/greenlet/_greenlet
.cpython-311-darwin.so, 0x0002): tried: '/Users/aa/.talon/.venv/lib/python3.11/site-packag
es/greenlet/_greenlet.cpython-311-darwin.so' (code signature in <15BDBF13-5042-3A35-8830-0
1D988DFCA4A> '/Users/aa/.talon/.venv/lib/python3.11/site-packages/greenlet/_greenlet.cpyth
on-311-darwin.so' not valid for use in process: mapping process and mapped file (non-platf
orm) have different Team IDs),
[SNIPPED]

However if I load the same module using talon's provided python, it imports fine:

❯ ./python
Python 3.11.8 (main, Apr 21 2024, 20:55:39) [Clang 15.0.0 (clang-1500.1.0.2.5)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import pynvim

So if we look at the actual error in detail:

not valid for use in process: mapping process and mapped file (non-platform) have different Team IDs

In this case the mapping process would be talon, and its I guess in forcing code signing for libraries, but because the greenlet library is also signed but with a different signature (just guessing), this strict checking is preventing it. It seems like this may be related to the [Disable Library Validation Entitlement(]https://developer.apple.com/documentation/bundleresources/entitlements/com_apple_security_cs_disable-library-validation).

If we look at the entitlements talon uses:

❯ codesign -d --entitlements :- /Applications/Talon.app/ | tidy -xml
Executable=/Applications/Talon.app/Contents/MacOS/Talon
warning: Specifying ':' in the path is deprecated and will not work in a future release
No warnings or errors were found.

<?xml version="1.0" encoding="utf-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN"
  "https://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
<key>com.apple.security.automation.apple-events</key>
<true />
<key>com.apple.security.cs.allow-jit</key>
<true />
<key>com.apple.security.cs.allow-unsigned-executable-memory</key>
<true />
<key>com.apple.security.device.audio-input</key>
<true />
</dict>
</plist>

It doesn't explicitly disable that, although the fact that it's using the com.apple.security.cs.allow-unsigned-executable-memory may suggest that aegis will be open to it assuming that's the actual issue.

[fidgetingbits]

Quick chat with aegis and note 2 import things: 1) he doesn't like the idea of vendoring the package 2) pip installing anything with binaries won't be supported on mac. so gonna close this and will just fule an issue to replace pynvim instead.