builder.py: check that Kconfig overrides are applied

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)
 

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"')