[BuildLibrary] misc simplifications

masque/library.py

@@ -1488,15 +1488,11 @@ class _SourceDeclaration:
     """
     Imported source-backed names registered with a `BuildLibrary`.
 
-    The declaration stores visible-name remapping plus pre-scanned graph
+    The declaration stores visible-name remapping. Underlying source cells stay
-    metadata. Underlying source cells stay lazy until a build session
+    lazy until a build session materializes or copies them through.
-    materializes or copies them through.
     """
     library: ILibraryView
     source_to_visible: Mapping[str, str]
-    visible_to_source: Mapping[str, str]
-    child_graph: Mapping[str, set[str]]
-    order: tuple[str, ...]
 
 
 def cell(func: Callable[..., 'Pattern']) -> _CellFactory:
@@ -1555,8 +1551,7 @@ class BuildLibrary(ILibrary):
     built library plus a `BuildReport`.
     """
 
-    def __init__(self, *, check_on_register: bool = False) -> None:
+    def __init__(self) -> None:
-        self.check_on_register = check_on_register
         self.cells = BuildCellsView(self)
         self._frozen = False
         self._declarations: dict[str, 'Pattern | _BuildRecipe'] = {}
@@ -1629,15 +1624,6 @@ class BuildLibrary(ILibrary):
         self._names.add(key)
         self._order.append(key)
 
-        if self.check_on_register:
-            try:
-                self.validate(names=(key,))
-            except Exception:
-                del self._declarations[key]
-                self._names.remove(key)
-                self._order.remove(key)
-                raise
-
     def __delitem__(self, key: str) -> None:
         session = self._active_session()
         if session is not None:
@@ -1706,10 +1692,16 @@ class BuildLibrary(ILibrary):
                 'Builder-level renames only change the visible imported name.'
                 )
 
-        source_index = next(
+        source_index: int | None = None
-            (idx for idx, spec in enumerate(self._sources) if old_name in spec.visible_to_source),
+        source_name: str | None = None
-            None,
+        for idx, spec in enumerate(self._sources):
-            )
+            for candidate_source, candidate_visible in spec.source_to_visible.items():
+                if candidate_visible == old_name:
+                    source_index = idx
+                    source_name = candidate_source
+                    break
+            if source_index is not None:
+                break
         if source_index is None:
             raise BuildError(
                 f'Cannot rename "{old_name}" during authoring because only imported source-backed '
@@ -1717,21 +1709,14 @@ class BuildLibrary(ILibrary):
                 )
 
         spec = self._sources[source_index]
-        source_name = spec.visible_to_source[old_name]
         source_to_visible = dict(spec.source_to_visible)
-        visible_to_source = dict(spec.visible_to_source)
+        assert source_name is not None
-        order = list(spec.order)
 
         source_to_visible[source_name] = new_name
-        del visible_to_source[old_name]
-        visible_to_source[new_name] = source_name
-        order[order.index(old_name)] = new_name
 
         self._sources[source_index] = replace(
             spec,
             source_to_visible = source_to_visible,
-            visible_to_source = visible_to_source,
-            order = tuple(order),
             )
         self._names.remove(old_name)
         self._names.add(new_name)
@@ -1753,51 +1738,69 @@ class BuildLibrary(ILibrary):
             source: Mapping[str, 'Pattern'] | ILibraryView,
             *,
             rename_theirs: Callable[[ILibraryView, str], str] | None = None,
+            rename_when: Literal['conflict', 'always'] = 'conflict',
             ) -> dict[str, str]:
         """
         Register an imported source-backed library with the builder.
 
-        The source is not materialized immediately. Instead, its names and
+        The source is not materialized immediately. Its names are scanned once
-        child graph are scanned once and stored as an import declaration. The
+        to reserve visible builder names, then the source is read again when a
-        source may be renamed on entry to avoid collisions with existing
+        build session starts. The source's cell membership must not be
+        structurally mutated between `add_source()` and `build()`/`validate()`.
+        Source cells may be renamed on entry to avoid collisions with existing
         declarations or other imported sources.
 
