From 794a681438cad6d9e9bdccaefbf9210fa325f146 Mon Sep 17 00:00:00 2001 From: raiden00pl Date: Wed, 7 Oct 2026 21:11:07 +0200 Subject: [PATCH] builder.py: check that Kconfig overrides are applied kconfig-tweak upper-cases symbol names unless --keep-case is given, so a 'kv' override of a mixed-case symbol (e.g. CONFIG_NET_IPv6) was written under a wrong name and silently dropped by olddefconfig. The same happened to overrides of unknown symbols, with unmet dependencies or with invalid values: the build used a configuration nobody asked for. * Pass --keep-case to kconfig-tweak. * After olddefconfig, compare every override with the final .config and fail the build with the list of overrides that were not applied. Assisted-by: Claude:claude-opus-5-5 Signed-off-by: raiden00pl --- src/ntfc/builder.py | 91 ++++++++++++++++++++++++++++++++++++++++++- tests/test_builder.py | 70 +++++++++++++++++++++++++++++++++ 2 files changed, 160 insertions(+), 1 deletion(-) diff --git a/src/ntfc/builder.py b/src/ntfc/builder.py index 22d10b1..3a18fb4 100644 --- a/src/ntfc/builder.py +++ b/src/ntfc/builder.py @@ -47,6 +47,7 @@ class NuttXBuilder: _KCONFIG_DISABLED_RE = re.compile( r"^#\s+(CONFIG_[A-Za-z0-9_]+)\s+is not set" ) + _KCONFIG_VALUE_RE = re.compile(r"^(CONFIG_[A-Za-z0-9_]+)=(.*)$") def __init__(self, config: Dict[str, Any], rebuild: bool = True): """Initialize NuttX builder.""" @@ -173,7 +174,10 @@ def _build_kconfig_tweak_cmd( self, tool: str, conf_path: str, key: str, value: Any ) -> List[str]: """Build one ``kconfig-tweak`` command for a Kconfig override.""" - cmd = [tool, "--file", conf_path] + # Kconfig symbols are case-sensitive (e.g. CONFIG_NET_IPv6) and + # kconfig-tweak upper-cases them unless asked not to + + cmd = [tool, "--file", conf_path, "--keep-case"] if value is False or value == "n": cmd.extend(["--disable", key]) elif value is True or value == "y": @@ -242,6 +246,85 @@ def _format_kconfig_line(self, key: str, value: Any) -> str: return f'{key}="{value}"\n' + def _read_kconfig_values(self, conf_path: str) -> Dict[str, str]: + """Read symbol values from a ``.config`` file. + + :param conf_path: path to ``.config`` + :return: map of symbol to value; ``n`` for "is not set" symbols + """ + values: Dict[str, str] = {} + with open(conf_path, "r", encoding="utf-8") as f: + for line in f: + line = line.rstrip("\n") + disabled = self._KCONFIG_DISABLED_RE.match(line) + if disabled: + values[disabled.group(1)] = "n" + continue + match = self._KCONFIG_VALUE_RE.match(line) + if match: + values[match.group(1)] = match.group(2) + + return values + + @staticmethod + def _kconfig_values_equal(requested: str, actual: str) -> bool: + """Compare two Kconfig values, numbers by value. + + :param requested: value as written by the override + :param actual: value found in ``.config`` + :return: True if the values are the same + """ + if requested == actual: + return True + + try: + return int(requested, 0) == int(actual, 0) + except ValueError: + return False + + def _check_kconfig_overrides( + self, conf_path: str, overrides: Dict[str, Any] + ) -> None: + """Check that every Kconfig override is in the final ``.config``. + + Kconfig drops or resets an override silently: an unknown symbol, + unmet dependencies or an invalid value. Building anyway would test + a configuration nobody asked for. + + :param conf_path: path to ``.config`` after ``olddefconfig`` + :param overrides: requested Kconfig overrides + :raises AssertionError: if any override is not applied + """ + values = self._read_kconfig_values(conf_path) + failed: List[str] = [] + + for key, value in overrides.items(): + line = self._format_kconfig_line(key, value).rstrip("\n") + match = self._KCONFIG_VALUE_RE.match(line) + requested = match.group(2) if match else "n" + actual = values.get(key) + + if actual is None and requested == "n": + continue + + if actual is None: + failed.append( + f"{key}: requested {requested}, not in .config " + "(unknown symbol or unmet dependencies)" + ) + elif not self._kconfig_values_equal(requested, actual): + failed.append( + f"{key}: requested {requested}, got {actual} " + "(unmet dependencies or invalid value)" + ) + + if failed: + for item in failed: + logger.error(f"Kconfig override not applied: {item}") + raise AssertionError( + "'kv' Kconfig overrides not applied: " + "; ".join(failed) + ) + def _apply_kconfig_overrides( self, conf_path: str, overrides: Dict[str, Any], cfg_cwd: str = "" ) -> None: @@ -462,6 +545,12 @@ def _build_core( # that enable new applications take effect configure() + # fail instead of building a configuration that + # silently lost some overrides + self._check_kconfig_overrides( + nuttx_conf_path, kv_overrides + ) + # build self._run_build(build_path, env=build_env) diff --git a/tests/test_builder.py b/tests/test_builder.py index cd5f7a0..b62461f 100644 --- a/tests/test_builder.py +++ b/tests/test_builder.py @@ -159,6 +159,9 @@ def run_command_capture(cmd, env): monkeypatch.setattr( b, "_apply_kconfig_overrides", lambda *args, **kwargs: None ) + monkeypatch.setattr( + b, "_check_kconfig_overrides", lambda *args, **kwargs: None + ) b.build_all() @@ -303,6 +306,7 @@ def test_builder_regenerates_config_after_kconfig_overrides() -> None: builder._run_command = lambda command, env: calls.append(command) builder._make_dir = builder_make_dir_dummy builder._apply_kconfig_overrides = lambda *_args: None + builder._check_kconfig_overrides = lambda *_args: None builder.build_all() assert calls[1] == [ @@ -488,6 +492,7 @@ def fake_run(cmd): "/usr/bin/kconfig-tweak", "--file", "/tmp/.config", + "--keep-case", "--disable", "CONFIG_FALSE", ] @@ -761,3 +766,68 @@ def test_builder_flash_supports_elf_placeholder(tmp_path) -> None: builder._flash_core("core0", {"core0": core}) assert commands == [["pxe-stage", str(image)]] + + +def test_builder_check_kconfig_overrides(tmp_path) -> None: + b = NuttXBuilder(copy.deepcopy(conf_dir)) + conf = tmp_path / ".config" + conf.write_text( + "# comment\n" + "CONFIG_NET_IPv6=y\n" + "# CONFIG_OFF is not set\n" + "CONFIG_HEX=0x10\n" + "CONFIG_INT=8192\n" + 'CONFIG_STR="abc"\n' + "CONFIG_MOD=m\n", + encoding="utf-8", + ) + + assert b._read_kconfig_values(str(conf)) == { + "CONFIG_NET_IPv6": "y", + "CONFIG_OFF": "n", + "CONFIG_HEX": "0x10", + "CONFIG_INT": "8192", + "CONFIG_STR": '"abc"', + "CONFIG_MOD": "m", + } + + # everything applied: mixed case, disabled and absent "n", numbers by + # value, strings with and without quotes + b._check_kconfig_overrides( + str(conf), + { + "CONFIG_NET_IPv6": True, + "CONFIG_OFF": "n", + "CONFIG_ABSENT": False, + "CONFIG_HEX": "0x0010", + "CONFIG_INT": 8192, + "CONFIG_STR": "abc", + "CONFIG_MOD": "m", + }, + ) + + with patch("ntfc.builder.logger.error") as error_mock: + with pytest.raises(AssertionError) as excinfo: + b._check_kconfig_overrides( + str(conf), + { + "CONFIG_NET_IPV6": "y", + "CONFIG_OFF": "y", + "CONFIG_HEX": "0x20", + "CONFIG_STR": "xyz", + }, + ) + msg = str(excinfo.value) + assert "CONFIG_NET_IPV6: requested y, not in .config" in msg + assert "CONFIG_OFF: requested y, got n" in msg + assert "CONFIG_HEX: requested 0x20, got 0x10" in msg + assert 'CONFIG_STR: requested "xyz", got "abc"' in msg + assert error_mock.call_count == 4 + + +def test_builder_kconfig_values_equal() -> None: + assert NuttXBuilder._kconfig_values_equal("y", "y") + assert NuttXBuilder._kconfig_values_equal("0x0010", "0x10") + assert NuttXBuilder._kconfig_values_equal("16", "0x10") + assert not NuttXBuilder._kconfig_values_equal("y", "n") + assert not NuttXBuilder._kconfig_values_equal('"a"', '"b"')