core: a bad relation must not take healthy modules with it
A cycle rejected everything still waiting, not just the modules on it. One that merely says 'after B' is not itself circular, and once B is dropped its relation has nothing to wait for -- the same as naming a module that was never installed. Now only the ids that can reach themselves are dropped, and a dropped id stops being a dependency so the damage stops rather than spreading downstream. A relation could also overturn the stage order: 'load after conditioning' was honoured, putting a whole stage out of sequence to satisfy one module. Stage is the coarse order; an impossible relation is dropped and logged, and the module itself is kept. panels.js now keeps what a module's setup() returns, which is its unsubscribe -- discarding it leaves a live listener behind on every re-mount.
This commit is contained in:
+12
-2
@@ -36,6 +36,7 @@ export async function mountAll(manifest, { load = (path) => import(path) } = {})
|
||||
|
||||
let ui = null;
|
||||
if (spec.ui) ui = await load(spec.ui);
|
||||
let teardown = null;
|
||||
|
||||
const panel = renderPanel(spec, {
|
||||
values: values.valuesOf(spec.id),
|
||||
@@ -46,7 +47,9 @@ export async function mountAll(manifest, { load = (path) => import(path) } = {})
|
||||
host.appendChild(panel.node);
|
||||
|
||||
if (ui && typeof ui.setup === "function") {
|
||||
ui.setup({
|
||||
// Kept, not discarded: setup() returns the unsubscribe from on(), and
|
||||
// dropping it means every re-mount leaves another live listener behind.
|
||||
teardown = ui.setup({
|
||||
composer,
|
||||
values: {
|
||||
get: () => values.valuesOf(spec.id),
|
||||
@@ -65,7 +68,14 @@ export async function mountAll(manifest, { load = (path) => import(path) } = {})
|
||||
});
|
||||
}
|
||||
|
||||
mounted.push({ id: spec.id, panel });
|
||||
mounted.push({
|
||||
id: spec.id,
|
||||
panel,
|
||||
destroy() {
|
||||
if (typeof teardown === "function") teardown();
|
||||
panel.destroy();
|
||||
},
|
||||
});
|
||||
} catch (err) {
|
||||
hidden.push({ id: spec.id, why: `${err.name}: ${err.message}` });
|
||||
console.warn(`[FunPack] ${spec.id} did not mount: ${err.message}`);
|
||||
|
||||
+63
-13
@@ -6,8 +6,9 @@ about each other. A relation is local -- a module names only the neighbours it
|
||||
actually cares about, and everything else sorts itself.
|
||||
"""
|
||||
|
||||
from typing import Dict, Iterable, List, Tuple
|
||||
from typing import Dict, Iterable, List, Set, Tuple
|
||||
|
||||
from . import log
|
||||
from .contract import STAGES, ModuleSpec
|
||||
|
||||
|
||||
@@ -26,22 +27,35 @@ def order(specs: Iterable[ModuleSpec]) -> Tuple[List[ModuleSpec], List[Tuple[Mod
|
||||
by_id = {spec.id: spec for spec in specs}
|
||||
rejected: List[Tuple[ModuleSpec, str]] = []
|
||||
|
||||
stage_of = {spec.id: STAGES.index(spec.stage) for spec in specs}
|
||||
|
||||
# Edges point from "runs earlier" to "runs later". `before` is expressed as
|
||||
# the same edge from the other end, so the two spellings cannot disagree.
|
||||
edges: Dict[str, set] = {spec.id: set() for spec in specs}
|
||||
|
||||
def _link(later: str, earlier: str) -> None:
|
||||
# Stage is the coarse order and a relation may not overturn it: a load
|
||||
# module cannot meaningfully run after a conditioning one, and honouring
|
||||
# that would put a whole stage out of sequence to satisfy one module.
|
||||
# The relation is dropped and said out loud rather than silently obeyed.
|
||||
if stage_of[later] < stage_of[earlier]:
|
||||
log.note(
|
||||
f"ignoring {later!r} after {earlier!r}: {later!r} is in stage "
|
||||
f"{STAGES[stage_of[later]]!r}, which runs before {STAGES[stage_of[earlier]]!r}"
|
||||
)
|
||||
return
|
||||
edges[later].add(earlier)
|
||||
|
||||
for spec in specs:
|
||||
# A relation naming an absent module is not an error: modules come and
|
||||
# go, and "after audio_clock" has nothing to wait for when audio_clock
|
||||
# failed to load or was never installed.
|
||||
for earlier in spec.after:
|
||||
if earlier in by_id:
|
||||
edges[spec.id].add(earlier)
|
||||
_link(spec.id, earlier)
|
||||
for later in spec.before:
|
||||
if later in by_id:
|
||||
edges[later].add(spec.id)
|
||||
|
||||
# A relation naming an absent module is not an error: modules come and go,
|
||||
# and "after audio_clock" simply has nothing to wait for when audio_clock
|
||||
# failed to load or was never installed.
|
||||
|
||||
stage_of = {spec.id: STAGES.index(spec.stage) for spec in specs}
|
||||
_link(later, spec.id)
|
||||
ordered: List[ModuleSpec] = []
|
||||
placed: set = set()
|
||||
|
||||
@@ -52,10 +66,28 @@ def order(specs: Iterable[ModuleSpec]) -> Tuple[List[ModuleSpec], List[Tuple[Mod
|
||||
while remaining:
|
||||
ready = [s for s in remaining if edges[s.id] <= placed]
|
||||
if not ready:
|
||||
for spec in remaining:
|
||||
unmet = sorted(edges[spec.id] - placed)
|
||||
rejected.append((spec, f"ordering cycle with {', '.join(unmet)}"))
|
||||
break
|
||||
# Drop only the modules actually ON a cycle. Dropping everything
|
||||
# still waiting takes healthy modules with it -- one that merely
|
||||
# says "after B" is not itself circular, and once B is gone its
|
||||
# relation simply has nothing to wait for.
|
||||
live = {s.id for s in remaining}
|
||||
looping = _on_a_cycle(edges, live)
|
||||
for spec in [s for s in remaining if s.id in looping]:
|
||||
others = sorted(edges[spec.id] & looping)
|
||||
rejected.append((spec, f"ordering cycle with {', '.join(others)}"))
|
||||
remaining.remove(spec)
|
||||
# A dropped module stops being something to wait for. Otherwise
|
||||
# everything downstream of the cycle waits on an id that will never
|
||||
# arrive, and the collateral damage spreads instead of stopping.
|
||||
for waiting in edges.values():
|
||||
waiting -= looping
|
||||
if not looping:
|
||||
# Nothing is circular and nothing is ready: unreachable, but
|
||||
# looping forever would be worse than saying so.
|
||||
for spec in remaining:
|
||||
rejected.append((spec, "could not be ordered"))
|
||||
break
|
||||
continue
|
||||
# Among the ready, respect stage so a later stage never jumps ahead.
|
||||
nxt = min(ready, key=lambda s: (stage_of[s.id], s.id))
|
||||
ordered.append(nxt)
|
||||
@@ -63,3 +95,21 @@ def order(specs: Iterable[ModuleSpec]) -> Tuple[List[ModuleSpec], List[Tuple[Mod
|
||||
remaining.remove(nxt)
|
||||
|
||||
return ordered, rejected
|
||||
|
||||
|
||||
def _on_a_cycle(edges: Dict[str, set], live: Set[str]) -> Set[str]:
|
||||
"""Ids that can reach themselves through the edges still in play."""
|
||||
on_cycle: Set[str] = set()
|
||||
for start in live:
|
||||
seen: Set[str] = set()
|
||||
stack = [n for n in edges[start] if n in live]
|
||||
while stack:
|
||||
node = stack.pop()
|
||||
if node == start:
|
||||
on_cycle.add(start)
|
||||
break
|
||||
if node in seen:
|
||||
continue
|
||||
seen.add(node)
|
||||
stack.extend(n for n in edges.get(node, ()) if n in live)
|
||||
return on_cycle
|
||||
|
||||
@@ -158,3 +158,66 @@ def test_traits_are_names_the_model_offers_not_model_names(scanned):
|
||||
for spec in scanned.specs.values():
|
||||
for trait in spec.requires:
|
||||
assert trait.islower() and " " not in trait
|
||||
|
||||
|
||||
# --- ordering: what a bad relation must not take with it -------------------
|
||||
|
||||
def test_a_cycle_does_not_drop_the_modules_that_merely_depend_on_it():
|
||||
# A module saying "after B" is not itself circular. Once B is gone its
|
||||
# relation has nothing to wait for -- exactly like a relation naming a
|
||||
# module that was never installed.
|
||||
from core.schema import validate
|
||||
b = validate({"id": "b", "title": "B", "mount": "m", "after": ["c"]})
|
||||
c = validate({"id": "c", "title": "C", "mount": "m", "after": ["b"]})
|
||||
a = validate({"id": "a", "title": "A", "mount": "m", "after": ["b"]})
|
||||
d = validate({"id": "d", "title": "D", "mount": "m"})
|
||||
|
||||
ordered, rejected = order([a, b, c, d])
|
||||
assert {s.id for s in ordered} == {"a", "d"}
|
||||
assert {s.id for s, _ in rejected} == {"b", "c"}
|
||||
|
||||
|
||||
def test_a_longer_cycle_is_found_whole():
|
||||
from core.schema import validate
|
||||
ring = [
|
||||
validate({"id": "x", "title": "X", "mount": "m", "after": ["z"]}),
|
||||
validate({"id": "y", "title": "Y", "mount": "m", "after": ["x"]}),
|
||||
validate({"id": "z", "title": "Z", "mount": "m", "after": ["y"]}),
|
||||
]
|
||||
healthy = validate({"id": "ok", "title": "OK", "mount": "m", "after": ["x"]})
|
||||
ordered, rejected = order(ring + [healthy])
|
||||
assert [s.id for s in ordered] == ["ok"]
|
||||
assert {s.id for s, _ in rejected} == {"x", "y", "z"}
|
||||
|
||||
|
||||
def test_a_relation_may_not_overturn_the_stage_order():
|
||||
# Stage is the coarse order. Honouring "load after conditioning" would put a
|
||||
# whole stage out of sequence to satisfy one module's relation.
|
||||
from core.schema import validate
|
||||
late = validate({"id": "late", "title": "L", "mount": "m", "stage": "post"})
|
||||
early = validate({"id": "early", "title": "E", "mount": "m", "stage": "load",
|
||||
"after": ["mid"]})
|
||||
mid = validate({"id": "mid", "title": "M", "mount": "m", "stage": "conditioning"})
|
||||
|
||||
ordered, rejected = order([late, early, mid])
|
||||
ids = [s.id for s in ordered]
|
||||
assert rejected == [], "the module is kept; only its impossible relation is dropped"
|
||||
assert ids == ["early", "mid", "late"], f"stages out of order: {ids}"
|
||||
|
||||
|
||||
def test_before_may_not_overturn_the_stage_order_either():
|
||||
from core.schema import validate
|
||||
early = validate({"id": "early", "title": "E", "mount": "m", "stage": "load"})
|
||||
late = validate({"id": "late", "title": "L", "mount": "m", "stage": "post",
|
||||
"before": ["early"]})
|
||||
ordered, _ = order([late, early])
|
||||
assert [s.id for s in ordered] == ["early", "late"]
|
||||
|
||||
|
||||
def test_a_relation_within_a_stage_is_still_honoured():
|
||||
from core.schema import validate
|
||||
first = validate({"id": "first", "title": "F", "mount": "m", "stage": "latent"})
|
||||
second = validate({"id": "second", "title": "S", "mount": "m", "stage": "latent",
|
||||
"after": ["first"]})
|
||||
ordered, _ = order([second, first])
|
||||
assert [s.id for s in ordered] == ["first", "second"]
|
||||
|
||||
Reference in New Issue
Block a user