Compare commits

..
Author SHA1 Message Date
ChuckandClaude Opus 5.5 686b00d224 fix(fonts): call the font manager's forget methods without a None-able local
Pylint E1102 (Codacy) read getattr(..., None) as possibly not callable.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-04 17:11:09 -04:00
ChuckandClaude Opus 5.5 10ac608b9d fix(fonts): unloading a plugin forgets its manifest fonts
PluginManager.unload_plugin() and the failed-load cleanup only called
FontManager.forget_manager_fonts(), which drops usage data. The plugin's
manifest registrations stayed: plugin_fonts / plugin_font_catalogs, its
plugin_id::family entries in font_catalog, and cached font objects for them
-- so its fonts kept resolving after unload and a family a reinstalled
manifest dropped stayed registered. The deprecated unregister_plugin_fonts
did this cleanup but nothing called it; it was removed in #708.

Add FontManager.forget_plugin_fonts(plugin_id) and call it from both
paths alongside forget_manager_fonts (each guarded on its own, so a font
manager stub with only one still works). FontManager takes no locks, so
like forget_manager_fonts it uses atomic pops over snapshots. A reload
(unload + load) registers the manifest fonts again and they resolve.

Raised by CodeRabbit on #709.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-04 17:00:28 -04:00
12 changed files with 223 additions and 552 deletions
+8 -11
View File
@@ -21,6 +21,14 @@ accepts both, but the store flags the old spelling as deprecated
### Fixed ### Fixed
- Unloading a plugin now forgets the fonts its manifest registered, not only
the fonts it reported using. Its `plugin_id::family` entries kept resolving
and their cached font objects stayed alive until a restart, and a family a
reinstalled plugin's manifest dropped stayed registered. The new
`FontManager.forget_plugin_fonts(plugin_id)` does the cleanup;
`PluginManager.unload_plugin()` and a failed load call it alongside
`forget_manager_fonts()`, and a reload registers the manifest's fonts again.
- The web preview and `/api/v3/display/current` no longer stay black for a - The web preview and `/api/v3/display/current` no longer stay black for a
whole screen that draws its card once and then holds it. The snapshot is whole screen that draws its card once and then holds it. The snapshot is
written from `update_display()` at most once per write interval, so a frame written from `update_display()` at most once per write interval, so a frame
@@ -1417,17 +1425,6 @@ read any of them:
### Fixes ### Fixes
- Updating a plugin from the store no longer deletes the files it wrote
beside itself. A monorepo update replaces the plugin directory with the
fresh download and deletes the old copy, so calendar's Google OAuth files
(`token.pickle`, `credentials.json`) were lost on every update and the
calendar stopped until they were restored by hand. Before the old copy is
removed, the update now copies over anything the plugin's `.gitignore`
excludes plus known secret/state files (`*.pickle`, `token.json`,
`credentials.json`, `config_secrets.json`, `.pkce_code_verifier`); files the
new release ships are never overwritten, and byte code is not carried. A
plugin updated with `git pull` no longer sweeps an untracked token into the
auto-stash, which is never popped (`src/plugin_system/plugin_local_files.py`).
- Quieter routine logging. Every rotation logged each mode twice - Quieter routine logging. Every rotation logged each mode twice
("Switching to mode", then "Processing mode"), and a mode with nothing to ("Switching to mode", then "Processing mode"), and a mode with nothing to
show added "display() returned False" and "No content to display". Those show added "display() returned False" and "No content to display". Those
+3 -1
View File
@@ -203,6 +203,7 @@ Current methods:
| `measure_text(text, font)` | `(width, height, baseline)` | | `measure_text(text, font)` | `(width, height, baseline)` |
| `get_font_height(font)` | Line height | | `get_font_height(font)` | Line height |
| `register_plugin_fonts(plugin_id, font_manifest)` | Register a plugin's fonts (core calls it at load) | | `register_plugin_fonts(plugin_id, font_manifest)` | Register a plugin's fonts (core calls it at load) |
| `forget_plugin_fonts(plugin_id)` | Drop a plugin's manifest fonts and their cached objects (core calls it when a plugin unloads) |
| `clear_cache()` | Drop cached fonts and metrics | | `clear_cache()` | Drop cached fonts and metrics |
| `font_catalog` (attribute) | Family name → file path | | `font_catalog` (attribute) | Family name → file path |
@@ -218,5 +219,6 @@ Removed in 3.8.0, after logging a deprecation warning on first call since
| `get_performance_stats()` | — | | `get_performance_stats()` | — |
| `set_override()`, `remove_override()`, `get_overrides()` | a font field in your plugin's config schema | | `set_override()`, `remove_override()`, `get_overrides()` | a font field in your plugin's config schema |
| `get_manager_fonts()`, `get_detected_fonts()` | — | | `get_manager_fonts()`, `get_detected_fonts()` | — |
| `get_plugin_fonts()`, `unregister_plugin_fonts()` | — | | `get_plugin_fonts()` | — |
| `unregister_plugin_fonts()` | `forget_plugin_fonts()` (core calls it on unload) |
| `add_font()`, `remove_font()`, `validate_font()` | the web UI's Fonts tab | | `add_font()`, `remove_font()`, `validate_font()` | the web UI's Fonts tab |
+33
View File
@@ -222,6 +222,39 @@ class FontManager:
logger.error(f"Error registering fonts for plugin {plugin_id}: {e}", exc_info=True) logger.error(f"Error registering fonts for plugin {plugin_id}: {e}", exc_info=True)
return False return False
def forget_plugin_fonts(self, plugin_id: str) -> bool:
"""Drop the fonts ``plugin_id``'s manifest registered: its manifest
and catalog, its ``plugin_id::family`` entries in font_catalog, and
cached font objects for those families. Called by core when a plugin
is unloaded, so a reload registers from its current manifest and a
removed plugin's fonts stop resolving.
FontManager takes no locks; like forget_manager_fonts this relies on
single dict operations being atomic and iterates snapshots, so a
render thread calling get_font() meanwhile cannot break it. Returns
True if the plugin had registered fonts.
"""
prefix = f"{plugin_id}::"
manifest = self.plugin_fonts.pop(plugin_id, None)
catalog = self.plugin_font_catalogs.pop(plugin_id, None)
# Every namespaced entry, not just the families in the catalog: one
# whose file failed to load never made it into the catalog, and a
# caller may have added one directly.
for family in list(self.font_catalog):
if family.startswith(prefix):
self.font_catalog.pop(family, None)
# get_font() keys the cache f"{family}_{size_px}".
dropped = [key for key in list(self.font_cache) if key.startswith(prefix)]
for key in dropped:
self.font_cache.pop(key, None)
if dropped:
# Font objects someone may hold were dropped; see cache_generation.
self.cache_generation += 1
if manifest is None and catalog is None:
return False
logger.info("Forgot fonts of plugin %s", plugin_id)
return True
def _validate_font_manifest(self, font_manifest: Dict[str, Any]) -> bool: def _validate_font_manifest(self, font_manifest: Dict[str, Any]) -> bool:
"""Validate the structure of a plugin's font manifest.""" """Validate the structure of a plugin's font manifest."""
required_fields = ["fonts"] required_fields = ["fonts"]
-204
View File
@@ -1,204 +0,0 @@
"""
Files a plugin writes beside itself at runtime, which an update must keep.
A store update replaces a plugin's directory with a fresh download and then
deletes the old copy. Anything the plugin created there -- OAuth tokens, a
client-secrets file, a PKCE verifier, cached state -- is in no release, so the
fresh download does not contain it and deleting the old copy destroys it. On
2026-10-04 updating calendar 1.2.9 -> 1.2.12 that way deleted its
``token.pickle`` and ``credentials.json``, and the calendar stopped until they
were restored from a backup.
What counts as "the plugin's own local file" is the union of:
* :data:`KNOWN_STATE_PATTERNS` -- secret and state files plugins are known to
write, kept even when a plugin forgot to gitignore them; and
* whatever the plugin's own ``.gitignore`` (old copy or new) excludes. A file
the author ignores is by definition not part of a release.
A file the new release ships is never overwritten: tracked content wins. Byte
code (``__pycache__``, ``*.pyc``) and ``.git`` are never carried, since they
belong to the old code rather than to the user.
"""
from __future__ import annotations
import fnmatch
import os
import re
import shutil
from pathlib import Path
from typing import Iterable, List, Optional, Pattern, Tuple
__all__ = [
'KNOWN_STATE_PATTERNS',
'carry_over_local_files',
'is_known_state_file',
'local_files_to_keep',
]
# Basename globs. Kept even when the plugin's .gitignore does not list them.
KNOWN_STATE_PATTERNS: Tuple[str, ...] = (
'token.pickle',
'*.pickle',
'token.json',
'credentials.json',
'config_secrets.json',
'.pkce_code_verifier',
)
_NEVER_CARRY_DIRS = frozenset({'.git', '__pycache__'})
_NEVER_CARRY_SUFFIXES = ('.pyc', '.pyo')
def is_known_state_file(rel_path: str) -> bool:
"""True when ``rel_path``'s basename is a known secret/state file."""
name = rel_path.replace('\\', '/').rsplit('/', 1)[-1]
return any(fnmatch.fnmatchcase(name, p) for p in KNOWN_STATE_PATTERNS)
class _GitIgnore:
"""The subset of gitignore semantics plugin .gitignore files use.
Supports comments, ``!`` negation (last match wins), a trailing ``/`` for
directory-only patterns, anchoring by a leading or embedded ``/``, ``*``,
``?``, ``[...]`` and ``**``. As in git, a file under an ignored directory
is ignored regardless of later negations.
"""
def __init__(self, lines: Iterable[str]):
self._rules: List[Tuple[Pattern[str], bool, bool]] = []
for raw in lines:
line = raw.rstrip('\n').rstrip()
if not line or line.startswith('#'):
continue
negate = line.startswith('!')
if negate:
line = line[1:]
elif line.startswith('\\'):
line = line[1:]
dir_only = line.endswith('/')
line = line.rstrip('/')
if not line:
continue
anchored = '/' in line
line = line.lstrip('/')
body = self._translate(line)
regex = body if anchored else r'(?:.*/)?' + body
self._rules.append((re.compile(r'\A' + regex + r'\Z'), negate, dir_only))
@staticmethod
def _translate(pattern: str) -> str:
out, i, n = [], 0, len(pattern)
while i < n:
if pattern.startswith('**/', i):
out.append(r'(?:.*/)?')
i += 3
elif pattern.startswith('/**', i) and i + 3 == n:
out.append(r'/.*')
i += 3
elif pattern.startswith('**', i):
out.append(r'.*')
i += 2
elif pattern[i] == '*':
out.append(r'[^/]*')
i += 1
elif pattern[i] == '?':
out.append(r'[^/]')
i += 1
elif pattern[i] == '[':
end = pattern.find(']', i + 1)
if end == -1:
out.append(re.escape('['))
i += 1
else:
cls = pattern[i + 1:end]
if cls.startswith('!'):
cls = '^' + cls[1:]
out.append('[' + cls.replace('\\', '\\\\') + ']')
i = end + 1
else:
out.append(re.escape(pattern[i]))
i += 1
return ''.join(out)
def _decide(self, rel: str, is_dir: bool) -> Optional[bool]:
verdict = None
for regex, negate, dir_only in self._rules:
if dir_only and not is_dir:
continue
if regex.match(rel):
verdict = not negate
return verdict
def ignores(self, rel_path: str) -> bool:
if not self._rules:
return False
parts = rel_path.replace('\\', '/').split('/')
for depth in range(1, len(parts)):
if self._decide('/'.join(parts[:depth]), True):
return True
return bool(self._decide('/'.join(parts), False))
def _read_gitignore(plugin_dir: Path) -> List[str]:
try:
return (plugin_dir / '.gitignore').read_text(
encoding='utf-8', errors='replace').splitlines()
except OSError:
return []
def local_files_to_keep(old_dir: Path, new_dir: Path) -> List[str]:
"""Relative paths (``/``-separated) in ``old_dir`` to copy into ``new_dir``.
Regular files only; symlinks and anything the new release already ships
are skipped.
"""
old_dir, new_dir = Path(old_dir), Path(new_dir)
ignore = _GitIgnore(_read_gitignore(old_dir) + _read_gitignore(new_dir))
keep: List[str] = []
for root, dirs, files in os.walk(old_dir):
dirs[:] = sorted(d for d in dirs if d not in _NEVER_CARRY_DIRS
and not os.path.islink(os.path.join(root, d)))
rel_root = os.path.relpath(root, old_dir)
for name in sorted(files):
if name.endswith(_NEVER_CARRY_SUFFIXES):
continue
full = os.path.join(root, name)
if os.path.islink(full) or not os.path.isfile(full):
continue
rel = name if rel_root == '.' else f"{rel_root}/{name}".replace('\\', '/')
if not (is_known_state_file(rel) or ignore.ignores(rel)):
continue
if os.path.lexists(new_dir / rel):
continue
keep.append(rel)
return keep
def carry_over_local_files(
old_dir: Path, new_dir: Path
) -> Tuple[List[str], List[Tuple[str, str]]]:
"""Copy the plugin's local files from ``old_dir`` into ``new_dir``.
Copies rather than moves, so ``old_dir`` stays a complete copy until the
caller deletes it. Returns ``(copied, failed)`` where ``failed`` pairs a
relative path with the error; the caller should keep ``old_dir`` when
anything failed.
"""
copied: List[str] = []
failed: List[Tuple[str, str]] = []
try:
candidates = local_files_to_keep(old_dir, new_dir)
except OSError as e:
return copied, [('.', str(e))]
for rel in candidates:
dest = Path(new_dir) / rel
try:
dest.parent.mkdir(parents=True, exist_ok=True)
shutil.copy2(Path(old_dir) / rel, dest)
copied.append(rel)
except OSError as e:
failed.append((rel, str(e)))
return copied, failed
+16 -9
View File
@@ -597,11 +597,21 @@ class PluginManager:
self.plugin_loader.unregister_plugin_modules(plugin_id) self.plugin_loader.unregister_plugin_modules(plugin_id)
except Exception as e: # pragma: no cover - defensive except Exception as e: # pragma: no cover - defensive
self.logger.debug("Could not drop modules of %s: %s", plugin_id, e) self.logger.debug("Could not drop modules of %s: %s", plugin_id, e)
self._forget_plugin_fonts(plugin_id)
def _forget_plugin_fonts(self, plugin_id: str) -> None:
"""Drop what the FontManager holds for a plugin: the fonts its
instance reported using (the Fonts tab's "Used by") and the fonts its
manifest registered. Never raises."""
if self.font_manager is None:
return
for name in ('forget_manager_fonts', 'forget_plugin_fonts'):
if not hasattr(self.font_manager, name):
continue
try: try:
if self.font_manager is not None and hasattr(self.font_manager, 'forget_manager_fonts'): getattr(self.font_manager, name)(plugin_id)
self.font_manager.forget_manager_fonts(plugin_id)
except Exception as e: except Exception as e:
self.logger.debug("Could not forget fonts of %s: %s", plugin_id, e) self.logger.debug("Could not forget fonts of %s (%s): %s", plugin_id, name, e)
#: Config keys the **core** reads out of a plugin's own config block. The #: Config keys the **core** reads out of a plugin's own config block. The
#: plugin never declares them, so a schema with #: plugin never declares them, so a schema with
@@ -846,12 +856,9 @@ class PluginManager:
# Delegate sub-module and cached-module cleanup to the loader # Delegate sub-module and cached-module cleanup to the loader
self.plugin_loader.unregister_plugin_modules(plugin_id) self.plugin_loader.unregister_plugin_modules(plugin_id)
# Its font registrations go with it (the Fonts tab's "Used by"). # Its font registrations go with it: the fonts it reported using
try: # and the ones its manifest registered.
if self.font_manager is not None and hasattr(self.font_manager, 'forget_manager_fonts'): self._forget_plugin_fonts(plugin_id)
self.font_manager.forget_manager_fonts(plugin_id)
except Exception as e:
self.logger.debug("Could not forget fonts of %s: %s", plugin_id, e)
# Update state # Update state
self.state_manager.set_state(plugin_id, PluginState.UNLOADED) self.state_manager.set_state(plugin_id, PluginState.UNLOADED)
+4 -33
View File
@@ -22,7 +22,6 @@ from src.plugin_system.plugin_loader import (
contained_plugin_dir, requirements_to_install, contained_plugin_dir, requirements_to_install,
) )
from src.plugin_system.plugin_dirs import BACKUP_MARKER from src.plugin_system.plugin_dirs import BACKUP_MARKER
from src.plugin_system.plugin_local_files import carry_over_local_files
from src.plugin_system.repo_urls import ( from src.plugin_system.repo_urls import (
USER_AGENT, github_api_headers, github_owner_repo, normalize_repo_url, USER_AGENT, github_api_headers, github_owner_repo, normalize_repo_url,
) )
@@ -93,9 +92,7 @@ class _InstallMixin:
raise raise
if installed: if installed:
self._discard_backup( self._discard_backup(plugin_id, backup_path, "install")
plugin_id, backup_path, "install",
new_path=self._existing_install(plugin_id) or plugin_path)
return True return True
self._restore_backup(plugin_id, plugin_path, backup_path, "Install") self._restore_backup(plugin_id, plugin_path, backup_path, "Install")
@@ -136,33 +133,8 @@ class _InstallMixin:
return f"could not set aside {plugin_path}: {e}" return f"could not set aside {plugin_path}: {e}"
return None return None
def _discard_backup( def _discard_backup(self, plugin_id: str, backup_path: Path, action: str) -> None:
self, plugin_id: str, backup_path: Path, action: str, """Remove the set-aside copy after a successful (re)install."""
new_path: Optional[Path] = None,
) -> None:
"""Remove the set-aside copy after a successful (re)install.
With ``new_path`` (where the new copy landed), first carries the
plugin's own runtime files -- OAuth tokens, client secrets, anything
its .gitignore excludes -- from the old copy into the new one: no
release contains them, so deleting the old copy would destroy them.
See src/plugin_system/plugin_local_files.py. If any could not be
copied the old copy is kept, so nothing is lost.
"""
if new_path is not None and new_path.is_dir():
copied, failed = carry_over_local_files(backup_path, new_path)
if copied:
self.logger.info(
"Kept %d local file(s) of %s across the %s: %s",
len(copied), plugin_id, action, ", ".join(copied))
if failed:
self.logger.error(
"Could not carry %s's local files into the new copy (%s); "
"the previous copy is kept at %s -- copy them back by hand",
plugin_id,
"; ".join(f"{rel}: {err}" for rel, err in failed),
backup_path)
return
if not self._safe_remove_directory(backup_path): if not self._safe_remove_directory(backup_path):
self.logger.warning( self.logger.warning(
"%s of %s succeeded but the previous copy at %s could not be " "%s of %s succeeded but the previous copy at %s could not be "
@@ -570,8 +542,7 @@ class _InstallMixin:
raise raise
temp_dir = None # Prevent cleanup since we moved it temp_dir = None # Prevent cleanup since we moved it
if backup_path is not None: if backup_path is not None:
self._discard_backup( self._discard_backup(plugin_id, backup_path, "install")
plugin_id, backup_path, "install", new_path=final_path)
# Install dependencies # Install dependencies
self._install_dependencies(final_path) self._install_dependencies(final_path)
+4 -23
View File
@@ -10,9 +10,6 @@ import subprocess # nosec B404 - list-form argv only, no shell # nosemgrep
from pathlib import Path from pathlib import Path
from typing import Dict, Optional, Tuple from typing import Dict, Optional, Tuple
from src.plugin_system.plugin_dirs import BACKUP_MARKER from src.plugin_system.plugin_dirs import BACKUP_MARKER
from src.plugin_system.plugin_local_files import (
KNOWN_STATE_PATTERNS, is_known_state_file,
)
from src.plugin_system.repo_urls import same_repo from src.plugin_system.repo_urls import same_repo
@@ -305,11 +302,7 @@ class _UpdateMixin:
installed = False installed = False
if installed: if installed:
# install_plugin may land the new copy under the manifest id self._discard_backup(plugin_id, backup_path, "update")
# rather than the old directory name.
self._discard_backup(
plugin_id, backup_path, "update",
new_path=self._existing_install(plugin_id) or plugin_path)
return True return True
# Bad network, registry error...: the user keeps a working plugin. # Bad network, registry error...: the user keeps a working plugin.
@@ -516,11 +509,7 @@ class _UpdateMixin:
for line in untracked_result.stdout.strip().split('\n'): for line in untracked_result.stdout.strip().split('\n'):
if line.startswith('??'): if line.startswith('??'):
# Untracked file # Untracked file
file_path = line[3:].strip().strip('"') file_path = line[3:].strip()
# Tokens and secrets stay out of the
# stash (see below), so they alone are
# not a reason to stash.
if not is_known_state_file(file_path):
untracked_files.append(file_path) untracked_files.append(file_path)
# Check for tracked file changes # Check for tracked file changes
@@ -548,17 +537,9 @@ class _UpdateMixin:
if has_changes: if has_changes:
self.logger.info(f"Stashing local changes in {plugin_id} before update") self.logger.info(f"Stashing local changes in {plugin_id} before update")
try: try:
# Use -u to include untracked files in stash -- # Use -u to include untracked files in stash
# except the plugin's tokens and secrets, which a
# repo may have forgotten to gitignore. The stash
# is never popped, so a stashed token.pickle would
# vanish from the plugin and break it.
stash_cmd = (
['git', '-C', str(plugin_path), 'stash', 'push', '-u',
'-m', f'LEDMatrix auto-stash before update {plugin_id}', '--', '.']
+ [f':(exclude,glob)**/{p}' for p in KNOWN_STATE_PATTERNS])
stash_result = subprocess.run( stash_result = subprocess.run(
stash_cmd, ['git', '-C', str(plugin_path), 'stash', 'push', '-u', '-m', f'LEDMatrix auto-stash before update {plugin_id}'],
capture_output=True, capture_output=True,
text=True, text=True,
timeout=30, timeout=30,
+143
View File
@@ -185,6 +185,149 @@ class TestPluginFonts:
assert fm.font_catalog["my-plugin::bundled"] == str(plugin_dir / "fonts" / "Bundled.ttf") assert fm.font_catalog["my-plugin::bundled"] == str(plugin_dir / "fonts" / "Bundled.ttf")
class TestForgetPluginFonts:
"""forget_plugin_fonts drops what a plugin's manifest registered. Before
it, unloading a plugin left its fonts resolvable and its cached font
objects alive until a restart."""
@staticmethod
def _register(fm, root, plugin_id, family="bundled"):
plugin_dir = root / plugin_id
(plugin_dir / "fonts").mkdir(parents=True, exist_ok=True)
font_file = plugin_dir / "fonts" / f"{family}.ttf"
if not font_file.exists(): # a loaded font may hold it open (Windows)
shutil.copy(resolve_asset_path("assets/fonts/PressStart2P-Regular.ttf"), font_file)
manifest = {"fonts": [{"family": family, "source": f"plugin://fonts/{family}.ttf"}]}
assert fm.register_plugin_fonts(plugin_id, manifest, plugin_dir=plugin_dir)
return plugin_dir
@staticmethod
def _entries_of(fm, plugin_id):
prefix = f"{plugin_id}::"
return {
"plugin_fonts": plugin_id in fm.plugin_fonts,
"plugin_font_catalogs": plugin_id in fm.plugin_font_catalogs,
"font_catalog": [k for k in fm.font_catalog if k.startswith(prefix)],
"font_cache": [k for k in fm.font_cache if k.startswith(prefix)],
}
NONE = {"plugin_fonts": False, "plugin_font_catalogs": False,
"font_catalog": [], "font_cache": []}
def test_unload_leaves_no_plugin_entries(self, fm, tmp_path):
self._register(fm, tmp_path, "alpha")
fm.resolve_font("alpha.title", "bundled", 8, plugin_id="alpha")
fm.get_font("alpha::bundled", 10)
assert self._entries_of(fm, "alpha")["font_cache"] # cached before
gen = fm.cache_generation
assert fm.forget_plugin_fonts("alpha") is True
assert self._entries_of(fm, "alpha") == self.NONE
assert fm.cache_generation == gen + 1
# The family no longer resolves to the plugin's file.
assert fm.font_catalog.get("alpha::bundled") is None
def test_other_plugins_and_core_fonts_are_untouched(self, fm, tmp_path):
self._register(fm, tmp_path, "alpha")
self._register(fm, tmp_path, "beta")
# A plugin whose id is a prefix of another's must not take it along.
self._register(fm, tmp_path, "alpha-two")
for pid in ("alpha", "beta", "alpha-two"):
fm.get_font(f"{pid}::bundled", 8)
core_font = fm.get_font("press_start", 8)
beta_before = self._entries_of(fm, "beta")
alpha_two_before = self._entries_of(fm, "alpha-two")
fm.forget_plugin_fonts("alpha")
assert self._entries_of(fm, "beta") == beta_before
assert self._entries_of(fm, "alpha-two") == alpha_two_before
assert fm.get_font("press_start", 8) is core_font
def test_reload_re_registers_cleanly(self, fm, tmp_path):
plugin_dir = self._register(fm, tmp_path, "alpha")
old = fm.get_font("alpha::bundled", 8)
fm.forget_plugin_fonts("alpha")
self._register(fm, tmp_path, "alpha")
assert fm.font_catalog["alpha::bundled"] == str(plugin_dir / "fonts" / "bundled.ttf")
font = fm.resolve_font("alpha.title", "bundled", 8, plugin_id="alpha")
assert isinstance(font, ImageFont.FreeTypeFont)
assert font is not old # loaded fresh, not the dropped cache entry
def test_a_family_the_new_manifest_drops_stops_resolving(self, fm, tmp_path):
self._register(fm, tmp_path, "alpha", family="old_face")
fm.forget_plugin_fonts("alpha")
self._register(fm, tmp_path, "alpha", family="new_face")
assert "alpha::old_face" not in fm.font_catalog
assert "alpha::new_face" in fm.font_catalog
def test_unknown_plugin_is_a_no_op(self, fm):
catalog = dict(fm.font_catalog)
gen = fm.cache_generation
assert fm.forget_plugin_fonts("never-registered") is False
assert fm.font_catalog == catalog
assert fm.cache_generation == gen
class TestPluginManagerReloadFonts:
"""Through PluginManager: unloading a plugin forgets its manifest fonts,
and reload_plugin (unload + load) registers them again so they resolve."""
PLUGIN_ID = "font-reload-demo"
MODULE = "plugin_font_reload_demo"
def test_unload_forgets_and_reload_resolves(self, tmp_path):
import sys
from src.plugin_system.plugin_manager import PluginManager
plugins_dir = tmp_path / "plugins"
plugin_dir = plugins_dir / self.PLUGIN_ID
(plugin_dir / "fonts").mkdir(parents=True)
shutil.copy(resolve_asset_path("assets/fonts/PressStart2P-Regular.ttf"),
plugin_dir / "fonts" / "Bundled.ttf")
manifest = {"id": self.PLUGIN_ID, "name": "Demo", "class_name": "Demo",
"entry_point": "manager.py",
"fonts": {"fonts": [{"family": "bundled",
"source": "plugin://fonts/Bundled.ttf"}]}}
(plugin_dir / "manifest.json").write_text(json.dumps(manifest), encoding="utf-8")
(plugin_dir / "manager.py").write_text(
"class Demo:\n"
" def __init__(self, plugin_id, config, display_manager, cache_manager, plugin_manager):\n"
" self.enabled = True\n", encoding="utf-8")
pm = PluginManager(plugins_dir=str(plugins_dir))
fm = FontManager({})
pm.font_manager = fm
pm.plugin_manifests[self.PLUGIN_ID] = manifest
key = f"{self.PLUGIN_ID}::bundled"
try:
assert pm.load_plugin(self.PLUGIN_ID) is True
assert key in fm.font_catalog
fm.register_manager_font(self.PLUGIN_ID, "demo.title", "bundled", 8)
old = fm.resolve_font("demo.title", "bundled", 8, plugin_id=self.PLUGIN_ID)
assert pm.unload_plugin(self.PLUGIN_ID) is True
assert self.PLUGIN_ID not in fm.plugin_fonts
assert self.PLUGIN_ID not in fm.plugin_font_catalogs
assert key not in fm.font_catalog
assert not [k for k in fm.font_cache if k.startswith(f"{self.PLUGIN_ID}::")]
assert self.PLUGIN_ID not in fm.manager_fonts
assert pm.reload_plugin(self.PLUGIN_ID) is True
assert fm.font_catalog[key] == str(plugin_dir / "fonts" / "Bundled.ttf")
font = fm.resolve_font("demo.title", "bundled", 8, plugin_id=self.PLUGIN_ID)
assert isinstance(font, ImageFont.FreeTypeFont)
assert font is not old
finally:
sys.modules.pop(self.MODULE, None)
class TestDownloadFont: class TestDownloadFont:
"""_download_font: plugin fonts declared by URL, cached in temp_font_dir.""" """_download_font: plugin fonts declared by URL, cached in temp_font_dir."""
+6 -14
View File
@@ -21,18 +21,6 @@ SPORTS_MODES = ['nfl_live', 'nfl_recent', 'nfl_upcoming',
'ncaa_fb_live', 'ncaa_fb_recent', 'ncaa_fb_upcoming'] 'ncaa_fb_live', 'ncaa_fb_recent', 'ncaa_fb_upcoming']
def _last_write(cache_manager, key):
"""The last ``cache_manager.set(key, ...)`` call.
Not simply the last ``set`` call: the controller's font-usage publisher
thread writes ``font_usage_snapshot`` to the same cache manager whenever
it wakes, so on a slow runner it can land after the write under test.
"""
writes = [c for c in cache_manager.set.call_args_list if c.args and c.args[0] == key]
assert writes, f"nothing was written to {key!r}"
return writes[-1]
def _sports_plugin(has_live_content=False): def _sports_plugin(has_live_content=False):
plugin = MagicMock(spec=['display', 'has_live_content', 'has_live_priority', plugin = MagicMock(spec=['display', 'has_live_content', 'has_live_priority',
'get_live_modes']) 'get_live_modes'])
@@ -99,7 +87,8 @@ class TestANamedLiveModeIsShown:
def test_the_named_mode_survives_a_restart(self, football): def test_the_named_mode_survives_a_restart(self, football):
football._activate_on_demand({'plugin_id': 'football-scoreboard', football._activate_on_demand({'plugin_id': 'football-scoreboard',
'mode': 'ncaa_fb_live'}) 'mode': 'ncaa_fb_live'})
saved = _last_write(football.cache_manager, 'display_on_demand_config') saved = football.cache_manager.set.call_args_list[-1]
assert saved.args[0] == 'display_on_demand_config'
config = saved.args[1] config = saved.args[1]
assert config['named_mode'] == 'ncaa_fb_live' assert config['named_mode'] == 'ncaa_fb_live'
@@ -131,7 +120,10 @@ class TestARestoreWithNothingToResume:
def test_it_is_reported_as_an_error(self, restored): def test_it_is_reported_as_an_error(self, restored):
assert restored.on_demand_status == 'error' assert restored.on_demand_status == 'error'
assert restored.on_demand_last_error == 'restore-failed' assert restored.on_demand_last_error == 'restore-failed'
published = _last_write(restored.cache_manager, 'display_on_demand_state') # The last on-demand state write, not the last write of any key: the
# font-usage publisher thread writes its own key at its own pace.
published = [c for c in restored.cache_manager.set.call_args_list
if c.args and c.args[0] == 'display_on_demand_state'][-1]
assert published.args[1]['status'] == 'error' assert published.args[1]['status'] == 'error'
assert published.args[1]['error'] == 'restore-failed' assert published.args[1]['error'] == 'restore-failed'
@@ -69,6 +69,7 @@ def test_fixed_plugin_loads_new_code_after_failed_load(plugin_env, first_source)
assert MODULE_NAME not in sys.modules assert MODULE_NAME not in sys.modules
assert PLUGIN_ID not in pm.plugin_loader._loaded_modules assert PLUGIN_ID not in pm.plugin_loader._loaded_modules
pm.font_manager.forget_manager_fonts.assert_called_with(PLUGIN_ID) pm.font_manager.forget_manager_fonts.assert_called_with(PLUGIN_ID)
pm.font_manager.forget_plugin_fonts.assert_called_with(PLUGIN_ID)
(plugin_dir / "manager.py").write_text(_FIXED, encoding="utf-8") (plugin_dir / "manager.py").write_text(_FIXED, encoding="utf-8")
assert pm.load_plugin(PLUGIN_ID) is True assert pm.load_plugin(PLUGIN_ID) is True
-244
View File
@@ -1,244 +0,0 @@
"""A plugin update must keep the files the plugin wrote beside itself.
Field incident, 2026-10-04: updating calendar 1.2.9 -> 1.2.12 from the web UI
replaced plugin-repos/calendar/ with the fresh download and deleted the old
copy -- and with it token.pickle and credentials.json, the plugin's Google
OAuth files. No release contains them (the repo gitignores them), so the hot
reload logged "Credentials file not found" and the calendar stayed broken
until the files were restored by hand.
Both update routes are covered: a monorepo plugin (registry ``plugin_path``),
which is reinstalled into a fresh directory, and a plugin installed from its
own git repository, which is updated with ``git pull`` after an auto-stash.
"""
import json
import shutil
import subprocess
import pytest
from src.plugin_system.plugin_local_files import (
is_known_state_file, local_files_to_keep,
)
from src.plugin_system.store_manager import PluginStoreManager
PLUGIN_ID = "calendar"
def _manifest(version):
return {"id": PLUGIN_ID, "name": "Calendar", "class_name": "CalendarPlugin",
"display_modes": ["calendar"], "version": version}
def _write_release(target, version):
"""What a download of ``version`` puts on disk."""
target.mkdir(parents=True, exist_ok=True)
(target / "manifest.json").write_text(json.dumps(_manifest(version)))
(target / "manager.py").write_text(f"VERSION = {version!r}\n")
(target / ".gitignore").write_text("credentials.json\ntoken.pickle\ncache/\n")
def _drop_local_files(plugin_dir):
"""What the plugin writes at runtime: OAuth files plus cached state."""
(plugin_dir / "token.pickle").write_bytes(b"\x80\x04oauth-token")
(plugin_dir / "credentials.json").write_text('{"installed": {}}')
(plugin_dir / "cache").mkdir()
(plugin_dir / "cache" / "events.json").write_text("[]")
def _assert_local_files_kept(plugin_dir):
assert (plugin_dir / "token.pickle").read_bytes() == b"\x80\x04oauth-token"
assert (plugin_dir / "credentials.json").read_text() == '{"installed": {}}'
assert (plugin_dir / "cache" / "events.json").read_text() == "[]"
def _leftover_backups(plugins_dir):
return [p.name for p in plugins_dir.iterdir() if "standalone-backup" in p.name]
@pytest.fixture
def store(tmp_path, monkeypatch):
mgr = PluginStoreManager(
plugins_dir=str(tmp_path / "plugin-repos"),
uninstalled_registry_path=str(tmp_path / "uninstalled.json"))
mgr.plugins_dir.mkdir(parents=True, exist_ok=True)
monkeypatch.setattr(mgr, "_install_dependencies", lambda *a, **k: True)
monkeypatch.setattr(mgr, "fetch_registry", lambda *a, **k: {"plugins": []})
return mgr
class TestMonorepoUpdate:
@pytest.fixture
def installed(self, store, monkeypatch):
registry_entry = {
"id": PLUGIN_ID, "repo": "https://github.com/ChuckBuilds/ledmatrix-plugins",
"plugin_path": "plugins/calendar", "branch": "main",
"latest_version": "1.2.9",
}
monkeypatch.setattr(store, "get_plugin_info", lambda *a, **k: registry_entry)
release = {"version": "1.2.9"}
def fake_monorepo_download(download_url, plugin_subpath, target):
assert plugin_subpath == "plugins/calendar"
_write_release(target, release["version"])
return True
monkeypatch.setattr(store, "_install_from_monorepo", fake_monorepo_download)
assert store.install_plugin(PLUGIN_ID) is True
def publish(version):
registry_entry["latest_version"] = release["version"] = version
return store, store.plugins_dir / PLUGIN_ID, publish
def test_update_keeps_token_and_gitignored_files(self, installed):
store, plugin_dir, publish = installed
_drop_local_files(plugin_dir)
publish("1.2.12")
assert store.update_plugin(PLUGIN_ID) is True
assert json.loads((plugin_dir / "manifest.json").read_text())["version"] == "1.2.12"
_assert_local_files_kept(plugin_dir)
assert _leftover_backups(store.plugins_dir) == []
def test_token_is_kept_even_when_the_release_does_not_gitignore_it(self, installed):
store, plugin_dir, publish = installed
(plugin_dir / ".gitignore").unlink()
(plugin_dir / "token.pickle").write_bytes(b"tok")
(plugin_dir / "config_secrets.json").write_text("{}")
publish("1.2.12")
assert store.update_plugin(PLUGIN_ID) is True
assert (plugin_dir / "token.pickle").read_bytes() == b"tok"
assert (plugin_dir / "config_secrets.json").read_text() == "{}"
def test_release_content_wins_and_old_code_is_not_carried(self, installed):
store, plugin_dir, publish = installed
# A file the old copy had that the new release dropped, byte code, and
# an old copy of a file the new release also ships.
(plugin_dir / "removed_module.py").write_text("OLD = True\n")
(plugin_dir / "__pycache__").mkdir()
(plugin_dir / "__pycache__" / "manager.cpython-313.pyc").write_bytes(b"pyc")
publish("1.2.12")
assert store.update_plugin(PLUGIN_ID) is True
assert not (plugin_dir / "removed_module.py").exists()
assert not (plugin_dir / "__pycache__").exists()
assert "1.2.12" in (plugin_dir / "manager.py").read_text()
def test_reinstall_over_an_existing_copy_keeps_them_too(self, installed):
store, plugin_dir, publish = installed
_drop_local_files(plugin_dir)
assert store.install_plugin(PLUGIN_ID) is True
_assert_local_files_kept(plugin_dir)
assert _leftover_backups(store.plugins_dir) == []
class TestInstallFromUrlReplace:
def test_replacing_an_installed_copy_keeps_the_token(self, store, monkeypatch):
plugin_dir = store.plugins_dir / PLUGIN_ID
_write_release(plugin_dir, "1.0.0")
_drop_local_files(plugin_dir)
def fake_clone(repo_url, target, branches):
_write_release(target, "2.0.0")
return "main"
monkeypatch.setattr(store, "_install_via_git", fake_clone)
result = store.install_from_url(
"https://github.com/example/ledmatrix-calendar", plugin_id=PLUGIN_ID)
assert result["success"] is True
assert json.loads((plugin_dir / "manifest.json").read_text())["version"] == "2.0.0"
_assert_local_files_kept(plugin_dir)
def _git(*args, cwd):
subprocess.run(["git", "-c", "user.email=t@example.com", "-c", "user.name=t",
"-c", "core.autocrlf=false", *args],
cwd=cwd, check=True, capture_output=True)
@pytest.mark.skipif(shutil.which("git") is None, reason="git not installed")
class TestGitRepoUpdate:
@pytest.fixture
def cloned(self, store, tmp_path, monkeypatch):
monkeypatch.setattr(store, "get_plugin_info", lambda *a, **k: None)
upstream = tmp_path / "upstream"
_write_release(upstream, "1.0.0")
# This repo does NOT gitignore the token: an untracked, non-ignored
# file is exactly what `git stash push -u` used to sweep away.
(upstream / ".gitignore").write_text("cache/\n")
_git("init", "-q", "-b", "main", cwd=upstream)
_git("add", ".", cwd=upstream)
_git("commit", "-qm", "1.0.0", cwd=upstream)
plugin_dir = store.plugins_dir / PLUGIN_ID
_git("clone", "-q", str(upstream), str(plugin_dir), cwd=tmp_path)
def publish(version):
(upstream / "manifest.json").write_text(json.dumps(_manifest(version)))
_git("commit", "-qam", version, cwd=upstream)
return store, plugin_dir, publish
def test_pull_update_keeps_untracked_token(self, cloned):
store, plugin_dir, publish = cloned
_drop_local_files(plugin_dir)
# An unrelated untracked file, so the update really does stash.
(plugin_dir / "notes.txt").write_text("scratch")
publish("1.1.0")
assert store.update_plugin(PLUGIN_ID) is True
assert json.loads((plugin_dir / "manifest.json").read_text())["version"] == "1.1.0"
_assert_local_files_kept(plugin_dir)
def test_token_alone_does_not_trigger_a_stash(self, cloned):
store, plugin_dir, publish = cloned
(plugin_dir / "token.pickle").write_bytes(b"tok")
publish("1.1.0")
assert store.update_plugin(PLUGIN_ID) is True
assert (plugin_dir / "token.pickle").read_bytes() == b"tok"
stashes = subprocess.run(["git", "-C", str(plugin_dir), "stash", "list"],
capture_output=True, text=True, check=True)
assert stashes.stdout.strip() == ""
class TestWhatIsKept:
@pytest.mark.parametrize("path,expected", [
("token.pickle", True),
("data/session.pickle", True),
("credentials.json", True),
("token.json", True),
("config_secrets.json", True),
(".pkce_code_verifier", True),
("manager.py", False),
("config.json", False),
])
def test_known_state_files(self, path, expected):
assert is_known_state_file(path) is expected
def test_gitignore_rules(self, tmp_path):
old, new = tmp_path / "old", tmp_path / "new"
new.mkdir()
for rel in ["a.log", "logs/x.txt", "sub/deep/b.log", "keep.log",
"anchored.txt", "sub/anchored.txt", "assets/x/y_backup/z.png",
"manager.py", "shipped.log"]:
(old / rel).parent.mkdir(parents=True, exist_ok=True)
(old / rel).write_text("x")
(new / "shipped.log").write_text("new")
(old / ".gitignore").write_text(
"# comment\n*.log\n!keep.log\nlogs/\n/anchored.txt\n"
"assets/**/*_backup/\n")
assert local_files_to_keep(old, new) == [
"a.log", "anchored.txt", "assets/x/y_backup/z.png",
"logs/x.txt", "sub/deep/b.log",
]
@@ -1131,16 +1131,8 @@ class TestPixletEditorHostDefaultsButDoesNotOverride:
class FakeProcess: class FakeProcess:
pid = 424242 pid = 424242
real_popen = mod.subprocess.Popen
def fake_popen(cmd, *args, env=None, **kwargs): def fake_popen(cmd, *args, env=None, **kwargs):
# Only the editor launch is faked. Patching subprocess.Popen if env is not None:
# patches it for the whole request, and the captive-portal
# before_request hook runs `systemctl is-active hostapd` through
# subprocess.run whenever its 30s cache has expired -- which
# needs a real process (run() uses it as a context manager).
if str(script) not in cmd:
return real_popen(cmd, *args, env=env, **kwargs)
captured['env'] = env captured['env'] = env
return FakeProcess() return FakeProcess()