]> git.ipfire.org Git - thirdparty/mkosi.git/commitdiff
Only write CLI arguments to history instead of full config
authorDaan De Meyer <daan.j.demeyer@gmail.com>
Sun, 13 Apr 2025 20:42:55 +0000 (22:42 +0200)
committerDaan De Meyer <daan.j.demeyer@gmail.com>
Wed, 16 Apr 2025 09:55:55 +0000 (11:55 +0200)
The huge downside of writing the full config to the history is that
any modifications to config files are ignored until the image is
rebuilt.

If we make the argument that changes to config files are usually
supposed to be permanent rather than ephemeral, it doesn't make sense
to not take them into account when reusing the history. Especially since
the primary purpose of the history is to avoid specifying the same CLI
arguments over and over again without encoding those CLI arguments into
a config file.

So instead of saving the full config to the history and not reparsing
the configuration when reusing the history, let's only save the CLI
arguments to the history and always reparse the configuration, even when
we end up reusing the history.

This keeps the crucial benefit of the history intact (not having to repeat
yourself endlessly) while still making sure any new settings added to the
configuration files are taken into account immediately.

It also simplifies the implementation quite a bit, as we can get away
again with maintaining a single history file instead of two.

Additionally, we opt to completely ignore the history when using the sandbox
verb. The idea here is that the history only applies to the final image (and
subimages). This was already the case as any --tools-tree-xxx CLI options aren't
saved in the history as they are not fields of the Config object.

mkosi/__init__.py
mkosi/config.py
mkosi/resources/man/mkosi.1.md

