diff --git a/scripts/ci/bindings_propertys_name_wl.yaml b/scripts/ci/bindings_propertys_name_wl.yaml new file mode 100644 index 000000000000000..351c0f1ca0110d0 --- /dev/null +++ b/scripts/ci/bindings_propertys_name_wl.yaml @@ -0,0 +1,11 @@ +# This file is a YAML whitelist of property names that are allowed to +# bypass the underscore check in bindings. These properties are exempt +# from the rule that requires using '-' instead of '_'. +# +# The file content can be as shown below: +# - propname1 +# - propname2 +# - ... + +- mmc-hs200-1_8v +- mmc-hs400-1_8v diff --git a/scripts/ci/check_compliance.py b/scripts/ci/check_compliance.py index aa4ec915b158e7d..7ac7244a9bd9fce 100755 --- a/scripts/ci/check_compliance.py +++ b/scripts/ci/check_compliance.py @@ -22,6 +22,7 @@ import shutil import textwrap import unidiff +import yaml from yamllint import config, linter @@ -31,13 +32,35 @@ from west.manifest import Manifest from west.manifest import ManifestProject +# Let the user run this script as ./scripts/ci/check_compliance.py without +# making them set ZEPHYR_BASE. +ZEPHYR_BASE = os.environ.get('ZEPHYR_BASE') +if not ZEPHYR_BASE: + ZEPHYR_BASE = str(Path(__file__).resolve().parents[2]) + # Propagate this decision to child processes. + os.environ['ZEPHYR_BASE'] = ZEPHYR_BASE + +# Zephyr local directory import path sys.path.insert(0, str(Path(__file__).resolve().parents[1])) +sys.path.insert(0, os.path.join(ZEPHYR_BASE, 'scripts', 'dts', 'python-devicetree', 'src')) + from get_maintainer import Maintainers, MaintainersError import list_boards import list_hardware +from devicetree import edtlib + logger = None +# Initialize the propertys name whitelist +BINDINGS_PROPERTYS_WL = None +with open(Path(__file__).parent / 'bindings_propertys_name_wl.yaml', 'r') as f: + wl = yaml.safe_load(f.read()) + if wl != None: + BINDINGS_PROPERTYS_WL = set(wl) + else: + BINDINGS_PROPERTYS_WL = {0,} + def git(*args, cwd=None, ignore_non_zero=False): # Helper for running a Git command. Returns the rstrip()ed stdout output. # Called like git("diff"). Exits with SystemError (raised by sys.exit()) on @@ -335,10 +358,27 @@ class DevicetreeBindingsCheck(ComplianceTest): path_hint = "" def run(self, full=True): - dts_bindings = self.parse_dt_bindings() + diff_binding_paths = self.parse_dt_bindings() - for dts_binding in dts_bindings: - self.required_false_check(dts_binding) + # If no bindings are changed, skip this check. + try: + subprocess.check_call(['git', 'diff', '--quiet', COMMIT_RANGE] + + diff_binding_paths) + nodiff = True + except subprocess.CalledProcessError: + nodiff = False + if nodiff or len(diff_binding_paths) == 0: + self.skip('no changes to bindings were made') + + for binding in edtlib.bindings_from_paths(diff_binding_paths): + self.check(binding, self.check_yaml_property_name) + self.check(binding, self.required_false_check) + + @staticmethod + def check(binding, callback): + while binding is not None: + callback(binding) + binding = binding.child_binding def parse_dt_bindings(self): """ @@ -352,15 +392,29 @@ def parse_dt_bindings(self): return dt_bindings - def required_false_check(self, dts_binding): - with open(dts_binding) as file: - for line_number, line in enumerate(file, 1): - if 'required: false' in line: - self.fmtd_failure( - 'warning', 'Devicetree Bindings', dts_binding, - line_number, col=None, - desc="'required: false' is redundant, please remove") + def check_yaml_property_name(self, binding): + """ + Checks if the property names in the binding file contain underscores. + """ + for prop_name in binding.prop2specs: + if '_' in prop_name and prop_name not in BINDINGS_PROPERTYS_WL: + better_prop = prop_name.replace('_', '-') + self.failure( + f"{binding.path}: property '{prop_name}' contains underscores.\n" + f"\tUse '{better_prop}' instead unless this property name is from Linux.\n" + "Or another authoritative upstream source of bindings for " + f"compatible '{binding.compatible}'.\n" + "\tHint: update 'bindings_propertys_name_wl.yaml' if you need to " + "override this check for this property." + ) + def required_false_check(self, binding): + raw_props = binding.raw.get('properties', {}) + for prop_name, raw_prop in raw_props.items(): + if raw_prop.get('required') is False: + self.failure( + f'{binding.path}: property "{prop_name}": ' + "'required: false' is redundant, please remove") class KconfigCheck(ComplianceTest): """ @@ -1802,16 +1856,6 @@ def _main(args): # The "real" main(), which is wrapped to catch exceptions and report them # to GitHub. Returns the number of test failures. - global ZEPHYR_BASE - ZEPHYR_BASE = os.environ.get('ZEPHYR_BASE') - if not ZEPHYR_BASE: - # Let the user run this script as ./scripts/ci/check_compliance.py without - # making them set ZEPHYR_BASE. - ZEPHYR_BASE = str(Path(__file__).resolve().parents[2]) - - # Propagate this decision to child processes. - os.environ['ZEPHYR_BASE'] = ZEPHYR_BASE - # The absolute path of the top-level git directory. Initialize it here so # that issues running Git can be reported to GitHub. global GIT_TOP