+        Args:
+            rename_theirs: Function used to choose visible names for imported
+                source cells.
+            rename_when: If `'conflict'`, only conflicting names are renamed.
+                If `'always'`, every imported source name is passed through
+                `rename_theirs`.
+
         Returns:
             Mapping of `{source_name: visible_name}` for imported names that
             were renamed while being added.
         """
+        if rename_when not in ('conflict', 'always'):
+            raise ValueError(f'Unknown source rename mode: {rename_when!r}')
+        if rename_when == 'always' and rename_theirs is None:
+            raise TypeError('rename_theirs is required when rename_when="always"')
         if self._active_session() is not None:
             raise BuildError('BuildLibrary.add_source() is only available while authoring, not during validate() or build().')
         self._assert_editable()
 
         view = source if isinstance(source, ILibraryView) else LibraryView(source)
         source_order = tuple(view.source_order())
-        child_graph = view.child_graph(dangling='include')
 
         source_to_visible: dict[str, str] = {}
-        visible_to_source: dict[str, str] = {}
+        visible_names: set[str] = set()
         rename_map: dict[str, str] = {}
         new_names: list[str] = []
 
         for name in source_order:
             visible = name
-            if visible in self._names or visible in visible_to_source:
+            renamed = False
+            if rename_when == 'always':
+                visible = cast('Callable[[ILibraryView, str], str]', rename_theirs)(self, name)
+                renamed = True
+            elif visible in self._names or visible in visible_names:
                 if rename_theirs is None:
                     raise LibraryError(f'Conflicting name while adding source: {name!r}')
                 visible = rename_theirs(self, name)
-                if visible in self._names or visible in visible_to_source:
+                renamed = True
+            if visible in self._names or visible in visible_names:
+                if not renamed:
+                    raise LibraryError(f'Conflicting name while adding source: {name!r}')
                 raise LibraryError(f'Unresolved duplicate key encountered while adding source: {name!r} -> {visible!r}')
+            if visible != name:
                 rename_map[name] = visible
             source_to_visible[name] = visible
-            visible_to_source[visible] = name
+            visible_names.add(visible)
             new_names.append(visible)
 
         self._sources.append(_SourceDeclaration(
             library = view,
             source_to_visible = dict(source_to_visible),
-            visible_to_source = dict(visible_to_source),
-            child_graph = {name: set(children) for name, children in child_graph.items()},
-            order = tuple(source_to_visible[name] for name in source_order),
             ))
         for visible in new_names:
             self._names.add(visible)
@@ -1884,12 +1887,10 @@ class _BuildSessionLibrary(ILibrary):
     """
 
     def __init__(self, builder: BuildLibrary) -> None:
-        from .file.gdsii_lazy_core import OverlayLibrary, _SourceEntry, _SourceLayer      # noqa: PLC0415
+        from .file.gdsii_lazy_core import OverlayLibrary      # noqa: PLC0415
 
         self._builder = builder
         self._overlay = OverlayLibrary()
-        self._source_entry_type = _SourceEntry
-        self._source_layer_type = _SourceLayer
         self._states: dict[str, Literal['unbuilt', 'building', 'built']] = {
             name: 'unbuilt' for name in builder._declarations
             }
@@ -1902,23 +1903,34 @@ class _BuildSessionLibrary(ILibrary):
 
     def _install_sources(self) -> None:
         for spec in self._builder._sources:
