scripts: edt: Add support for include property filtering

Add the ability to filter which properties get imported when we do an
include.  We add a new YAML form for this:

include:
  - name: other.yaml
    property-blocklist:
      - prop-to-block

or

include:
  - name: other.yaml
    property-allowlist:
      - prop-to-allow

These lists can intermix simple file names with maps, like:

include:
  - foo.yaml
  - name: bar.yaml
    property-allowlist:
      - prop-to-allow

And you can filter from child bindings like this:

include:
  - name: bar.yaml
    child-binding:
      property-allowlist:
        - child-prop-to-allow

Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
This commit is contained in:
Kumar Gala 2020-10-21 22:38:30 -05:00 committed by Carles Cufí
commit 6e01c6abb6
20 changed files with 400 additions and 15 deletions

View file

@ -60,7 +60,50 @@ compatible: "manufacturer,device"
# When including multiple files, any overlapping 'required' keys on properties # When including multiple files, any overlapping 'required' keys on properties
# in the included files are ORed together. This makes sure that a # in the included files are ORed together. This makes sure that a
# 'required: true' is always respected. # 'required: true' is always respected.
include: other.yaml # or [other1.yaml, other2.yaml] #
# When 'include:' is a list, its elements can be either filenames as
# strings, or maps. Each map element must have a 'name' key which is
# the filename to include, and may have 'property-allowlist' and
# 'property-blocklist' keys that filter which properties are included.
# You can freely intermix strings and maps in a single 'include:' list.
# You cannot have a single map element with both 'property-allowlist' and
# 'property-blocklist' keys. A map element with neither 'property-allowlist'
# nor 'property-blocklist' is valid; no additional filtering is done.
include: other.yaml
# To include multiple files:
#
# include:
# - other1.yaml
# - other2.yaml
#
# To filter the included properties:
#
# include:
# - name: other1.yaml
# property-allowlist:
# - i-want-this-one
# - and-this-one
# - other2.yaml
# property-blocklist:
# - do-not-include-this-one
# - or-this-one
#
# You can intermix these types of includes:
#
# include:
# - other1.yaml
# - other2.yaml
# property-blocklist:
# - do-not-include-this-one
# - or-this-one
#
# And you can filter from a child binding like this:
#
# include:
# - name: bar.yaml
# child-binding:
# property-allowlist:
# - child-prop-to-allow
# If the node describes a bus, then the bus type should be given, like below # If the node describes a bus, then the bus type should be given, like below
bus: <string describing bus type, e.g. "i2c"> bus: <string describing bus type, e.g. "i2c">

View file

