Skip to content

Commit 4240853

Browse files
rruuaannghenrikbrixandersen
authored andcommitted
scripts: ci: Add CI bindings style checker
Implement a check in the CI pipeline to enforce that property names in device tree bindings do not contain underscores. Signed-off-by: James Roy <rruuaanng@outlook.com> Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
1 parent 9cf9416 commit 4240853

File tree

2 files changed

+98
-30
lines changed

2 files changed

+98
-30
lines changed
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
# This file is a YAML allowlist of property names that are allowed to
2+
# bypass the underscore check in bindings. These properties are exempt
3+
# from the rule that requires using '-' instead of '_'.
4+
#
5+
# The file content can be as shown below:
6+
# - propname1
7+
# - propname2
8+
# - ...
9+
10+
- mmc-hs200-1_8v
11+
- mmc-hs400-1_8v

scripts/ci/check_compliance.py

Lines changed: 87 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import shutil
2222
import textwrap
2323
import unidiff
24+
import yaml
2425

2526
from yamllint import config, linter
2627

@@ -35,6 +36,30 @@
3536
import list_boards
3637
import list_hardware
3738

39+
sys.path.insert(0, str(Path(__file__).resolve().parents[2]
40+
/ "scripts" / "dts" / "python-devicetree" / "src"))
41+
from devicetree import edtlib
42+
43+
44+
# Let the user run this script as ./scripts/ci/check_compliance.py without
45+
# making them set ZEPHYR_BASE.
46+
ZEPHYR_BASE = os.environ.get('ZEPHYR_BASE')
47+
if ZEPHYR_BASE:
48+
ZEPHYR_BASE = Path(ZEPHYR_BASE)
49+
else:
50+
ZEPHYR_BASE = Path(__file__).resolve().parents[2]
51+
# Propagate this decision to child processes.
52+
os.environ['ZEPHYR_BASE'] = str(ZEPHYR_BASE)
53+
54+
# Initialize the property names allowlist
55+
BINDINGS_PROPERTIES_AL = None
56+
with open(Path(__file__).parents[1] / 'bindings_properties_allowlist.yaml') as f:
57+
allowlist = yaml.safe_load(f.read())
58+
if allowlist is not None:
59+
BINDINGS_PROPERTIES_AL = set(allowlist)
60+
else:
61+
BINDINGS_PROPERTIES_AL = set()
62+
3863
logger = None
3964

4065
def git(*args, cwd=None, ignore_non_zero=False):
@@ -380,32 +405,75 @@ class DevicetreeBindingsCheck(ComplianceTest):
380405
doc = "See https://docs.zephyrproject.org/latest/build/dts/bindings.html for more details."
381406

382407
def run(self, full=True):
383-
dts_bindings = self.parse_dt_bindings()
384-
385-
for dts_binding in dts_bindings:
386-
self.required_false_check(dts_binding)
408+
bindings_diff, bindings = self.get_yaml_bindings()
387409

388-
def parse_dt_bindings(self):
410+
# If no bindings are changed, skip this check.
411+
try:
412+
subprocess.check_call(['git', 'diff', '--quiet', COMMIT_RANGE]
413+
+ bindings_diff)
414+
nodiff = True
415+
except subprocess.CalledProcessError:
416+
nodiff = False
417+
if nodiff:
418+
self.skip('no changes to bindings were made')
419+
420+
for binding in bindings:
421+
self.check(binding, self.check_yaml_property_name)
422+
self.check(binding, self.required_false_check)
423+
424+
@staticmethod
425+
def check(binding, callback):
426+
while binding is not None:
427+
callback(binding)
428+
binding = binding.child_binding
429+
430+
def get_yaml_bindings(self):
389431
"""
390-
Returns a list of dts/bindings/**/*.yaml files
432+
Returns a list of 'dts/bindings/**/*.yaml'
391433
"""
434+
from glob import glob
435+
BINDINGS_PATH = 'dts/bindings/'
436+
bindings_diff_dir, bindings = set(), []
392437

393-
dt_bindings = []
394-
for file_name in get_files(filter="d"):
395-
if 'dts/bindings/' in file_name and file_name.endswith('.yaml'):
396-
dt_bindings.append(file_name)
438+
for file_name in get_files(filter='d'):
439+
if BINDINGS_PATH in file_name:
440+
p = file_name.partition(BINDINGS_PATH)
441+
bindings_diff_dir.add(os.path.join(p[0], p[1]))
397442

398-
return dt_bindings
443+
for path in bindings_diff_dir:
444+
yamls = glob(f'{os.fspath(path)}/**/*.yaml', recursive=True)
445+
bindings.extend(yamls)
399446

400-
def required_false_check(self, dts_binding):
401-
with open(dts_binding) as file:
402-
for line_number, line in enumerate(file, 1):
403-
if 'required: false' in line:
404-
self.fmtd_failure(
405-
'warning', 'Devicetree Bindings', dts_binding,
406-
line_number, col=None,
407-
desc="'required: false' is redundant, please remove")
447+
bindings = edtlib.bindings_from_paths(bindings, ignore_errors=True)
448+
return list(bindings_diff_dir), bindings
408449

450+
def check_yaml_property_name(self, binding):
451+
"""
452+
Checks if the property names in the binding file contain underscores.
453+
"""
454+
for prop_name in binding.prop2specs:
455+
if '_' in prop_name and prop_name not in BINDINGS_PROPERTIES_AL:
456+
better_prop = prop_name.replace('_', '-')
457+
print(f"Required: In '{binding.path}', "
458+
f"the property '{prop_name}' "
459+
f"should be renamed to '{better_prop}'.")
460+
self.failure(
461+
f"{binding.path}: property '{prop_name}' contains underscores.\n"
462+
f"\tUse '{better_prop}' instead unless this property name is from Linux.\n"
463+
"Or another authoritative upstream source of bindings for "
464+
f"compatible '{binding.compatible}'.\n"
465+
"\tHint: update 'bindings_properties_allowlist.yaml' if you need to "
466+
"override this check for this property."
467+
)
468+
469+
def required_false_check(self, binding):
470+
raw_props = binding.raw.get('properties', {})
471+
for prop_name, raw_prop in raw_props.items():
472+
if raw_prop.get('required') is False:
473+
self.failure(
474+
f'{binding.path}: property "{prop_name}": '
475+
"'required: false' is redundant, please remove"
476+
)
409477

410478
class KconfigCheck(ComplianceTest):
411479
"""
@@ -2001,17 +2069,6 @@ def _main(args):
20012069
# The "real" main(), which is wrapped to catch exceptions and report them
20022070
# to GitHub. Returns the number of test failures.
20032071

2004-
global ZEPHYR_BASE
2005-
ZEPHYR_BASE = os.environ.get('ZEPHYR_BASE')
2006-
if not ZEPHYR_BASE:
2007-
# Let the user run this script as ./scripts/ci/check_compliance.py without
2008-
# making them set ZEPHYR_BASE.
2009-
ZEPHYR_BASE = str(Path(__file__).resolve().parents[2])
2010-
2011-
# Propagate this decision to child processes.
2012-
os.environ['ZEPHYR_BASE'] = ZEPHYR_BASE
2013-
ZEPHYR_BASE = Path(ZEPHYR_BASE)
2014-
20152072
# The absolute path of the top-level git directory. Initialize it here so
20162073
# that issues running Git can be reported to GitHub.
20172074
global GIT_TOP

0 commit comments

Comments
 (0)