config: sanity check via types (MR 2252)

Replace the "sanity_check" code with type checking built into the Config
__setattr__ operator.

This keeps all the Config related code in one place.

Signed-off-by: Caleb Connolly <caleb@postmarketos.org>

pmb/config/__init__.py

@@ -11,7 +11,7 @@ from typing import Dict, List, Sequence
 #
 # FIXME (#2324): this sucks, we should re-organise this and not rely on "lifting"
 # this functions this way
-from pmb.config.load import load, sanity_checks, save, serialize
+from pmb.config.load import load, save, serialize
 from pmb.config.sudo import which_sudo
 from pmb.config.other import is_systemd_selected
 
@@ -113,10 +113,6 @@ defaults = {
     "iter_time": "200",
 }
 
-allowed_values = {
-    "systemd": ["default", "always", "never"],
-}
-
 # Whether we're connected to a TTY (which allows things like e.g. printing
 # progress bars)
 is_interactive = sys.stdout.isatty() and \

pmb/config/init.py

@@ -2,6 +2,7 @@
 # SPDX-License-Identifier: GPL-3.0-or-later
 from pmb.core import get_context
 from pmb.core.chroot import Chroot
+from pmb.core.config import SystemdConfig
 from pmb.core.context import Context
 from pmb.core.pkgrepo import pkgrepo_default_path
 from pmb.helpers import logging
@@ -211,7 +212,7 @@ def ask_for_systemd(config: Config, ui):
     logging.info("Based on your UI selection, 'default' will result"
                  f" in{not_str}installing systemd.")
 
