mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-10-04 06:15:09 +00:00
fix(plugins): put a (re)loading plugin's directory first on sys.path (#663)
Plugins import their own files by bare name (`from sports import ...`), which resolves to the first directory on sys.path that has the file. The loader added a plugin's directory only if it was missing, so on a reload -- a live re-enable from the web UI -- the plugin's directory stayed behind every plugin loaded since, and its bare imports found their files first. Seen on ledpi: re-enabling UFC with hockey running failed with "cannot import name '_status_is_final' from 'sports'" (it got hockey's sports.py). A loading plugin's directory is now always moved to the front. Every scoreboard ships its own sports.py, so any of them was exposed on reload. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -19,6 +19,7 @@ accepts both, but the store flags the old spelling as deprecated
|
||||
|
||||
## Unreleased
|
||||
|
||||
- A plugin that is reloaded (switched off and on again from the web UI) imports its own modules again, not another plugin's. Plugins import their own files by bare name (`from sports import ...`), which resolves to the first plugin directory on `sys.path` that has the file; the loader only added a directory that was missing, so a reloaded plugin's directory stayed behind any loaded since. On a Pi, re-enabling UFC with hockey running failed with "cannot import name '_status_is_final' from 'sports'". A loading plugin's directory is now always moved to the front.
|
||||
- A mypy ratchet in CI. `mypy-clean.txt` lists the 71 modules under `src/` that type-check clean, and the new "Type check (mypy ratchet)" job runs `python scripts/check_types.py` (mypy 1.20.2 on exactly those files) so they stay clean; add a module when you make it clean (see CONTRIBUTING.md). The manual pre-commit `mypy` hook runs the same script. 35 modules were made clean for it with annotation-only fixes, no behaviour change. Their public signatures only widened (`declared_min_version()` now says it returns the manifest's value as-is, `Any`); `DynamicTeamResolver._rankings_cache` is annotated as the abbreviation-to-rank dict it holds. `mypy.ini` treats numpy and orjson as `Any`, so it parses with `python_version = 3.10` against numpy 2.3+ stubs and gives the same result whether orjson is installed or not.
|
||||
- CI runs the web UI's DOM test suites (jsdom against the real server-rendered pages and API) in a new **Web UI JS tests** job, with the web interface started in emulator mode; `REQUIRE_DOM=1` makes a suite that can't run fail instead of being skipped. Two suites that had gone stale were fixed: the Tools suite now installs `LEDEscape` the way `base.html` does and supplies sample Starlark apps when the server has none, and the Store suite no longer assumes the registry has 48 plugins or fewer.
|
||||
- `src/plugin_system/store_manager.py` (2,977 lines) is split into mixins: `store_registry.py` (registry, GitHub metadata, search, manifest validation), `store_install.py` (install paths and dependencies) and `store_update.py` (updates, rollback, local git state). `PluginStoreManager` is still imported from `store_manager.py` and has exactly the same methods and attributes; every method body is byte-identical.
|
||||
|
||||
@@ -592,11 +592,21 @@ class PluginLoader:
|
||||
raise PluginError(error_msg, plugin_id=plugin_id, context={'entry_file': str(entry_file)})
|
||||
|
||||
with self._module_load_lock:
|
||||
# Add plugin directory to sys.path if not already there
|
||||
# Put this plugin's directory first on sys.path -- moving it there
|
||||
# if it is already present. Plugins import their own modules by
|
||||
# bare name (``from sports import ...``), and those resolve to the
|
||||
# first directory that has the file. A directory added on an
|
||||
# earlier load stays where it was, so reloading a plugin (a live
|
||||
# re-enable from the web UI) after another scoreboard had loaded
|
||||
# found that one's sports.py first and failed on a name only its
|
||||
# own copy has.
|
||||
plugin_dir_str = str(plugin_dir)
|
||||
if plugin_dir_str not in sys.path:
|
||||
sys.path.insert(0, plugin_dir_str)
|
||||
self.logger.debug("Added plugin %s's directory to sys.path", plugin_id)
|
||||
try:
|
||||
sys.path.remove(plugin_dir_str)
|
||||
except ValueError:
|
||||
pass
|
||||
sys.path.insert(0, plugin_dir_str)
|
||||
self.logger.debug("Put plugin %s's directory first on sys.path", plugin_id)
|
||||
|
||||
# Import the plugin module
|
||||
module_name = f"plugin_{plugin_id.replace('-', '_')}"
|
||||
|
||||
@@ -0,0 +1,60 @@
|
||||
"""A reloaded plugin imports its own bare-name modules, not another plugin's.
|
||||
|
||||
Scoreboard plugins each ship a ``sports.py`` and import it by bare name. Each
|
||||
plugin's directory goes on sys.path when it loads, and a bare import resolves
|
||||
to the first directory that has the file. The loader used to add a directory
|
||||
only if it was missing, so after alpha, then beta, loaded, re-enabling alpha
|
||||
from the web UI left beta's directory in front: alpha's ``from sports import
|
||||
...`` got beta's copy. On a Pi, re-enabling UFC with hockey running failed
|
||||
with "cannot import name '_status_is_final' from 'sports'".
|
||||
"""
|
||||
|
||||
import sys
|
||||
|
||||
import pytest
|
||||
|
||||
from src.plugin_system.plugin_loader import PluginLoader
|
||||
|
||||
|
||||
def _write_plugin(root, name):
|
||||
d = root / name
|
||||
d.mkdir(parents=True)
|
||||
(d / "sports.py").write_text(f"WHO = {name!r}\n", encoding="utf-8")
|
||||
(d / "manager.py").write_text("from sports import WHO\n", encoding="utf-8")
|
||||
return d
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def plugins(tmp_path):
|
||||
before_path = list(sys.path)
|
||||
before_modules = set(sys.modules)
|
||||
dirs = {name: _write_plugin(tmp_path, name) for name in ("alpha", "beta")}
|
||||
yield dirs
|
||||
sys.path[:] = before_path
|
||||
for key in set(sys.modules) - before_modules:
|
||||
sys.modules.pop(key, None)
|
||||
|
||||
|
||||
def _unload(loader, plugin_id):
|
||||
# What PluginManager.unload_plugin does to the module entries.
|
||||
sys.modules.pop(f"plugin_{plugin_id}", None)
|
||||
loader.unregister_plugin_modules(plugin_id)
|
||||
|
||||
|
||||
def test_each_plugin_gets_its_own_bare_module(plugins):
|
||||
loader = PluginLoader()
|
||||
assert loader.load_module("alpha", plugins["alpha"], "manager.py").WHO == "alpha"
|
||||
assert loader.load_module("beta", plugins["beta"], "manager.py").WHO == "beta"
|
||||
|
||||
|
||||
def test_a_reloaded_plugin_still_gets_its_own_bare_module(plugins):
|
||||
loader = PluginLoader()
|
||||
loader.load_module("alpha", plugins["alpha"], "manager.py")
|
||||
loader.load_module("beta", plugins["beta"], "manager.py")
|
||||
|
||||
_unload(loader, "alpha")
|
||||
reloaded = loader.load_module("alpha", plugins["alpha"], "manager.py")
|
||||
|
||||
assert reloaded.WHO == "alpha"
|
||||
assert sys.path.index(str(plugins["alpha"])) < sys.path.index(str(plugins["beta"]))
|
||||
assert sys.path.count(str(plugins["alpha"])) == 1
|
||||
Reference in New Issue
Block a user