diff --git a/.github/workflows/pypi-release.yml b/.github/workflows/pypi-release.yml index 35bebda807..bfc4007ebb 100644 --- a/.github/workflows/pypi-release.yml +++ b/.github/workflows/pypi-release.yml @@ -81,7 +81,7 @@ jobs: fail-fast: false matrix: os: [macos-11, windows-2019] - python-version: ['3.7', '3.8', '3.9', '3.10', '3.11'] + python-version: ['3.7', '3.8', '3.9', '3.10', '3.11', '3.12.0-rc.1'] steps: - uses: actions/checkout@v3 - name: Set up Python ${{ matrix.python-version }} diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml index 015c889433..8f72c83f40 100644 --- a/.github/workflows/run-tests.yml +++ b/.github/workflows/run-tests.yml @@ -9,7 +9,7 @@ jobs: strategy: matrix: os: [macos-latest, ubuntu-latest, windows-2019] - python-version: ['3.7', '3.8', '3.9', '3.10', '3.11'] + python-version: ['3.7', '3.8', '3.9', '3.10', '3.11', '3.12.0-rc.1'] include: - os: macos-11 python-version: '3.7' diff --git a/picard/pluginmanager.py b/picard/pluginmanager.py index 492d2b3dc3..3ffbcb9f39 100644 --- a/picard/pluginmanager.py +++ b/picard/pluginmanager.py @@ -25,11 +25,12 @@ from functools import partial -import imp import importlib +from importlib.abc import MetaPathFinder import json import os.path import shutil +import sys import tempfile import zipfile import zipimport @@ -46,6 +47,7 @@ PLUGINS_API, USER_PLUGIN_DIR, ) +from picard.const.sys import IS_FROZEN from picard.plugin import ( _PLUGIN_MODULE_PREFIX, PluginData, @@ -156,30 +158,25 @@ def _plugin_name_from_path(path): return None -def load_manifest(archive_path): - archive = zipfile.ZipFile(archive_path) - manifest_data = None - with archive.open('MANIFEST.json') as f: - manifest_data = json.loads(str(f.read().decode())) - return manifest_data +def load_zip_manifest(archive_path): + if is_zipped_package(archive_path): + try: + archive = zipfile.ZipFile(archive_path) + with archive.open('MANIFEST.json') as f: + return json.loads(str(f.read().decode())) + except Exception as why: + log.warning("Failed to load manifest data from json: %s", why) + return None def zip_import(path): if (not is_zip(path) or not os.path.isfile(path)): - return (None, None, None) + return None try: - zip_importer = zipimport.zipimporter(path) - plugin_name = _plugin_name_from_path(path) - manifest_data = None - if is_zipped_package(path): - try: - manifest_data = load_manifest(path) - except Exception as why: - log.warning("Failed to load manifest data from json: %s", why) - return (zip_importer, plugin_name, manifest_data) + return zipimport.zipimporter(path) except zipimport.ZipImportError as why: log.error("ZIP import error: %s", why) - return (None, None, None) + return None def _compatible_api_versions(api_versions): @@ -187,6 +184,45 @@ def _compatible_api_versions(api_versions): return set(versions) & set(picard.api_versions_tuple) +_plugin_dirs = [] + + +def plugin_dirs(): + yield from _plugin_dirs + + +def init_default_plugin_dirs(): + # Add user plugin dir first + if not os.path.exists(USER_PLUGIN_DIR): + os.makedirs(USER_PLUGIN_DIR) + register_plugin_dir(USER_PLUGIN_DIR) + + # Register system wide plugin dir + if IS_FROZEN: + toppath = sys.argv[0] + else: + toppath = os.path.abspath(__file__) + + topdir = os.path.dirname(toppath) + plugin_dir = os.path.join(topdir, "plugins") + register_plugin_dir(plugin_dir) + + +def register_plugin_dir(path): + if path not in _plugin_dirs: + _plugin_dirs.append(path) + + +def plugin_dir_for_path(path): + for plugin_dir in plugin_dirs(): + try: + if os.path.commonpath((path, plugin_dir)) == plugin_dir: + return plugin_dir + except ValueError: + pass + return path + + class PluginManager(QtCore.QObject): plugin_installed = QtCore.pyqtSignal(PluginWrapper, bool) @@ -201,6 +237,7 @@ def __init__(self, plugins_directory=None): if plugins_directory is None: plugins_directory = USER_PLUGIN_DIR self.plugins_directory = os.path.normpath(plugins_directory) + init_default_plugin_dirs() @property def available_plugins(self): @@ -262,7 +299,7 @@ def load_plugins_from_directory(self, plugindir): len(names)) for name in sorted(names): try: - self._load_plugin_from_directory(name, plugindir) + self._load_plugin(name) except Exception: self.plugin_error(name, _("Unable to load plugin '%s'"), name, log_func=log.exception) @@ -272,50 +309,57 @@ def _get_plugin_index_by_name(self, name): return (plugin, index) return (None, None) - def _load_plugin_from_directory(self, name, plugindir): - module_file = None - zipfilename = os.path.join(plugindir, name + '.zip') - (zip_importer, module_name, manifest_data) = zip_import(zipfilename) - if zip_importer: - name = module_name - if not zip_importer.find_module(name): - errorfmt = _('Failed loading zipped plugin "%(plugin)s" from "%(filename)s"') - self.plugin_error(name, errorfmt, params={ - 'plugin': name, - 'filename': zipfilename, - }) - return None - module_pathname = zip_importer.get_filename(name) - else: - try: - info = imp.find_module(name, [plugindir]) - module_file = info[0] - module_pathname = info[1] - except ImportError: - errorfmt = _('Failed loading plugin "%(plugin)s" in "%(dirname)s"') + def _load_plugin(self, name): + existing_plugin, existing_plugin_index = self._get_plugin_index_by_name(name) + if existing_plugin: + log.debug("Ignoring already loaded plugin %r (version %r at %r)", + existing_plugin.module_name, + existing_plugin.version, + existing_plugin.file) + return + + spec = None + module_pathname = None + zip_importer = None + manifest_data = None + full_module_name = _PLUGIN_MODULE_PREFIX + name + + # Legacy loading of ZIP plugins. In Python >= 3.10 this is all handled + # by PluginMetaPathFinder. Remove once Python 3.9 is no longer supported. + if not hasattr(zipimport.zipimporter, 'find_spec'): + (zip_importer, plugin_dir, module_pathname, manifest_data) = self._legacy_load_zip_plugin(name) + + if not module_pathname: + spec = PluginMetaPathFinder().find_spec(full_module_name, []) + if not spec or not spec.loader: + errorfmt = _('Failed loading plugin "%(plugin)s"') self.plugin_error(name, errorfmt, params={ 'plugin': name, - 'dirname': plugindir, }) return None + module_pathname = spec.origin + if isinstance(spec.loader, zipimport.zipimporter): + manifest_data = load_zip_manifest(spec.loader.archive) + if os.path.basename(module_pathname) == '__init__.py': + module_pathname = os.path.dirname(module_pathname) + plugin_dir = plugin_dir_for_path(module_pathname) + plugin = None try: - existing_plugin, existing_plugin_index = self._get_plugin_index_by_name(name) - if existing_plugin: - log.warning("Module %r conflict: unregistering previously" - " loaded %r version %s from %r", - existing_plugin.module_name, - existing_plugin.name, - existing_plugin.version, - existing_plugin.file) - _unregister_module_extensions(name) - full_module_name = _PLUGIN_MODULE_PREFIX + name - if zip_importer: + if zip_importer: # Legacy ZIP import for Python < 3.10 plugin_module = zip_importer.load_module(full_module_name) else: - plugin_module = imp.load_module(full_module_name, *info) - plugin = PluginWrapper(plugin_module, plugindir, + plugin_module = importlib.util.module_from_spec(spec) + # This is kind of a hack. The module will be in sys.modules + # after exec_module has run. But if inside of the loaded plugin + # there are relative imports it would load the same plugin + # module twice. This executes the plugins code twice and leads + # to potential side effects. + sys.modules[full_module_name] = plugin_module + spec.loader.exec_module(plugin_module) + + plugin = PluginWrapper(plugin_module, plugin_dir, file=module_pathname, manifest_data=manifest_data) compatible_versions = _compatible_api_versions(plugin.api_versions) if compatible_versions: @@ -345,10 +389,25 @@ def _load_plugin_from_directory(self, name, plugindir): errorfmt = _('Plugin "%(plugin)s"') self.plugin_error(name, errorfmt, log_func=log.exception, params={'plugin': name}) - if module_file is not None: - module_file.close() return plugin + def _legacy_load_zip_plugin(self, name): + for plugin_dir in plugin_dirs(): + zipfilename = os.path.join(plugin_dir, name + '.zip') + zip_importer = zip_import(zipfilename) + if zip_importer: + if not zip_importer.find_module(name): + errorfmt = _('Failed loading zipped plugin "%(plugin)s" from "%(filename)s"') + self.plugin_error(name, errorfmt, params={ + 'plugin': name, + 'filename': zipfilename, + }) + return (None, None, None, None) + module_pathname = zip_importer.get_filename(name) + manifest_data = load_zip_manifest(zip_importer.archive) + return (zip_importer, plugin_dir, module_pathname, manifest_data) + return (None, None, None, None) + def _get_existing_paths(self, plugin_name, fileexts): dirpath = os.path.join(self.plugins_directory, plugin_name) if not os.path.isdir(dirpath): @@ -454,7 +513,7 @@ def install_plugin(self, path, update=False, plugin_name=None, plugin_data=None) if not update: try: - installed_plugin = self._load_plugin_from_directory(plugin_name, self.plugins_directory) + installed_plugin = self._load_plugin(plugin_name) if not installed_plugin: raise RuntimeError("Failed loading newly installed plugin %s" % plugin_name) except Exception as e: @@ -553,3 +612,37 @@ def _display_update(): self.query_available_plugins(_display_update) else: _display_update() + + +class PluginMetaPathFinder(MetaPathFinder): + def find_spec(self, fullname, path, target=None): + if not fullname.startswith(_PLUGIN_MODULE_PREFIX): + return None + plugin_name = fullname[len(_PLUGIN_MODULE_PREFIX):] + for plugin_dir in plugin_dirs(): + for file_path in self._plugin_file_paths(plugin_dir, plugin_name): + if os.path.exists(file_path): + spec = self._spec_from_path(fullname, file_path) + if spec and spec.loader: + return spec + + def _spec_from_path(self, fullname, file_path): + if file_path.endswith('.zip'): + return self._spec_from_zip(fullname, file_path) + else: + return importlib.util.spec_from_file_location(fullname, file_path) + + def _spec_from_zip(self, fullname, file_path): + zip_importer = zip_import(file_path) + if zip_importer: + return zip_importer.find_spec(fullname) + + @staticmethod + def _plugin_file_paths(plugin_dir, plugin_name): + yield os.path.join(plugin_dir, plugin_name, '__init__.py') + yield os.path.join(plugin_dir, plugin_name + '.py') + if hasattr(zipimport.zipimporter, 'find_spec'): # Python >= 3.10 + yield os.path.join(plugin_dir, plugin_name + '.zip') + + +sys.meta_path.append(PluginMetaPathFinder()) diff --git a/picard/tagger.py b/picard/tagger.py index f363446a94..9e0ae56a33 100644 --- a/picard/tagger.py +++ b/picard/tagger.py @@ -94,12 +94,8 @@ setup_config, ) from picard.config_upgrade import upgrade_config -from picard.const import ( - USER_DIR, - USER_PLUGIN_DIR, -) +from picard.const import USER_DIR from picard.const.sys import ( - IS_FROZEN, IS_HAIKU, IS_MACOS, IS_WIN, @@ -114,7 +110,10 @@ from picard.file import File from picard.formats import open_ as open_file from picard.i18n import setup_gettext -from picard.pluginmanager import PluginManager +from picard.pluginmanager import ( + PluginManager, + plugin_dirs, +) from picard.releasegroup import ReleaseGroup from picard.track import ( NonAlbumTrack, @@ -175,21 +174,6 @@ def _patched_shutil_copystat(src, dst, *, follow_symlinks=True): shutil.copystat = _patched_shutil_copystat -def plugin_dirs(): - if IS_FROZEN: - toppath = sys.argv[0] - else: - toppath = os.path.abspath(__file__) - - topdir = os.path.dirname(toppath) - plugin_dir = os.path.join(topdir, "plugins") - yield plugin_dir - - if not os.path.exists(USER_PLUGIN_DIR): - os.makedirs(USER_PLUGIN_DIR) - yield USER_PLUGIN_DIR - - class ParseItemsToLoad: WINDOWS_DRIVE_TEST = re.compile(r"^[a-z]\:", re.IGNORECASE) diff --git a/test/test_plugins.py b/test/test_plugins.py index 56c9244205..f56bfb7aba 100644 --- a/test/test_plugins.py +++ b/test/test_plugins.py @@ -3,7 +3,7 @@ # Picard, the next-generation MusicBrainz tagger # # Copyright (C) 2019-2021 Laurent Monin -# Copyright (C) 2019-2021 Philipp Wolfer +# Copyright (C) 2019-2021, 2023 Philipp Wolfer # # This program is free software; you can redistribute it and/or # modify it under the terms of the GNU General Public License @@ -30,14 +30,15 @@ import picard from picard.const import USER_PLUGIN_DIR from picard.plugin import ( - _PLUGIN_MODULE_PREFIX, PluginWrapper, _unregister_module_extensions, ) from picard.pluginmanager import ( PluginManager, _compatible_api_versions, + _plugin_dirs, _plugin_name_from_path, + register_plugin_dir, ) from picard.version import ( Version, @@ -92,9 +93,8 @@ def unload_plugin(plugin_name): _unregister_module_extensions(plugin_name) if hasattr(picard.plugins, plugin_name): delattr(picard.plugins, plugin_name) - key = _PLUGIN_MODULE_PREFIX + plugin_name - if key in sys.modules: - del sys.modules[key] + if plugin_name in sys.modules: + del sys.modules[plugin_name] class TestPicardPluginsCommon(PicardTestCase): @@ -112,6 +112,7 @@ class TestPicardPluginsCommonTmpDir(TestPicardPluginsCommon): def setUp(self): super().setUp() self.tmp_directory = self.mktmpdir() + register_plugin_dir(self.tmp_directory) class TestPicardPluginManager(TestPicardPluginsCommon): @@ -146,10 +147,6 @@ def test_plugin_name_from_path(self): class TestPicardPluginsInstall(TestPicardPluginsCommonTmpDir): def _test_plugin_install(self, name): - unload_plugin('dummyplugin') - with self.assertRaises(ImportError): - from picard.plugins.dummyplugin import DummyPlugin - plugin_path = _testplugins[name] pm = PluginManager(plugins_directory=self.tmp_directory) @@ -159,14 +156,18 @@ def _test_plugin_install(self, name): self.assertEqual(pm.plugins[0].name, 'Dummy plugin', msg) # if module is properly loaded, this should work - from picard.plugins.dummyplugin import DummyPlugin # noqa: F811 + from picard.plugins.dummyplugin import DummyPlugin DummyPlugin() - def _test_plugin_install_data(self, name): - unload_plugin('dummyplugin') + # Remove plugin again + pm.remove_plugin('dummyplugin') + unload_plugin('picard.plugins.dummyplugin') with self.assertRaises(ImportError): - from picard.plugins.dummyplugin import DummyPlugin + from picard.plugins.dummyplugin import ( # noqa: F811 # pylint: disable=reimported + DummyPlugin, + ) + def _test_plugin_install_data(self, name): # simulate installation from UI using data from picard plugins api web service with open(_testplugins[name], 'rb') as f: data = f.read() @@ -179,9 +180,17 @@ def _test_plugin_install_data(self, name): self.assertEqual(pm.plugins[0].name, 'Dummy plugin', msg) # if module is properly loaded, this should work - from picard.plugins.dummyplugin import DummyPlugin # noqa: F811 + from picard.plugins.dummyplugin import DummyPlugin DummyPlugin() + # Remove plugin again + pm.remove_plugin('dummyplugin') + unload_plugin('picard.plugins.dummyplugin') + with self.assertRaises(ImportError): + from picard.plugins.dummyplugin import ( # noqa: F811 # pylint: disable=reimported + DummyPlugin, + ) + # module def test_plugin_install_module(self): self._test_plugin_install('module') @@ -221,13 +230,10 @@ def test_plugin_install_no_path_no_plugin_name(self): class TestPicardPluginsLoad(TestPicardPluginsCommonTmpDir): def _test_plugin_load_from_directory(self, name): - unload_plugin('dummyplugin') - with self.assertRaises(ImportError): - from picard.plugins.dummyplugin import DummyPlugin - pm = PluginManager(plugins_directory=self.tmp_directory) src_dir = os.path.dirname(_testplugins[name]) + register_plugin_dir(src_dir) msg = "plugins_load_from_directory: %s %r" % (name, src_dir) pm.load_plugins_from_directory(src_dir) @@ -235,9 +241,11 @@ def _test_plugin_load_from_directory(self, name): self.assertEqual(pm.plugins[0].name, 'Dummy plugin', msg) # if module is properly loaded, this should work - from picard.plugins.dummyplugin import DummyPlugin # noqa: F811 + from picard.plugins.dummyplugin import DummyPlugin DummyPlugin() + _plugin_dirs.remove(src_dir) + # singlefile def test_plugin_load_from_directory_singlefile(self): self._test_plugin_load_from_directory('singlefile')