From c420ac8085626c5459f1c4bfa54d0855e6c9afa4 Mon Sep 17 00:00:00 2001 From: Jan Petykiewicz Date: Fri, 19 Jun 2026 22:48:21 -0700 Subject: [PATCH] [BuildLibrary] misc simplifications --- masque/library.py | 132 ++++++++++++++++-------------- masque/test/test_build_library.py | 58 +++++++++---- 2 files changed, 115 insertions(+), 75 deletions(-) diff --git a/masque/library.py b/masque/library.py index 59a1655..8f8d842 100644 --- a/masque/library.py +++ b/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 - metadata. Underlying source cells stay lazy until a build session - materializes or copies them through. + The declaration stores visible-name remapping. Underlying source cells stay + lazy until a build session 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: - self.check_on_register = check_on_register + def __init__(self) -> None: 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( - (idx for idx, spec in enumerate(self._sources) if old_name in spec.visible_to_source), - None, - ) + source_index: int | None = None + source_name: str | 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) - order = list(spec.order) + assert source_name is not None 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 - child graph are scanned once and stored as an import declaration. The - source may be renamed on entry to avoid collisions with existing + The source is not materialized immediately. Its names are scanned once + to reserve visible builder names, then the source is read again when a + 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: - raise LibraryError(f'Unresolved duplicate key encountered while adding source: {name!r} -> {visible!r}') + 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( - library = spec.library, - source_to_visible = dict(spec.source_to_visible), - visible_to_source = dict(spec.visible_to_source), - child_graph = {name: set(children) for name, children in spec.child_graph.items()}, - order = list(spec.order), - ) - layer_index = len(self._overlay._layers) - self._overlay._layers.append(layer) - - for source_name, visible_name in spec.source_to_visible.items(): - self._overlay._entries[visible_name] = self._source_entry_type( - layer_index = layer_index, - source_name = source_name, + source_order = spec.library.source_order() + expected_names = set(spec.source_to_visible) + actual_names = set(source_order) + if actual_names != expected_names: + added_names = sorted(actual_names - expected_names) + 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().' ) - if visible_name not in self._overlay._order: - self._overlay._order.append(visible_name) + + def rename_source(_lib: ILibraryView, name: str, *, mapping: Mapping[str, str] = spec.source_to_visible) -> str: + return mapping[name] + + self._overlay.add_source( + spec.library, + rename_theirs = rename_source, + rename_when = 'always', + ) + + 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', diff --git a/masque/test/test_build_library.py b/masque/test/test_build_library.py index 4bd645f..2e7cb27 100644 --- a/masque/test/test_build_library.py +++ b/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()