]> git.ipfire.org Git - thirdparty/mkosi.git/commitdiff
Make matches work more like systemd conditions 1613/head
authorDaan De Meyer <daan.j.demeyer@gmail.com>
Tue, 6 Jun 2023 09:59:55 +0000 (11:59 +0200)
committerDaan De Meyer <daan.j.demeyer@gmail.com>
Wed, 7 Jun 2023 13:08:51 +0000 (15:08 +0200)
Let's make matches behave like systemd conditions. We drop support
for list matches. Instead, we add support for match negation and
trigger matches. A match is a trigger match if it's prefixed with
the pipe symbol (|). A match is satisfied if all its regular matches
and one of its trigger matches are satisfied.

mkosi.md
mkosi/config.py
tests/test_parse_load_args.py

index c3daae112959c73795165365bfb0ccc90e787863..15f0270f6065dd617e6c328c1f24705651ff23ad 100644 (file)
--- a/mkosi.md
+++ b/mkosi.md
@@ -272,73 +272,57 @@ prefixed with `!`, if any later assignment of that setting tries to add
 the same value, that value is ignored. Values prefixed with `!` can be
 globs to ignore more than one value.
 
-To conditionally include configuration files, the `[Match]` section can
-be used. A configuration file is only included if all the conditions in the
-`[Match]` block are satisfied. If a condition in `[Match]` depends on a
-setting and the setting has not been explicitly configured when the condition
-is evaluated, the setting is assigned its default value.
+To conditionally include configuration files, the `[Match]` section can be used. Matches can use a pipe
+symbol ("|") after the equals sign ("…=|…"), which causes the match to become a triggering match. The config
+file will be included if the logical AND of all non-triggering matches and the logical OR of all triggering
+matches is satisfied. To negate the result of a match, prefix the argument with an exclamation mark. If an
+argument is prefixed with the pipe symbol and an exclamation mark, the pipe symbol must be passed first, and
+the exclamation second.
 
-Command line options that take no argument are shown without "=" in
-their long version. In the config files, they should be specified with
-a boolean argument: either "1", "yes", or "true" to enable, or "0",
-"no", "false" to disable.
+Command line options that take no argument are shown without "=" in their long version. In the config files,
+they should be specified with a boolean argument: either "1", "yes", or "true" to enable, or "0", "no",
+"false" to disable.
 
 ### [Match] Section.
 
 `Distribution=`
 
-: Matches against the configured distribution. Multiple distributions may
-  be specified, separated by spaces. If multiple distributions are specified,
-  the condition is satisfied if the current distribution equals any of the
-  specified distributions.
+: Matches against the configured distribution.
 
 `Release=`
 
-: Matches against the configured distribution release. If this condition
-  is used and no distribution has been explicitly configured yet, the
-  host distribution and release are used. Multiple releases may be specified,
-  separated by spaces. If multiple releases are specified, the condition is
-  satisfied if the current release equals any of the specified releases.
+: Matches against the configured distribution release. If this condition is used and no distribution has been
+  explicitly configured yet, the host distribution and release are used.
 
 `PathExists=`
 
-: This condition is satisfied if the given path exists. Relative paths are
-  interpreted relative to the parent directory of the config file that the
-  condition is read from.
+: This condition is satisfied if the given path exists. Relative paths are interpreted relative to the parent
+  directory of the config file that the condition is read from.
 
 `ImageId=`
 
-: Matches against the configured image ID, supporting globs. If this condition
-  is used and no image ID has been explicitly configured yet, this condition
-  fails. Multiple image IDs may be specified, separated by spaces. If multiple
-  image IDs are specified, the condition is satisfied if the configured image ID
-  equals any of the specified image IDs.
+: Matches against the configured image ID, supporting globs. If this condition is used and no image ID has
+  been explicitly configured yet, this condition fails.
 
 `ImageVersion=`
 
-: Matches against the configured image version. Image versions can be prepended
-  by the operators `==`, `!=`, `>=`, `<=`, `<`, `>` for rich version comparisons
-  according to the UAPI group version format specification. If no operator is
-  prepended, the equality operator is assumed by default If this condition is
-  used and no image Version has be explicitly configured yet, this condition
-  fails. Multiple image version constraints can be specified as a
-  space-separated list. If multiple image version constraints are specified, all
-  must be satisfied for the match to succeed.
+: Matches against the configured image version. Image versions can be prepended by the operators `==`, `!=`,
+  `>=`, `<=`, `<`, `>` for rich version comparisons according to the UAPI group version format specification.
+  If no operator is prepended, the equality operator is assumed by default If this condition is used and no
+  image version has been explicitly configured yet, this condition fails.
 
 `Bootable=`
 