@ -68,6 +68,7 @@ bindings_from_paths() helper function.
# variables. See the existing @properties for a template. # variables. See the existing @properties for a template.
from collections import OrderedDict, defaultdict from collections import OrderedDict, defaultdict
from copy import deepcopy
import logging import logging
import os import os
import re import re
@ -1607,26 +1608,51 @@ class Binding:
return raw return raw
include = raw.pop("include") include = raw.pop("include")
fnames = []
if isinstance(include, str):
fnames.append(include)
elif isinstance(include, list):
if not all(isinstance(elem, str) for elem in include):
_err(f"all elements in 'include:' in {binding_path} "
"should be strings")
fnames += include
else:
_err(f"'include:' in {binding_path} "
"should be a string or a list of strings")
# First, merge the included files together. If more than one included # First, merge the included files together. If more than one included
# file has a 'required:' for a particular property, OR the values # file has a 'required:' for a particular property, OR the values
# together, so that 'required: true' wins. # together, so that 'required: true' wins.
merged = {} merged = {}
for fname in fnames:
_merge_props(merged, self._load_raw(fname), None, binding_path, if isinstance(include, str):
check_required=False) # Simple scalar string case
_merge_props(merged, self._load_raw(include), None, binding_path,
False)
elif isinstance(include, list):
# List of strings and maps. These types may be intermixed.
for elem in include:
if isinstance(elem, str):
_merge_props(merged, self._load_raw(elem), None,
binding_path, False)
elif isinstance(elem, dict):
name = elem.pop('name', None)
allowlist = elem.pop('property-allowlist', None)
blocklist = elem.pop('property-blocklist', None)
child_filter = elem.pop('child-binding', None)
if elem:
# We've popped out all the valid keys.
_err(f"'include:' in {binding_path} should not have "
f"these unexpected contents: {elem}")
_check_include_dict(name, allowlist, blocklist,
child_filter, binding_path)
contents = self._load_raw(name)
_filter_properties(contents, allowlist, blocklist,
child_filter, binding_path)
_merge_props(merged, contents, None, binding_path, False)
else:
_err(f"all elements in 'include:' in {binding_path} "
"should be either strings or maps with a 'name' key "
"and optional 'property-allowlist' or "
f"'property-blocklist' keys, but got: {elem}")
else:
# Invalid item.
_err(f"'include:' in {binding_path} "
f"should be a string or list, but has type {type(include)}")
# Next, merge the merged included files into 'raw'. Error out if # Next, merge the merged included files into 'raw'. Error out if
# 'raw' has 'required: false' while the merged included files have # 'raw' has 'required: false' while the merged included files have
@ -1952,6 +1978,89 @@ def _binding_inc_error(msg):
raise yaml.constructor.ConstructorError(None, None, "error: " + msg) raise yaml.constructor.ConstructorError(None, None, "error: " + msg)
def _check_include_dict(name, allowlist, blocklist, child_filter,
binding_path):
# Check that an 'include:' named 'name' with property-allowlist
# 'allowlist', property-blocklist 'blocklist', and
# child-binding filter 'child_filter' has valid structure.
if name is None:
_err(f"'include:' element in {binding_path} "
"should have a 'name' key")
if allowlist is not None and blocklist is not None:
_err(f"'include:' of file '{name}' in {binding_path} "
"should not specify both 'property-allowlist:' "
"and 'property-blocklist:'")
while child_filter is not None:
child_copy = deepcopy(child_filter)
child_allowlist = child_copy.pop('property-allowlist', None)
child_blocklist = child_copy.pop('property-blocklist', None)
next_child_filter = child_copy.pop('child-binding', None)
if child_copy:
# We've popped out all the valid keys.
_err(f"'include:' of file '{name}' in {binding_path} "
"should not have these unexpected contents in a "
f"'child-binding': {child_copy}")
if child_allowlist is not None and child_blocklist is not None:
_err(f"'include:' of file '{name}' in {binding_path} "
"should not specify both 'property-allowlist:' and "
"'property-blocklist:' in a 'child-binding:'")
child_filter = next_child_filter
def _filter_properties(raw, allowlist, blocklist, child_filter,
binding_path):
# Destructively modifies 'raw["properties"]' and
# 'raw["child-binding"]', if they exist, according to
# 'allowlist', 'blocklist', and 'child_filter'.
props = raw.get('properties')
_filter_properties_helper(props, allowlist, blocklist, binding_path)
child_binding = raw.get('child-binding')
while child_filter is not None and child_binding is not None:
_filter_properties_helper(child_binding.get('properties'),
child_filter.get('property-allowlist'),
child_filter.get('property-blocklist'),
binding_path)
child_filter = child_filter.get('child-binding')
child_binding = child_binding.get('child-binding')
def _filter_properties_helper(props, allowlist, blocklist, binding_path):
if props is None or (allowlist is None and blocklist is None):
return
_check_prop_filter('property-allowlist', allowlist, binding_path)
_check_prop_filter('property-blocklist', blocklist, binding_path)
if allowlist is not None:
allowset = set(allowlist)
to_del = [prop for prop in props if prop not in allowset]
else:
blockset = set(blocklist)
to_del = [prop for prop in props if prop in blockset]
for prop in to_del:
del props[prop]
def _check_prop_filter(name, value, binding_path):
# Ensure an include: ... property-allowlist or property-blocklist
# is a list.
if value is None:
return
if not isinstance(value, list):
_err(f"'{name}' value {value} in {binding_path} should be a list")
def _merge_props(to_dict, from_dict, parent, binding_path, check_required): def _merge_props(to_dict, from_dict, parent, binding_path, check_required):
# Recursively merges 'from_dict' into 'to_dict', to implement 'include:'. # Recursively merges 'from_dict' into 'to_dict', to implement 'include:'.
# #

View file

@ -0,0 +1 @@
This directory contains bindings used to test the 'include:' feature.

