mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-10-04 14:25:08 +00:00
fix(store): read the core version from disk, not from a stale import (#518)
* fix(store): read the core version from disk, not from a stale import
Updating the core to 3.3.0 and then updating plugins refused all eight sports
scoreboards:
Refusing to install nrl-scoreboard: NRL Scoreboard supports LEDMatrix
>=3.3.0, but this system is running 3.2.0.
while src/__init__.py on that machine read 3.3.0. Observed on hardware, not
theorised.
The gate ran `from src import __version__ as core_version`, which binds
whatever the process loaded at start. The plugin store's gate lives in the web
UI, a long-lived service of its own, and the update route deliberately restarts
nothing -- it replaces files on disk and asks the user to restart. Its prompt
named only the *display* service, so a user who followed it left the web
process holding the previous number.
Stale by exactly one release is the case that bites: every plugin flooring on
the release you just installed is refused, blaming a core version that is
already correct on disk. It reads as a broken plugin store. 3.3.0 is the first
release where this hits a whole family at once, since all eight scoreboards
floor there.
compatibility.current_core_version() reads the version from the file instead,
falling back to the imported value on any failure -- so it can only ever be as
correct as before, never worse. All four gate call sites use it: three in
store_manager (install, the git-pull update path, install_from_url) and one in
plugin_loader's advisory warning.
The restart prompt now names both services.
Twelve tests, including the hardware failure itself: a process holding 3.2.0
while disk says 3.3.0 refuses hockey, and reading fresh allows it. The inverse
is asserted too -- a genuinely old core still refuses, so the gate has not
become permissive. One test greps both modules for the old import-bound read;
reintroducing that line fails it, which is what stops this coming back.
Not changed: web_interface/__init__.py also imports __version__, but for
display rather than gating, and the API endpoint already reports a fresh
git describe.
* fix: drop the unused os import
Left over from a first draft that joined paths by hand before this used
pathlib. Flagged by CodeRabbit on #518; confirmed dead -- no os. reference
remains in the module.
This commit is contained in:
@@ -28,6 +28,7 @@ fixes their version string. See `docs/SPORTS_UNIFICATION.md`, phase B4.
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import re
|
import re
|
||||||
|
from pathlib import Path
|
||||||
from typing import Any, Dict, Optional, Tuple
|
from typing import Any, Dict, Optional, Tuple
|
||||||
|
|
||||||
# Below this, the core's self-reported version is not evidence of anything.
|
# Below this, the core's self-reported version is not evidence of anything.
|
||||||
@@ -35,6 +36,45 @@ from typing import Any, Dict, Optional, Tuple
|
|||||||
TRUSTWORTHY_FLOOR: Tuple[int, int, int] = (2, 0, 0)
|
TRUSTWORTHY_FLOOR: Tuple[int, int, int] = (2, 0, 0)
|
||||||
|
|
||||||
|
|
||||||
|
# ``src/__init__.py`` relative to this file: src/plugin_system/ -> src/
|
||||||
|
_VERSION_FILE = Path(__file__).resolve().parent.parent / "__init__.py"
|
||||||
|
_VERSION_RE = re.compile(r'^__version__\s*=\s*["\']([^"\']+)["\']', re.M)
|
||||||
|
|
||||||
|
|
||||||
|
def current_core_version() -> str:
|
||||||
|
"""The core version as it is on disk right now, not as it was at import.
|
||||||
|
|
||||||
|
``from src import __version__`` binds whatever the process loaded at start.
|
||||||
|
The web UI runs as its own long-lived service (``ledmatrix-web.service``),
|
||||||
|
and updating the core replaces files on disk without restarting it -- the
|
||||||
|
update route says so explicitly and asks the user to restart. Its prompt
|
||||||
|
names the *display* service, so a user who follows it leaves the web
|
||||||
|
process holding the old number.
|
||||||
|
|
||||||
|
The plugin store's gate lives in that web process. Stale by one release is
|
||||||
|
exactly the case that matters: every plugin flooring on the release you
|
||||||
|
just installed gets refused, with a message blaming a core version that is
|
||||||
|
already correct on disk. 3.3.0 is the first release where that hits a whole
|
||||||
|
plugin family at once -- all eight sports scoreboards floor there.
|
||||||
|
|
||||||
|
Reading the file costs one stat and a small read per call, and only on the
|
||||||
|
install/update path. Any failure falls back to the imported value, so this
|
||||||
|
can only ever be as wrong as before, never worse.
|
||||||
|
"""
|
||||||
|
try:
|
||||||
|
text = _VERSION_FILE.read_text(encoding="utf-8")
|
||||||
|
match = _VERSION_RE.search(text)
|
||||||
|
if match:
|
||||||
|
return match.group(1)
|
||||||
|
except (OSError, UnicodeDecodeError):
|
||||||
|
pass
|
||||||
|
try:
|
||||||
|
from src import __version__ as imported
|
||||||
|
return imported
|
||||||
|
except Exception: # noqa: BLE001 - never let this raise
|
||||||
|
return "0.0.0"
|
||||||
|
|
||||||
|
|
||||||
def parse_semver(value: Any) -> Optional[Tuple[int, int, int]]:
|
def parse_semver(value: Any) -> Optional[Tuple[int, int, int]]:
|
||||||
"""Parse ``X.Y.Z`` (extra parts and suffixes ignored) into a comparable
|
"""Parse ``X.Y.Z`` (extra parts and suffixes ignored) into a comparable
|
||||||
3-tuple, or ``None`` when unparseable. A leading ``v`` is tolerated."""
|
3-tuple, or ``None`` when unparseable. A leading ``v`` is tolerated."""
|
||||||
|
|||||||
@@ -772,8 +772,8 @@ class PluginLoader:
|
|||||||
newer than the running core. Advisory only — never raises — so a
|
newer than the running core. Advisory only — never raises — so a
|
||||||
plugin that guards optional features with try/except keeps working.
|
plugin that guards optional features with try/except keeps working.
|
||||||
"""
|
"""
|
||||||
from src import __version__ as core_version
|
|
||||||
from src.plugin_system import compatibility
|
from src.plugin_system import compatibility
|
||||||
|
core_version = compatibility.current_core_version()
|
||||||
|
|
||||||
compatible, _reason = compatibility.check(manifest, core_version)
|
compatible, _reason = compatibility.check(manifest, core_version)
|
||||||
if compatible:
|
if compatible:
|
||||||
|
|||||||
@@ -1467,8 +1467,11 @@ class PluginStoreManager:
|
|||||||
# already had. Allowing it costs them a plugin that raises
|
# already had. Allowing it costs them a plugin that raises
|
||||||
# ModuleNotFoundError at load and is reported only as one line
|
# ModuleNotFoundError at load and is reported only as one line
|
||||||
# in the journal. See docs/SPORTS_UNIFICATION.md (phase B4/B6).
|
# in the journal. See docs/SPORTS_UNIFICATION.md (phase B4/B6).
|
||||||
from src import __version__ as core_version
|
|
||||||
from src.plugin_system import compatibility
|
from src.plugin_system import compatibility
|
||||||
|
# On disk, not as imported: this process may predate the core
|
||||||
|
# update that made the plugin compatible. See
|
||||||
|
# compatibility.current_core_version.
|
||||||
|
core_version = compatibility.current_core_version()
|
||||||
|
|
||||||
compatible, reason = compatibility.check(manifest, core_version)
|
compatible, reason = compatibility.check(manifest, core_version)
|
||||||
if not compatible:
|
if not compatible:
|
||||||
@@ -1612,8 +1615,8 @@ class PluginStoreManager:
|
|||||||
#
|
#
|
||||||
# Before the move, so the `finally` below removes the temp tree and
|
# Before the move, so the `finally` below removes the temp tree and
|
||||||
# nothing half-installed is left behind.
|
# nothing half-installed is left behind.
|
||||||
from src import __version__ as core_version
|
|
||||||
from src.plugin_system import compatibility
|
from src.plugin_system import compatibility
|
||||||
|
core_version = compatibility.current_core_version()
|
||||||
|
|
||||||
compatible, reason = compatibility.check(manifest, core_version)
|
compatible, reason = compatibility.check(manifest, core_version)
|
||||||
if not compatible:
|
if not compatible:
|
||||||
@@ -2644,8 +2647,8 @@ class PluginStoreManager:
|
|||||||
manifest_path, plugin_id, e)
|
manifest_path, plugin_id, e)
|
||||||
return True
|
return True
|
||||||
|
|
||||||
from src import __version__ as core_version
|
|
||||||
from src.plugin_system import compatibility
|
from src.plugin_system import compatibility
|
||||||
|
core_version = compatibility.current_core_version()
|
||||||
|
|
||||||
compatible, reason = compatibility.check(manifest, core_version)
|
compatible, reason = compatibility.check(manifest, core_version)
|
||||||
if compatible:
|
if compatible:
|
||||||
|
|||||||
@@ -0,0 +1,122 @@
|
|||||||
|
"""The gate must read the core version from disk, not from its own import.
|
||||||
|
|
||||||
|
`from src import __version__` binds whatever the process loaded at start. The
|
||||||
|
web UI is a long-lived service of its own (ledmatrix-web.service), and updating
|
||||||
|
the core replaces files on disk without restarting it -- the update route says
|
||||||
|
so and asks the user to restart, but its prompt named only the *display*
|
||||||
|
service, so a user who followed it left the web process holding the old number.
|
||||||
|
|
||||||
|
The plugin store's gate lives in that web process, so being stale by exactly
|
||||||
|
one release is the case that bites: every plugin flooring on the release you
|
||||||
|
just installed is refused, with a message blaming a core version that is
|
||||||
|
already correct on disk.
|
||||||
|
|
||||||
|
Observed on hardware: after updating a rig to 3.3.0 and restarting only the
|
||||||
|
display service, all eight sports scoreboards were refused with
|
||||||
|
"supports LEDMatrix >=3.3.0, but this system is running 3.2.0" while
|
||||||
|
src/__init__.py on that machine read 3.3.0.
|
||||||
|
"""
|
||||||
|
|
||||||
|
import importlib
|
||||||
|
import sys
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
from src.plugin_system import compatibility
|
||||||
|
|
||||||
|
|
||||||
|
class TestCurrentCoreVersion:
|
||||||
|
def test_it_reads_the_file_rather_than_the_imported_value(self, monkeypatch, tmp_path):
|
||||||
|
# Simulate a process whose import predates the update: the module
|
||||||
|
# object says 3.2.0 while the file on disk says 3.3.0.
|
||||||
|
import src
|
||||||
|
monkeypatch.setattr(src, "__version__", "3.2.0")
|
||||||
|
fake = tmp_path / "__init__.py"
|
||||||
|
fake.write_text('__version__ = "3.3.0"\n', encoding="utf-8")
|
||||||
|
monkeypatch.setattr(compatibility, "_VERSION_FILE", fake)
|
||||||
|
assert compatibility.current_core_version() == "3.3.0"
|
||||||
|
|
||||||
|
def test_it_matches_the_real_file_by_default(self):
|
||||||
|
import src
|
||||||
|
assert compatibility.current_core_version() == src.__version__
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("body", [
|
||||||
|
"__version__ = '3.4.1'\n",
|
||||||
|
'__version__="3.4.1"\n',
|
||||||
|
'"""doc"""\n\n__version__ = "3.4.1" # trailing comment\n',
|
||||||
|
])
|
||||||
|
def test_it_tolerates_the_ways_that_line_gets_written(self, monkeypatch, tmp_path, body):
|
||||||
|
fake = tmp_path / "__init__.py"
|
||||||
|
fake.write_text(body, encoding="utf-8")
|
||||||
|
monkeypatch.setattr(compatibility, "_VERSION_FILE", fake)
|
||||||
|
assert compatibility.current_core_version() == "3.4.1"
|
||||||
|
|
||||||
|
def test_a_missing_file_falls_back_to_the_import(self, monkeypatch, tmp_path):
|
||||||
|
# Never worse than before: an unreadable file returns what the old
|
||||||
|
# code would have returned.
|
||||||
|
import src
|
||||||
|
monkeypatch.setattr(src, "__version__", "3.2.0")
|
||||||
|
monkeypatch.setattr(compatibility, "_VERSION_FILE", tmp_path / "gone.py")
|
||||||
|
assert compatibility.current_core_version() == "3.2.0"
|
||||||
|
|
||||||
|
def test_a_file_without_the_line_falls_back(self, monkeypatch, tmp_path):
|
||||||
|
import src
|
||||||
|
monkeypatch.setattr(src, "__version__", "3.2.0")
|
||||||
|
fake = tmp_path / "__init__.py"
|
||||||
|
fake.write_text("# no version here\n", encoding="utf-8")
|
||||||
|
monkeypatch.setattr(compatibility, "_VERSION_FILE", fake)
|
||||||
|
assert compatibility.current_core_version() == "3.2.0"
|
||||||
|
|
||||||
|
def test_it_never_raises(self, monkeypatch, tmp_path):
|
||||||
|
# This runs on the install path; an exception here would surface as a
|
||||||
|
# failed update rather than a version mismatch.
|
||||||
|
bad = tmp_path / "__init__.py"
|
||||||
|
bad.write_bytes(b"\xff\xfe\x00 not utf-8 \xff")
|
||||||
|
monkeypatch.setattr(compatibility, "_VERSION_FILE", bad)
|
||||||
|
assert isinstance(compatibility.current_core_version(), str)
|
||||||
|
|
||||||
|
|
||||||
|
class TestTheBugItFixes:
|
||||||
|
def test_a_stale_import_no_longer_refuses_a_compatible_plugin(self, monkeypatch, tmp_path):
|
||||||
|
"""The exact hardware failure, as a test."""
|
||||||
|
import src
|
||||||
|
monkeypatch.setattr(src, "__version__", "3.2.0") # what the process holds
|
||||||
|
fake = tmp_path / "__init__.py"
|
||||||
|
fake.write_text('__version__ = "3.3.0"\n', encoding="utf-8") # what is on disk
|
||||||
|
monkeypatch.setattr(compatibility, "_VERSION_FILE", fake)
|
||||||
|
|
||||||
|
manifest = {"name": "Hockey Scoreboard", "min_ledmatrix_version": "3.3.0"}
|
||||||
|
|
||||||
|
stale_ok, _ = compatibility.check(manifest, src.__version__)
|
||||||
|
assert stale_ok is False, "precondition: the stale value is what refused it"
|
||||||
|
|
||||||
|
fresh_ok, reason = compatibility.check(
|
||||||
|
manifest, compatibility.current_core_version())
|
||||||
|
assert fresh_ok is True, f"the disk version must allow it, got: {reason}"
|
||||||
|
|
||||||
|
def test_it_still_refuses_when_the_core_really_is_too_old(self, monkeypatch, tmp_path):
|
||||||
|
# The gate must not become permissive: a genuinely old core still says no.
|
||||||
|
fake = tmp_path / "__init__.py"
|
||||||
|
fake.write_text('__version__ = "3.2.0"\n', encoding="utf-8")
|
||||||
|
monkeypatch.setattr(compatibility, "_VERSION_FILE", fake)
|
||||||
|
ok, reason = compatibility.check(
|
||||||
|
{"name": "Hockey", "min_ledmatrix_version": "3.3.0"},
|
||||||
|
compatibility.current_core_version())
|
||||||
|
assert ok is False
|
||||||
|
assert "3.2.0" in (reason or "")
|
||||||
|
|
||||||
|
|
||||||
|
class TestCallSites:
|
||||||
|
@pytest.mark.parametrize("module", [
|
||||||
|
"src.plugin_system.store_manager",
|
||||||
|
"src.plugin_system.plugin_loader",
|
||||||
|
])
|
||||||
|
def test_no_gate_binds_the_version_at_import(self, module):
|
||||||
|
"""Catch a future call site reintroducing the stale read."""
|
||||||
|
import inspect
|
||||||
|
mod = importlib.import_module(module)
|
||||||
|
source = inspect.getsource(mod)
|
||||||
|
assert "from src import __version__ as core_version" not in source, (
|
||||||
|
f"{module} binds __version__ at import; use "
|
||||||
|
f"compatibility.current_core_version() so a long-lived process "
|
||||||
|
f"sees a core update.")
|
||||||
@@ -1164,9 +1164,16 @@
|
|||||||
// The pull replaced files on disk; the running services still
|
// The pull replaced files on disk; the running services still
|
||||||
// hold the code they loaded at boot. Ask for the restart that
|
// hold the code they loaded at boot. Ask for the restart that
|
||||||
// makes the update actually take effect.
|
// makes the update actually take effect.
|
||||||
|
//
|
||||||
|
// Both services, not just the display. The plugin store's
|
||||||
|
// compatibility gate runs in the web process, so a web service
|
||||||
|
// still holding the previous version refuses every plugin that
|
||||||
|
// floors on the release just installed -- blaming a core
|
||||||
|
// version that is already correct on disk.
|
||||||
if (data.restart_required && typeof window.showRestartPending === 'function') {
|
if (data.restart_required && typeof window.showRestartPending === 'function') {
|
||||||
window.showRestartPending(
|
window.showRestartPending(
|
||||||
'Update installed \u2014 restart the display to run the new code');
|
'Update installed \u2014 restart the display and web ' +
|
||||||
|
'services to run the new code');
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if (typeof showNotification === 'function') {
|
if (typeof showNotification === 'function') {
|
||||||
|
|||||||
Reference in New Issue
Block a user