From 083135363be859070aff75ffb3b4503339464c4b Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Mon, 28 Aug 2023 13:24:59 +0200 Subject: [PATCH] Rework ini file parsing Python's configparser module is rather inadequate for our purposes. Specifically, it's built around every setting only being specified once in a config file and even though we got it to kind of parse more than one of the same setting with our own custom dict type, there's a lot of limitations involved. Specifically, assigning the empty value is impossible with the current approach. To avoid all these issues, let's introduce our own ini parser. This also splits off the matching logic into a new method match_config() and moves setting the debugging level just after we've parsed CLI options so we get debug messages from the parser as well. --- mkosi/__main__.py | 3 - mkosi/config.py | 173 ++++++++++++++++++++++++++++--------------- tests/test_config.py | 33 ++++++++- 3 files changed, 146 insertions(+), 63 deletions(-) diff --git a/mkosi/__main__.py b/mkosi/__main__.py index 1ca236af4..8442cecc0 100644 --- a/mkosi/__main__.py +++ b/mkosi/__main__.py @@ -44,9 +44,6 @@ def main() -> None: log_setup() args, presets = MkosiConfigParser().parse() - if ARG_DEBUG.get(): - logging.getLogger().setLevel(logging.DEBUG) - try: run_verb(args, presets) finally: diff --git a/mkosi/config.py b/mkosi/config.py index 81b57c756..5a307c35e 100644 --- a/mkosi/config.py +++ b/mkosi/config.py @@ -2,7 +2,6 @@ import argparse import base64 -import configparser import copy import dataclasses import enum @@ -22,7 +21,7 @@ import textwrap import uuid from collections.abc import Iterable, Sequence from pathlib import Path -from typing import Any, Callable, Optional, Type, Union, cast +from typing import Any, Callable, Iterator, Optional, Type, Union, cast from mkosi.architecture import Architecture from mkosi.distributions import Distribution, detect_distribution @@ -226,7 +225,7 @@ def config_parse_boolean(value: Optional[str], old: Optional[bool]) -> bool: def parse_feature(value: Optional[str]) -> ConfigFeature: - if not value or value == ConfigFeature.auto.name: + if value is None or value == ConfigFeature.auto.name: return ConfigFeature.auto return ConfigFeature.enabled if parse_boolean(value) else ConfigFeature.disabled @@ -241,7 +240,7 @@ def config_match_feature(match: Optional[str], value: Optional[ConfigFeature]) - def config_parse_compression(value: Optional[str], old: Optional[Compression]) -> Optional[Compression]: - if not value: + if value is None: return None try: @@ -251,7 +250,7 @@ def config_parse_compression(value: Optional[str], old: Optional[Compression]) - def config_parse_seed(value: Optional[str], old: Optional[str]) -> Optional[uuid.UUID]: - if not value or value == "random": + if value is None or value == "random": return None try: @@ -362,7 +361,7 @@ def config_make_list_parser(delimiter: str, new = old.copy() if old else [] # Empty strings reset the list. - if not value: + if value is None: return [] if unescape: @@ -461,7 +460,7 @@ def config_make_path_parser(*, def config_parse_filename(value: Optional[str], old: Optional[str]) -> Optional[str]: - if not value: + if value is None: return None if value == "." or value == "..": @@ -481,7 +480,7 @@ def match_path_exists(value: str) -> bool: def config_parse_root_password(value: Optional[str], old: Optional[tuple[str, bool]]) -> Optional[tuple[str, bool]]: - if not value: + if value is None: return None value = value.strip() @@ -816,6 +815,69 @@ class MkosiConfig: return manifest +def parse_ini(path: Path, only_sections: Sequence[str] = ()) -> Iterator[tuple[str, str, str]]: + """ + We have our own parser instead of using configparser as the latter does not support specifying the same + setting multiple times in the same configuration file. + """ + section: Optional[str] = None + setting: Optional[str] = None + value: Optional[str] = None + + for l in textwrap.dedent(path.read_text()).splitlines(): + # Systemd unit files allow both '#' and ';' to indicate comments so we do the same. + for c in ("#", ";"): + comment = l.find(c) + if comment >= 0: + l = l[:comment] + + if not l.strip(): + continue + + # If we have a section, setting and value, any line that's indented is considered part of the + # setting's value. + if section and setting and value is not None and l[0].isspace(): + value = f"{value}\n{l.strip()}" + continue + + # So the line is not indented, that means we either found a new section or a new setting. Either way, + # let's yield the previous setting and its value before parsing the new section/setting. + if section and setting and value is not None: + yield section, setting, value + setting = value = None + + l = l.strip() + + if l[0] == '[': + if l[-1] != ']': + die(f"{l} is not a valid section") + + section = l[1:-1].strip() + if not section: + die("Section name cannot be empty or whitespace") + + continue + + if not section: + die(f"Setting {l} is located outside of section") + + if only_sections and section not in only_sections: + continue + + setting, delimiter, value = l.partition("=") + if not delimiter: + die(f"Setting {setting} must be followed by '='") + if not setting: + die(f"Missing setting name before '=' in {l}") + + setting = setting.strip() + value = value.strip() + + # Make sure we yield any final setting and its value. + if section and setting and value is not None: + yield section, setting, value + + class MkosiConfigParser: SETTINGS = ( MkosiConfigSetting( @@ -1585,72 +1647,62 @@ class MkosiConfigParser: self.settings_lookup_by_dest = {s.dest: s for s in self.SETTINGS} self.match_lookup = {m.name: m for m in self.MATCHES} - def parse_config(self, path: Path, namespace: argparse.Namespace, defaults: argparse.Namespace) -> bool: - extras = path.is_dir() + def match_config(self, path: Path, namespace: argparse.Namespace, defaults: argparse.Namespace) -> bool: + triggered = None - if path.is_dir(): - path = path / "mkosi.conf" + # If the config file does not exist, we assume it matches so that we look at the other files in the + # directory as well (mkosi.conf.d/ and extra files). + if not path.exists(): + return True - parser = configparser.ConfigParser( - delimiters="=", - comment_prefixes="#", - inline_comment_prefixes="#", - empty_lines_in_values=True, - interpolation=None, - strict=False, - dict_type=ConfigParserMultipleValues, - ) + for _, k, v in parse_ini(path, only_sections=["Match"]): + trigger = v.startswith("|") + v = v.removeprefix("|") + negate = v.startswith("!") + v = v.removeprefix("!") - parser.optionxform = lambda optionstr: optionstr # type: ignore + if not v: + die("Match value cannot be empty") - if path.exists(): - parser.read(path) + if (s := self.settings_lookup_by_name.get(k)): + if not s.match: + die(f"{k} cannot be used in [Match]") - if "Match" in parser.sections(): - triggered = None + # 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. + self.finalize_default(s, namespace, defaults) - 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("!") + result = s.match(v, getattr(namespace, s.dest)) - if not v: - die("Match value cannot be empty") + elif (m := self.match_lookup.get(k)): + result = m.match(v) + else: + die(f"{k} cannot be used in [Match]") - if (s := self.settings_lookup_by_name.get(k)): - if not (match := s.match): - die(f"{k} cannot be used in [Match]") + if negate: + result = not result + if not trigger and not result: + return False + if trigger: + triggered = bool(triggered) or result - # 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. - self.finalize_default(s, namespace, defaults) + return triggered is not False - result = match(v, getattr(namespace, s.dest)) - elif (m := self.match_lookup.get(k)): - result = m.match(v) - else: - die(f"{k} cannot be used in [Match]") + def parse_config(self, path: Path, namespace: argparse.Namespace, defaults: argparse.Namespace) -> bool: + extras = path.is_dir() - if negate: - result = not result - if not trigger and not result: - return False - if trigger: - triggered = bool(triggered) or result + if path.is_dir(): + path = path / "mkosi.conf" - if triggered is False: - return False + if not self.match_config(path, namespace, defaults): + return False - parser.remove_section("Match") + if path.exists(): + logging.debug(f"Including configuration file {Path.cwd() / path}") - for section in parser.sections(): - for k, v in parser.items(section): + for _, k, v in parse_ini(path, only_sections=["Distribution", "Output", "Content", "Validation", "Host"]): ns = defaults if k.startswith("@") else namespace if not (s := self.settings_lookup_by_name.get(k.removeprefix("@"))): @@ -1943,6 +1995,9 @@ class MkosiConfigParser: args = load_args(namespace) + if ARG_DEBUG.get(): + logging.getLogger().setLevel(logging.DEBUG) + if args.verb == Verb.help: PagerHelpAction.__call__(None, argparser, namespace) # type: ignore diff --git a/tests/test_config.py b/tests/test_config.py index c4f74cd65..290c2618c 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -10,7 +10,7 @@ from typing import Iterator import pytest from mkosi.architecture import Architecture -from mkosi.config import Compression, MkosiConfig, MkosiConfigParser, OutputFormat +from mkosi.config import Compression, MkosiConfig, MkosiConfigParser, OutputFormat, parse_ini CONF_DIR = Path(__file__).parent.absolute() / "test-config" @@ -190,3 +190,34 @@ def test_path_inheritence() -> None: # Confirm output directory changes assert presets[0].preset == "00-test-preset" assert presets[0].output_dir == Path(tmpdir) / "mkosi.presets" / "00-test-preset" / "mkosi.output" + + +def test_parse_ini() -> None: + with tempfile.TemporaryDirectory() as d: + p = Path(d) / "ini" + p.write_text( + """\ + [MySection] + Value=abc + Other=def + ALLCAPS=txt + + # Comment + ; Another comment + [EmptySection] + [AnotherSection] + EmptyValue= + Multiline=abc + def + qed + ord + """ + ) + + g = parse_ini(p) + + assert next(g) == ("MySection", "Value", "abc") + assert next(g) == ("MySection", "Other", "def") + assert next(g) == ("MySection", "ALLCAPS", "txt") + assert next(g) == ("AnotherSection", "EmptyValue", "") + assert next(g) == ("AnotherSection", "Multiline", "abc\ndef\nqed\nord") -- 2.47.2