-            layer = self._source_layer_type(
+            source_order = spec.library.source_order()
-                library = spec.library,
+            expected_names = set(spec.source_to_visible)
-                source_to_visible = dict(spec.source_to_visible),
+            actual_names = set(source_order)
-                visible_to_source = dict(spec.visible_to_source),
+            if actual_names != expected_names:
-                child_graph = {name: set(children) for name, children in spec.child_graph.items()},
+                added_names = sorted(actual_names - expected_names)
-                order = list(spec.order),
+                removed_names = sorted(expected_names - actual_names)
+                detail = []
+                if added_names:
+                    detail.append(f'added={added_names}')
+                if removed_names:
+                    detail.append(f'removed={removed_names}')
+                raise BuildError(
+                    'Imported source library changed after add_source() was called '
+                    f'({", ".join(detail)}). '
+                    'Do not structurally mutate source libraries between add_source() and build()/validate().'
                     )
-            layer_index = len(self._overlay._layers)
-            self._overlay._layers.append(layer)
 
-            for source_name, visible_name in spec.source_to_visible.items():
+            def rename_source(_lib: ILibraryView, name: str, *, mapping: Mapping[str, str] = spec.source_to_visible) -> str:
-                self._overlay._entries[visible_name] = self._source_entry_type(
+                return mapping[name]
-                    layer_index = layer_index,
+
-                    source_name = source_name,
+            self._overlay.add_source(
+                spec.library,
+                rename_theirs = rename_source,
+                rename_when = 'always',
             )
-                if visible_name not in self._overlay._order:
+
-                    self._overlay._order.append(visible_name)
+            for source_name in source_order:
+                visible_name = spec.source_to_visible[source_name]
                 self._provenance[visible_name] = CellProvenance(
                     requested_name = source_name,
                     kind = 'source',

masque/test/test_build_library.py

@@ -100,21 +100,6 @@ def test_build_library_validate_is_retryable_after_failure() -> None:
     assert report.dependency_graph["parent"] == frozenset({"child"})
 
 
-def test_build_library_check_on_register_rolls_back_failed_declarations() -> None:
-    builder = BuildLibrary(check_on_register=True)
-
-    def make_parent(lib: BuildLibrary) -> Pattern:
-        pat = Pattern()
-        pat.ref("child")
-        lib.abstract("child")
-        return pat
-
-    with pytest.raises(BuildError, match='Failed while building declared cell "parent"'):
-        builder.cells.parent = cell(make_parent)(builder)
-
-    assert "parent" not in builder
-
-
 def test_build_library_depends_on_supports_hidden_dependencies_for_partial_validation() -> None:
     builder = BuildLibrary()
     builder["child"] = Pattern()
@@ -179,6 +164,49 @@ def test_build_library_preserves_source_cells_and_records_source_provenance() ->
     assert report.provenance["src"].kind == "source"
 
 
+def test_build_library_add_source_can_rename_every_source_cell() -> None:
+    source = Library()
+    source["child"] = Pattern()
+    parent = Pattern()
+    parent.ref("child")
+    source["parent"] = parent
+
+    builder = BuildLibrary()
+    rename_map = builder.add_source(
+        source,
+        rename_theirs=lambda _lib, name: f"mapped_{name}",
+        rename_when="always",
+    )
+    built, report = builder.build()
+
+    assert rename_map == {
+        "child": "mapped_child",
+        "parent": "mapped_parent",
+    }
+    assert "mapped_child" in built["mapped_parent"].refs
+    assert report.provenance["mapped_child"].source_name == "child"
+
+
+def test_build_library_rejects_source_cells_added_after_add_source() -> None:
+    source = Library({"src": Pattern()})
+    builder = BuildLibrary()
+    builder.add_source(source)
+    source["late"] = Pattern()
+
+    with pytest.raises(BuildError, match="Do not structurally mutate source libraries"):
+        builder.build()
+
+
+def test_build_library_rejects_source_cells_removed_after_add_source() -> None:
+    source = Library({"src": Pattern()})
+    builder = BuildLibrary()
+    builder.add_source(source)
+    del source["src"]
+
+    with pytest.raises(BuildError, match="Do not structurally mutate source libraries"):
+        builder.build()
+
+
 def test_build_library_rejects_add_source_during_build() -> None:
     builder = BuildLibrary()