-    choices = pmb.config.allowed_values["systemd"]
+    choices = SystemdConfig.choices()
     answer = pmb.helpers.cli.ask("Install systemd?",
                                  choices,
                                  config.systemd,

pmb/config/load.py

@@ -1,35 +1,13 @@
 # Copyright 2023 Oliver Smith
 # SPDX-License-Identifier: GPL-3.0-or-later
 from pathlib import Path, PosixPath
-from typing import Any, Dict, List, Mapping, Optional
+from typing import List
 from pmb.helpers import logging
 import configparser
 import os
-import sys
-import pmb.config
 from pmb.core import Config
 
 
-def sanity_check(cfg: Config, key, allowed, path: Optional[Path] = None):
-    value = getattr(cfg, key)
-
-    if value in allowed:
-        return
-
-    logging.error(f"pmbootstrap.cfg: invalid value for {key}: '{value}'")
-    logging.error(f"Allowed: {', '.join(allowed)}")
-
-    if path:
-        logging.error(f"Fix it here and try again: {path}")
-
-    sys.exit(1)
-
-
-def sanity_checks(cfg: Config, path: Optional[Path] = None):
-    for key, allowed in pmb.config.allowed_values.items():
-        sanity_check(cfg, key, allowed, path)
-
-
 def load(path: Path) -> Config:
     config = Config()
 
@@ -67,7 +45,6 @@ def load(path: Path) -> Config:
         elif key in cfg["pmbootstrap"]:
             setattr(config, key, cfg["pmbootstrap"][key])
 
-    sanity_checks(config, path)
 
     return config
 

pmb/config/other.py

@@ -1,6 +1,7 @@
 # Copyright 2024 Oliver Smith
 # SPDX-License-Identifier: GPL-3.0-or-later
 from pmb.core import Config
+from pmb.core.config import SystemdConfig
 import pmb.helpers.ui
 import pmb.config.pmaports
 
@@ -10,9 +11,9 @@ def is_systemd_selected(config: Config):
         return False
     if pmb.helpers.ui.check_option(config.ui, "pmb:systemd-never", skip_extra_repos=True):
         return False
-    if config.systemd == "always":
+    if config.systemd == SystemdConfig.ALWAYS:
         return True
-    if config.systemd == "never":
+    if config.systemd == SystemdConfig.NEVER:
         return False
     return pmb.helpers.ui.check_option(config.ui, "pmb:systemd", skip_extra_repos=True)
 
@@ -22,9 +23,9 @@ def systemd_selected_str(config: Config):
         return "no", "not supported by pmaports branch"
     if pmb.helpers.ui.check_option(config.ui, "pmb:systemd-never"):
         return "no", "not supported by selected UI"
-    if config.systemd == "always":
+    if config.systemd == SystemdConfig.ALWAYS:
         return "yes", "'always' selected in 'pmbootstrap init'"
-    if config.systemd == "never":
+    if config.systemd == SystemdConfig.NEVER:
         return "no", "'never' selected in 'pmbootstrap init'"
     if pmb.helpers.ui.check_option(config.ui, "pmb:systemd"):
         return "yes", "default for selected UI"

pmb/core/config.py

@@ -1,5 +1,6 @@
 
 from copy import deepcopy
+import enum
 import multiprocessing
 from typing import Any, List, Dict, TypedDict
 from pathlib import Path
@@ -11,6 +12,20 @@ class Mirrors(TypedDict):
     systemd: str
 
 
+class SystemdConfig(enum.Enum):
+    DEFAULT = "default"
+    ALWAYS = "always"
+    NEVER = "never"
+
+
+    def __str__(self) -> str:
+        return self.value
+
+    @staticmethod
+    def choices() -> List[str]:
+        return [e.value for e in SystemdConfig]
+
+
 class Config():
     aports: List[Path] = [Path(os.path.expanduser("~") +
                         "/.local/var/pmbootstrap/cache_git/pmaports")]
@@ -41,7 +56,7 @@ class Config():
     ssh_key_glob: str = "~/.ssh/id_*.pub"
     ssh_keys: bool = False
     sudo_timer: bool = False
-    systemd: str = "default"
+    systemd: SystemdConfig = SystemdConfig.DEFAULT
     timezone: str = "GMT"
     ui: str = "console"
     ui_extras: bool = False
@@ -55,7 +70,7 @@ class Config():
         # Make sure we aren't modifying the class defaults
         for key in Config.__annotations__.keys():
             setattr(self, key, deepcopy(Config.get_default(key)))
-    
+
 
     @staticmethod
     def keys() -> List[str]:
@@ -78,22 +93,29 @@ class Config():
             raise ValueError(f"Invalid dotted key: {dotted_key}")
 
 
-    def __setattr__(self, key: str, value: str):
+    def __setattr__(self, key: str, value: Any):
         """Allow for setattr() to be used with a dotted key
         to set nested dictionaries (e.g. "mirrors.alpine")."""
         keys = key.split(".")
         if len(keys) == 1:
-            super(Config, self).__setattr__(key, value)
+            _type = type(getattr(Config, key))
+            try:
+                super(Config, self).__setattr__(key, _type(value))
+            except ValueError:
+                msg = f"Invalid value for '{key}': '{value}' "
+                if issubclass(_type, enum.Enum):
+                    valid = [x.value for x in _type]
+                    msg += f"(valid values: {', '.join(valid)})"
+                else:
+                    msg += f"(expected {_type}, got {type(value)})"
+                raise ValueError(msg)
         elif len(keys) == 2:
-            #print(f"cfgset, before: {super(Config, self).__getattribute__(keys[0])[keys[1]]}")
             super(Config, self).__getattribute__(keys[0])[keys[1]] = value
-            #print(f"cfgset, after: {super(Config, self).__getattribute__(keys[0])[keys[1]]}")
         else:
             raise ValueError(f"Invalid dotted key: {key}")
 
 
-    def __getattribute__(self, key: str) -> str:
+    def __getattribute__(self, key: str) -> Any:
-        #print(repr(self))
         """Allow for getattr() to be used with a dotted key
         to get nested dictionaries (e.g. "mirrors.alpine")."""
         keys = key.split(".")

pmb/helpers/frontend.py

@@ -228,7 +228,6 @@ def config(args: PmbArgs):
         pmb.config.save(args.config, config)
     elif args.value is not None:
         setattr(config, args.name, args.value)
-        pmb.config.sanity_checks(config)
         logging.info("Config changed: " + args.name + "='" + args.value + "'")
         pmb.config.save(args.config, config)
     elif args.name: