From 47a29e7a58f0ddf4bef8e84f0a8283dc5208ccbf Mon Sep 17 00:00:00 2001 From: James Roy Date: Thu, 12 Jun 2025 20:31:47 +0800 Subject: [PATCH 1/7] [nrf fromtree] scripts: ci: Add CI bindings style checker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Martí Bolívar (cherry picked from commit 4240853a06b83a0d5d49a5e80697f4a6485a3a76) --- scripts/bindings_properties_allowlist.yaml | 11 ++ scripts/ci/check_compliance.py | 117 +++++++++++++++------ 2 files changed, 98 insertions(+), 30 deletions(-) create mode 100644 scripts/bindings_properties_allowlist.yaml diff --git a/scripts/bindings_properties_allowlist.yaml b/scripts/bindings_properties_allowlist.yaml new file mode 100644 index 00000000000..100d4d380fb --- /dev/null +++ b/scripts/bindings_properties_allowlist.yaml @@ -0,0 +1,11 @@ +# This file is a YAML allowlist 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 3ee7b62c9b5..7ad88630e17 100755 --- a/scripts/ci/check_compliance.py +++ b/scripts/ci/check_compliance.py @@ -21,6 +21,7 @@ import shutil import textwrap import unidiff +import yaml from yamllint import config, linter @@ -35,6 +36,30 @@ import list_boards import list_hardware +sys.path.insert(0, str(Path(__file__).resolve().parents[2] + / "scripts" / "dts" / "python-devicetree" / "src")) +from devicetree import edtlib + + +# 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 ZEPHYR_BASE: + ZEPHYR_BASE = Path(ZEPHYR_BASE) +else: + ZEPHYR_BASE = Path(__file__).resolve().parents[2] + # Propagate this decision to child processes. + os.environ['ZEPHYR_BASE'] = str(ZEPHYR_BASE) + +# Initialize the property names allowlist +BINDINGS_PROPERTIES_AL = None +with open(Path(__file__).parents[1] / 'bindings_properties_allowlist.yaml') as f: + allowlist = yaml.safe_load(f.read()) + if allowlist is not None: + BINDINGS_PROPERTIES_AL = set(allowlist) + else: + BINDINGS_PROPERTIES_AL = set() + logger = None def git(*args, cwd=None, ignore_non_zero=False): @@ -380,32 +405,75 @@ class DevicetreeBindingsCheck(ComplianceTest): doc = "See https://docs.zephyrproject.org/latest/build/dts/bindings.html for more details." def run(self, full=True): - dts_bindings = self.parse_dt_bindings() - - for dts_binding in dts_bindings: - self.required_false_check(dts_binding) + bindings_diff, bindings = self.get_yaml_bindings() - def parse_dt_bindings(self): + # If no bindings are changed, skip this check. + try: + subprocess.check_call(['git', 'diff', '--quiet', COMMIT_RANGE] + + bindings_diff) + nodiff = True + except subprocess.CalledProcessError: + nodiff = False + if nodiff: + self.skip('no changes to bindings were made') + + for binding in bindings: + 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 get_yaml_bindings(self): """ - Returns a list of dts/bindings/**/*.yaml files + Returns a list of 'dts/bindings/**/*.yaml' """ + from glob import glob + BINDINGS_PATH = 'dts/bindings/' + bindings_diff_dir, bindings = set(), [] - dt_bindings = [] - for file_name in get_files(filter="d"): - if 'dts/bindings/' in file_name and file_name.endswith('.yaml'): - dt_bindings.append(file_name) + for file_name in get_files(filter='d'): + if BINDINGS_PATH in file_name: + p = file_name.partition(BINDINGS_PATH) + bindings_diff_dir.add(os.path.join(p[0], p[1])) - return dt_bindings + for path in bindings_diff_dir: + yamls = glob(f'{os.fspath(path)}/**/*.yaml', recursive=True) + bindings.extend(yamls) - 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") + bindings = edtlib.bindings_from_paths(bindings, ignore_errors=True) + return list(bindings_diff_dir), bindings + 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_PROPERTIES_AL: + better_prop = prop_name.replace('_', '-') + print(f"Required: In '{binding.path}', " + f"the property '{prop_name}' " + f"should be renamed to '{better_prop}'.") + 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_properties_allowlist.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): """ @@ -1984,17 +2052,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 - ZEPHYR_BASE = Path(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 From 175f136a99dfcc6a8f8127b6d8801cc129ddf76a Mon Sep 17 00:00:00 2001 From: Jamie McCrae Date: Mon, 7 Jul 2025 08:10:58 +0100 Subject: [PATCH 2/7] [nrf noup] scripts: ci: check_compliance: Add NCS sysbuild Kconfig nrf-squash! [nrf noup] scripts: ci: check_compliance: Add NCS sysbuild Kconfigs Adds Kconfigs to the allow list as they are used in documentation but are not defined Signed-off-by: Jamie McCrae --- scripts/ci/check_compliance.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts/ci/check_compliance.py b/scripts/ci/check_compliance.py index 7ad88630e17..e40455b6c97 100755 --- a/scripts/ci/check_compliance.py +++ b/scripts/ci/check_compliance.py @@ -1320,11 +1320,13 @@ class SysbuildKconfigCheck(KconfigCheck): "COMP_DATA_LAYOUT_SINGLE", # Used by test "DTM_NO_DFE", # Used by DTM application "DTM_TRANSPORT_HCI", # Used by DTM application + "FIRMWARE_LOADER_IMAGE_ABC", # Used in documentation "INCLUDE_REMOTE_IMAGE", # Used by machine learning application "MCUBOOT_FPROTECT_ALLOW_COMBINED_REGIONS", # Used in migration documentation "ML_APP_INCLUDE_REMOTE_IMAGE", # Used by machine learning application "ML_APP_REMOTE_BOARD", # Used by machine learning application "MY_APP_IMAGE_ABC", # Used in documentation + "NETCORE_ABC", # Used in documentation "REMOTE_GLOBAL_DOMAIN_CLOCK_FREQUENCY_SWITCHING", # Used in tests "SOC_FLASH_NRF_RADIO_SYNC_RPC", # Used in documentation "SUIT_ENVELOPE_", # Used by jinja From 26161ea7919d53eb98a7241f4f3f965d4f44d526 Mon Sep 17 00:00:00 2001 From: Jamie McCrae Date: Tue, 8 Jul 2025 13:59:15 +0100 Subject: [PATCH 3/7] [nrf noup] scripts: ci: check_compliance: Add undefined Kconfigs Adds undefined Kconfigs used in NCS to the allow list for Kconfig compliance checks Signed-off-by: Jamie McCrae --- scripts/ci/check_compliance.py | 43 ++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/scripts/ci/check_compliance.py b/scripts/ci/check_compliance.py index e40455b6c97..decce399f63 100755 --- a/scripts/ci/check_compliance.py +++ b/scripts/ci/check_compliance.py @@ -1239,6 +1239,49 @@ def check_no_undef_outside_kconfig(self, kconf): # documentation "ZTEST_FAIL_TEST_", # regex in tests/ztest/fail/CMakeLists.txt # zephyr-keep-sorted-stop + + # NCS-specific allow list + # zephyr-keep-sorted-start re(^\s+") + "BAR", # Example documentation + "BT_ADV_PROV_", # Documentation + "CHANNEL_FETCHED_DATA_MAX_SIZE", # NRF desktop + "CHANNEL_TRANSPORT_DISABLED", # NRF desktop + "CHANNEL_TRANSPORT_IDLE", # NRF desktop + "CHANNEL_TRANSPORT_RSP_READY", # NRF desktop + "CHANNEL_TRANSPORT_WAIT_RSP", # NRF desktop + "DESKTOP_DVFS_STATE_", # NRF desktop + "DESKTOP_DVFS_STATE_CONFIG_CHANNEL_ENABLE", # NRF desktop + "DESKTOP_DVFS_STATE_INITIALIZING_ENABLE", # NRF desktop + "DESKTOP_DVFS_STATE_LLPM_CONNECTED_ENABLE", # NRF desktop + "DESKTOP_DVFS_STATE_SMP_TRANSFER_ENABLE", # NRF desktop + "DESKTOP_DVFS_STATE_USB_CONNECTED_ENABLE", # NRF desktop + "MY_CUSTOM_CONFIG", # Example documentation + "MY_EXT_API_ENABLED", # Example documentation + "MY_EXT_API_REQUIRED", # Example documentation + "NCS_IS_VARIANT_IMAGE", # Build system defined symbol + "NCS_VARIANT_MERGE_KCONFIG", # Build system defined symbol + "NRF_MODEM_LIB_TRACE_BACKEND_MY_TRACE_BACKEND", # Documentation + "SOC_NRF54H20_CPUSEC", # Internal + "SSF_SERVER_PSA_CRYPTO_SERVICE_ENABLED", # Internal + "STATUS_", # NRF desktop + "STATUS_COUNT", # NRF desktop + "STATUS_DISCONNECTED", # NRF desktop + "STATUS_FETCH", # NRF desktop + "STATUS_GET_BOARD_NAME", # NRF desktop + "STATUS_GET_HWID", # NRF desktop + "STATUS_GET_MAX_MOD_ID", # NRF desktop + "STATUS_GET_PEER", # NRF desktop + "STATUS_GET_PEERS_CACHE", # NRF desktop + "STATUS_INDEX_PEERS", # NRF desktop + "STATUS_LIST", # NRF desktop + "STATUS_PENDING", # NRF desktop + "STATUS_POS", # NRF desktop + "STATUS_REJECT", # NRF desktop + "STATUS_SET", # NRF desktop + "STATUS_SUCCESS", # NRF desktop + "STATUS_TIMEOUT", # NRF desktop + "STATUS_WRITE_FAIL", # NRF desktop + # zephyr-keep-sorted-stop } From b8959b7be57b7d30945109ce9869e56ad0fad801 Mon Sep 17 00:00:00 2001 From: Jamie McCrae Date: Tue, 4 Mar 2025 09:11:24 +0000 Subject: [PATCH 4/7] [nrf fromtree] scripts: ci: check_compliance: Add support for modules for Kconfig Adds support for checking modules for disallow Kconfig's in boards and SoCs, which have been defined in a Zephyr module file Signed-off-by: Jamie McCrae (cherry picked from commit 6d73a9c45aee4a1040bf4f94d41a27463ceca027) --- scripts/ci/check_compliance.py | 59 +++++++++++++++++++++++++++++++--- 1 file changed, 54 insertions(+), 5 deletions(-) diff --git a/scripts/ci/check_compliance.py b/scripts/ci/check_compliance.py index decce399f63..f6ee08383e3 100755 --- a/scripts/ci/check_compliance.py +++ b/scripts/ci/check_compliance.py @@ -10,7 +10,7 @@ import json import logging import os -from pathlib import Path +from pathlib import Path, PurePath import platform import re import subprocess @@ -31,6 +31,11 @@ from west.manifest import Manifest from west.manifest import ManifestProject +try: + from yaml import CSafeLoader as SafeLoader +except ImportError: + from yaml import SafeLoader + sys.path.insert(0, str(Path(__file__).resolve().parents[1])) from get_maintainer import Maintainers, MaintainersError import list_boards @@ -781,6 +786,23 @@ def get_logging_syms(self, kconf): return set(kconf_syms) + def module_disallowed_check(self, module_path, type, folder, meta, regex): + # Returns a list with lines from git grep which includes Kconfigs from defconfig files + entry = type + '_root' + git_folder = ":" + folder + + if entry in meta['build']['settings']: + tmp_path = module_path.joinpath(meta['build']['settings'][entry]) + + if Path(tmp_path.joinpath(folder)).is_dir(): + tmp_output = git("grep", "--line-number", "-I", "--null", + "--perl-regexp", regex, "--", git_folder, + cwd=tmp_path, ignore_non_zero=True) + + if len(tmp_output) > 0: + return tmp_output.splitlines() + return [] + def check_disallowed_defconfigs(self, kconf): """ Checks that there are no disallowed Kconfigs used in board/SoC defconfig files @@ -823,14 +845,41 @@ def check_disallowed_defconfigs(self, kconf): grep_stdout_boards = git("grep", "--line-number", "-I", "--null", "--perl-regexp", regex_boards, "--", ":boards", - cwd=ZEPHYR_BASE) + cwd=ZEPHYR_BASE).splitlines() grep_stdout_socs = git("grep", "--line-number", "-I", "--null", "--perl-regexp", regex_socs, "--", ":soc", - cwd=ZEPHYR_BASE) + cwd=ZEPHYR_BASE).splitlines() + + manifest = Manifest.from_file() + for project in manifest.get_projects([]): + if not manifest.is_active(project): + continue + + if not project.is_cloned(): + continue + + module_path = PurePath(project.abspath) + module_yml = module_path.joinpath('zephyr/module.yml') + + if not Path(module_yml).is_file(): + module_yml = module_path.joinpath('zephyr/module.yaml') + + if Path(module_yml).is_file(): + with Path(module_yml).open('r', encoding='utf-8') as f: + meta = yaml.load(f.read(), Loader=SafeLoader) + + if 'build' in meta and 'settings' in meta['build']: + grep_stdout_boards.extend(self.module_disallowed_check(module_path, + 'board', + 'boards', meta, + regex_boards)) + grep_stdout_socs.extend(self.module_disallowed_check(module_path, 'soc', + 'soc', meta, + regex_socs)) # Board processing # splitlines() supports various line terminators - for grep_line in grep_stdout_boards.splitlines(): + for grep_line in grep_stdout_boards: path, lineno, line = grep_line.split("\0") # Extract symbol references (might be more than one) within the line @@ -847,7 +896,7 @@ def check_disallowed_defconfigs(self, kconf): # SoCs processing # splitlines() supports various line terminators - for grep_line in grep_stdout_socs.splitlines(): + for grep_line in grep_stdout_socs: path, lineno, line = grep_line.split("\0") # Extract symbol references (might be more than one) within the line From eabf969eedbc9bf45e94e31b9d100678e60ce1cb Mon Sep 17 00:00:00 2001 From: Jamie McCrae Date: Mon, 7 Jul 2025 10:22:42 +0100 Subject: [PATCH 5/7] [nrf fromlist] zephyr: Add zephyr module file Adds a module file which specifies the path to samples and tests inside of zephyr Upstream PR #: 92765 Signed-off-by: Jamie McCrae --- zephyr/module.yml | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 zephyr/module.yml diff --git a/zephyr/module.yml b/zephyr/module.yml new file mode 100644 index 00000000000..9faf4e207c4 --- /dev/null +++ b/zephyr/module.yml @@ -0,0 +1,4 @@ +samples: + - samples +tests: + - tests From 1d3dd6340468cf0bc9f466bb6b4e81c88b3fa4f5 Mon Sep 17 00:00:00 2001 From: Jamie McCrae Date: Mon, 7 Jul 2025 08:51:44 +0100 Subject: [PATCH 6/7] [nrf fromlist] scripts: ci: check_compliance: Add support for module Kconfigs Adds support for checking module samples and tests for additional Kconfigs, as well as logging Kconfigs, so that this check can be reused more easily with out of tree manifest repos Upstream PR #: 92765 Signed-off-by: Jamie McCrae --- scripts/ci/check_compliance.py | 62 ++++++++++++++++++++++++++++++---- 1 file changed, 56 insertions(+), 6 deletions(-) diff --git a/scripts/ci/check_compliance.py b/scripts/ci/check_compliance.py index f6ee08383e3..19e61a21d7f 100755 --- a/scripts/ci/check_compliance.py +++ b/scripts/ci/check_compliance.py @@ -753,6 +753,58 @@ def parse_kconfig(self): # Clean up the temporary directory shutil.rmtree(kconfiglib_dir) + def module_kconfigs(self, regex): + manifest = Manifest.from_file() + kconfigs = "" + + for project in manifest.get_projects([]): + if not manifest.is_active(project): + continue + + if not project.is_cloned(): + continue + + module_path = PurePath(project.abspath) + module_yml = module_path.joinpath('zephyr/module.yml') + + if not Path(module_yml).is_file(): + module_yml = module_path.joinpath('zephyr/module.yaml') + + if Path(module_yml).is_file(): + dirs = [] + + with Path(module_yml).open('r', encoding='utf-8') as f: + meta = yaml.load(f.read(), Loader=SafeLoader) + + for folder_type in ['samples', 'tests']: + if folder_type in meta: + for path_ext in meta[folder_type]: + path_full = module_path.joinpath(path_ext) + + if Path(path_full).is_dir(): + dirs.append(":" + path_ext) + + # Add ext. module root, if one is defined. For zephyr, the base one is added + # directly since in CMake it is added statically instead of by parsing the + # zephyr module file + if module_path == ZEPHYR_BASE: + dirs.append(":" + str(module_path / "modules")) + elif 'build' in meta and 'settings' in meta['build'] and \ + 'module_ext_root' in meta['build']['settings']: + path_full = module_path.joinpath(meta['build']['settings']['module_ext_root']) + + if Path(path_full).is_dir(): + dirs.append(":" + meta['build']['settings']['module_ext_root']) + + if len(dirs) > 0: + tmp_output = git("grep", "-I", "-h", "--perl-regexp", regex, "--", + *dirs, cwd=module_path, ignore_non_zero=True) + + if len(tmp_output) > 0: + kconfigs += tmp_output + "\n" + + return kconfigs + def get_logging_syms(self, kconf): # Returns a set() with the names of the Kconfig symbols generated with # logging template in samples/tests folders. The Kconfig symbols doesn't @@ -773,9 +825,8 @@ def get_logging_syms(self, kconf): # Warning: Needs to work with both --perl-regexp and the 're' module. regex = r"^\s*(?:module\s*=\s*)([A-Z0-9_]+)\s*(?:#|$)" - # Grep samples/ and tests/ for symbol definitions - grep_stdout = git("grep", "-I", "-h", "--perl-regexp", regex, "--", - ":samples", ":tests", cwd=ZEPHYR_BASE) + # Grep samples/ and tests/ for symbol definitions in all modules + grep_stdout = self.module_kconfigs(regex) names = re.findall(regex, grep_stdout, re.MULTILINE) @@ -922,9 +973,8 @@ def get_defined_syms(self, kconf): # (?:...) is a non-capturing group. regex = r"^\s*(?:menu)?config\s*([A-Z0-9_]+)\s*(?:#|$)" - # Grep samples/ and tests/ for symbol definitions - grep_stdout = git("grep", "-I", "-h", "--perl-regexp", regex, "--", - ":samples", ":tests", cwd=ZEPHYR_BASE) + # Grep samples/ and tests/ for symbol definitions in all modules + grep_stdout = self.module_kconfigs(regex) # Generate combined list of configs and choices from the main Kconfig tree. kconf_syms = kconf.unique_defined_syms + kconf.unique_choices From 2dedad04e4ddeb1eafdb08723a03cd08d4478801 Mon Sep 17 00:00:00 2001 From: Jamie McCrae Date: Mon, 7 Jul 2025 09:02:08 +0100 Subject: [PATCH 7/7] [nrf noup] scripts: ci: check_compliance: Exclude release docs Adds the NCS release notes folder to the exclusion list for undefined Kconfigs so that old Kconfigs can be used e.g. for old release notes, and lwm2m carrier library changelog Signed-off-by: Jamie McCrae --- scripts/ci/check_compliance.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts/ci/check_compliance.py b/scripts/ci/check_compliance.py index 19e61a21d7f..50496c2ced4 100755 --- a/scripts/ci/check_compliance.py +++ b/scripts/ci/check_compliance.py @@ -1156,6 +1156,8 @@ def check_no_undef_outside_kconfig(self, kconf): grep_stdout = git("grep", "--line-number", "-I", "--null", "--perl-regexp", regex, "--", ":!/doc/releases", ":!/doc/security/vulnerabilities.rst", + ":!/doc/nrf/releases_and_maturity", + ":!/doc/nrf/libraries/bin/lwm2m_carrier/CHANGELOG.rst", cwd=GIT_TOP) # splitlines() supports various line terminators