View file

@ -0,0 +1,11 @@
# SPDX-License-Identifier: BSD-3-Clause
description: |
An include must not give both an allowlist and a blocklist in a
child binding. This binding should cause an error.
compatible: allow-and-blocklist-child
include:
- name: include.yaml
child-binding:
property-blocklist: [x]
property-allowlist: [y]

View file

@ -0,0 +1,10 @@
# SPDX-License-Identifier: BSD-3-Clause
description: |
An include must not give both an allowlist and a blocklist.
This binding should cause an error.
compatible: allow-and-blocklist
include:
- name: include.yaml
property-blocklist: [x]
property-allowlist: [y]

View file

@ -0,0 +1,10 @@
# SPDX-License-Identifier: BSD-3-Clause
description: |
A property-allowlist, if given, must be a list. This binding should
cause an error.
compatible: allow-not-list
include:
- name: include.yaml
property-allowlist:
foo:

View file

@ -0,0 +1,7 @@
# SPDX-License-Identifier: BSD-3-Clause
description: Valid property-allowlist.
compatible: allowlist
include:
- name: include.yaml
property-allowlist: [x]

View file

@ -0,0 +1,10 @@
# SPDX-License-Identifier: BSD-3-Clause
description: |
A property-blocklist, if given, must be a list. This binding should
cause an error.
compatible: block-not-list
include:
- name: include.yaml
property-blocklist:
foo:

View file

@ -0,0 +1,7 @@
# SPDX-License-Identifier: BSD-3-Clause
description: Valid property-blocklist.
compatible: blocklist
include:
- name: include.yaml
property-blocklist: [x]

View file

@ -0,0 +1,7 @@
# SPDX-License-Identifier: BSD-3-Clause
description: An empty property-allowlist is valid.
compatible: empty-allowlist
include:
- name: include.yaml
property-allowlist: []

View file

@ -0,0 +1,7 @@
# SPDX-License-Identifier: BSD-3-Clause
description: An empty property-blocklist is valid.
compatible: empty-blocklist
include:
- name: include.yaml
property-blocklist: []

View file

@ -0,0 +1,11 @@
description: Test binding for filtering 'child-binding' properties
include:
- name: include.yaml
property-allowlist: [x]
child-binding:
property-blocklist: [child-prop-1]
child-binding:
property-allowlist: [grandchild-prop-1]
compatible: filter-child-bindings

View file

@ -0,0 +1,7 @@
# SPDX-License-Identifier: BSD-3-Clause
description: Second file for testing "intermixed" includes.
compatible: include-2
properties:
a:
type: int

View file

@ -0,0 +1,10 @@
# SPDX-License-Identifier: BSD-3-Clause
description: |
Invalid include element: invalid keys are present.
compatible: include-invalid-keys
include:
- name: include.yaml
property-allowlist: [x]
bad-key-1: 3
bad-key-2: 3

View file

@ -0,0 +1,5 @@
description: |
Invalid include: wrong top level type.
compatible: include-invalid-type
include:
a-map-is-not-allowed-here: 3

View file

@ -0,0 +1,6 @@
# SPDX-License-Identifier: BSD-3-Clause
description: A map element with just a name is valid, and has no filters.
compatible: include-no-list
include:
- name: include.yaml

View file

@ -0,0 +1,7 @@
# SPDX-License-Identifier: BSD-3-Clause
description: |
Invalid include element: no name key is present.
compatible: include-no-name
include:
- property-allowlist: [x]

View file

@ -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

View file

@ -0,0 +1,8 @@
# SPDX-License-Identifier: BSD-3-Clause
description: Including intermixed file names and maps is valid.
compatible: intermixed
include:
- name: include.yaml
property-allowlist: [x]
- include-2.yaml

View file

