diff --git a/src/devicetree/edtlib.py b/src/devicetree/edtlib.py index b7d5366..7f212ed 100644 --- a/src/devicetree/edtlib.py +++ b/src/devicetree/edtlib.py @@ -240,29 +240,32 @@ def __init__( # this binding itself defines or modifies self.prop2specs: Dict[str, "PropertySpec"] = {} + # This binding's child-binding, + # using the same approach as above for property specifications: + # - we initially have no child-binding + # - we may inherit child-binding definitions from other bindings + # when merging included files + # - eventually, we may update inherited child-binding definitions + # with the content of the "child-binding" element of this binding's file + self.child_binding: Optional[Binding] = None + # Save child-binding element before merging. + if "child-binding" in raw: + raw_child_binding = deepcopy(raw["child-binding"]) + else: + raw_child_binding = None + # Merge any included files into self.raw. This also pulls in # inherited child binding definitions, so it has to be done - # before initializing those. + # before initializing this binding's child-binding. self.raw: dict = self._merge_includes(raw, self.path) - # Recursively initialize any child bindings. These don't - # require a 'compatible' or 'description' to be well defined, - # but they must be dicts. - if "child-binding" in raw: - if not isinstance(raw["child-binding"], dict): - _err( - f"malformed 'child-binding:' in {self.path}, " - "expected a binding (dictionary with keys/values)" - ) - self.child_binding: Optional["Binding"] = Binding( - path, - fname2path, - raw=raw["child-binding"], - require_compatible=False, - require_description=False, - ) - else: - self.child_binding = None + # When this binding's file contains a child-binding element, + # its content will: + # - update the child-binding definitions inherited from included files, + # if any + # - or initialize this binding's child-binding + if raw_child_binding: + self._update_child_binding(raw_child_binding) # Make sure this is a well defined object. self._check(require_compatible, require_description) @@ -446,6 +449,18 @@ def _load_raw( # Register included property specs. self._add_included_prop2specs(fname, contents, allowlist, blocklist) + # Pull in child-binding definitions we may inherit + # from the included file. + if "child-binding" in contents: + self._add_included_child_binding( + path, + contents["child-binding"], + # Properties in child-binding have already been filtered, + # child_filter is only used to determine if the including + # binding takes the "Last modified here" semantic. + child_filter, + ) + return self._merge_includes(contents, path) def _add_included_prop2specs( @@ -491,6 +506,126 @@ def _add_included_prop2specs( if prop not in self.prop2specs: self.prop2specs[prop] = spec + # Add child-binding definitions inherited from an included binding. + # - path: YAML file containing the "child-binding" element + # - raw: The content of the "child-binding" element + # - child_filter: filters applied by the including binding + def _add_included_child_binding( + self, + path: Optional[str], + raw: Dict[str, Any], + child_filter: Optional[dict], + ) -> None: + inc_child_binding = self._mk_child_binding(path, raw) + + if self.child_binding or child_filter: + # Take "Last modified here" semantic when: + # - we've already included a child-binding + # - the including binding applies some filter + # to the included child-binding's properties + child_binding = self._mk_child_binding(self.path, {}) + if self.child_binding: + self._merge_child_binding(child_binding, self.child_binding) + self.child_binding = child_binding + + if self.child_binding: + self._merge_child_binding(self.child_binding, inc_child_binding) + else: + self.child_binding = inc_child_binding + + # Either: + # - update child-binding definitions we may inherited from included bindings + # - initialize this binding's child binding with its child-binding element + # + # raw_child_binding: content of the child-binding element + # in this binding file. + def _update_child_binding(self, raw_child_binding: Dict[str, Any]) -> None: + if self.child_binding: + raw_properties = raw_child_binding.get("properties", {}) + if raw_properties: + # This binding's child-binding element may redefine properties + # we've already inherited from included bindings. + # This will typically happen when the including file + # (this binding) set "required: true" on the inherited property. + # We can't create a child binding based solely on this latest + # definition: we before have to update the redefined properties + # with the attributes (type, description) we inherited. + for prop, raw in raw_properties.items(): + if prop in self.child_binding.prop2specs: + # A bit convoluted, but avoid accessing private + # member PropertySpec._raw. + inc_raw_prop = self.child_binding.prop2specs[ + prop + ].binding.raw.get("properties", {})[prop] + raw.update(inc_raw_prop) + + # Last modified child-binding. + last_child_binding = self._mk_child_binding( + self.path, raw_child_binding + ) + + if self.child_binding: + # Merge this file's child-binding element with the child-binding + # definitions we inherited from included files. + if self.child_binding.path != self.path: + # Taking "Last modified here" semantic if not already taken + # when we merged the included bindings, e.g. if the + # including file filtered the included child-binding's properties. + child_binding = self._mk_child_binding(self.path, {}) + self._merge_child_binding(child_binding, self.child_binding) + self.child_binding = child_binding + + self._merge_child_binding(self.child_binding, last_child_binding) + else: + self.child_binding = last_child_binding + + # Merge child bindings: + # - update child_binding's properties with properties + # from other_child_binding + # - update child_binding's cell specifiers with specifiers + # from other_child_binding + # - recursively merge child bindings of child_binding + # and other_child_binding + def _merge_child_binding( + self, + child_binding: "Binding", + other_child_binding: "Binding", + ) -> None: + child_binding.prop2specs.update(other_child_binding.prop2specs) + child_binding.specifier2cells.update( + other_child_binding.specifier2cells + ) + + other_grandchild_binding = other_child_binding.child_binding + if other_grandchild_binding: + if child_binding.child_binding: + self._merge_child_binding( + child_binding.child_binding, other_grandchild_binding + ) + else: + child_binding.child_binding = other_grandchild_binding + + def _mk_child_binding( + self, path: Optional[str], raw: Dict[str, Any] + ) -> "Binding": + # These don't require a 'compatible' or 'description' to be well defined, + # but they must be dicts. + if not isinstance(raw, dict): + _err( + f"malformed 'child-binding:' in {path}, " + "expected a binding (dictionary with keys/values)" + ) + return Binding( + path, + self._fname2path, + raw, + require_compatible=False, + require_description=False, + # Filters already applied. + inc_allowlist=None, + inc_blocklist=None, + ) + def _check(self, require_compatible: bool, require_description: bool): # Does sanity checking on the binding. diff --git a/tests/res/edtlib/allow-and-blocklist-anylevel.yaml b/tests/res/edtlib/allow-and-blocklist-anylevel.yaml new file mode 100644 index 0000000..604dad4 --- /dev/null +++ b/tests/res/edtlib/allow-and-blocklist-anylevel.yaml @@ -0,0 +1,71 @@ +# SPDX-License-Identifier: Apache-2.0 + +description: | + Extends edtlib's upstream allow-and-blocklist-multilevel.yaml test case, adds: + - check of bindings and property specs paths + - check of grand-grandchild-binding level + - check grandchild-binding filters + - check setting "required: true" won't truncate included + child-binding's property definitions + + All child-binding definitions come from a single file, + included multiple times, at different levels, + with different filters. + +compatible: vnd,allow-and-blocklist-anylevel + +# child-binding: +# + child-prop-1 +# +# grandchild-binding: +# + gandchild-prop-1 +# + gandchild-prop-2 +include: + - name: base.yaml + property-allowlist: [x] + + child-binding: + property-allowlist: [child-prop-1] + + child-binding: + property-blocklist: [grandchild-prop-blocked] + +properties: + ceil-prop: + type: int + +child-binding: + include: + # child-binding: + # ~ child-prop-1 + # + x + # + z + - name: base.yaml + property-blocklist: [y] + + # grandchild-binding: + # ~ gandchild-prop-1 + # ~ gandchild-prop-2 + # + child-prop-1 + # + child-prop-1 + child-binding: + property-blocklist: [child-prop-blocked] + + # grandgrandchild-binding: + # + gandchild-prop-1 + # + gandchild-prop-2 + child-binding: + property-blocklist: [grandchild-prop-blocked] + + properties: + child-prop-1: + required: true + z: + required: true + cb-prop: + type: int + + child-binding: + properties: + gcbb-prop: + type: int diff --git a/tests/res/edtlib/allow-and-blocklist-multilevel.yaml b/tests/res/edtlib/allow-and-blocklist-multilevel.yaml new file mode 100644 index 0000000..cd33ff8 --- /dev/null +++ b/tests/res/edtlib/allow-and-blocklist-multilevel.yaml @@ -0,0 +1,16 @@ +# SPDX-License-Identifier: BSD-3-Clause + +description: | + Includes can be added at any level, so can property-allowlist and + property-blocklist. + +compatible: allow-and-blocklist-multilevel + +include: + - name: include.yaml + property-allowlist: [x] + +child-binding: + include: + - name: include.yaml + property-blocklist: [y] diff --git a/tests/res/edtlib/base.yaml b/tests/res/edtlib/base.yaml new file mode 100644 index 0000000..af959fe --- /dev/null +++ b/tests/res/edtlib/base.yaml @@ -0,0 +1,30 @@ +# SPDX-License-Identifier: Apache-2.0 + +description: | + Base include file for the allow-and-blocklist-anylevel.yaml test case. + +properties: + x: + type: int + y: + type: int + z: + type: int + +child-binding: + properties: + child-prop-1: + type: int + child-prop-2: + type: int + child-prop-blocked: + type: int + + child-binding: + properties: + grandchild-prop-1: + type: int + grandchild-prop-2: + type: int + grandchild-prop-blocked: + type: int diff --git a/tests/res/edtlib/include.yaml b/tests/res/edtlib/include.yaml new file mode 100644 index 0000000..4559920 --- /dev/null +++ b/tests/res/edtlib/include.yaml @@ -0,0 +1,24 @@ +# SPDX-License-Identifier: BSD-3-Clause + +description: Test file for including other bindings +compatible: include +properties: + x: + type: int + y: + type: int + z: + type: int +child-binding: + properties: + child-prop-1: + type: int + child-prop-2: + type: int + + child-binding: + properties: + grandchild-prop-1: + type: int + grandchild-prop-2: + type: int diff --git a/tests/res/edtlib/left.yaml b/tests/res/edtlib/left.yaml new file mode 100644 index 0000000..a9e3348 --- /dev/null +++ b/tests/res/edtlib/left.yaml @@ -0,0 +1,17 @@ +# SPDX-License-Identifier: Apache-2.0 + +description: | + Left include file for left_rigt.yaml. + +properties: + left-prop-1: + type: int + left-prop-2: + type: int + +child-binding: + properties: + left-child-prop-1: + type: int + left-child-prop-2: + type: int diff --git a/tests/res/edtlib/left_right.yaml b/tests/res/edtlib/left_right.yaml new file mode 100644 index 0000000..69f3bd3 --- /dev/null +++ b/tests/res/edtlib/left_right.yaml @@ -0,0 +1,35 @@ +# SPDX-License-Identifier: Apache-2.0 + +description: | + Complements allow-and-blocklist-anylevel.yaml test case, + here child-binding definitions come from two files. + +compatible: vnd,left_right + +include: + - name: left.yaml + property-allowlist: [left-prop-1] + child-binding: + property-allowlist: [left-child-prop-1] + + - name: right.yaml + property-allowlist: [right-prop-2] + child-binding: + property-allowlist: [right-child-prop-2] + +properties: + left-prop-1: + required: true + left-right-prop: + type: int + +child-binding: + properties: + left-child-prop-1: + required: true + child-prop: + type: int + child-binding: + properties: + grandchild-prop: + type: int diff --git a/tests/res/edtlib/props_def.yaml b/tests/res/edtlib/props_def.yaml new file mode 100644 index 0000000..90a7784 --- /dev/null +++ b/tests/res/edtlib/props_def.yaml @@ -0,0 +1,13 @@ +# SPDX-License-Identifier: Apache-2.0 + +description: | + Include file for testing property redefinition. + +properties: + x: + type: int + +child-binding: + properties: + child-prop-1: + type: int diff --git a/tests/res/edtlib/props_redef.yaml b/tests/res/edtlib/props_redef.yaml new file mode 100644 index 0000000..5233dfa --- /dev/null +++ b/tests/res/edtlib/props_redef.yaml @@ -0,0 +1,23 @@ +# SPDX-License-Identifier: Apache-2.0 + +description: | + Testing property redefinition.. + +compatible: vnd,props-redef + +# Properties: +# + x +# child-binding: +# + child-prop-1 +include: + - name: props_def.yaml + +properties: + x: + required: true + const: 1 + +child-binding: + properties: + child-prop-1: + required: true diff --git a/tests/res/edtlib/right.yaml b/tests/res/edtlib/right.yaml new file mode 100644 index 0000000..f7b56d8 --- /dev/null +++ b/tests/res/edtlib/right.yaml @@ -0,0 +1,17 @@ +# SPDX-License-Identifier: Apache-2.0 + +description: | + Right include file for left_rigt.yaml. + +properties: + right-prop-1: + type: int + right-prop-2: + type: int + +child-binding: + properties: + right-child-prop-1: + type: int + right-child-prop-2: + type: int diff --git a/tests/test_edtlib.py b/tests/test_edtlib.py new file mode 100644 index 0000000..e83e4ab --- /dev/null +++ b/tests/test_edtlib.py @@ -0,0 +1,273 @@ +# Copyright (c) 2023 Christophe Dufaza +# SPDX-License-Identifier: Apache-2.0 + +"""Check our understanding of edtlib, regression tests for related issues.""" + +from typing import Optional +import os + +from devicetree import edtlib + +from .dtsh_uthelpers import DTShTests + + +def _basename(path: Optional[str]) -> str: + return os.path.basename(path or "?") + + +def test_props_redef() -> None: + fname2path = { + "props_def.yaml": "props_def.yaml", + } + + binding = None + with DTShTests.from_there("edtlib"): + binding = edtlib.Binding( + "props_redef.yaml", + fname2path, + ) + + assert {"x"} == set(binding.prop2specs.keys()) + assert "int" == binding.prop2specs["x"].type + assert binding.prop2specs["x"].required + + child_binding = binding.child_binding + assert child_binding + assert {"child-prop-1"} == set(child_binding.prop2specs.keys()) + assert "int" == child_binding.prop2specs["child-prop-1"].type + assert child_binding.prop2specs["child-prop-1"].required + + +def test_allow_and_blocklist_multilevel() -> None: + """Similar to edtlib's allow-and-blocklist-multilevel.yaml test case, + similar test case, plus: + - check of bindings and property specs paths + - check of grandchild-binding level + """ + fname2path = { + "include.yaml": "include.yaml", + } + + binding = None + with DTShTests.from_there("edtlib"): + binding = edtlib.Binding( + "allow-and-blocklist-multilevel.yaml", + fname2path, + ) + assert "allow-and-blocklist-multilevel.yaml" == binding.path + + assert {"x"} == set(binding.prop2specs.keys()) + assert "include.yaml" == _basename(binding.prop2specs["x"].path) + + child_binding = binding.child_binding + assert child_binding + assert "allow-and-blocklist-multilevel.yaml" == binding.path + + assert {"x", "z", "child-prop-1", "child-prop-2"} == set( + child_binding.prop2specs.keys() + ) + + assert "include.yaml" == _basename(child_binding.prop2specs["x"].path) + assert "include.yaml" == _basename(child_binding.prop2specs["z"].path) + assert "include.yaml" == _basename( + child_binding.prop2specs["child-prop-1"].path + ) + assert "include.yaml" == _basename( + child_binding.prop2specs["child-prop-2"].path + ) + + grandchild_binding = child_binding.child_binding + assert grandchild_binding + assert "include.yaml" == grandchild_binding.path + + assert { + # From nested child-binding. + "child-prop-1", + "child-prop-2", + # From top level child-binding. + "grandchild-prop-1", + "grandchild-prop-2", + } == set(grandchild_binding.prop2specs.keys()) + + assert "include.yaml" == _basename( + grandchild_binding.prop2specs["child-prop-1"].path + ) + assert "include.yaml" == _basename( + grandchild_binding.prop2specs["child-prop-2"].path + ) + assert "include.yaml" == _basename( + grandchild_binding.prop2specs["grandchild-prop-1"].path + ) + assert "include.yaml" == _basename( + grandchild_binding.prop2specs["grandchild-prop-2"].path + ) + + +def test_included_child_binding_any_level() -> None: + """Extends edtlib's upstream allow-and-blocklist-multilevel.yaml test case. + Adds: + - check of bindings and property specs paths + - check of grand-grandchild-binding level + - check grandchild-binding filters + - check setting "required: true" won't truncate included + child-binding's property definitions + + All child-binding definitions come from a single file, + included multiple times, at different levels, + with different filters.""" + fname2path = { + "base.yaml": "base.yaml", + } + + binding = None + with DTShTests.from_there("edtlib"): + binding = edtlib.Binding( + "allow-and-blocklist-anylevel.yaml", + fname2path, + ) + + # Top-level binding. + assert "allow-and-blocklist-anylevel.yaml" == binding.path + + assert {"x", "ceil-prop"} == set(binding.prop2specs.keys()) + assert "base.yaml" == _basename(binding.prop2specs["x"].path) + assert "allow-and-blocklist-anylevel.yaml" == _basename( + binding.prop2specs["ceil-prop"].path + ) + + assert "int" == binding.prop2specs["x"].type + assert not binding.prop2specs["x"].required + + # Child-binding. + child_binding = binding.child_binding + assert child_binding + # Last modified semantic. + assert "allow-and-blocklist-anylevel.yaml" == child_binding.path + + assert {"x", "z", "child-prop-1", "cb-prop"} == set( + child_binding.prop2specs.keys() + ) + + assert "base.yaml" == _basename(child_binding.prop2specs["x"].path) + assert "allow-and-blocklist-anylevel.yaml" == _basename( + child_binding.prop2specs["child-prop-1"].path + ) + + assert "int" == child_binding.prop2specs["x"].type + assert not child_binding.prop2specs["x"].required + + assert "int" == child_binding.prop2specs["z"].type + assert child_binding.prop2specs["z"].required + + # GrandChild-binding. + grandchild_binding = child_binding.child_binding + assert grandchild_binding + assert "base.yaml" == grandchild_binding.path + + assert { + "child-prop-1", + "child-prop-2", + "grandchild-prop-1", + "grandchild-prop-2", + "gcbb-prop", + } == set(grandchild_binding.prop2specs.keys()) + + assert "base.yaml" == _basename( + grandchild_binding.prop2specs["child-prop-1"].path + ) + assert "base.yaml" == _basename( + grandchild_binding.prop2specs["grandchild-prop-1"].path + ) + assert "allow-and-blocklist-anylevel.yaml" == _basename( + grandchild_binding.prop2specs["gcbb-prop"].path + ) + + # GrandGrandChild-binding. + ggchild_binding = grandchild_binding.child_binding + assert ggchild_binding + assert "base.yaml" == ggchild_binding.path + + assert { + "grandchild-prop-1", + "grandchild-prop-2", + } == set(ggchild_binding.prop2specs.keys()) + + assert "base.yaml" == _basename( + ggchild_binding.prop2specs["grandchild-prop-1"].path + ) + assert "base.yaml" == _basename( + ggchild_binding.prop2specs["grandchild-prop-2"].path + ) + + +def test_multi_include_child_binding() -> None: + """Complement test case above, child-binding definitions come from two files.""" + + fname2path = { + "left.yaml": "left.yaml", + "right.yaml": "right.yaml", + } + + binding = None + with DTShTests.from_there("edtlib"): + binding = edtlib.Binding( + "left_right.yaml", + fname2path, + ) + + # Top-level binding. + assert "left_right.yaml" == binding.path + assert {"left-right-prop", "left-prop-1", "right-prop-2"} == set( + binding.prop2specs.keys() + ) + assert "left_right.yaml" == _basename( + binding.prop2specs["left-right-prop"].path + ) + # Taken "Last modified here" semantic. + assert "left_right.yaml" == _basename( + binding.prop2specs["left-prop-1"].path + ) + assert "right.yaml" == _basename(binding.prop2specs["right-prop-2"].path) + + assert "int" == binding.prop2specs["left-prop-1"].type + assert binding.prop2specs["left-prop-1"].required + assert "int" == binding.prop2specs["right-prop-2"].type + assert not binding.prop2specs["right-prop-2"].required + + # Child-binding. + child_binding = binding.child_binding + assert child_binding + # Last modified semantic. + assert "left_right.yaml" == _basename(child_binding.path) + + assert {"left-child-prop-1", "right-child-prop-2", "child-prop"} == set( + child_binding.prop2specs.keys() + ) + + assert "left_right.yaml" == _basename( + child_binding.prop2specs["left-child-prop-1"].path + ) + assert "right.yaml" == _basename( + child_binding.prop2specs["right-child-prop-2"].path + ) + assert "left_right.yaml" == _basename( + child_binding.prop2specs["child-prop"].path + ) + + assert "int" == child_binding.prop2specs["left-child-prop-1"].type + assert child_binding.prop2specs["left-child-prop-1"].required + assert "int" == child_binding.prop2specs["right-child-prop-2"].type + assert not child_binding.prop2specs["right-child-prop-2"].required + + # GrandChild-binding. + grandchild_binding = child_binding.child_binding + assert grandchild_binding + assert "left_right.yaml" == _basename(grandchild_binding.path) + + assert { + "grandchild-prop", + } == set(grandchild_binding.prop2specs.keys()) + + assert "left_right.yaml" == _basename( + grandchild_binding.prop2specs["grandchild-prop"].path + )