Skip to content

Commit 6c1afd6

Browse files
ThVergolofk
authored andcommitted
flows: properly merge EDAM from multiple dependencies
configure_tools' merge_edam was a placeholder that returned only the second argument ('Yeah, I know. It's just a temporary hack'), so a node with several dependencies received the EDAM of whichever dep happened to be merged last and lost all files/hooks/options from the others. Replace it with a real recursive merge: dicts merge key-wise, lists concatenate while skipping entries already present (so files inherited from a common ancestor node aren't duplicated once per dependency), and scalars from later dependencies win. Inputs are left unmodified. Single-dependency nodes see the same content as before.
1 parent e8e25d0 commit 6c1afd6

2 files changed

Lines changed: 68 additions & 4 deletions

File tree

edalize/flows/edaflow.py

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,29 @@ def merge_dict(d1, d2):
7272
return d1
7373

7474

75+
def merge_edam(a, b):
76+
"""Merge two EDAM dicts coming from different flow graph dependencies.
77+
78+
Dicts are merged recursively and scalar values from the second EDAM
79+
win. Lists are concatenated, but entries from the second EDAM that
80+
already exist in the first one are skipped, so that files, hooks etc.
81+
inherited from a common ancestor node don't show up once per
82+
dependency. Neither input is modified.
83+
"""
84+
85+
def _merge(x, y):
86+
if isinstance(x, dict) and isinstance(y, dict):
87+
merged = dict(x)
88+
for key, value in y.items():
89+
merged[key] = _merge(merged[key], value) if key in merged else value
90+
return merged
91+
if isinstance(x, list) and isinstance(y, list):
92+
return x + [e for e in y if e not in x]
93+
return y
94+
95+
return _merge(a, b)
96+
97+
7598
class Node(object):
7699
def __init__(self, name, deps=[], fdto={}, tool=None):
77100
self.deps = deps
@@ -225,10 +248,6 @@ def extract_tool_options(self):
225248
self.edam["tool_options"] = tool_options
226249

227250
def configure_tools(self, graph):
228-
def merge_edam(a, b):
229-
# Yeah, I know. It's just a temporary hack
230-
return b
231-
232251
# Instantiate each node and add to list of unconfigured nodes
233252
unconfigured_nodes = list(graph.get_nodes().values())
234253

tests/test_flow_merge_edam.py

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
from edalize.flows.edaflow import merge_edam
2+
3+
4+
def test_merge_edam():
5+
common = [{"name": "common.v", "file_type": "verilogSource"}]
6+
a = {
7+
"name": "design",
8+
"files": common + [{"name": "a.v", "file_type": "verilogSource"}],
9+
"hooks": {"pre_build": [{"name": "s", "cmd": ["true"]}]},
10+
"tool_options": {"toola": {"opt": ["x"]}},
11+
"toplevel": "top",
12+
}
13+
b = {
14+
"name": "design",
15+
"files": common + [{"name": "b.v", "file_type": "verilogSource"}],
16+
"hooks": {"pre_build": [{"name": "s", "cmd": ["true"]}]},
17+
"tool_options": {"toolb": {"opt": ["y"]}},
18+
"toplevel": "top",
19+
}
20+
21+
merged = merge_edam(a, b)
22+
23+
# Files inherited from a common ancestor only appear once
24+
assert [f["name"] for f in merged["files"]] == ["common.v", "a.v", "b.v"]
25+
assert merged["hooks"]["pre_build"] == [{"name": "s", "cmd": ["true"]}]
26+
# Dicts are merged recursively
27+
assert merged["tool_options"] == {
28+
"toola": {"opt": ["x"]},
29+
"toolb": {"opt": ["y"]},
30+
}
31+
assert merged["name"] == "design"
32+
assert merged["toplevel"] == "top"
33+
34+
# Inputs are not modified
35+
assert [f["name"] for f in a["files"]] == ["common.v", "a.v"]
36+
assert [f["name"] for f in b["files"]] == ["common.v", "b.v"]
37+
38+
39+
def test_merge_edam_scalar_conflict():
40+
assert merge_edam({"toplevel": "a"}, {"toplevel": "b"})["toplevel"] == "b"
41+
42+
43+
def test_merge_edam_empty():
44+
edam = {"name": "design", "files": [{"name": "a.v"}]}
45+
assert merge_edam({}, edam) == edam

0 commit comments

Comments
 (0)