Skip to content

Commit

Permalink
scripts: ci: Add CI bindings style checker
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
rruuaanng committed Dec 26, 2024
1 parent 1e6f48d commit 23bb613
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 21 deletions.
11 changes: 11 additions & 0 deletions scripts/ci/bindings_propertys_name_wl.yaml
Original file line number Diff line number Diff line change
@@ -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
86 changes: 65 additions & 21 deletions scripts/ci/check_compliance.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import shutil
import textwrap
import unidiff
import yaml

from yamllint import config, linter

Expand All @@ -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
Expand Down Expand Up @@ -335,10 +358,27 @@ class DevicetreeBindingsCheck(ComplianceTest):
path_hint = "<zephyr-base>"

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):
"""
Expand All @@ -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):
"""
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 23bb613

Please sign in to comment.