[data_to_ports] don't leave the pattern dirty if we error out part-way

masque/test/test_ports2data.py

@@ -98,3 +98,21 @@ def test_data_to_ports_hierarchical_repeated_ref_warns_and_keeps_best_effort(
     assert "A" in parent.ports
     assert_allclose(parent.ports["A"].offset, [5, 0], atol=1e-10)
     assert any("importing only the base instance ports" in record.message for record in caplog.records)
+
+
+def test_data_to_ports_hierarchical_collision_is_atomic() -> None:
+    lib = Library()
+
+    child = Pattern()
+    layer = (10, 0)
+    child.label(layer=layer, string="A:type1 0", offset=(5, 0))
+    lib["child"] = child
+
+    parent = Pattern()
+    parent.ref("child", offset=(0, 0))
+    parent.ref("child", offset=(10, 0))
+
+    with pytest.raises(PortError, match="Device ports conflict with existing ports"):
+        data_to_ports([layer], lib, parent, max_depth=1)
+
+    assert not parent.ports

masque/utils/ports2data.py

@@ -122,6 +122,7 @@ def data_to_ports(
     if not found_ports:
         return pattern
 
+    imported_ports: dict[str, Port] = {}
     for target, refs in pattern.refs.items():
         if target is None:
             continue
@@ -137,8 +138,10 @@ def data_to_ports(
                 logger.warning(f'Pattern {name if name else pattern} has repeated ref to {target!r}; '
                                'data_to_ports() is importing only the base instance ports')
             aa.apply_ref_transform(ref)
-            pattern.check_ports(other_names=aa.ports.keys())
-            pattern.ports.update(aa.ports)
+            Pattern(ports={**pattern.ports, **imported_ports}).check_ports(other_names=aa.ports.keys())
+            imported_ports.update(aa.ports)
+
+    pattern.ports.update(imported_ports)
     return pattern
 
 
@@ -193,4 +196,3 @@ def data_to_ports_flat(
 
     pattern.ports.update(local_ports)
     return pattern
-