-: Matches against the configured value for the `Bootable=` feature. Takes a boolean value or `auto`. Multiple
-  values may be specified, separated by commas. If multiple values are specified, the condition is satisfied
-  if the current value of the `Bootable=` feature matches any of the specified values.
-
-| Matcher         | Multiple Values | Globs | Rich Comparisons | Default                 |
-|-----------------|-----------------|-------|------------------|-------------------------|
-| `Distribution=` | yes             | no    | no               | match host distribution |
-| `Release=`      | yes             | no    | no               | match host release      |
-| `PathExists=`   | no              | no    | no               | match fails             |
-| `ImageId=`      | yes             | yes   | no               | match fails             |
-| `ImageVersion=` | yes             | no    | yes              | match fails             |
-| `Bootable=`     | yes             | no    | no               | match auto feature      |
+: Matches against the configured value for the `Bootable=` feature. Takes a boolean value or `auto`.
+
+| Matcher         | Globs | Rich Comparisons | Default                 |
+|-----------------|-------|------------------|-------------------------|
+| `Distribution=` | no    | no               | match host distribution |
+| `Release=`      | no    | no               | match host release      |
+| `PathExists=`   | no    | no               | match fails             |
+| `ImageId=`      | yes   | no               | match fails             |
+| `ImageVersion=` | no    | yes              | match fails             |
+| `Bootable=`     | no    | no               | match auto feature      |
 
 ### [Distribution] Section
 
