From 5b2eddb28ed626b5bf1986afb036b29158e299f6 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Tue, 6 Jun 2023 11:59:55 +0200 Subject: [PATCH] Make matches work more like systemd conditions 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 | 76 +++++-------- mkosi/config.py | 199 +++++++++++++++++----------------- tests/test_parse_load_args.py | 15 ++- 3 files changed, 140 insertions(+), 150 deletions(-) diff --git a/mkosi.md b/mkosi.md index c3daae112..15f0270f6 100644 --- 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 diff --git a/mkosi/config.py b/mkosi/config.py index 3c526e60f..8dfffd2dd 100644 --- a/mkosi/config.py +++ b/mkosi/config.py @@ -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") diff --git a/tests/test_parse_load_args.py b/tests/test_parse_load_args.py index 3d5ec0b55..362c9f014 100644 --- a/tests/test_parse_load_args.py +++ b/tests/test_parse_load_args.py @@ -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 -- 2.47.2