Warn when a setting is in the wrong section

Except for the `[Match]` section, a setting can be set in any of the
valid sections because the actual section is ignored. This fuzziness
works because each setting is used in only one section so there is not
conflict.

Instead of silently parsing such a file, a warning is now displayed with
a hint on how to fix it. In the future, this could become an error.

diff --git a/.github/mkosi.conf.d/10-common.conf b/.github/mkosi.conf.d/10-common.conf
index e612e0c449fa4783fc4f097129dac6929023e78a..132accf374f15261062342ee763df4bd2f627572 100644 (file)
--- a/.github/mkosi.conf.d/10-common.conf
+++ b/.github/mkosi.conf.d/10-common.conf
@@ -1,13 +1,13 @@
 [Output]
 CacheDirectory=mkosi.cache
+
+[Content]
+Autologin=yes
+BiosBootloader=grub
 KernelCommandLine=console=ttyS0
                   systemd.unit=mkosi-check-and-shutdown.service
                   systemd.log_target=console
                   systemd.default_standard_output=journal+console
 
-[Content]
-BiosBootloader=grub
-
 [Host]
-Autologin=yes
 QemuVsock=yes

diff --git a/mkosi/config.py b/mkosi/config.py
index 13c30fca7c6e87f328ecf62de488319dd2cd8884..daa088dd0dffe1626300c76270cc3249fca8fc58 100644 (file)
--- a/mkosi/config.py
+++ b/mkosi/config.py
@@ -1976,12 +1976,15 @@ def parse_config(argv: Sequence[str] = ()) -> tuple[MkosiArgs, tuple[MkosiConfig
         if path.exists():
             logging.debug(f"Including configuration file {Path.cwd() / path}")
 
-            for _, k, v in parse_ini(path, only_sections=["Distribution", "Output", "Content", "Validation", "Host", "Preset"]):
+            for section, k, v in parse_ini(path, only_sections=["Distribution", "Output", "Content", "Validation", "Host", "Preset"]):
                 ns = defaults if k.startswith("@") else namespace
 
                 if not (s := settings_lookup_by_name.get(k.removeprefix("@"))):
                     die(f"Unknown setting {k}")
 
+                if section != s.section:
+                    logging.warning(f"Setting {k} should be configured in [{s.section}], not [{section}].")
+
                 setattr(ns, s.dest, s.parse(v, getattr(ns, s.dest, None)))
 
         if extras:

diff --git a/tests/test_config.py b/tests/test_config.py
index 082759c7e361ea62b7aaf54fa4067240fab3cab2..7f4608a277958211242349ecdb415be4e19edb33 100644 (file)
--- a/tests/test_config.py
+++ b/tests/test_config.py
@@ -2,6 +2,7 @@
 
 import argparse
 import itertools
+import logging
 import operator
 from pathlib import Path
 from typing import Optional
@@ -148,6 +149,8 @@ def test_parse_config(tmp_path: Path) -> None:
         """\
         [Content]
         Packages=
+
+        [Output]
         ImageId=
         """
     )
@@ -362,6 +365,8 @@ def test_match_imageid(tmp_path: Path, image1: str, image2: str) -> None:
             f"""\
             [Distribution]
             Distribution=fedora
+
+            [Output]
             ImageId={image1}
             """
         )
@@ -442,6 +447,8 @@ def test_match_imageversion(tmp_path: Path, op: str, version: str) -> None:
             """\
             [Distribution]
             Distribution=fedora
+
+            [Output]
             ImageId=testimage
             ImageVersion=123
             """
@@ -523,3 +530,41 @@ def test_package_manager_tree(tmp_path: Path, skel: Optional[Path], pkgmngr: Opt
 
         assert conf.skeleton_trees == skel_expected
         assert conf.package_manager_trees == pkgmngr_expected
+
+
+@pytest.mark.parametrize(
+    "sections,args,warning_count",
+    [
+        (["Output"], [], 0),
+        (["Content"], [], 1),
+        (["Content", "Output"], [], 1),
+        (["Output", "Content"], [], 1),
+        (["Output", "Content", "Distribution"], [], 2),
+        (["Content"], ["--image-id=testimage"], 1),
+    ],
+)
+def test_wrong_section_warning(
+    tmp_path: Path,
+    caplog: pytest.LogCaptureFixture,
+    sections: list[str],
+    args: list[str],
+    warning_count: int,
+) -> None:
+    with chdir(tmp_path):
+        # Create a config with ImageId in the wrong section,
+        # and sometimes in the correct section
+        Path("mkosi.conf").write_text(
+            "\n".join(
+                f"""\
+                [{section}]
+                ImageId=testimage
+                """
+                for section in sections
+            )
+        )
+
+        with caplog.at_level(logging.WARNING):
+            # Parse the config, with --image-id sometimes given on the command line
+            parse_config(args)
+
+        assert len(caplog.records) == warning_count