@ -132,6 +132,91 @@ def test_include():
assert str(edt.get_node("/binding-include").props) == \ assert str(edt.get_node("/binding-include").props) == \
"OrderedDict([('foo', <Property, name: foo, type: int, value: 0>), ('bar', <Property, name: bar, type: int, value: 1>), ('baz', <Property, name: baz, type: int, value: 2>), ('qaz', <Property, name: qaz, type: int, value: 3>)])" "OrderedDict([('foo', <Property, name: foo, type: int, value: 0>), ('bar', <Property, name: bar, type: int, value: 1>), ('baz', <Property, name: baz, type: int, value: 2>), ('qaz', <Property, name: qaz, type: int, value: 3>)])"
def test_include_filters():
'''Test property-allowlist and property-blocklist in an include.'''
fname2path = {'include.yaml': 'test-bindings-include/include.yaml',
'include-2.yaml': 'test-bindings-include/include-2.yaml'}
with pytest.raises(edtlib.EDTError) as e:
with from_here():
edtlib.Binding("test-bindings-include/allow-and-blocklist.yaml", fname2path)
assert ("should not specify both 'property-allowlist:' and 'property-blocklist:'"
in str(e.value))
with pytest.raises(edtlib.EDTError) as e:
with from_here():
edtlib.Binding("test-bindings-include/allow-and-blocklist-child.yaml", fname2path)
assert ("should not specify both 'property-allowlist:' and 'property-blocklist:'"
in str(e.value))
with pytest.raises(edtlib.EDTError) as e:
with from_here():
edtlib.Binding("test-bindings-include/allow-not-list.yaml", fname2path)
value_str = str(e.value)
assert value_str.startswith("'property-allowlist' value")
assert value_str.endswith("should be a list")
with pytest.raises(edtlib.EDTError) as e:
with from_here():
edtlib.Binding("test-bindings-include/block-not-list.yaml", fname2path)
value_str = str(e.value)
assert value_str.startswith("'property-blocklist' value")
assert value_str.endswith("should be a list")
with pytest.raises(edtlib.EDTError) as e:
with from_here():
binding = edtlib.Binding("test-bindings-include/include-invalid-keys.yaml", fname2path)
value_str = str(e.value)
assert value_str.startswith(
"'include:' in test-bindings-include/include-invalid-keys.yaml should not have these "
"unexpected contents: ")
assert 'bad-key-1' in value_str
assert 'bad-key-2' in value_str
with pytest.raises(edtlib.EDTError) as e:
with from_here():
binding = edtlib.Binding("test-bindings-include/include-invalid-type.yaml", fname2path)
value_str = str(e.value)
assert value_str.startswith(
"'include:' in test-bindings-include/include-invalid-type.yaml "
"should be a string or list, but has type ")
with pytest.raises(edtlib.EDTError) as e:
with from_here():
binding = edtlib.Binding("test-bindings-include/include-no-name.yaml", fname2path)
value_str = str(e.value)
assert value_str.startswith("'include:' element")
assert value_str.endswith(
"in test-bindings-include/include-no-name.yaml should have a 'name' key")
with from_here():
binding = edtlib.Binding("test-bindings-include/allowlist.yaml", fname2path)
assert set(binding.prop2specs.keys()) == {'x'} # 'x' is allowed
binding = edtlib.Binding("test-bindings-include/empty-allowlist.yaml", fname2path)
assert set(binding.prop2specs.keys()) == set() # nothing is allowed
binding = edtlib.Binding("test-bindings-include/blocklist.yaml", fname2path)
assert set(binding.prop2specs.keys()) == {'y', 'z'} # 'x' is blocked
binding = edtlib.Binding("test-bindings-include/empty-blocklist.yaml", fname2path)
assert set(binding.prop2specs.keys()) == {'x', 'y', 'z'} # nothing is blocked
binding = edtlib.Binding("test-bindings-include/intermixed.yaml", fname2path)
assert set(binding.prop2specs.keys()) == {'x', 'a'}
binding = edtlib.Binding("test-bindings-include/include-no-list.yaml", fname2path)
assert set(binding.prop2specs.keys()) == {'x', 'y', 'z'}
binding = edtlib.Binding("test-bindings-include/filter-child-bindings.yaml", fname2path)
child = binding.child_binding
grandchild = child.child_binding
assert set(binding.prop2specs.keys()) == {'x'}
assert set(child.prop2specs.keys()) == {'child-prop-2'}
assert set(grandchild.prop2specs.keys()) == {'grandchild-prop-1'}
def test_bus(): def test_bus():
'''Test 'bus:' and 'on-bus:' in bindings''' '''Test 'bus:' and 'on-bus:' in bindings'''
with from_here(): with from_here():