From: Daan De Meyer Date: Fri, 7 Apr 2023 10:32:58 +0000 (+0200) Subject: Rework configuration parsing ordering and overrides X-Git-Tag: v15~266^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=refs%2Fpull%2F1424%2Fhead;p=thirdparty%2Fmkosi.git Rework configuration parsing ordering and overrides Currently, aside from list based settings, if a setting is used multiple times across different configuration files, the later assignments override earlier assignments. On top of that, the CLI arguments are parsed last and override everything from config files. This has worked very well until now, but breaks now that we have [Match] sections. If we override a setting using the CLI, we want any configured [Match] sections to compare against the value specified via the CLI. Because the CLI values are applied last, this currently isn't the case. Similarly, if we add 90-local.conf to override the distribution release used for Debian, all the config files that are processed earlier will still compare against the default release configured for Debian in 50-debian.conf. If we rename 90-local.conf to 00-local.conf, the default release in 50-debian.conf will still override that value. Only by putting the override after the config file that assigns the default release but before the first config file that matches on that release can we make this work, but depending on the configuration this might require a lot of different override files which isn't ideal. Also, because later values can override earlier ones, it's possible to have [Match] sections that match against different values of the same setting both apply, if the setting happens to be reassigned inbetween, which is not intuitive and error prone. Ideally, [Match] settings only apply to the final value of a setting. To fix these problems, this commit reworks the way we order and override settings. Instead of having later assignments override earlier ones, for all settings aside from list settings, the first assignment is the one that is used. All later assignments of the same setting are ignored. Along with, we switch to parsing CLI arguments first, which means that any settings assigned in CLI args take precedence over those configured in config files. Because the first value is immediately the final value, any [Match] sections will only ever see the final value. One caveat is default values for settings. We only want to assign these at the end of parsing, if no value has been explicitly configured, but we also want [Match] sections to apply to default values if no value is explicitly configured. The solution to this is that if we encounter a setting in a [Match] section and it has not been explicitly assigned a value yet, it is assigned its default value. For list based settings, ! now configures an ignore glob, which means that if any later assignments try to assign values that match an ignore glob, those values are ignored. We also prepend list values instead of appending so that list values that are configured in a preceeding config file appear later in the final list value than values configured in a later assignment. Implementation wise, this commit reworks config parser functions to return the new value that should be assigned instead of assigning it themselves. This makes the config parsing functions slightly more generic. --- diff --git a/mkosi.md b/mkosi.md index 9d830e123..6d9309566 100644 --- a/mkosi.md +++ b/mkosi.md @@ -150,6 +150,7 @@ in consecutive runs with data from the cached one. * Run SELinux relabel is a SELinux policy is installed * Generate unified kernel image * Generate final output format + ## Supported output formats The following output formats are supported: @@ -176,11 +177,55 @@ single-letter shortcut is also allowed. In the configuration files, the setting must be in the appropriate section, so the settings are grouped by section below. +Configuration is parsed in the following order: + +* The command line arguments are parsed +* `mkosi.conf` is parsed if it exists in the directory set with + `--directory=` or the current working directory if `--directory=` is + not used. +* `mkosi.conf.d/` is parsed in the same directory if it exists. Each + directory and each file with the `.conf` extension in `mkosi.conf.d/` + is parsed. Any directory in `mkosi.conf.d` is parsed as if it were + a regular top level directory. +* Any default paths (depending on the option) are configured if the + corresponding path exists. + +If a setting is specified multiple times across the different sources +of configuration, the first assignment that is found is used. For example, +a setting specified on the command line will always take precedence over +the same setting configured in a config file. To override settings in a +dropin file, make sure your dropin file is alphanumerically ordered +before the config file that you're trying to override. + +Settings that take a list of values are merged by prepending each value +to the previously configured values. If a value of a list setting is +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. + 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. + +`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. + ### [Distribution] Section `Distribution=`, `--distribution=`, `-d` diff --git a/mkosi/__init__.py b/mkosi/__init__.py index 6f070b919..1cd3db431 100644 --- a/mkosi/__init__.py +++ b/mkosi/__init__.py @@ -45,6 +45,7 @@ from mkosi.backend import ( from mkosi.config import ( MkosiConfigParser, MkosiConfigSetting, + config_default_release, config_make_action, config_make_enum_matcher, config_make_enum_parser, @@ -54,7 +55,6 @@ from mkosi.config import ( config_parse_base_packages, config_parse_boolean, config_parse_compression, - config_parse_distribution, config_parse_feature, config_parse_script, config_parse_string, @@ -1079,7 +1079,7 @@ SETTINGS = ( MkosiConfigSetting( dest="distribution", section="Distribution", - parse=config_parse_distribution, + parse=config_make_enum_parser(Distribution), match=config_make_enum_matcher(Distribution), default=detect_distribution()[0], ), @@ -1088,7 +1088,7 @@ SETTINGS = ( section="Distribution", parse=config_parse_string, match=config_match_string, - default=detect_distribution()[1], + default_factory=config_default_release, ), MkosiConfigSetting( dest="architecture", @@ -1483,6 +1483,7 @@ def create_argument_parser() -> argparse.ArgumentParser: usage=USAGE, add_help=False, allow_abbrev=False, + argument_default=argparse.SUPPRESS, ) parser.add_argument( @@ -1998,18 +1999,11 @@ def create_argument_parser() -> argparse.ArgumentParser: return parser -def parse_args( - argv: Optional[Sequence[str]] = None, - directory: Optional[Path] = None, - namespace: Optional[argparse.Namespace] = None -) -> argparse.Namespace: +def parse_args(argv: Optional[Sequence[str]] = None) -> argparse.Namespace: if argv is None: argv = sys.argv[1:] argv = list(argv) # make a copy 'cause we'll be modifying the list later on - if namespace is None: - namespace = argparse.Namespace() - # Make sure the verb command gets explicitly passed. Insert a -- before the positional verb argument # otherwise it might be considered as an argument of a parameter with nargs='?'. For example mkosi -i # summary would be treated as -i=summary. @@ -2025,24 +2019,33 @@ def parse_args( else: argv += ["--", "build"] + argparser = create_argument_parser() + namespace = argparser.parse_args(argv) + + if namespace.verb == Verb.help: + argparser.print_help() + argparser.exit() + + if "directory" not in namespace: + setattr(namespace, "directory", None) + + if namespace.directory and not namespace.directory.is_dir(): + die(f"Error: {namespace.directory} is not a directory!") + + namespace = MkosiConfigParser(SETTINGS).parse(namespace.directory or Path("."), namespace) + for s in SETTINGS: if s.dest in namespace: continue - if s.default is None: - s.parse(s.dest, None, namespace) + if s.default_factory: + default = s.default_factory(namespace) + elif s.default is None: + default = s.parse(s.dest, None, namespace) else: - setattr(namespace, s.dest, s.default) - - if directory: - namespace = MkosiConfigParser(SETTINGS, directory).parse(namespace) - - argparser = create_argument_parser() - namespace = argparser.parse_args(argv, namespace) + default = s.default - if namespace.verb == Verb.help: - argparser.print_help() - argparser.exit() + setattr(namespace, s.dest, default) return namespace diff --git a/mkosi/__main__.py b/mkosi/__main__.py index d6f390b47..af8a69381 100644 --- a/mkosi/__main__.py +++ b/mkosi/__main__.py @@ -5,7 +5,6 @@ import contextlib import os import sys from collections.abc import Iterator -from pathlib import Path from subprocess import CalledProcessError from mkosi import parse_args, run_verb @@ -34,7 +33,6 @@ def main() -> None: else: die(f"Error: {args.directory} is not a directory!") - args = parse_args(directory=Path(".")) run_verb(args) diff --git a/mkosi/config.py b/mkosi/config.py index 93642e649..6e6d142ef 100644 --- a/mkosi/config.py +++ b/mkosi/config.py @@ -2,12 +2,13 @@ import argparse import configparser import dataclasses import enum +import fnmatch import os from collections.abc import Sequence from pathlib import Path from typing import Any, Callable, Optional, Type, Union, cast -from mkosi.backend import Distribution +from mkosi.backend import Distribution, detect_distribution from mkosi.log import die @@ -30,61 +31,85 @@ def parse_source_target_paths(value: str) -> tuple[Path, Optional[Path]]: return Path(src), Path(target) if target else None -def config_parse_string(dest: str, value: Optional[str], namespace: argparse.Namespace) -> None: - setattr(namespace, dest, value) +def config_parse_string(dest: str, value: Optional[str], namespace: argparse.Namespace) -> Optional[str]: + if dest in namespace: + return getattr(namespace, dest) # type: ignore + + 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_parse_script(dest: str, value: Optional[str], namespace: argparse.Namespace) -> None: - if value is not None: +def config_parse_script(dest: str, value: Optional[str], namespace: argparse.Namespace) -> Optional[Path]: + if dest in namespace: + return getattr(namespace, dest) # type: ignore + + if value: if not Path(value).exists(): die(f"{value} does not exist") if not os.access(value, os.X_OK): die(f"{value} is not executable") - config_make_path_parser(required=True)(dest, value, namespace) + return Path(value) if value else None -def config_parse_boolean(dest: str, value: Optional[str], namespace: argparse.Namespace) -> None: - setattr(namespace, dest, parse_boolean(value) if value is not None else False) +def config_parse_boolean(dest: str, value: Optional[str], namespace: argparse.Namespace) -> bool: + if dest in namespace: + return getattr(namespace, dest) # type: ignore + + return parse_boolean(value) if value else False def config_match_boolean(dest: str, value: str, namespace: argparse.Namespace) -> bool: return cast(bool, getattr(namespace, dest) == parse_boolean(value)) -def config_parse_feature(dest: str, value: Optional[str], namespace: argparse.Namespace) -> None: - if value is None: - value = "auto" - setattr(namespace, dest, parse_boolean(value) if value != "auto" else None) +def config_parse_feature(dest: str, value: Optional[str], namespace: argparse.Namespace) -> Optional[bool]: + if dest in namespace: + return getattr(namespace, dest) # type: ignore + + if value and value == "auto": + return None + return parse_boolean(value) if value else None + + +def config_parse_compression(dest: str, value: Optional[str], namespace: argparse.Namespace) -> Union[None, str, bool]: + if dest in namespace: + return getattr(namespace, dest) # type: ignore -def config_parse_compression(dest: str, value: Optional[str], namespace: argparse.Namespace) -> None: if value in ("zlib", "lzo", "zstd", "lz4", "xz"): - setattr(namespace, dest, value) - else: - setattr(namespace, dest, parse_boolean(value) if value is not None else None) + return value + + return parse_boolean(value) if value else None + +def config_parse_base_packages(dest: str, value: Optional[str], namespace: argparse.Namespace) -> Union[bool, str]: + if dest in namespace: + return getattr(namespace, dest) # type: ignore -def config_parse_base_packages(dest: str, value: Optional[str], namespace: argparse.Namespace) -> None: if value == "conditional": - setattr(namespace, dest, value) - else: - setattr(namespace, dest, parse_boolean(value) if value is not None else False) + return value + return parse_boolean(value) if value else False -def config_parse_distribution(dest: str, value: Optional[str], namespace: argparse.Namespace) -> None: - assert value is not None - try: - d = Distribution[value] - except KeyError: - die(f"Invalid distribution {value}") +def config_default_release(namespace: argparse.Namespace) -> Any: + # If we encounter Release in [Match] and no distribution has been set yet, configure the default + # distribution as well since the default release depends on the selected distribution. + if "distribution" not in namespace: + setattr(namespace, "distribution", detect_distribution()[0]) - r = { + d = getattr(namespace, "distribution") + + # If the configured distribution matches the host distribution, use the same release as the host. + hd, hr = detect_distribution() + if d == hd: + return hr + + return { Distribution.fedora: "37", Distribution.centos: "9", Distribution.rocky: "9", @@ -97,12 +122,10 @@ def config_parse_distribution(dest: str, value: Optional[str], namespace: argpar Distribution.gentoo: "17.1", }.get(d, "rolling") - setattr(namespace, dest, d) - setattr(namespace, "release", r) - -ConfigParseCallback = Callable[[str, Optional[str], argparse.Namespace], None] +ConfigParseCallback = Callable[[str, Optional[str], argparse.Namespace], Any] ConfigMatchCallback = Callable[[str, str, argparse.Namespace], bool] +ConfigDefaultCallback = Callable[[argparse.Namespace], Any] @dataclasses.dataclass(frozen=True) @@ -113,6 +136,7 @@ class MkosiConfigSetting: match: Optional[ConfigMatchCallback] = None name: str = "" default: Any = None + default_factory: Optional[ConfigDefaultCallback] = None paths: tuple[str, ...] = tuple() def __post_init__(self) -> None: @@ -121,12 +145,11 @@ class MkosiConfigSetting: class MkosiConfigParser: - def __init__(self, settings: Sequence[MkosiConfigSetting], directory: Path) -> None: + def __init__(self, settings: Sequence[MkosiConfigSetting]) -> None: self.settings = settings - self.directory = directory self.lookup = {s.name: s for s in settings} - def _parse_config(self, path: Path, namespace: argparse.Namespace) -> None: + def parse(self, path: Path, namespace: argparse.Namespace) -> argparse.Namespace: extras = path.is_dir() if path.is_dir(): @@ -150,32 +173,46 @@ class MkosiConfigParser: if not (s := self.lookup.get(k)): die(f"Unknown setting {k}") - if s.match and not s.match(s.dest, v, namespace): - return + if not (match := s.match): + die(f"{k} cannot be used in [Match]") - parser.remove_section("Match") + # If we encounter a setting in [Match] that has not been explicitly configured yet, we assign + # it it's 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 - if extras: - for s in self.settings: - for f in s.paths: - if path.parent.joinpath(f).exists(): - s.parse(s.dest, str(path.parent / f), namespace) + setattr(namespace, s.dest, default) + + if not match(s.dest, v, namespace): + return namespace + + parser.remove_section("Match") for section in parser.sections(): for k, v in parser.items(section): if not (s := self.lookup.get(k)): die(f"Unknown setting {k}") - s.parse(s.dest, v, namespace) + setattr(namespace, s.dest, s.parse(s.dest, v, namespace)) - if extras and path.parent.joinpath("mkosi.conf.d").exists(): - for p in sorted(path.parent.joinpath("mkosi.conf.d").iterdir()): - if p.is_dir() or p.suffix == ".conf": - self._parse_config(p, namespace) + if extras: + # Dropin configuration has priority over any default paths. + if path.parent.joinpath("mkosi.conf.d").exists(): + for p in sorted(path.parent.joinpath("mkosi.conf.d").iterdir()): + if p.is_dir() or p.suffix == ".conf": + namespace = self.parse(p, namespace) + + for s in self.settings: + for f in s.paths: + if path.parent.joinpath(f).exists(): + setattr(namespace, s.dest, s.parse(s.dest, str(path.parent / f), namespace)) - def parse(self, namespace: argparse.Namespace = argparse.Namespace()) -> argparse.Namespace: - self._parse_config(self.directory, namespace) return namespace @@ -201,11 +238,11 @@ def config_make_action(settings: Sequence[MkosiConfigSetting]) -> Type[argparse. die(f"Unknown setting {option_string}") if values is None or isinstance(values, str): - s.parse(self.dest, values, namespace) + setattr(namespace, s.dest, s.parse(self.dest, values, namespace)) else: for v in values: assert isinstance(v, str) - s.parse(self.dest, v, namespace) + setattr(namespace, s.dest, s.parse(self.dest, v, namespace)) return MkosiAction @@ -215,14 +252,17 @@ def make_enum_parser(type: Type[enum.Enum]) -> Callable[[str], enum.Enum]: try: return type[value] except KeyError: - die(f"Invalid enum value {value}") + die(f"Invalid {type.__name__} value \"{value}\"") return parse_enum def config_make_enum_parser(type: Type[enum.Enum]) -> ConfigParseCallback: - def config_parse_enum(dest: str, value: Optional[str], namespace: argparse.Namespace) -> None: - setattr(namespace, dest, make_enum_parser(type)(value) if value else None) + def config_parse_enum(dest: str, value: Optional[str], namespace: argparse.Namespace) -> Optional[enum.Enum]: + if dest in namespace: + return getattr(namespace, dest) # type: ignore + + return make_enum_parser(type)(value) if value else None return config_parse_enum @@ -235,30 +275,45 @@ def config_make_enum_matcher(type: Type[enum.Enum]) -> ConfigMatchCallback: def config_make_list_parser(delimiter: str, parse: Callable[[str], Any] = str) -> ConfigParseCallback: - def config_parse_list(dest: str, value: Optional[str], namespace: argparse.Namespace) -> None: + ignore: set[str] = set() + + def config_parse_list(dest: str, value: Optional[str], namespace: argparse.Namespace) -> list[Any]: + if dest not in namespace: + ignore.clear() + l = [] + else: + l = getattr(namespace, dest).copy() + if not value: - setattr(namespace, dest, []) - return + return l # type: ignore value = value.replace("\n", delimiter) values = [v for v in value.split(delimiter) if v] for v in values: - if v == "!*": - getattr(namespace, dest).clear() - elif v.startswith("!"): - setattr(namespace, dest, [i for i in getattr(namespace, dest) if i == parse(v[1:])]) + if v.startswith("!"): + ignore.add(v[1:]) + continue + + for i in ignore: + if fnmatch.fnmatchcase(v, i): + break else: - getattr(namespace, dest).append(parse(v)) + l.insert(0, parse(v)) + + return l return config_parse_list def config_make_path_parser(required: bool) -> ConfigParseCallback: - def config_parse_path(dest: str, value: Optional[str], namespace: argparse.Namespace) -> None: - if value is not None and required and not Path(value).exists(): + def config_parse_path(dest: str, value: Optional[str], namespace: argparse.Namespace) -> Optional[Path]: + if dest in namespace: + return getattr(namespace, dest) # type: ignore + + if value and required and not Path(value).exists(): die(f"{value} does not exist") - setattr(namespace, dest, Path(value) if value else None) + return Path(value) if value else None return config_parse_path diff --git a/tests/test_parse_load_args.py b/tests/test_parse_load_args.py index 0a53f3be9..0fe97a269 100644 --- a/tests/test_parse_load_args.py +++ b/tests/test_parse_load_args.py @@ -27,7 +27,7 @@ def cd_temp_dir() -> Iterator[None]: def parse(argv: Optional[List[str]] = None) -> MkosiConfig: - return mkosi.load_args(mkosi.parse_args(argv, directory=Path("."))) + return mkosi.load_args(mkosi.parse_args(argv)) def test_parse_load_verb() -> None: