From 5ac50b2d7efe1e1cef127977d8863fd544019c5c Mon Sep 17 00:00:00 2001 From: Jongwook Choi Date: Thu, 21 Sep 2023 19:46:51 -0400 Subject: [PATCH] fix: fix broken dynamic import of rplugin modules The removal of `imp` package (#461) in order to supprot Python 3.12 had a bug where rplugins can't be loaded and the ImportError exception was silenced, making the remote provider throwing lots of errors. This commit fixes broken dynamic import of python modules from the registered rplugins. We add tests for Host._load, with loading rplugins consisting of: (1) single-file module (e.g., `rplugins/simple_plugin.py`) (2) package (e.g., `rplugins/mymodule/__init__.py`) --- pynvim/plugin/host.py | 46 ++++++++++--------- .../rplugin/python3/mymodule/__init__.py | 9 ++++ .../rplugin/python3/mymodule/plugin.py | 13 ++++++ .../rplugin/python3/simple_nvim.py | 13 ++++++ test/test_host.py | 25 +++++++++- 5 files changed, 83 insertions(+), 23 deletions(-) create mode 100644 test/fixtures/module_plugin/rplugin/python3/mymodule/__init__.py create mode 100644 test/fixtures/module_plugin/rplugin/python3/mymodule/plugin.py create mode 100644 test/fixtures/simple_plugin/rplugin/python3/simple_nvim.py diff --git a/pynvim/plugin/host.py b/pynvim/plugin/host.py index ea47ee20..ea4c1df6 100644 --- a/pynvim/plugin/host.py +++ b/pynvim/plugin/host.py @@ -1,10 +1,13 @@ # type: ignore """Implements a Nvim host for python plugins.""" + +import importlib import inspect import logging import os import os.path import re +import sys from functools import partial from traceback import format_exc from typing import Any, Sequence @@ -23,25 +26,18 @@ host_method_spec = {"poll": {}, "specs": {"nargs": 1}, "shutdown": {}} -def handle_import(directory, name): - """Import a python file given a known location. +def _handle_import(path: str, name: str): + """Import python module `name` from a known file path or module directory. - Currently works on both python2 or 3. + The path should be the base directory from which the module can be imported. + To support python 3.12, the use of `imp` should be avoided. + @see https://docs.python.org/3.12/whatsnew/3.12.html#imp """ - try: # Python3 - from importlib.util import module_from_spec, spec_from_file_location - except ImportError: # Python2.7 - import imp - from pynvim.compat import find_module - file, pathname, descr = find_module(name, [directory]) - module = imp.load_module(name, file, pathname, descr) - return module - else: - spec = spec_from_file_location(name, location=directory) - if spec is not None: - return module_from_spec(spec) - else: - raise ImportError + if not name: + raise ValueError("Missing module name.") + + sys.path.append(path) + return importlib.import_module(name) class Host(object): @@ -167,11 +163,20 @@ def _missing_handler_error(self, name, kind): return msg def _load(self, plugins: Sequence[str]) -> None: + """Load the remote plugins and register handlers defined in the plugins. + + Args: + plugins: List of plugin paths to rplugin python modules + registered by remote#host#RegisterPlugin('python3', ...) + (see the generated rplugin.vim manifest) + """ + # self.nvim.err_write("host init _load\n", async_=True) has_script = False for path in plugins: + path = os.path.normpath(path) # normalize path err = None if path in self._loaded: - error('{} is already loaded'.format(path)) + warn('{} is already loaded'.format(path)) continue try: if path == "script_host.py": @@ -179,10 +184,7 @@ def _load(self, plugins: Sequence[str]) -> None: has_script = True else: directory, name = os.path.split(os.path.splitext(path)[0]) - try: - module = handle_import(directory, name) - except ImportError: - return + module = _handle_import(directory, name) handlers = [] self._discover_classes(module, handlers, path) self._discover_functions(module, handlers, path, False) diff --git a/test/fixtures/module_plugin/rplugin/python3/mymodule/__init__.py b/test/fixtures/module_plugin/rplugin/python3/mymodule/__init__.py new file mode 100644 index 00000000..703517cb --- /dev/null +++ b/test/fixtures/module_plugin/rplugin/python3/mymodule/__init__.py @@ -0,0 +1,9 @@ +"""The `mymodule` package for the fixture module plugin.""" +# pylint: disable=all + +# Somehow the plugin might be using relative imports. +from .plugin import MyPlugin as MyPlugin + +# ... or absolute import (assuming this is the root package) +import mymodule.plugin # noqa: I100 +assert mymodule.plugin.MyPlugin is MyPlugin diff --git a/test/fixtures/module_plugin/rplugin/python3/mymodule/plugin.py b/test/fixtures/module_plugin/rplugin/python3/mymodule/plugin.py new file mode 100644 index 00000000..d25b3a41 --- /dev/null +++ b/test/fixtures/module_plugin/rplugin/python3/mymodule/plugin.py @@ -0,0 +1,13 @@ +"""Actual implement lies here.""" +import pynvim as neovim +import pynvim.api + + +@neovim.plugin +class MyPlugin: + def __init__(self, nvim: pynvim.api.Nvim): + self.nvim = nvim + + @neovim.command("ModuleHelloWorld") + def hello_world(self) -> None: + self.nvim.command("echom 'MyPlugin: Hello World!'") diff --git a/test/fixtures/simple_plugin/rplugin/python3/simple_nvim.py b/test/fixtures/simple_plugin/rplugin/python3/simple_nvim.py new file mode 100644 index 00000000..e3db4bd0 --- /dev/null +++ b/test/fixtures/simple_plugin/rplugin/python3/simple_nvim.py @@ -0,0 +1,13 @@ +import neovim + +import pynvim.api + + +@neovim.plugin +class SimplePlugin: + def __init__(self, nvim: pynvim.api.Nvim): + self.nvim = nvim + + @neovim.command("SimpleHelloWorld") + def hello_world(self) -> None: + self.nvim.command("echom 'SimplePlugin: Hello World!'") diff --git a/test/test_host.py b/test/test_host.py index 98e34055..016d48b7 100644 --- a/test/test_host.py +++ b/test/test_host.py @@ -1,8 +1,13 @@ -# -*- coding: utf-8 -*- # type: ignore +# pylint: disable=protected-access +import os +from typing import Sequence + from pynvim.plugin.host import Host, host_method_spec from pynvim.plugin.script_host import ScriptHost +__PATH__ = os.path.abspath(os.path.dirname(__file__)) + def test_host_imports(vim): h = ScriptHost(vim) @@ -11,6 +16,24 @@ def test_host_imports(vim): assert h.module.__dict__['sys'] +def test_host_import_rplugin_modules(vim): + # Test whether a Host can load and import rplugins (#461). + # See also $VIMRUNTIME/autoload/provider/pythonx.vim. + h = Host(vim) + plugins: Sequence[str] = [ # plugin paths like real rplugins + os.path.join(__PATH__, "./fixtures/simple_plugin/rplugin/python3/simple_nvim.py"), + os.path.join(__PATH__, "./fixtures/module_plugin/rplugin/python3/mymodule/"), + os.path.join(__PATH__, "./fixtures/module_plugin/rplugin/python3/mymodule"), # duplicate + ] + h._load(plugins) + assert len(h._loaded) == 2 + + # pylint: disable-next=unbalanced-tuple-unpacking + simple_nvim, mymodule = list(h._loaded.values()) + assert simple_nvim['module'].__name__ == 'simple_nvim' + assert mymodule['module'].__name__ == 'mymodule' + + def test_host_clientinfo(vim): h = Host(vim) assert h._request_handlers.keys() == host_method_spec.keys()