From: Septatrix <24257556+Septatrix@users.noreply.github.com>
Date: Wed, 20 Nov 2024 22:10:45 +0000 (+0100)
Subject: Keep track of collection resets from CLI arguments (fixes #3208)
X-Git-Tag: v25~155
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3d2fbda95c243d16d04b2a917f2baa45bc48159d;p=thirdparty%2Fmkosi.git

Keep track of collection resets from CLI arguments (fixes #3208)

Previously it was only possible to completely reset but not append,
or only append but not reset to collection/list settings
via command line arguments.
Now we track if a setting has ever been set to None (i.e. reset)
during command line parsing.
This information is used during value finalization
to decide whether to merge both collections or only keep the CLI value.
---

diff --git a/mkosi/config.py b/mkosi/config.py
index ca88a1e3e..0cb6cb336 100644
--- a/mkosi/config.py
+++ b/mkosi/config.py
@@ -3950,11 +3950,14 @@ class ConfigAction(argparse.Action):
         s = SETTINGS_LOOKUP_BY_DEST[self.dest]
 
         if values is None or isinstance(values, str):
-            setattr(namespace, s.dest, s.parse(values, getattr(namespace, self.dest, None)))
-        else:
-            for v in values:
-                assert isinstance(v, str)
-                setattr(namespace, s.dest, s.parse(v, getattr(namespace, self.dest, None)))
+            values = [values]
+
+        for v in values:
+            assert isinstance(v, str) or v is None
+            parsed_value = s.parse(v, getattr(namespace, self.dest, None))
+            if parsed_value is None:
+                setattr(namespace, f"{s.dest}_was_none", True)
+            setattr(namespace, s.dest, parsed_value)
 
 
 class ParseContext:
@@ -4066,8 +4069,10 @@ class ParseContext:
         # If a value was specified on the CLI, it always takes priority. If the setting is a collection of
         # values, we merge the value from the CLI with the value from the configuration, making sure that the
         # value from the CLI always takes priority.
-        if hasattr(self.cli, setting.dest) and (v := getattr(self.cli, setting.dest)) is not None:
-            if isinstance(v, list):
+        if (v := getattr(self.cli, setting.dest, None)) is not None:
+            if getattr(self.cli, f"{setting.dest}_was_none", False):
+                return v
+            elif isinstance(v, list):
                 return (getattr(self.config, setting.dest, None) or []) + v
             elif isinstance(v, dict):
                 return (getattr(self.config, setting.dest, None) or {}) | v
diff --git a/tests/test_config.py b/tests/test_config.py
index 13765dced..f572739b2 100644
--- a/tests/test_config.py
+++ b/tests/test_config.py
@@ -1341,3 +1341,27 @@ def test_split_artifacts_compat(tmp_path: Path) -> None:
     with chdir(d):
         _, [config] = parse_config()
         assert config.split_artifacts == ArtifactOutput.compat_yes()
+
+
+def test_cli_collection_reset(tmp_path: Path) -> None:
+    d = tmp_path
+
+    (d / "mkosi.conf").write_text(
+        """
+        [Content]
+        Packages=abc
+        """
+    )
+
+    with chdir(d):
+        _, [config] = parse_config(["--package", ""])
+        assert config.packages == []
+
+        _, [config] = parse_config(["--package", "", "--package", "foo"])
+        assert config.packages == ["foo"]
+
+        _, [config] = parse_config(["--package", "foo", "--package", "", "--package", "bar"])
+        assert config.packages == ["bar"]
+
+        _, [config] = parse_config(["--package", "foo", "--package", ""])
+        assert config.packages == []