vars: simplify graph implementation, remove obsolete closure functions

- full_closure is obsolete since it is the same as calling requested_closure with the full list of generators.
- minimal_closure is obsolete as well. Since the recent addition of dependents to the closure via 3d2127ce1e it is essentially the same as the all_missing_closure

pkgs/clan-cli/clan_cli/vars/graph.py

@@ -87,31 +87,27 @@ def toposort_closure(
     return [generators[gen_key] for gen_key in result]
 
 
-# all generators in topological order
-def full_closure(generators: dict[GeneratorKey, Generator]) -> list[Generator]:
-    """From a set of generators, return all generators in topological order.
-    This includes all dependencies and dependents of the generators.
-    Returns all generators in topological order.
-    """
-    return toposort_closure(generators.keys(), generators)
-
-
 # just the missing generators including their dependents
-def all_missing_closure(generators: dict[GeneratorKey, Generator]) -> list[Generator]:
+def all_missing_closure(
+    requested_generators: Iterable[GeneratorKey],
+    generators: dict[GeneratorKey, Generator],
+) -> list[Generator]:
     """From a set of generators, return all incomplete generators in topological order.
 
     incomplete
     : A generator is missing if at least one of its files is missing.
     """
     # collect all generators that are missing from disk
-    closure = {gen_key for gen_key, gen in generators.items() if not gen.exists}
+    closure = {
+        gen_key for gen_key in requested_generators if not generators[gen_key].exists
+    }
     closure = add_dependents(closure, generators)
     return toposort_closure(closure, generators)
 
 
 # only a selected list of generators including their missing dependencies and their dependents
 def requested_closure(
-    requested_generators: list[GeneratorKey],
+    requested_generators: Iterable[GeneratorKey],
     generators: dict[GeneratorKey, Generator],
 ) -> list[Generator]:
     closure = set(requested_generators)
@@ -119,23 +115,3 @@ def requested_closure(
     closure = add_missing_dependencies(closure, generators)
     closure = add_dependents(closure, generators)
     return toposort_closure(closure, generators)
-
-
-# just enough to ensure that the list of selected generators are in a consistent state.
-# empty if nothing is missing.
-# Theoretically we could have a more minimal closure that does not include dependents of
-# the requested generators, but this would introduce the potential for previously
-# generated dependents being out of sync.
-def minimal_closure(
-    requested_generators: list[GeneratorKey],
-    generators: dict[GeneratorKey, Generator],
-) -> list[Generator]:
-    closure = set(requested_generators)
-    final_closure = missing_dependency_closure(closure, generators)
-    # add requested generators if not already exist
-    for gen_key in closure:
-        if not generators[gen_key].exists:
-            final_closure.add(gen_key)
-    # add dependents of final_closure
-    final_closure = add_dependents(final_closure, generators)
-    return toposort_closure(final_closure, generators)

pkgs/clan-cli/clan_cli/vars/graph_test.py

@@ -5,6 +5,10 @@ from clan_cli.vars.generator import (
 from clan_cli.vars.graph import all_missing_closure, requested_closure
 
 
+def generator_names(generator: list[Generator]) -> list[str]:
+    return [gen.name for gen in generator]
+
+
 def test_required_generators() -> None:
     # Create generators with proper machine context
     machine_name = "test_machine"
@@ -33,9 +37,6 @@ def test_required_generators() -> None:
         generator.key: generator for generator in [gen_1, gen_2, gen_2a, gen_2b]
     }
 
-    def generator_names(generator: list[Generator]) -> list[str]:
-        return [gen.name for gen in generator]
-
     assert generator_names(requested_closure([gen_1.key], generators)) == [
         "gen_1",
         "gen_2",
@@ -58,8 +59,50 @@ def test_required_generators() -> None:
         "gen_2b",
     ]
 
-    assert generator_names(all_missing_closure(generators)) == [
+    assert generator_names(all_missing_closure(generators.keys(), generators)) == [
         "gen_2",
         "gen_2a",
         "gen_2b",
     ]
+
+
+def test_shared_generator_invalidates_multiple_machines_dependents() -> None:
+    # Create generators with proper machine context
+    machine_1 = "machine_1"
+    machine_2 = "machine_2"
+    shared_gen = Generator(
+        name="shared_gen",
+        dependencies=[],
+        machine=None,  # Shared generator
+    )
+    gen_1 = Generator(
+        name="gen_1",
+        dependencies=[shared_gen.key],
+        machine=machine_1,
+    )
+    gen_2 = Generator(
+        name="gen_2",
+        dependencies=[shared_gen.key],
+        machine=machine_2,
+    )
+
+    shared_gen.exists = False
+    gen_1.exists = True
+    gen_2.exists = True
+    generators: dict[GeneratorKey, Generator] = {
+        generator.key: generator for generator in [shared_gen, gen_1, gen_2]
+    }
+
+    assert generator_names(all_missing_closure(generators.keys(), generators)) == [
+        "shared_gen",
+        "gen_1",
+        "gen_2",
+    ], (
+        "All generators should be included in all_missing_closure due to shared dependency"
+    )
+
+    assert generator_names(requested_closure([shared_gen.key], generators)) == [
+        "shared_gen",
+        "gen_1",
+        "gen_2",
+    ], "All generators should be included in requested_closure due to shared dependency"