index 3c526e60fbd5809c4ba40bbef6e49fbe9789c1a6..8dfffd2dd99532effec19bb1a59ebf3e7b7a332c 100644 (file)
@@ -133,8 +133,17 @@ def config_parse_string(dest: str, value: Optional[str], namespace: argparse.Nam
     return value if value else None
 
 
-def config_match_string(dest: str, value: str, namespace: argparse.Namespace) -> bool:
-    return cast(bool, value == getattr(namespace, dest))
+def config_make_string_matcher(allow_globs: bool = False) -> ConfigMatchCallback:
+    def config_match_string(dest: str, value: str, namespace: argparse.Namespace) -> bool:
+        if getattr(namespace, dest, None) is None:
+            return False
+
+        if allow_globs:
+            return fnmatch.fnmatchcase(getattr(namespace, dest), value)
+        else:
+            return cast(bool, value == getattr(namespace, dest))
+
+    return config_match_string
 
 
 def config_parse_script(dest: str, value: Optional[str], namespace: argparse.Namespace) -> Optional[Path]:
@@ -175,6 +184,10 @@ def config_parse_feature(dest: str, value: Optional[str], namespace: argparse.Na
     return parse_feature(value)
 
 
+def config_match_feature(dest: str, value: Optional[str], namespace: argparse.Namespace) -> bool:
+    return cast(bool, getattr(namespace, dest) == parse_feature(value))
+
+
 def config_parse_compression(dest: str, value: Optional[str], namespace: argparse.Namespace) -> Optional[Compression]:
     if dest in namespace:
         return getattr(namespace, dest) # type: ignore
@@ -316,81 +329,35 @@ def config_make_list_parser(delimiter: str,
     return config_parse_list
 
 
-def config_make_list_matcher(
-    delimiter: str,
-    *,
-    unescape: bool = False,
-    allow_globs: bool = False,
-    all: bool = False,
-    parse: Callable[[str], Any] = str,
-) -> ConfigMatchCallback:
-    def config_match_list(dest: str, value: str, namespace: argparse.Namespace) -> bool:
-        if unescape:
-            lex = shlex.shlex(value, posix=True)
-            lex.whitespace_split = True
-            lex.whitespace = f"\n{delimiter}"
-            lex.commenters = ""
-            values = list(lex)
-        else:
-            values = value.replace(delimiter, "\n").split("\n")
-
-        for v in values:
-            current_value = getattr(namespace, dest)
-            comparison_value = parse(v)
-            if allow_globs:
-                # check if the option has been set, since fnmatch wants strings
-                if isinstance(current_value, str):
-                    m = fnmatch.fnmatchcase(current_value, comparison_value)
-                else:
-                    m = False
-            else:
-                m = current_value == comparison_value
-
-            if not all and m:
-                return True
-            if all and not m:
-                return False
-
-        return all
-
-    return config_match_list
-
-
-def config_make_image_version_list_matcher(delimiter: str) -> ConfigMatchCallback:
-    def config_match_image_version_list(dest: str, value: str, namespace: argparse.Namespace) -> bool:
-        version_specs = value.replace(delimiter, "\n").splitlines()
-
-        image_version = getattr(namespace, dest)
-        # If the version is not set it cannot positively compare to anything
-        if image_version is None:
-            return False
-        image_version = GenericVersion(image_version)
-
-        for v in version_specs:
-            for sigil, opfunc in {
-                "==": operator.eq,
-                "!=": operator.ne,
-                "<=": operator.le,
-                ">=": operator.ge,
-                ">": operator.gt,
-                "<": operator.lt,
-            }.items():
-                if v.startswith(sigil):
-                    op = opfunc
-                    comp_version = GenericVersion(v[len(sigil):])
-                    break
-            else:
-                # default to equality if no operation is specified
-                op = operator.eq
-                comp_version = GenericVersion(v)
-
-            # all constraints must be fulfilled
-            if not op(image_version, comp_version):
-                return False
+def config_match_image_version(dest: str, value: str, namespace: argparse.Namespace) -> bool:
+    image_version = getattr(namespace, dest)
+    # If the version is not set it cannot positively compare to anything
+    if image_version is None:
+        return False
+    image_version = GenericVersion(image_version)
+
+    for sigil, opfunc in {
+        "==": operator.eq,
+        "!=": operator.ne,
+        "<=": operator.le,
+        ">=": operator.ge,
+        ">": operator.gt,
+        "<": operator.lt,
+    }.items():
+        if value.startswith(sigil):
+            op = opfunc
+            comp_version = GenericVersion(value[len(sigil):])
+            break
+    else:
+        # default to equality if no operation is specified
+        op = operator.eq
+        comp_version = GenericVersion(value)
 
-        return True
+    # all constraints must be fulfilled
+    if not op(image_version, comp_version):
+        return False
 
-    return config_match_image_version_list
+    return True
 
 
 def make_path_parser(*,
@@ -471,6 +438,14 @@ def config_parse_root_password(dest: str, value: Optional[str], namespace: argpa
     return (value, hashed)
 
 
+class ConfigParserMultipleValues(dict[str, Any]):
+    def __setitem__(self, key: str, value: Any) -> None:
+        if key in self and isinstance(value, list):
+            self[key].extend(value)
+        else:
+            super().__setitem__(key, value)
+
+
 @dataclasses.dataclass(frozen=True)
 class MkosiConfigSetting:
     dest: str
@@ -764,14 +739,14 @@ class MkosiConfigParser:
             dest="distribution",
             section="Distribution",
             parse=config_make_enum_parser(Distribution),
-            match=config_make_list_matcher(delimiter=" ", parse=make_enum_parser(Distribution)),
+            match=config_make_enum_matcher(Distribution),
             default=detect_distribution()[0],
         ),
         MkosiConfigSetting(
             dest="release",
             section="Distribution",
             parse=config_parse_string,
-            match=config_make_list_matcher(delimiter=" "),
+            match=config_make_string_matcher(),
             default_factory=config_default_release,
         ),
         MkosiConfigSetting(
@@ -868,12 +843,12 @@ class MkosiConfigParser:
         ),
         MkosiConfigSetting(
             dest="image_version",
-            match=config_make_image_version_list_matcher(delimiter=" "),
+            match=config_match_image_version,
             section="Output",
         ),
         MkosiConfigSetting(
             dest="image_id",
-            match=config_make_list_matcher(delimiter=" ", allow_globs=True),
+            match=config_make_string_matcher(allow_globs=True),
             section="Output",
         ),
         MkosiConfigSetting(
@@ -934,7 +909,7 @@ class MkosiConfigParser:
             dest="bootable",
             section="Content",
             parse=config_parse_feature,
-            match=config_make_list_matcher(delimiter=",", parse=parse_feature),
+            match=config_match_feature,
         ),
         MkosiConfigSetting(
             dest="autologin",
@@ -1255,6 +1230,8 @@ class MkosiConfigParser:
             inline_comment_prefixes="#",
             empty_lines_in_values=True,
             interpolation=None,
+            strict=False,
+            dict_type=ConfigParserMultipleValues,
         )
 
         parser.optionxform = lambda optionstr: optionstr # type: ignore
@@ -1263,29 +1240,53 @@ class MkosiConfigParser:
             parser.read(path)
 
         if "Match" in parser.sections():
-            for k, v in parser.items("Match"):
-                if (s := self.settings_lookup.get(k)):
-                    if not (match := s.match):
+            triggered = None
+
+            for k, values in parser.items("Match"):
+                # If a match is specified multiple times, the values are returned concatenated by newlines
+                # for some reason.
+                for v in values.split("\n"):
+                    trigger = v.startswith("|")
+                    v = v.removeprefix("|")
+                    negate = v.startswith("!")
+                    v = v.removeprefix("!")
+
+                    if not v:
+                        die("Match value cannot be empty")
+
+                    if (s := self.settings_lookup.get(k)):
+                        if not (match := s.match):
+                            die(f"{k} cannot be used in [Match]")
+
+                        # If we encounter a setting in [Match] that has not been explicitly configured yet,
+                        # we assign the default value first so that we can [Match] on default values for
+                        # settings.
+                        if s.dest not in namespace:
+                            if s.default_factory:
+                                default = s.default_factory(namespace)
+                            elif s.default is None:
+                                default = s.parse(s.dest, None, namespace)
+                            else:
+                                default = s.default
+
+                            setattr(namespace, s.dest, default)
+
+                        result = match(s.dest, v, namespace)
+
+                    elif (m := self.match_lookup.get(k)):
+                        result = m.match(v)
+                    else:
                         die(f"{k} cannot be used in [Match]")
 
-                    # If we encounter a setting in [Match] that has not been explicitly configured yet, we assign
-                    # the default value first so that we can [Match] on default values for settings.
-                    if s.dest not in namespace:
-                        if s.default_factory:
-                            default = s.default_factory(namespace)
-                        elif s.default is None:
-                            default = s.parse(s.dest, None, namespace)
-                        else:
-                            default = s.default
-
-                        setattr(namespace, s.dest, default)
-
-                    if not match(s.dest, v, namespace):
+                    if negate:
+                        result = not result
+                    if not trigger and not result:
                         return False
+                    if trigger:
+                        triggered = bool(triggered) or result
 
-                elif (m := self.match_lookup.get(k)):
-                    if not m.match(v):
-                        return False
+                if triggered is False:
+                    return False
 
         parser.remove_section("Match")
 
index 3d5ec0b55c2228e283f2b8b29bdcad3965e43aba..362c9f014e77ba218a9d93e2508e8203c011b9ed 100644 (file)
@@ -125,7 +125,8 @@ def test_match_distribution(dist1: Distribution, dist2: Distribution) -> None:
             dedent(
                 f"""\
                 [Match]
-                Distribution={dist1} {dist2}
+                Distribution=|{dist1}
+                Distribution=|{dist2}
 
                 [Content]
                 Packages=testpkg3
@@ -189,7 +190,8 @@ def test_match_release(release1: int, release2: int) -> None:
             dedent(
                 f"""\
                 [Match]
-                Release={release1} {release2}
+                Release=|{release1}
+                Release=|{release2}
 
                 [Content]
                 Packages=testpkg3
@@ -255,7 +257,8 @@ def test_match_imageid(image1: str, image2: str) -> None:
             dedent(
                 f"""\
                 [Match]
-                ImageId={image1} {image2}
+                ImageId=|{image1}
+                ImageId=|{image2}
 
                 [Content]
                 Packages=testpkg3
@@ -331,7 +334,8 @@ def test_match_imageversion(op: str, version: str) -> None:
             dedent(
                 f"""\
                 [Match]
-                ImageVersion=<200 {op}{version}
+                ImageVersion=<200
+                ImageVersion={op}{version}
 
                 [Content]
                 Packages=testpkg2
@@ -343,7 +347,8 @@ def test_match_imageversion(op: str, version: str) -> None:
             dedent(
                 f"""\
                 [Match]
-                ImageVersion=>9000 {op}{version}
+                ImageVersion=>9000
+                ImageVersion={op}{version}
 
                 [Content]
                 Packages=testpkg3