index 3c922b1aebeb17e9ffa7d5716dadce518dea18b0..82a860b0e80d15c2655a9421f5e36305d262177f 100644 (file)
@@ -73,7 +73,6 @@ from mkosi.config import (
     expand_delayed_specifiers,
     finalize_configdir,
     format_bytes,
-    have_history,
     in_sandbox,
     parse_boolean,
     parse_config,
@@ -5070,11 +5069,6 @@ def run_verb(args: Args, tools: Optional[Config], images: Sequence[Config], *, r
 
         _, _, manifest = cache_tree_paths(tools)
         manifest.write_text(dump_json(tools.cache_manifest()))
-        Path(".mkosi-private/history/tools.json").unlink(missing_ok=True)
-
-    if tools and last.history and not Path(".mkosi-private/history/tools.json").exists():
-        Path(".mkosi-private/history").mkdir(parents=True, exist_ok=True)
-        Path(".mkosi-private/history/tools.json").write_text(dump_json(tools.to_dict()))
 
     if args.verb.needs_tools():
         return {
@@ -5165,27 +5159,22 @@ def run_verb(args: Args, tools: Optional[Config], images: Sequence[Config], *, r
             if args.force > 1 or not have_cache(initrd):
                 remove_cache_entries(initrd)
 
-    if not have_history(args):
-        for i, config in enumerate(images):
-            if args.verb != Verb.build:
-                check_tools(config, args.verb)
+    for i, config in enumerate(images):
+        if args.verb != Verb.build:
+            check_tools(config, args.verb)
 
-            images[i] = config = run_configure_scripts(config)
+        images[i] = config = run_configure_scripts(config)
 
-        # The images array has been modified so we need to reevaluate last again.
-        # Also ensure that all other images are reordered in case their dependencies were modified.
-        last = images[-1]
-        images = resolve_deps(images[:-1], last.dependencies) + [last]
+    # The images array has been modified so we need to reevaluate last again.
+    # Also ensure that all other images are reordered in case their dependencies were modified.
+    last = images[-1]
+    images = resolve_deps(images[:-1], last.dependencies) + [last]
 
     if (
         args.rerun_build_scripts
         or last.output_format == OutputFormat.none
         or not (last.output_dir_or_cwd() / last.output).exists()
     ):
-        history = (last.output_format == OutputFormat.none and not args.rerun_build_scripts) or (
-            last.output_format != OutputFormat.none and not (last.output_dir_or_cwd() / last.output).exists()
-        )
-
         for config in images:
             if any(
                 source.type != KeySourceType.file
@@ -5268,12 +5257,6 @@ def run_verb(args: Args, tools: Optional[Config], images: Sequence[Config], *, r
         if args.auto_bump:
             bump_image_version()
 
-        if last.history and history:
-            Path(".mkosi-private/history").mkdir(parents=True, exist_ok=True)
-            Path(".mkosi-private/history/latest.json").write_text(
-                dump_json({"Images": [config.to_dict() for config in images]})
-            )
-
     if args.verb == Verb.build:
         return
 
index 8f9399c51bcc1b17b49c070a2c15e12c1b066b36..8c7b13ecf4e8952e966b615dd1d9c060e0234d5b 100644 (file)
@@ -2344,7 +2344,14 @@ class Config:
         return d
 
     @classmethod
-    def from_json(cls, s: Union[str, dict[str, Any], SupportsRead[str], SupportsRead[bytes]]) -> "Config":
+    def to_partial_dict(cls, partial: dict[str, Any]) -> dict[str, Any]:
+        return dict_with_capitalised_keys_factory([(k, v) for k, v in partial.items() if k in cls.fields()])
+
+    @classmethod
+    def from_partial_json(
+        cls,
+        s: Union[str, dict[str, Any], SupportsRead[str], SupportsRead[bytes]],
+    ) -> dict[str, Any]:
         """Instantiate a Config object from a (partial) JSON dump."""
         if isinstance(s, str):
             j = json.loads(s)
@@ -2374,9 +2381,13 @@ class Config:
                 )
 
         value_transformer = json_type_transformer(cls)
-        j = {(tk := key_transformer(k)): value_transformer(tk, v) for k, v in j.items()}
+        return {(tk := key_transformer(k)): value_transformer(tk, v) for k, v in j.items()}
 
-        return dataclasses.replace(cls.default(), **{k: v for k, v in j.items() if k in cls.fields()})
+    @classmethod
+    def from_json(cls, s: Union[str, dict[str, Any], SupportsRead[str], SupportsRead[bytes]]) -> "Config":
+        return dataclasses.replace(
+            cls.default(), **{k: v for k, v in cls.from_partial_json(s).items() if k in cls.fields()}
+        )
 
     def find_binary(self, *names: PathString, tools: bool = True) -> Optional[Path]:
         return find_binary(*names, root=self.tools() if tools else Path("/"), extra=self.extra_search_paths)
@@ -4510,7 +4521,6 @@ class ParseContext:
         self.defaults: dict[str, Any] = {}
         # Compare inodes instead of paths so we can't get tricked by bind mounts and such.
         self.includes: set[tuple[int, int]] = set()
-        self.only_sections: tuple[str, ...] = tuple()
 
     def setting_prohibited(self, setting: ConfigSetting[T]) -> bool:
         image = self.config["image"]
@@ -4791,9 +4801,6 @@ class ParseContext:
                 if self.setting_prohibited(s):
                     continue
 
-                if self.only_sections and s.section not in self.only_sections:
-                    continue
-
                 for f in s.path_suffixes:
                     f = f"mkosi.{f}"
 
@@ -4833,7 +4840,7 @@ class ParseContext:
 
             for section, k, v in parse_ini(
                 path,
-                only_sections=self.only_sections or {s.section for s in SETTINGS} | {"Host"},
+                only_sections={s.section for s in SETTINGS} | {"Host"},
             ):
                 if not k and not v:
                     continue
@@ -4899,13 +4906,30 @@ class ParseContext:
         return ns
 
 
-def have_history(args: Args, tools: bool = False) -> bool:
-    filename = "tools" if tools else "latest"
+def want_new_history(args: Args) -> bool:
+    if args.directory is None:
+        return False
+
+    if not args.verb.needs_build():
+        return False
+
+    if args.rerun_build_scripts:
+        return False
+
+    if args.verb != Verb.build and args.force == 0:
+        return False
+
+    return True
+
+
+def have_history(args: Args) -> bool:
+    if want_new_history(args):
+        return False
 
     if args.directory is None:
         return False
 
-    if args.verb in (Verb.build, Verb.cat_config, Verb.clean):
+    if args.verb in (Verb.clean, Verb.sandbox):
         return False
 
     if args.verb == Verb.summary and args.force > 0:
@@ -4914,10 +4938,13 @@ def have_history(args: Args, tools: bool = False) -> bool:
     if args.verb.needs_tools() and args.force > 0:
         return False
 
-    if args.verb.needs_build() and args.force > 0 and not args.rerun_build_scripts:
+    if args.verb.needs_build() and args.force > 0:
         return False
 
-    return Path(f".mkosi-private/history/{filename}.json").exists()
+    if args.verb == Verb.build and not args.rerun_build_scripts:
+        return False
+
+    return Path(".mkosi-private/history/latest.json").exists()
 
 
 def finalize_default_tools(
@@ -4989,7 +5016,6 @@ def parse_config(
     argv: Sequence[str] = (),
     *,
     resources: Path = Path("/"),
-    only_sections: Sequence[str] = (),
 ) -> tuple[Args, Optional[Config], tuple[Config, ...]]:
     argv = list(argv)
 
@@ -5035,14 +5061,7 @@ def parse_config(
         return args, None, ()
 
     if have_history(args):
-        try:
-            j = json.loads(Path(".mkosi-private/history/latest.json").read_text())
-            *subimages, prev = [Config.from_json(c) for c in j["Images"]]
-        except (KeyError, ValueError):
-            die(
-                "Unable to parse history from .mkosi-private/history/latest.json",
-                hint="Build with -f to generate a new history file from scratch",
-            )
+        history = Config.from_partial_json(Path(".mkosi-private/history/latest.json").read_text())
 
         # If we're operating on a previously built image (vm, boot, shell, ...), we're not rebuilding the
         # image and the configuration of the latest build is available, we load the config that was used to
@@ -5050,28 +5069,17 @@ def parse_config(
         # section settings which we allow changing without requiring a rebuild of the image.
         for s in SETTINGS:
             if s.section in ("Include", "Runtime"):
+                history.pop(s.dest, None)
                 continue
 
-            if s.dest in context.cli and context.cli[s.dest] != getattr(prev, s.dest):
+            if s.dest in context.cli and context.cli[s.dest] != history[s.dest]:
                 logging.warning(
                     f"Ignoring {s.long} from the CLI. Run with -f to rebuild the image with this setting"
                 )
 
-            if hasattr(prev, s.dest):
-                context.cli[s.dest] = getattr(prev, s.dest)
-                if s.dest in context.config:
-                    del context.config[s.dest]
+        context.cli |= history
 
-        context.only_sections = ("Include", "Runtime", "Host")
-    else:
-        context.only_sections = tuple(only_sections)
-        subimages = []
-        prev = None
-
-    if not in_sandbox() and have_history(args, tools=True):
-        tools = Config.from_json(json.loads(Path(".mkosi-private/history/tools.json").read_text()))
-    else:
-        tools = None
+    cli = copy.deepcopy(context.cli)
 
     # One of the specifiers needs access to the directory, so make sure it is available.
     context.config["directory"] = args.directory
@@ -5089,9 +5097,11 @@ def parse_config(
 
     config = context.finalize()
 
-    if prev:
-        return args, tools, (*subimages, Config.from_dict(config))
+    if config["history"] and want_new_history(args):
+        Path(".mkosi-private/history").mkdir(parents=True, exist_ok=True)
+        Path(".mkosi-private/history/latest.json").write_text(dump_json(Config.to_partial_dict(cli)))
 
+    tools = None
     if config.get("tools_tree") == Path("default"):
         if in_sandbox():
             config["tools_tree"] = Path(os.environ["MKOSI_DEFAULT_TOOLS_TREE_PATH"])
index 9ff4c9be4113a733d7efd446dbb6d67de9de3107..100dc0f303ee4cc886f882e03173902e2721942b 100644 (file)
@@ -1583,15 +1583,11 @@ boolean argument: either `1`, `yes`, or `true` to enable, or `0`, `no`,
     up in the generated XFS filesystem.
 
 `History=`, `--history=`
-:   Takes a boolean. If enabled, **mkosi** will write information about
-    the latest build to the `.mkosi-private` subdirectory in the
-    directory from which it was invoked. This information is then used
-    to restore the config of the latest build when running any verb that
-    does not rebuild the image or when running any verb that may rebuild
-    the image without specifying `--force`.
-
-    Note that configure scripts will not be executed if we reuse the
-    history from a previous build.
+:   Takes a boolean. If enabled, **mkosi** will write the configuration
+    provided via the CLI for the latest build to the `.mkosi-private`
+    subdirectory in the directory from which it was invoked. These
+    arguments are then reused as long as the image is not rebuilt to
+    avoid having to specify them over and over again.
 
     To give an example of why this is useful, if you run
     `mkosi -O my-custom-output-dir -f` followed by `mkosi vm`, **mkosi**